Merge pull request github#13141 from kaspersv/kaspersv/ql-explicit-this-receivers

QL: Enable implicit this receiver warnings

Author: kaspersv

ql/ql/src/codeql/Locations.qll

@@ -25,13 +25,13 @@ class Location extends @location {
   int getEndColumn() { locations_default(this, _, _, _, _, result) }
 
   /** Gets the number of lines covered by this location. */
-  int getNumLines() { result = getEndLine() - getStartLine() + 1 }
+  int getNumLines() { result = this.getEndLine() - this.getStartLine() + 1 }
 
   /** Gets a textual representation of this element. */
   cached
   string toString() {
     exists(string filepath, int startline, int startcolumn, int endline, int endcolumn |
-      hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
+      this.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn) and
       result = filepath + "@" + startline + ":" + startcolumn + ":" + endline + ":" + endcolumn
     )
   }

ql/ql/src/codeql_ql/style/UseSetLiteralQuery.qll

@@ -16,7 +16,7 @@ class DisjunctionChain extends Disjunction {
   Formula getOperand(int i) {
     result =
       rank[i + 1](Formula operand, Location l |
-        operand = getAnOperand*() and
+        operand = this.getAnOperand*() and
         not operand instanceof Disjunction and
         l = operand.getLocation()
       |
@@ -33,16 +33,16 @@ class DisjunctionChain extends Disjunction {
  */
 class EqualsLiteral extends ComparisonFormula {
   EqualsLiteral() {
-    getOperator() = "=" and
-    getAnOperand() instanceof Literal
+    this.getOperator() = "=" and
+    this.getAnOperand() instanceof Literal
   }
 
   AstNode getOther() {
-    result = getAnOperand() and
+    result = this.getAnOperand() and
     not result instanceof Literal
   }
 
-  Literal getLiteral() { result = getAnOperand() }
+  Literal getLiteral() { result = this.getAnOperand() }
 }
 
 /**
@@ -60,29 +60,33 @@ class DisjunctionEqualsLiteral extends DisjunctionChain {
   DisjunctionEqualsLiteral() {
     // VarAccess on the same variable
     exists(VarDef v |
-      forex(Formula f | f = getOperand(_) |
+      forex(Formula f | f = this.getOperand(_) |
         f.(EqualsLiteral).getAnOperand().(VarAccess).getDeclaration() = v
       ) and
-      firstOperand = getOperand(0).(EqualsLiteral).getAnOperand() and
+      firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand() and
       firstOperand.(VarAccess).getDeclaration() = v
     )
     or
     // FieldAccess on the same variable
     exists(FieldDecl v |
-      forex(Formula f | f = getOperand(_) |
+      forex(Formula f | f = this.getOperand(_) |
         f.(EqualsLiteral).getAnOperand().(FieldAccess).getDeclaration() = v
       ) and
-      firstOperand = getOperand(0).(EqualsLiteral).getAnOperand() and
+      firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand() and
       firstOperand.(FieldAccess).getDeclaration() = v
     )
     or
     // ThisAccess
-    forex(Formula f | f = getOperand(_) | f.(EqualsLiteral).getAnOperand() instanceof ThisAccess) and
-    firstOperand = getOperand(0).(EqualsLiteral).getAnOperand().(ThisAccess)
+    forex(Formula f | f = this.getOperand(_) |
+      f.(EqualsLiteral).getAnOperand() instanceof ThisAccess
+    ) and
+    firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand().(ThisAccess)
     or
     // ResultAccess
-    forex(Formula f | f = getOperand(_) | f.(EqualsLiteral).getAnOperand() instanceof ResultAccess) and
-    firstOperand = getOperand(0).(EqualsLiteral).getAnOperand().(ResultAccess)
+    forex(Formula f | f = this.getOperand(_) |
+      f.(EqualsLiteral).getAnOperand() instanceof ResultAccess
+    ) and
+    firstOperand = this.getOperand(0).(EqualsLiteral).getAnOperand().(ResultAccess)
     // (in principle something like GlobalValueNumbering could be used to generalize this)
   }
 
@@ -100,8 +104,8 @@ class DisjunctionEqualsLiteral extends DisjunctionChain {
  */
 class CallLiteral extends Call {
   CallLiteral() {
-    getNumberOfArguments() = 1 and
-    getArgument(0) instanceof Literal
+    this.getNumberOfArguments() = 1 and
+    this.getArgument(0) instanceof Literal
   }
 }
 
@@ -118,7 +122,7 @@ class DisjunctionPredicateLiteral extends DisjunctionChain {
   DisjunctionPredicateLiteral() {
     // Call to the same target
     exists(PredicateOrBuiltin target |
-      forex(Formula f | f = getOperand(_) | f.(CallLiteral).getTarget() = target)
+      forex(Formula f | f = this.getOperand(_) | f.(CallLiteral).getTarget() = target)
     )
   }
 }

ql/ql/src/qlpack.yml

@@ -8,3 +8,4 @@ extractor: ql
 dependencies:
   codeql/typos: ${workspace}
   codeql/util: ${workspace}
+warnOnImplicitThis: true

ql/ql/test/callgraph/Foo.qll

@@ -7,7 +7,7 @@ query predicate test() { foo() }
 class Foo extends AstNode {
   predicate bar() { none() }
 
-  predicate baz() { bar() }
+  predicate baz() { this.bar() }
 }
 
 class Sub extends Foo {

ql/ql/test/callgraph/callgraph.expected

@@ -5,7 +5,7 @@ getTarget
 | Bar.qll:30:12:30:32 | MemberCall | Bar.qll:19:7:19:18 | ClassPredicate getParameter |
 | Baz.qll:8:18:8:44 | MemberCall | Baz.qll:4:10:4:24 | ClassPredicate getImportedPath |
 | Foo.qll:5:26:5:30 | PredicateCall | Foo.qll:3:11:3:13 | ClasslessPredicate foo |
-| Foo.qll:10:21:10:25 | PredicateCall | Foo.qll:8:13:8:15 | ClassPredicate bar |
+| Foo.qll:10:21:10:30 | MemberCall | Foo.qll:8:13:8:15 | ClassPredicate bar |
 | Foo.qll:14:34:14:44 | MemberCall | Foo.qll:10:13:10:15 | ClassPredicate baz |
 | Foo.qll:17:27:17:42 | MemberCall | Foo.qll:8:13:8:15 | ClassPredicate bar |
 | Foo.qll:29:5:29:16 | PredicateCall | Foo.qll:20:13:20:20 | ClasslessPredicate myThing2 |