Merge pull request #130 from github/erik-krogh/more-types

Better type resolution

Author: erik-krogh

ql/consistency-queries/AstConsistency.ql

@@ -1,10 +1 @@
-import ql
-private import codeql_ql.ast.internal.AstNodes as AstNodes
-
-query AstNode nonTotalGetParent() {
-  exists(AstNodes::toQL(result).getParent()) and
-  not exists(result.getParent()) and
-  not result.getLocation().getStartColumn() = 1 and // startcolumn = 1 <=> top level in file <=> fine to have no parent
-  not result instanceof YAML::YAMLNode and // parents for YAML doens't work
-  not (result instanceof QLDoc and result.getLocation().getFile().getExtension() = "dbscheme") // qldoc in dbschemes are not hooked up
-}
+private import codeql_ql.ast.internal.AstNodes::AstConsistency

ql/src/codeql_ql/ast/Ast.qll

@@ -819,7 +819,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, PredicateOrBuiltin, TypeDeclaration {
+class NewTypeBranch extends TNewTypeBranch, Predicate, TypeDeclaration {
   QL::DatatypeBranch branch;
 
   NewTypeBranch() { this = TNewTypeBranch(branch) }
@@ -835,7 +835,7 @@ class NewTypeBranch extends TNewTypeBranch, PredicateOrBuiltin, TypeDeclaration
   }
 
   /** Gets the body of this branch. */
-  Formula getBody() { toQL(result) = branch.getChild(_).(QL::Body).getChild() }
+  override Formula getBody() { toQL(result) = branch.getChild(_).(QL::Body).getChild() }
 
   override NewTypeBranchType getReturnType() { result.getDeclaration() = this }
 
@@ -1551,7 +1551,7 @@ class ExprAggregate extends TExprAggregate, Aggregate {
 
   override Type getType() {
     exists(PrimitiveType prim | prim = result |
-      kind.regexpMatch("(strict)?count|sum|min|max|rank") and
+      kind.regexpMatch("(strict)?count") and
       result.getName() = "int"
       or
       kind.regexpMatch("(strict)?concat") and
@@ -1615,16 +1615,16 @@ class FullAggregate extends TFullAggregate, Aggregate {
 
   override Type getType() {
     exists(PrimitiveType prim | prim = result |
-      kind.regexpMatch("(strict)?(count|sum|min|max|rank)") and
+      kind = ["count", "strictcount"] and
       result.getName() = "int"
       or
       kind.regexpMatch("(strict)?concat") and
       result.getName() = "string"
     )
     or
-    kind = ["any", "min", "max", "unique"] and
+    kind = ["any", "min", "max", "unique", "rank", "sum", "strictsum"] and
     not exists(this.getExpr(_)) and
-    result = this.getArgument(0).getTypeExpr().getResolvedType()
+    result = this.getArgument(0).getType()
     or
     not kind = ["count", "strictcount"] and
     result = this.getExpr(0).getType()
@@ -1761,6 +1761,10 @@ class Super extends TSuper, Expr {
   Super() { this = TSuper(_) }
 
   override string getAPrimaryQlClass() { result = "Super" }
+
+  override Type getType() {
+    exists(MemberCall call | call.getBase() = this | result = call.getTarget().getDeclaringType())
+  }
 }
 
 /** An access to `result`. */
@@ -1794,6 +1798,7 @@ class Negation extends TNegation, Formula {
 
 /** An expression, such as `x+4`. */
 class Expr extends TExpr, AstNode {
+  cached
   Type getType() { none() }
 }
 
@@ -1874,15 +1879,14 @@ class AddSubExpr extends TAddSubExpr, BinOpExpr {
     result = this.getLeftOperand().getType() and
     result = this.getRightOperand().getType()
     or
-    // Both operands are subtypes of `int`
-    result.getName() = "int" and
-    result = this.getLeftOperand().getType().getASuperType*() and
-    result = this.getRightOperand().getType().getASuperType*()
+    // Both operands are subtypes of `int` / `string` / `float`
+    exprOfPrimitiveAddType(result) = this.getLeftOperand() and
+    exprOfPrimitiveAddType(result) = this.getRightOperand()
     or
     // Coercion to from `int` to `float`
     exists(PrimitiveType i | result.getName() = "float" and i.getName() = "int" |
       this.getAnOperand().getType() = result and
-      this.getAnOperand().getType().getASuperType*() = i
+      this.getAnOperand() = exprOfPrimitiveAddType(i)
     )
     or
     // Coercion to `string`
@@ -1899,6 +1903,25 @@ class AddSubExpr extends TAddSubExpr, BinOpExpr {
   }
 }
 
+pragma[noinline]
+private Expr exprOfPrimitiveAddType(PrimitiveType t) {
+  result.getType() = getASubTypeOfAddPrimitive(t)
+}
+
+/**
+ * Gets a subtype of the given primitive type `prim`.
+ * This predicate does not consider float to be a supertype of int.
+ */
+private Type getASubTypeOfAddPrimitive(PrimitiveType prim) {
+  result = prim and
+  result.getName() = ["int", "string", "float"]
+  or
+  exists(Type superType | superType = getASubTypeOfAddPrimitive(prim) |
+    result.getASuperType() = superType and
+    not (result.getName() = "int" and superType.getName() = "float")
+  )
+}
+
 /**
  * An addition expression, such as `x + y`.
  */
@@ -1939,15 +1962,18 @@ class MulDivModExpr extends TMulDivModExpr, BinOpExpr {
     this.getLeftOperand().getType() = result and
     this.getRightOperand().getType() = result
     or
-    // Both operands are subtypes of `int`
-    result.getName() = "int" and
-    result = this.getLeftOperand().getType().getASuperType*() and
-    result = this.getRightOperand().getType().getASuperType*()
+    // Both operands are subtypes of `int`/`float`
+    result.getName() = ["int", "float"] and
+    exprOfPrimitiveAddType(result) = this.getLeftOperand() and
+    exprOfPrimitiveAddType(result) = this.getRightOperand()
     or
     // Coercion from `int` to `float`
     exists(PrimitiveType i | result.getName() = "float" and i.getName() = "int" |
-      this.getAnOperand().getType() = result and
-      this.getAnOperand().getType().getASuperType*() = i
+      this.getLeftOperand() = exprOfPrimitiveAddType(result) and
+      this.getRightOperand() = exprOfPrimitiveAddType(i)
+      or
+      this.getRightOperand() = exprOfPrimitiveAddType(result) and
+      this.getLeftOperand() = exprOfPrimitiveAddType(i)
     )
   }
 
@@ -2281,6 +2307,8 @@ module YAML {
       )
     }
 
+    YAMLListItem getListItem() { toQL(result).getParent() = yamle }
+
     /** Gets the value of this YAML entry. */
     YAMLValue getValue() {
       exists(QL::YamlKeyvaluepair pair |
@@ -2393,7 +2421,7 @@ module YAML {
         deps.getLocation().getFile() = file and entry.getLocation().getFile() = file
       |
         deps.isRoot() and
-        deps.getKey().getQualifiedName() = "dependencies" and
+        deps.getKey().getQualifiedName() = ["dependencies", "libraryPathDependencies"] and
         entry.getLocation().getStartLine() = 1 + deps.getLocation().getStartLine() and
         entry.getLocation().getStartColumn() > deps.getLocation().getStartColumn()
       )
@@ -2408,8 +2436,11 @@ module YAML {
 
     predicate hasDependency(string name, string version) {
       exists(YAMLEntry entry | this.isADependency(entry) |
-        entry.getKey().getQualifiedName() = name and
+        entry.getKey().getQualifiedName().trim() = name and
         entry.getValue().getValue() = version
+        or
+        name = entry.getListItem().getValue().getValue().trim() and
+        version = "\"*\""
       )
     }
 
@@ -2431,7 +2462,7 @@ module YAML {
      */
     QLPack getADependency() {
       exists(string name | this.hasDependency(name, _) |
-        result.getName().replaceAll("-", "/") = name
+        result.getName().replaceAll("-", "/") = name.replaceAll("-", "/")
       )
     }
 

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

@@ -193,7 +193,8 @@ QL::AstNode toQL(AST::AstNode n) {
   n = TAnnotationArg(result)
 }
 
-class TPredicate = TCharPred or TClasslessPredicate or TClassPredicate or TDBRelation;
+class TPredicate =
+  TCharPred or TClasslessPredicate or TClassPredicate or TDBRelation or TNewTypeBranch;
 
 class TPredOrBuiltin = TPredicate or TNewTypeBranch or TBuiltin;
 
@@ -208,3 +209,16 @@ class TTypeDeclaration = TClass or TNewType or TNewTypeBranch;
 class TModuleDeclaration = TClasslessPredicate or TModule or TClass or TNewType;
 
 class TVarDef = TVarDecl or TAsExpr;
+
+module AstConsistency {
+  import ql
+
+  query predicate nonTotalGetParent(AstNode node) {
+    exists(toQL(node).getParent()) and
+    not exists(node.getParent()) and
+    not node.getLocation().getStartColumn() = 1 and // startcolumn = 1 <=> top level in file <=> fine to have no parent
+    exists(node.toString()) and // <- there are a few parse errors in "global-data-flow-java-1.ql", this way we filter them out.
+    not node instanceof YAML::YAMLNode and // parents for YAML doens't work
+    not (node instanceof QLDoc and node.getLocation().getFile().getExtension() = "dbscheme") // qldoc in dbschemes are not hooked up
+  }
+}

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

@@ -139,6 +139,7 @@ private predicate resolveQualifiedName(Import imp, ContainerOrModule m, int i) {
 }
 
 private predicate resolveSelectionName(Import imp, ContainerOrModule m, int i) {
+  (m.(File_).getFile().getExtension() = "qll" or not m instanceof File_) and
   exists(int last |
     resolveQualifiedName(imp, m, last) and
     last = count(int j | exists(imp.getQualifiedName(j))) - 1
@@ -281,7 +282,11 @@ module ModConsistency {
   query predicate multipleResolve(Import imp, int c, ContainerOrModule m) {
     c = strictcount(ContainerOrModule m0 | resolve(imp, m0)) and
     c > 1 and
-    resolve(imp, m)
+    resolve(imp, m) and
+    not imp.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate multipleResolveModuleExpr(ModuleExpr me, int c, ContainerOrModule m) {

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

@@ -3,44 +3,43 @@ private import Builtins
 private import codeql_ql.ast.internal.Module
 private import codeql_ql.ast.internal.AstNodes
 
-private class TClasslessPredicateOrNewTypeBranch = TClasslessPredicate or TNewTypeBranch;
-
-private string getPredicateName(TClasslessPredicateOrNewTypeBranch p) {
-  result = p.(ClasslessPredicate).getName() or
-  result = p.(NewTypeBranch).getName()
-}
-
 private predicate definesPredicate(
-  FileOrModule m, string name, int arity, TClasslessPredicateOrNewTypeBranch p, boolean public
+  FileOrModule m, string name, int arity, Predicate p, boolean public
 ) {
-  m = getEnclosingModule(p) and
-  name = getPredicateName(p) and
-  public = getPublicBool(p) and
-  arity = [p.(ClasslessPredicate).getArity(), count(p.(NewTypeBranch).getField(_))]
-  or
-  // import X
-  exists(Import imp, FileOrModule m0 |
-    m = getEnclosingModule(imp) and
-    m0 = imp.getResolvedModule() and
-    not exists(imp.importedAs()) and
-    definesPredicate(m0, name, arity, p, true) and
-    public = getPublicBool(imp)
-  )
-  or
-  // predicate X = Y
-  exists(ClasslessPredicate alias |
-    m = getEnclosingModule(alias) and
-    name = alias.getName() and
-    resolvePredicateExpr(alias.getAlias(), p) and
-    public = getPublicBool(alias) and
-    arity = alias.getArity()
+  (
+    p instanceof NewTypeBranch or
+    p instanceof ClasslessPredicate
+  ) and
+  (
+    m = getEnclosingModule(p) and
+    name = p.getName() and
+    public = getPublicBool(p) and
+    arity = p.getArity()
+    or
+    // import X
+    exists(Import imp, FileOrModule m0 |
+      m = getEnclosingModule(imp) and
+      m0 = imp.getResolvedModule() and
+      not exists(imp.importedAs()) and
+      definesPredicate(m0, name, arity, p, true) and
+      public = getPublicBool(imp)
+    )
+    or
+    // predicate X = Y
+    exists(ClasslessPredicate alias |
+      m = getEnclosingModule(alias) and
+      name = alias.getName() and
+      resolvePredicateExpr(alias.getAlias(), p) and
+      public = getPublicBool(alias) and
+      arity = alias.getArity()
+    )
   )
 }
 
 cached
 private module Cached {
   cached
-  predicate resolvePredicateExpr(PredicateExpr pe, ClasslessPredicate p) {
+  predicate resolvePredicateExpr(PredicateExpr pe, Predicate p) {
     exists(FileOrModule m, boolean public |
       not exists(pe.getQualifier()) and
       m = getEnclosingModule(pe).getEnclosing*() and
@@ -49,7 +48,7 @@ private module Cached {
       m = pe.getQualifier().getResolvedModule() and
       public = true
     |
-      definesPredicate(m, pe.getName(), count(p.getParameter(_)), p, public)
+      definesPredicate(m, pe.getName(), p.getArity(), p, public)
     )
   }
 

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

@@ -340,6 +340,12 @@ module TyConsistency {
     resolveTypeExpr(te, t)
   }
 
+  query predicate multiplePrimitives(TypeExpr te, int c, PrimitiveType t) {
+    c = strictcount(PrimitiveType t0 | resolveTypeExpr(te, t0)) and
+    c > 1 and
+    resolveTypeExpr(te, t)
+  }
+
   query predicate varDefNoType(VarDef def) {
     not exists(def.getType()) and
     not def.getLocation()
@@ -360,4 +366,10 @@ module TyConsistency {
         .getAbsolutePath()
         .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
+
+  query predicate multiplePrimitivesExpr(Expr e, int c, PrimitiveType t) {
+    c = strictcount(PrimitiveType t0 | t0 = e.getType()) and
+    c > 1 and
+    t = e.getType()
+  }
 }

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

@@ -1,5 +1,5 @@
 import ql
-import codeql_ql.ast.internal.AstNodes
+private import codeql_ql.ast.internal.AstNodes
 
 private class TScope =
   TClass or TAggregate or TQuantifier or TSelect or TPredicate or TNewTypeBranch;
@@ -89,4 +89,8 @@ module VarConsistency {
     decl = f.getDeclaration() and
     strictcount(f.getDeclaration()) > 1
   }
+
+  query predicate noFieldDef(FieldAccess f) { not exists(f.getDeclaration()) }
+
+  query predicate noVarDef(VarAccess v) { not exists(v.getDeclaration()) }
 }

ql/src/codeql_ql/printAstGenerated.qll

@@ -8,7 +8,7 @@
  * to hold for only the AST nodes you wish to view.
  */
 
-import ast.internal.TreeSitter::Generated
+import ast.internal.TreeSitter::QL
 private import codeql.Locations
 
 /**