Refactor

Author: aschackmull

ql/src/codeql_ql/ast/Ast.qll

@@ -31,6 +31,20 @@ class AstNode extends TAstNode {
     )
   }
 
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    if exists(getLocation())
+    then getLocation().hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+    else (
+      filepath = "" and
+      startline = 0 and
+      startcolumn = 0 and
+      endline = 0 and
+      endcolumn = 0
+    )
+  }
+
   /**
    * Gets the parent in the AST for this node.
    */
@@ -181,7 +195,7 @@ class Select extends TSelect, AstNode {
  * A QL predicate.
  * Either a classless predicate, a class predicate, or a characteristic predicate.
  */
-class Predicate extends TPredicate, AstNode, Declaration {
+class Predicate extends TPredicate, AstNode, PredicateOrBuiltin, Declaration {
   /**
    * Gets the body of the predicate.
    */
@@ -200,7 +214,7 @@ class Predicate extends TPredicate, AstNode, Declaration {
   /**
    * Gets the number of parameters.
    */
-  int getArity() {
+  override int getArity() {
     not this.(ClasslessPredicate).getAlias() instanceof PredicateExpr and
     result = count(getParameter(_))
     or
@@ -209,12 +223,19 @@ class Predicate extends TPredicate, AstNode, Declaration {
     )
   }
 
+  /**
+   * Holds if this predicate is private.
+   */
+  override predicate isPrivate() { hasAnnotation("private") }
+
   /**
    * Gets the return type (if any) of the predicate.
    */
   TypeExpr getReturnTypeExpr() { none() }
 
-  Type getReturnType() { result = this.getReturnTypeExpr().getResolvedType() }
+  override Type getReturnType() { result = this.getReturnTypeExpr().getResolvedType() }
+
+  override Type getParameterType(int i) { result = this.getParameter(i).getType() }
 
   override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
@@ -376,6 +397,8 @@ class ClasslessPredicate extends TClasslessPredicate, Predicate, ModuleDeclarati
     or
     pred_name = directMember("getReturnTypeExpr") and result = this.getReturnTypeExpr()
   }
+
+  override predicate isPrivate() { Predicate.super.isPrivate() }
 }
 
 /**
@@ -394,11 +417,6 @@ class ClassPredicate extends TClassPredicate, Predicate {
 
   override Class getParent() { result.getAClassPredicate() = this }
 
-  /**
-   * Holds if this predicate is private.
-   */
-  predicate isPrivate() { hasAnnotation("private") }
-
   /**
    * Holds if this predicate is annotated as overriding another predicate.
    */
@@ -416,7 +434,7 @@ class ClassPredicate extends TClassPredicate, Predicate {
   /**
    * Gets the type representing this class.
    */
-  ClassType getDeclaringType() { result.getDeclaration() = getParent() }
+  override ClassType getDeclaringType() { result.getDeclaration() = getParent() }
 
   predicate overrides(ClassPredicate other) { predOverrides(this, other) }
 
@@ -453,7 +471,7 @@ class CharPred extends TCharPred, Predicate {
     pred_name = directMember("getBody") and result = this.getBody()
   }
 
-  ClassType getDeclaringType() { result.getDeclaration() = getParent() }
+  override ClassType getDeclaringType() { result.getDeclaration() = getParent() }
 }
 
 /**
@@ -764,7 +782,7 @@ class NewType extends TNewType, TypeDeclaration, ModuleDeclaration {
  * A branch in a `newtype`.
  * E.g. `Bar()` or `Baz()` in `newtype Foo = Bar() or Baz()`.
  */
-class NewTypeBranch extends TNewTypeBranch, TypeDeclaration {
+class NewTypeBranch extends TNewTypeBranch, PredicateOrBuiltin, TypeDeclaration {
   Generated::DatatypeBranch branch;
 
   NewTypeBranch() { this = TNewTypeBranch(branch) }
@@ -786,6 +804,16 @@ class NewTypeBranch extends TNewTypeBranch, TypeDeclaration {
   /** Gets the body of this branch. */
   Formula getBody() { toGenerated(result) = branch.getChild(_).(Generated::Body).getChild() }
 
+  override NewTypeBranchType getReturnType() { result.getDeclaration() = this }
+
+  override Type getParameterType(int i) { result = this.getField(i).getType() }
+
+  override int getArity() { result = count(this.getField(_)) }
+
+  override Type getDeclaringType() { none() }
+
+  predicate isPrivate() { this.getNewType().isPrivate() }
+
   override QLDoc getQLDoc() { toGenerated(result) = branch.getChild(_) }
 
   NewType getNewType() { result.getABranch() = this }

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

@@ -1,5 +1,6 @@
 import codeql_ql.ast.Ast as AST
 import TreeSitter
+private import Builtins
 
 cached
 newtype TAstNode =
@@ -65,7 +66,11 @@ newtype TAstNode =
   TYamlEntry(Generated::YamlEntry ye) or
   TYamlKey(Generated::YamlKey yk) or
   TYamlListitem(Generated::YamlListitem yli) or
-  TYamlValue(Generated::YamlValue yv)
+  TYamlValue(Generated::YamlValue yv) or
+  TBuiltinClassless(string ret, string name, string args) { isBuiltinClassless(ret, name, args) } or
+  TBuiltinMember(string qual, string ret, string name, string args) {
+    isBuiltinMember(qual, ret, name, args)
+  }
 
 class TFormula =
   TDisjunction or TConjunction or TComparisonFormula or TQuantifier or TNegation or TIfFormula or
@@ -194,6 +199,8 @@ Generated::AstNode toGenerated(AST::AstNode n) {
 
 class TPredicate = TCharPred or TClasslessPredicate or TClassPredicate or TDBRelation;
 
+class TPredOrBuiltin = TPredicate or TNewTypeBranch or TBuiltinClassless or TBuiltinMember;
+
 class TModuleMember = TModuleDeclaration or TImport or TSelect or TQLDoc;
 
 class TDeclaration = TTypeDeclaration or TModuleDeclaration or TPredicate or TVarDecl;

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

@@ -1,12 +1,11 @@
 import ql
 private import Builtins
 private import codeql_ql.ast.internal.Module
-private import codeql_ql.ast.internal.AstNodes as AstNodes
+private import codeql_ql.ast.internal.AstNodes
 
-private class TClasslessPredicateOrNewTypeBranch =
-  AstNodes::TClasslessPredicate or AstNodes::TNewTypeBranch;
+private class TClasslessPredicateOrNewTypeBranch = TClasslessPredicate or TNewTypeBranch;
 
-string getPredicateName(TClasslessPredicateOrNewTypeBranch p) {
+private string getPredicateName(TClasslessPredicateOrNewTypeBranch p) {
   result = p.(ClasslessPredicate).getName() or
   result = p.(NewTypeBranch).getName()
 }
@@ -69,8 +68,7 @@ private module Cached {
       m = pc.getQualifier().getResolvedModule() and
       public = true
     |
-      definesPredicate(m, pc.getPredicateName(), pc.getNumberOfArguments(), p.getDeclaration(),
-        public)
+      definesPredicate(m, pc.getPredicateName(), pc.getNumberOfArguments(), p, public)
     )
   }
 
@@ -120,49 +118,13 @@ private module Cached {
     not resolvePredicateCall(c, _) and
     resolveDBRelation(c, p)
   }
-
-  cached
-  module NewTypeDef {
-    cached
-    newtype TPredOrBuiltin =
-      TPred(Predicate p) or
-      TNewTypeBranch(NewTypeBranch b) or
-      TBuiltinClassless(string ret, string name, string args) {
-        isBuiltinClassless(ret, name, args)
-      } or
-      TBuiltinMember(string qual, string ret, string name, string args) {
-        isBuiltinMember(qual, ret, name, args)
-      }
-  }
 }
 
 import Cached
-private import NewTypeDef
 
-class PredicateOrBuiltin extends TPredOrBuiltin {
+class PredicateOrBuiltin extends TPredOrBuiltin, AstNode {
   string getName() { none() }
 
-  string toString() { result = getName() }
-
-  predicate hasLocationInfo(
-    string filepath, int startline, int startcolumn, int endline, int endcolumn
-  ) {
-    if exists(getDeclaration())
-    then
-      getDeclaration()
-          .getLocation()
-          .hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
-    else (
-      filepath = "" and
-      startline = 0 and
-      startcolumn = 0 and
-      endline = 0 and
-      endcolumn = 0
-    )
-  }
-
-  AstNode getDeclaration() { none() }
-
   Type getDeclaringType() { none() }
 
   Type getParameterType(int i) { none() }
@@ -174,58 +136,14 @@ class PredicateOrBuiltin extends TPredOrBuiltin {
   predicate isPrivate() { none() }
 }
 
-private class DefinedPredicate extends PredicateOrBuiltin, TPred {
-  Predicate decl;
-
-  DefinedPredicate() { this = TPred(decl) }
-
-  override Predicate getDeclaration() { result = decl }
-
-  override string getName() { result = decl.getName() }
-
-  override Type getReturnType() { result = decl.getReturnType() }
-
-  override Type getParameterType(int i) { result = decl.getParameter(i).getType() }
-
-  // Can be removed when all types can be resolved
-  override int getArity() { result = decl.getArity() }
-
-  override Type getDeclaringType() {
-    result = decl.(ClassPredicate).getDeclaringType()
-    or
-    result = decl.(CharPred).getDeclaringType()
-  }
-
-  override predicate isPrivate() {
-    decl.(ClassPredicate).isPrivate() or decl.(ClassPredicate).isPrivate()
-  }
-}
-
-private class DefinedNewTypeBranch extends PredicateOrBuiltin, TNewTypeBranch {
-  NewTypeBranch b;
-
-  DefinedNewTypeBranch() { this = TNewTypeBranch(b) }
-
-  override NewTypeBranch getDeclaration() { result = b }
-
-  override string getName() { result = b.getName() }
-
-  override NewTypeBranchType getReturnType() { result.getDeclaration() = b }
-
-  override Type getParameterType(int i) { result = b.getField(i).getType() }
-
-  // Can be removed when all types can be resolved
-  override int getArity() { result = count(b.getField(_)) }
+private class TBuiltin = TBuiltinClassless or TBuiltinMember;
 
-  override Type getDeclaringType() { none() }
+class BuiltinPredicate extends PredicateOrBuiltin, TBuiltin {
+  override string toString() { result = getName() }
 
-  override predicate isPrivate() { b.getNewType().isPrivate() }
+  override string getAPrimaryQlClass() { result = "BuiltinPredicate" }
 }
 
-private class TBuiltin = TBuiltinClassless or TBuiltinMember;
-
-class BuiltinPredicate extends PredicateOrBuiltin, TBuiltin { }
-
 private class BuiltinClassless extends BuiltinPredicate, TBuiltinClassless {
   string name;
   string ret;
@@ -287,7 +205,7 @@ module PredConsistency {
       strictcount(PredicateOrBuiltin p0 |
         resolveCall(call, p0) and
         // aliases are expected to resolve to multiple.
-        not exists(p0.getDeclaration().(ClasslessPredicate).getAlias())
+        not exists(p0.(ClasslessPredicate).getAlias())
       ) and
     c > 1 and
     resolveCall(call, p)

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

@@ -127,8 +127,7 @@ private PredicateOrBuiltin inherClassPredCandidate(Type ty, string name, int ari
 }
 
 predicate predOverrides(ClassPredicate sub, ClassPredicate sup) {
-  sup =
-    inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity()).getDeclaration()
+  sup = inherClassPredCandidate(sub.getDeclaringType(), sub.getName(), sub.getArity())
 }
 
 private VarDecl declaredField(ClassType ty, string name) {