Skip to content

Commit 3ffda2f

Browse files
committed
Shared: Overhaul the AlertFiltering QLDoc
The documentation is now up-to-date with the new and more relaxed rules that allow overapproximating the results. I have also attempted to make a clearer distinction between the requirements of the specification and the behaviour of the implementation.
1 parent 5a1246a commit 3ffda2f

File tree

1 file changed

+29
-28
lines changed

1 file changed

+29
-28
lines changed

shared/util/codeql/util/AlertFiltering.qll

Lines changed: 29 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,14 @@ module;
88
private import codeql.util.Location
99

1010
/**
11-
* Holds if the query should produce alerts that match the given line ranges.
11+
* Holds if the query may restrict its computation to only produce alerts that match the given line
12+
* ranges. This predicate is used for implementing _diff-informed queries_ for pull requests in
13+
* GitHub Code Scanning.
1214
*
1315
* This predicate is active if and only if it is nonempty. If this predicate is inactive, it has no
14-
* effect. If it is active, it accepts any alert that has at least one matching location.
15-
*
16-
* Note that an alert that is not accepted by this filtering predicate may still be included in the
17-
* query results if it is accepted by another active filtering predicate in this module. An alert is
18-
* excluded from the query results if only if (1) there is at least one active filtering predicate,
19-
* and (2) it is not accepted by any active filtering predicate.
16+
* effect. If it is active, queries may omit alerts that don't have a _primary_ or _related_
17+
* location (in SARIF terminology) whose start line is within a specified range. Queries are allowed
18+
* to produce alerts outside this range.
2019
*
2120
* An alert location is a match if it matches a row in this predicate. If `startLineStart` and
2221
* `startLineEnd` are both 0, the row specifies a whole-file match, and a location is a match if
@@ -29,26 +28,24 @@ private import codeql.util.Location
2928
* - startLineStart: inclusive start of the range for alert location start line number (1-based).
3029
* - startLineEnd: inclusive end of the range for alert location start line number (1-based).
3130
*
32-
* A query should either perform no alert filtering, or adhere to all the filtering rules in this
33-
* module and return all and only the accepted alerts.
34-
*
35-
* This predicate is suitable for situations where we want to filter alerts at line granularity,
36-
* such as based on the pull request diff.
31+
* Note that an alert that is not accepted by this filtering predicate may still be included in the
32+
* query results if it is accepted by another active filtering predicate in this module. An alert is
33+
* excluded from the query results if only if (1) there is at least one active filtering predicate,
34+
* and (2) it is not accepted by any active filtering predicate.
3735
*
3836
* See also: `restrictAlertsToExactLocation`.
3937
*/
4038
extensible predicate restrictAlertsTo(string filePath, int startLineStart, int startLineEnd);
4139

4240
/**
43-
* Holds if the query should produce alerts that match the given locations.
41+
* Holds if the query may restrict its computation to only produce alerts that match the given
42+
* character ranges. This predicate is suitable for testing, where we want to filter by the exact
43+
* alert location, distinguishing between alerts on the same line.
4444
*
4545
* This predicate is active if and only if it is nonempty. If this predicate is inactive, it has no
46-
* effect. If it is active, it accepts any alert that has at least one matching location.
47-
*
48-
* Note that an alert that is not accepted by this filtering predicate may still be included in the
49-
* query results if it is accepted by another active filtering predicate in this module. An alert is
50-
* excluded from the query results if only if (1) there is at least one active filtering predicate,
51-
* and (2) it is not accepted by any active filtering predicate.
46+
* effect. If it is active, queries may omit alerts that don't have a _primary_ or _related_
47+
* location (in SARIF terminology) whose location equals a tuple from this predicate. Queries are
48+
* allowed to produce alerts outside this range.
5249
*
5350
* An alert location is a match if it matches a row in this predicate. Each row specifies an exact
5451
* location: an alert location is a match if its file path matches `filePath`, its start line and
@@ -61,11 +58,10 @@ extensible predicate restrictAlertsTo(string filePath, int startLineStart, int s
6158
* - endLine: alert location end line number (1-based).
6259
* - endColumn: alert location end column number (1-based).
6360
*
64-
* A query should either perform no alert filtering, or adhere to all the filtering rules in this
65-
* module and return all and only the accepted alerts.
66-
*
67-
* This predicate is suitable for situations where we want to filter by the exact alert location,
68-
* distinguishing between alerts on the same line.
61+
* Note that an alert that is not accepted by this filtering predicate may still be included in the
62+
* query results if it is accepted by another active filtering predicate in this module. An alert is
63+
* excluded from the query results if only if (1) there is at least one active filtering predicate,
64+
* and (2) it is not accepted by any active filtering predicate.
6965
*
7066
* See also: `restrictAlertsTo`.
7167
*/
@@ -87,10 +83,15 @@ module AlertFilteringImpl<LocationSig Location> {
8783
}
8884

8985
/**
90-
* Holds if the given location intersects the diff range. Testing for full
91-
* intersection rather than only matching the start line means that this
92-
* predicate is more broadly useful than just checking whether a specific
93-
* element is considered to be in the diff range of GitHub Code Scanning:
86+
* Holds if the given location intersects the diff range. When that is the
87+
* case, ensuring that alerts mentioning this location are included in the
88+
* query results is a valid overapproximation of the requirements for
89+
* _diff-informed queries_ as documented under `restrictAlertsTo`.
90+
*
91+
* Testing for full intersection rather than only matching the start line
92+
* means that this predicate is more broadly useful than just checking whether
93+
* a specific element is considered to be in the diff range of GitHub Code
94+
* Scanning:
9495
* - If it's inconvenient to pass the exact `Location` of the element of
9596
* interest, it's valid to use a `Location` of an enclosing element.
9697
* - This predicate could be useful for other systems of alert presentation

0 commit comments

Comments
 (0)