Skip to content

Commit e092cb0

Browse files
erik-krogherik-krogh
authored
Merge pull request #8937 from erik-krogh/qlFocusedLocations
QL: more precise alert locations
2 parents c18428f + 73b657c commit e092cb0

File tree

11 files changed

+136
-117
lines changed

11 files changed

+136
-117
lines changed

‎ql/ql/src/codeql_ql/ast/Ast.qll

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,13 @@ private string stringIndexedMember(string name, string index) {
2121
class AstNode extends TAstNode {
2222
string toString() { result = this.getAPrimaryQlClass() }
2323

24-
/**
25-
* Gets the location of the AST node.
26-
*/
24+
/** Gets the location of the AST node. */
2725
cached
28-
Location getLocation() {
26+
Location getLocation() { result = this.getFullLocation() } // overriden in some subclasses
27+
28+
/** Gets the location that spans the entire AST node. */
29+
cached
30+
final Location getFullLocation() {
2931
exists(QL::AstNode node | not node instanceof QL::ParExpr |
3032
node = toQL(this) and
3133
result = node.getLocation()
@@ -434,6 +436,8 @@ class ClasslessPredicate extends TClasslessPredicate, Predicate, ModuleDeclarati
434436

435437
ClasslessPredicate() { this = TClasslessPredicate(pred) }
436438

439+
override Location getLocation() { result = pred.getName().getLocation() }
440+
437441
/**
438442
* Gets the aliased value if this predicate is an alias
439443
* E.g. for `predicate foo = Module::bar/2;` gets `Module::bar/2`.
@@ -484,6 +488,8 @@ class ClassPredicate extends TClassPredicate, Predicate {
484488

485489
ClassPredicate() { this = TClassPredicate(pred) }
486490

491+
override Location getLocation() { result = pred.getName().getLocation() }
492+
487493
override string getName() { result = pred.getName().getValue() }
488494

489495
override Formula getBody() { toQL(result) = pred.getChild(_).(QL::Body).getChild() }
@@ -701,6 +707,8 @@ class Module extends TModule, ModuleDeclaration {
701707

702708
Module() { this = TModule(mod) }
703709

710+
override Location getLocation() { result = mod.getName().getLocation() }
711+
704712
override string getAPrimaryQlClass() { result = "Module" }
705713

706714
override string getName() { result = mod.getName().getChild().getValue() }
@@ -784,6 +792,8 @@ class Class extends TClass, TypeDeclaration, ModuleDeclaration {
784792

785793
Class() { this = TClass(cls) }
786794

795+
override Location getLocation() { result = cls.getName().getLocation() }
796+
787797
override string getAPrimaryQlClass() { result = "Class" }
788798

789799
override string getName() { result = cls.getName().getValue() }
@@ -871,6 +881,8 @@ class NewType extends TNewType, TypeDeclaration, ModuleDeclaration {
871881

872882
NewType() { this = TNewType(type) }
873883

884+
override Location getLocation() { result = type.getName().getLocation() }
885+
874886
override string getName() { result = type.getName().getValue() }
875887

876888
override string getAPrimaryQlClass() { result = "NewType" }
@@ -896,6 +908,8 @@ class NewTypeBranch extends TNewTypeBranch, Predicate, TypeDeclaration {
896908

897909
NewTypeBranch() { this = TNewTypeBranch(branch) }
898910

911+
override Location getLocation() { result = branch.getName().getLocation() }
912+
899913
override string getAPrimaryQlClass() { result = "NewTypeBranch" }
900914

901915
override string getName() { result = branch.getName().getValue() }

‎ql/ql/src/queries/performance/VarUnusedInDisjunct.ql

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,12 +119,14 @@ class EffectiveDisjunction extends AstNode {
119119
* Holds if `disj` only uses `var` in one of its branches.
120120
*/
121121
pragma[noinline]
122-
predicate onlyUseInOneBranch(EffectiveDisjunction disj, VarDef var) {
122+
predicate onlyUseInOneBranch(EffectiveDisjunction disj, VarDef var, AstNode notBoundIn) {
123123
alwaysBindsVar(var, disj.getLeft()) and
124-
not alwaysBindsVar(var, disj.getRight())
124+
not alwaysBindsVar(var, disj.getRight()) and
125+
notBoundIn = disj.getRight()
125126
or
126127
not alwaysBindsVar(var, disj.getLeft()) and
127-
alwaysBindsVar(var, disj.getRight())
128+
alwaysBindsVar(var, disj.getRight()) and
129+
notBoundIn = disj.getLeft()
128130
}
129131

130132
/**
@@ -170,7 +172,7 @@ class EffectiveConjunction extends AstNode {
170172
predicate varIsAlwaysBound(VarDef var, AstNode node) {
171173
// base case
172174
alwaysBindsVar(var, node) and
173-
onlyUseInOneBranch(_, var) // <- manual magic
175+
onlyUseInOneBranch(_, var, _) // <- manual magic
174176
or
175177
// recursive cases
176178
exists(AstNode parent | node.getParent() = parent | varIsAlwaysBound(var, parent))
@@ -194,8 +196,8 @@ predicate varIsAlwaysBound(VarDef var, AstNode node) {
194196
* Holds if `disj` only uses `var` in one of its branches.
195197
* And we should report it as being a bad thing.
196198
*/
197-
predicate badDisjunction(EffectiveDisjunction disj, VarDef var) {
198-
onlyUseInOneBranch(disj, var) and
199+
predicate badDisjunction(EffectiveDisjunction disj, VarDef var, AstNode notBoundIn) {
200+
onlyUseInOneBranch(disj, var, notBoundIn) and
199201
// it's fine if it's always bound further up
200202
not varIsAlwaysBound(var, disj) and
201203
// none() on one side makes everything fine. (this happens, it's a type-system hack)
@@ -213,9 +215,9 @@ predicate badDisjunction(EffectiveDisjunction disj, VarDef var) {
213215
not isTinyAssignment(disj.getAnOperand())
214216
}
215217

216-
from EffectiveDisjunction disj, VarDef var, string type
218+
from EffectiveDisjunction disj, VarDef var, AstNode notBoundIn, string type
217219
where
218-
badDisjunction(disj, var) and
219-
not badDisjunction(disj.getParent(), var) and // avoid duplicate reporting of the same error
220+
badDisjunction(disj, var, notBoundIn) and
221+
not badDisjunction(disj.getParent(), var, _) and // avoid duplicate reporting of the same error
220222
if var.getParent() instanceof FieldDecl then type = "field" else type = "variable"
221223
select disj, "The $@ is only used in one side of disjunct.", var, type + " " + var.getName()

‎ql/ql/src/queries/style/LibraryAnnotation.ql

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@
1010

1111
import ql
1212

13-
from AstNode n
14-
where n.hasAnnotation("library") and not n.hasAnnotation("deprecated")
13+
from AstNode n, Annotation library
14+
where
15+
library = n.getAnAnnotation() and
16+
library.getName() = "library" and
17+
not n.hasAnnotation("deprecated")
1518
select n, "Don't use the library annotation."

‎ql/ql/src/queries/style/UseInstanceofExtension.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,4 +18,4 @@ where
1818
usesFieldBasedInstanceof(c, any(TypeExpr te | te.getResolvedType() = type), _, _)
1919
) and
2020
message = "consider defining $@ as non-extending subtype of $@"
21-
select c, message, c, c.getName(), type, type.getName()
21+
select c, message, c, c.getName(), type.getDeclaration(), type.getName()

‎ql/ql/test/callgraph/callgraph.expected

Lines changed: 40 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,47 @@
11
getTarget
2-
| Bar.qll:5:38:5:47 | PredicateCall | Bar.qll:8:3:8:31 | ClasslessPredicate snapshot |
3-
| Bar.qll:24:12:24:32 | MemberCall | Bar.qll:19:3:19:47 | ClassPredicate getParameter |
4-
| Bar.qll:26:12:26:31 | MemberCall | Bar.qll:19:3:19:47 | ClassPredicate getParameter |
5-
| Bar.qll:30:12:30:32 | MemberCall | Bar.qll:19:3:19:47 | ClassPredicate getParameter |
6-
| Baz.qll:8:18:8:44 | MemberCall | Baz.qll:4:3:4:37 | ClassPredicate getImportedPath |
7-
| Foo.qll:5:26:5:30 | PredicateCall | Foo.qll:3:1:3:26 | ClasslessPredicate foo |
8-
| Foo.qll:10:21:10:25 | PredicateCall | Foo.qll:8:3:8:28 | ClassPredicate bar |
9-
| Foo.qll:14:30:14:40 | MemberCall | Foo.qll:10:3:10:27 | ClassPredicate baz |
10-
| Foo.qll:17:27:17:42 | MemberCall | Foo.qll:8:3:8:28 | ClassPredicate bar |
11-
| Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:20:3:20:54 | ClasslessPredicate myThing2 |
12-
| Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:26:3:26:32 | ClasslessPredicate alias2 |
13-
| Foo.qll:31:5:31:12 | PredicateCall | Foo.qll:22:3:22:32 | ClasslessPredicate myThing0 |
14-
| Foo.qll:31:5:31:12 | PredicateCall | Foo.qll:24:3:24:32 | ClasslessPredicate alias0 |
2+
| Bar.qll:5:38:5:47 | PredicateCall | Bar.qll:8:7:8:14 | ClasslessPredicate snapshot |
3+
| Bar.qll:24:12:24:32 | MemberCall | Bar.qll:19:7:19:18 | ClassPredicate getParameter |
4+
| Bar.qll:26:12:26:31 | MemberCall | Bar.qll:19:7:19:18 | ClassPredicate getParameter |
5+
| Bar.qll:30:12:30:32 | MemberCall | Bar.qll:19:7:19:18 | ClassPredicate getParameter |
6+
| Baz.qll:8:18:8:44 | MemberCall | Baz.qll:4:10:4:24 | ClassPredicate getImportedPath |
7+
| Foo.qll:5:26:5:30 | PredicateCall | Foo.qll:3:11:3:13 | ClasslessPredicate foo |
8+
| Foo.qll:10:21:10:25 | PredicateCall | Foo.qll:8:13:8:15 | ClassPredicate bar |
9+
| Foo.qll:14:30:14:40 | MemberCall | Foo.qll:10:13:10:15 | ClassPredicate baz |
10+
| Foo.qll:17:27:17:42 | MemberCall | Foo.qll:8:13:8:15 | ClassPredicate bar |
11+
| Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:20:13:20:20 | ClasslessPredicate myThing2 |
12+
| Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:26:13:26:18 | ClasslessPredicate alias2 |
13+
| Foo.qll:31:5:31:12 | PredicateCall | Foo.qll:22:13:22:20 | ClasslessPredicate myThing0 |
14+
| Foo.qll:31:5:31:12 | PredicateCall | Foo.qll:24:13:24:18 | ClasslessPredicate alias0 |
1515
| Foo.qll:36:36:36:65 | MemberCall | file://:0:0:0:0 | replaceAll |
1616
| Foo.qll:38:39:38:67 | MemberCall | file://:0:0:0:0 | regexpCapture |
17-
| Overrides.qll:8:30:8:39 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
18-
| Overrides.qll:16:39:16:48 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
19-
| Overrides.qll:16:39:16:48 | MemberCall | Overrides.qll:14:12:14:43 | ClassPredicate bar |
20-
| Overrides.qll:24:39:24:48 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
21-
| Overrides.qll:24:39:24:48 | MemberCall | Overrides.qll:22:12:22:44 | ClassPredicate bar |
22-
| Overrides.qll:28:3:28:9 | MemberCall | Overrides.qll:6:3:6:29 | ClassPredicate bar |
23-
| Overrides.qll:29:3:29:10 | MemberCall | Overrides.qll:8:3:8:41 | ClassPredicate baz |
24-
| ParamModules.qll:5:28:5:41 | PredicateCall | ParamModules.qll:2:13:2:36 | ClasslessPredicate fooSig |
25-
| ParamModules.qll:5:28:5:41 | PredicateCall | ParamModules.qll:8:3:8:35 | ClasslessPredicate myFoo |
26-
| ParamModules.qll:10:26:10:49 | PredicateCall | ParamModules.qll:5:5:5:43 | ClasslessPredicate bar |
27-
| ParamModules.qll:26:27:26:53 | PredicateCall | ParamModules.qll:17:5:17:42 | ClasslessPredicate getAnEven |
28-
| ParamModules.qll:26:27:26:61 | MemberCall | ParamModules.qll:23:5:23:39 | ClassPredicate myFoo |
29-
| ParamModules.qll:37:29:37:47 | PredicateCall | ParamModules.qll:33:5:33:17 | ClasslessPredicate getThing |
30-
| ParamModules.qll:37:29:37:47 | PredicateCall | ParamModules.qll:51:5:51:26 | ClasslessPredicate getThing |
31-
| ParamModules.qll:59:30:59:45 | PredicateCall | ParamModules.qll:37:5:37:49 | ClasslessPredicate getThing |
32-
| ParamModules.qll:59:30:59:53 | MemberCall | ParamModules.qll:46:7:46:41 | ClassPredicate myFoo |
33-
| ParamModules.qll:61:39:61:47 | MemberCall | ParamModules.qll:46:7:46:41 | ClassPredicate myFoo |
34-
| packs/other/OtherThing.qll:5:3:5:8 | PredicateCall | packs/lib/LibThing/Foo.qll:1:1:1:30 | ClasslessPredicate foo |
35-
| packs/other/OtherThing.qll:6:3:6:8 | PredicateCall | packs/src/SrcThing.qll:8:1:8:30 | ClasslessPredicate bar |
36-
| packs/src/SrcThing.qll:4:3:4:8 | PredicateCall | packs/lib/LibThing/Foo.qll:1:1:1:30 | ClasslessPredicate foo |
37-
| packs/src/SrcThing.qll:5:3:5:8 | PredicateCall | packs/src/SrcThing.qll:8:1:8:30 | ClasslessPredicate bar |
17+
| Overrides.qll:8:30:8:39 | MemberCall | Overrides.qll:6:7:6:9 | ClassPredicate bar |
18+
| Overrides.qll:16:39:16:48 | MemberCall | Overrides.qll:6:7:6:9 | ClassPredicate bar |
19+
| Overrides.qll:16:39:16:48 | MemberCall | Overrides.qll:14:16:14:18 | ClassPredicate bar |
20+
| Overrides.qll:24:39:24:48 | MemberCall | Overrides.qll:6:7:6:9 | ClassPredicate bar |
21+
| Overrides.qll:24:39:24:48 | MemberCall | Overrides.qll:22:16:22:18 | ClassPredicate bar |
22+
| Overrides.qll:28:3:28:9 | MemberCall | Overrides.qll:6:7:6:9 | ClassPredicate bar |
23+
| Overrides.qll:29:3:29:10 | MemberCall | Overrides.qll:8:13:8:15 | ClassPredicate baz |
24+
| ParamModules.qll:5:28:5:41 | PredicateCall | ParamModules.qll:2:23:2:28 | ClasslessPredicate fooSig |
25+
| ParamModules.qll:5:28:5:41 | PredicateCall | ParamModules.qll:8:13:8:17 | ClasslessPredicate myFoo |
26+
| ParamModules.qll:10:26:10:49 | PredicateCall | ParamModules.qll:5:15:5:17 | ClasslessPredicate bar |
27+
| ParamModules.qll:26:27:26:53 | PredicateCall | ParamModules.qll:17:13:17:21 | ClasslessPredicate getAnEven |
28+
| ParamModules.qll:26:27:26:61 | MemberCall | ParamModules.qll:23:12:23:16 | ClassPredicate myFoo |
29+
| ParamModules.qll:37:29:37:47 | PredicateCall | ParamModules.qll:33:7:33:14 | ClasslessPredicate getThing |
30+
| ParamModules.qll:37:29:37:47 | PredicateCall | ParamModules.qll:51:7:51:14 | ClasslessPredicate getThing |
31+
| ParamModules.qll:59:30:59:45 | PredicateCall | ParamModules.qll:37:7:37:14 | ClasslessPredicate getThing |
32+
| ParamModules.qll:59:30:59:53 | MemberCall | ParamModules.qll:46:14:46:18 | ClassPredicate myFoo |
33+
| ParamModules.qll:61:39:61:47 | MemberCall | ParamModules.qll:46:14:46:18 | ClassPredicate myFoo |
34+
| packs/other/OtherThing.qll:5:3:5:8 | PredicateCall | packs/lib/LibThing/Foo.qll:1:11:1:13 | ClasslessPredicate foo |
35+
| packs/other/OtherThing.qll:6:3:6:8 | PredicateCall | packs/src/SrcThing.qll:8:11:8:13 | ClasslessPredicate bar |
36+
| packs/src/SrcThing.qll:4:3:4:8 | PredicateCall | packs/lib/LibThing/Foo.qll:1:11:1:13 | ClasslessPredicate foo |
37+
| packs/src/SrcThing.qll:5:3:5:8 | PredicateCall | packs/src/SrcThing.qll:8:11:8:13 | ClasslessPredicate bar |
3838
dependsOn
3939
| packs/other/qlpack.yml:1:1:1:4 | ql-other-pack-thing | packs/src/qlpack.yml:1:1:1:4 | ql-testing-src-pack |
4040
| packs/src/qlpack.yml:1:1:1:4 | ql-testing-src-pack | packs/lib/qlpack.yml:1:1:1:4 | ql-testing-lib-pack |
4141
exprPredicate
42-
| Foo.qll:24:22:24:31 | predicate | Foo.qll:22:3:22:32 | ClasslessPredicate myThing0 |
43-
| Foo.qll:26:22:26:31 | predicate | Foo.qll:20:3:20:54 | ClasslessPredicate myThing2 |
44-
| Foo.qll:47:55:47:62 | predicate | Foo.qll:42:20:42:27 | NewTypeBranch MkRoot |
45-
| Foo.qll:47:65:47:70 | predicate | Foo.qll:44:9:44:56 | ClasslessPredicate edge |
46-
| ParamModules.qll:4:18:4:25 | predicate | ParamModules.qll:2:13:2:36 | ClasslessPredicate fooSig |
47-
| ParamModules.qll:10:34:10:40 | predicate | ParamModules.qll:8:3:8:35 | ClasslessPredicate myFoo |
42+
| Foo.qll:24:22:24:31 | predicate | Foo.qll:22:13:22:20 | ClasslessPredicate myThing0 |
43+
| Foo.qll:26:22:26:31 | predicate | Foo.qll:20:13:20:20 | ClasslessPredicate myThing2 |
44+
| Foo.qll:47:55:47:62 | predicate | Foo.qll:42:20:42:25 | NewTypeBranch MkRoot |
45+
| Foo.qll:47:65:47:70 | predicate | Foo.qll:44:19:44:22 | ClasslessPredicate edge |
46+
| ParamModules.qll:4:18:4:25 | predicate | ParamModules.qll:2:23:2:28 | ClasslessPredicate fooSig |
47+
| ParamModules.qll:10:34:10:40 | predicate | ParamModules.qll:8:13:8:17 | ClasslessPredicate myFoo |

0 commit comments

Comments
 (0)