don't require a member to call a range method before suggesting to use instanceof

Author: erik-krogh

ql/ql/src/codeql_ql/style/UseInstanceofExtensionQuery.qll

@@ -17,19 +17,6 @@ predicate instanceofThisInCharPred(Class c, Type type) {
   )
 }
 
-/**
- * Holds if `c` uses the casting based range pattern, which could be replaced with `instanceof type`.
- */
-predicate usesCastingBasedInstanceof(Class c, Type type) {
-  instanceofThisInCharPred(c, type) and
-  // require that there is a call to the range class that matches the name of the enclosing predicate
-  exists(InlineCast cast, MemberCall call |
-    cast = getAThisCast(c, type) and
-    call.getBase() = cast and
-    cast.getEnclosingPredicate().getName() = call.getMemberName()
-  )
-}
-
 /** Gets an inline cast that cases `this` to `type` inside a class predicate for `c`. */
 InlineCast getAThisCast(Class c, Type type) {
   exists(MemberCall call |
@@ -51,12 +38,6 @@ predicate usesFieldBasedInstanceof(Class c, TypeExpr type, FieldDecl field, Comp
     comp.getAnOperand() = fieldAccess and
     fieldAccess.getDeclaration() = field and
     field.getVarDecl().getTypeExpr() = type
-  ) and
-  // require that there is a call to the range field that matches the name of the enclosing predicate
-  exists(FieldAccess access, MemberCall call |
-    access = getARangeFieldAccess(c, field, _) and
-    call.getBase() = access and
-    access.getEnclosingPredicate().getName() = call.getMemberName()
   )
 }
 

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

@@ -14,7 +14,7 @@ import codeql_ql.style.UseInstanceofExtensionQuery
 from Class c, Type type, string message
 where
   (
-    usesCastingBasedInstanceof(c, type) or
+    instanceofThisInCharPred(c, type) or
     usesFieldBasedInstanceof(c, any(TypeExpr te | te.getResolvedType() = type), _, _)
   ) and
   message = "Consider defining this class as non-extending subtype of $@."

ql/ql/test/queries/style/UseInstanceofExtension/Foo.qll

@@ -0,0 +1,29 @@
+class Range extends string {
+  Range() { this = "ql" }
+
+  string getAChild() { result = "test" }
+}
+
+class Inst extends string {
+  Range range;
+
+  Inst() { this = range }
+
+  string getAChild() { result = range.getAChild() }
+}
+
+class Inst2 extends string {
+  Inst2() { this instanceof Range }
+
+  string getAChild() { result = this.(Range).getAChild() }
+}
+
+class Inst3 extends string {
+  Range range;
+
+  Inst3() { this = range }
+}
+
+class Inst4 extends string {
+  Inst4() { this instanceof Range }
+}

ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.expected

@@ -0,0 +1,4 @@
+| Foo.qll:7:7:7:10 | Class Inst | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range |
+| Foo.qll:15:7:15:11 | Class Inst2 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range |
+| Foo.qll:21:7:21:11 | Class Inst3 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range |
+| Foo.qll:27:7:27:11 | Class Inst4 | Consider defining this class as non-extending subtype of $@. | Foo.qll:1:7:1:11 | Class Range | Range |

ql/ql/test/queries/style/UseInstanceofExtension/UseInstanceofExtension.qlref

@@ -0,0 +1 @@
+queries/style/UseInstanceofExtension.ql