Merge pull request #13521 from hvitved/ql/final-extends

QL: Model `final extends`

Author: hvitved

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

@@ -611,6 +611,8 @@ class ClassPredicate extends TClassPredicate, Predicate {
 
   predicate overrides(ClassPredicate other) { predOverrides(this, other) }
 
+  predicate shadows(ClassPredicate other) { predShadows(this, other) }
+
   override TypeExpr getReturnTypeExpr() { toQL(result) = pred.getReturnType() }
 
   override AstNode getAChild(string pred_name) {
@@ -878,6 +880,9 @@ class Module extends TModule, ModuleDeclaration {
 class ModuleMember extends TModuleMember, AstNode {
   /** Holds if this member is declared as `private`. */
   predicate isPrivate() { this.hasAnnotation("private") }
+
+  /** Holds if this member is declared as `final`. */
+  predicate isFinal() { this.hasAnnotation("final") }
 }
 
 /** A declaration. E.g. a class, type, predicate, newtype... */

ql/ql/src/codeql_ql/ast/internal/Type.qll

@@ -21,7 +21,7 @@ private newtype TType =
 private predicate primTypeName(string s) { s = ["int", "float", "string", "boolean", "date"] }
 
 private predicate isActualClass(Class c) {
-  not exists(c.getAliasType()) and
+  (not exists(c.getAliasType()) or c.isFinal()) and
   not exists(c.getUnionMember())
 }
 
@@ -36,6 +36,10 @@ class Type extends TType {
   /**
    * Gets a supertype of this type. This follows the user-visible type hierarchy,
    * and doesn't include internal types like the characteristic and domain types of classes.
+   *
+   * For supertypes that are `final` aliases, this returns the alias itself, and for
+   * types that are `final` aliases, this returns the supertypes of the type that is
+   * being aliased.
    */
   Type getASuperType() { none() }
 
@@ -94,9 +98,23 @@ class ClassType extends Type, TClass {
 
   override Class getDeclaration() { result = decl }
 
-  override Type getASuperType() { result = decl.getASuperType().getResolvedType() }
+  override Type getASuperType() {
+    result = decl.getASuperType().getResolvedType()
+    or
+    exists(ClassType alias |
+      this.isFinalAlias(alias) and
+      result = alias.getASuperType()
+    )
+  }
 
-  Type getAnInstanceofType() { result = decl.getAnInstanceofType().getResolvedType() }
+  Type getAnInstanceofType() {
+    result = decl.getAnInstanceofType().getResolvedType()
+    or
+    exists(ClassType alias |
+      this.isFinalAlias(alias) and
+      result = alias.getAnInstanceofType()
+    )
+  }
 
   override Type getAnInternalSuperType() {
     result.(ClassCharType).getClassType() = this
@@ -110,6 +128,12 @@ class ClassType extends Type, TClass {
       other.getDeclaringType().getASuperType+() = result.getDeclaringType()
     )
   }
+
+  /** Holds if this class is a `final` alias of `c`. */
+  predicate isFinalAlias(ClassType c) {
+    decl.isFinal() and
+    decl.getAliasType().getResolvedType() = c
+  }
 }
 
 class FileType extends Type, TFile {
@@ -136,23 +160,37 @@ private PredicateOrBuiltin declaredPred(Type ty, string name, int arity) {
   result.getDeclaringType() = ty and
   result.getName() = name and
   result.getArity() = arity
+  or
+  exists(ClassType alias |
+    ty.(ClassType).isFinalAlias(alias) and
+    result = declaredPred(alias, name, arity)
+  )
 }
 
 pragma[nomagic]
-private PredicateOrBuiltin classPredCandidate(Type ty, string name, int arity) {
-  result = declaredPred(ty, name, arity)
+private PredicateOrBuiltin classPredCandidate(Type ty, string name, int arity, boolean isFinal) {
+  result = declaredPred(ty, name, arity) and
+  if ty.(ClassType).getDeclaration().isFinal() then isFinal = true else isFinal = false
   or
   not exists(declaredPred(ty, name, arity)) and
-  result = inherClassPredCandidate(ty, name, arity)
+  result = inherClassPredCandidate(ty, name, arity, isFinal)
 }
 
-private PredicateOrBuiltin inherClassPredCandidate(Type ty, string name, int arity) {
-  result = classPredCandidate(ty.getAnInternalSuperType(), name, arity) and
+private PredicateOrBuiltin classPredCandidate(Type ty, string name, int arity) {
+  result = classPredCandidate(ty, name, arity, _)
+}
+
+private PredicateOrBuiltin inherClassPredCandidate(Type ty, string name, int arity, boolean isFinal) {
+  result = classPredCandidate(ty.getAnInternalSuperType(), name, arity, isFinal) and
   not result.isPrivate()
 }
 
 predicate predOverrides(ClassPredicate sub, ClassPredicate sup) {
-  sup = inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity())
+  sup = inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity(), false)
+}
+
+predicate predShadows(ClassPredicate sub, ClassPredicate sup) {
+  sup = inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity(), true)
 }
 
 private VarDecl declaredField(ClassType ty, string name) {
@@ -376,7 +414,8 @@ private predicate defines(FileOrModule m, string name, Type t, boolean public) {
   exists(Class ty | t = ty.getAliasType().getResolvedType() |
     getEnclosingModule(ty) = m and
     ty.getName() = name and
-    public = getPublicBool(ty)
+    public = getPublicBool(ty) and
+    not ty.isFinal()
   )
   or
   exists(Import im |

ql/ql/src/queries/performance/AbstractClassImport.ql

@@ -38,14 +38,15 @@ Class getASubclassOfAbstract(Class ab) {
 
 /** Gets a non-abstract subclass of `ab` that contributes to the extent of `ab`. */
 Class concreteExternalSubclass(Class ab) {
-  ab.isAbstract() and
   not result.isAbstract() and
   result = getASubclassOfAbstract+(ab) and
   // Heuristic: An abstract class with subclasses in the same file and no other
   // imported subclasses is likely intentional.
   result.getLocation().getFile() != ab.getLocation().getFile() and
   // Exclude subclasses in tests and libraries that are only used in tests.
-  liveNonTestFile(result.getLocation().getFile())
+  liveNonTestFile(result.getLocation().getFile()) and
+  // exclude `final` aliases
+  not result.getType().isFinalAlias(_)
 }
 
 /** Holds if there is a bidirectional import between the abstract class `ab` and its subclass `sub` */

ql/ql/test/queries/performance/AbstractClassImport/AbstractClassImport.expected

@@ -0,0 +1,4 @@
+| AbstractClassImportTest1.qll:4:16:4:19 | Class Base | This abstract class doesn't import its subclass $@ but imports 2 other subclasses, such as $@. | AbstractClassImportTest3.qll:4:7:4:11 | Class Sub31 | Sub31 | AbstractClassImportTest2.qll:4:7:4:11 | Class Sub21 | Sub21 |
+| AbstractClassImportTest1.qll:4:16:4:19 | Class Base | This abstract class doesn't import its subclass $@ but imports 2 other subclasses, such as $@. | AbstractClassImportTest3.qll:8:7:8:11 | Class Sub32 | Sub32 | AbstractClassImportTest2.qll:4:7:4:11 | Class Sub21 | Sub21 |
+| AbstractClassImportTest1.qll:4:16:4:19 | Class Base | This abstract class imports its subclass $@ but doesn't import 2 other subclasses, such as $@. | AbstractClassImportTest2.qll:4:7:4:11 | Class Sub21 | Sub21 | AbstractClassImportTest3.qll:4:7:4:11 | Class Sub31 | Sub31 |
+| AbstractClassImportTest1.qll:4:16:4:19 | Class Base | This abstract class imports its subclass $@ but doesn't import 2 other subclasses, such as $@. | AbstractClassImportTest2.qll:8:7:8:11 | Class Sub22 | Sub22 | AbstractClassImportTest3.qll:4:7:4:11 | Class Sub31 | Sub31 |

ql/ql/test/queries/performance/AbstractClassImport/AbstractClassImport.qlref

@@ -0,0 +1 @@
+queries/performance/AbstractClassImport.ql

ql/ql/test/queries/performance/AbstractClassImport/AbstractClassImportTest1.qll

@@ -0,0 +1,4 @@
+import ql
+import AbstractClassImportTest2
+
+abstract class Base extends AstNode { }

ql/ql/test/queries/performance/AbstractClassImport/AbstractClassImportTest2.qll

@@ -0,0 +1,8 @@
+import ql
+import AbstractClassImportTest1
+
+class Sub21 extends Base {
+  Sub21() { this instanceof TopLevel }
+}
+
+class Sub22 extends Base instanceof Comment { }

ql/ql/test/queries/performance/AbstractClassImport/AbstractClassImportTest3.qll

@@ -0,0 +1,18 @@
+import ql
+import AbstractClassImportTest1
+
+class Sub31 extends Base {
+  Sub31() { this instanceof Comment }
+}
+
+class Sub32 extends Base instanceof Comment { }
+
+final class BaseFinal = Base;
+
+class Sub33 extends BaseFinal instanceof Comment { }
+
+abstract class Sub34 extends Base { }
+
+final class Sub34Final = Sub34;
+
+class Sub35 extends Sub34Final instanceof Comment { }

ql/ql/test/queries/performance/AbstractClassImport/AbstractClassImportTestQuery.expected

@@ -0,0 +1,6 @@
+import ql
+import AbstractClassImportTest3
+
+from AstNode n
+where none()
+select n