Merge pull request #36 from github/various-small-fixes

Various small fixes

ql/src/codeql_ql/ast/Ast.qll

@@ -1065,6 +1065,8 @@ class String extends Literal {
 
   override string getAPrimaryQlClass() { result = "String" }
 
+  override PrimitiveType getType() { result.getName() = "string" }
+
   /** Gets the string value of this literal. */
   string getValue() {
     exists(string raw | raw = lit.getChild().(Generated::String).getValue() |
@@ -1079,6 +1081,8 @@ class Integer extends Literal {
 
   override string getAPrimaryQlClass() { result = "Integer" }
 
+  override PrimitiveType getType() { result.getName() = "int" }
+
   /** Gets the integer value of this literal. */
   int getValue() { result = lit.getChild().(Generated::Integer).getValue().toInt() }
 }
@@ -1089,6 +1093,8 @@ class Float extends Literal {
 
   override string getAPrimaryQlClass() { result = "Float" }
 
+  override PrimitiveType getType() { result.getName() = "float" }
+
   /** Gets the float value of this literal. */
   float getValue() { result = lit.getChild().(Generated::Float).getValue().toFloat() }
 }
@@ -1105,6 +1111,8 @@ class Boolean extends Literal {
   /** Holds if the value is `false` */
   predicate isFalse() { bool.getChild() instanceof Generated::False }
 
+  override PrimitiveType getType() { result.getName() = "boolean" }
+
   override string getAPrimaryQlClass() { result = "Boolean" }
 }
 
@@ -1384,11 +1392,17 @@ class HigherOrderFormula extends THigherOrderFormula, Formula {
   }
 }
 
+class Aggregate extends TAggregate, Expr {
+  string getKind() { none() }
+
+  Generated::Aggregate getAggregate() { none() }
+}
+
 /**
  * An aggregate containing an expression.
  * E.g. `min(getAPredicate().getArity())`.
  */
-class ExprAggregate extends TExprAggregate, Expr {
+class ExprAggregate extends TExprAggregate, Aggregate {
   Generated::Aggregate agg;
   Generated::ExprAggregateBody body;
   string kind;
@@ -1403,7 +1417,9 @@ class ExprAggregate extends TExprAggregate, Expr {
    * Gets the kind of aggregate.
    * E.g. for `min(foo())` the result is "min".
    */
-  string getKind() { result = kind }
+  override string getKind() { result = kind }
+
+  override Generated::Aggregate getAggregate() { result = agg }
 
   /**
    * Gets the ith "as" expression of this aggregate, if any.
@@ -1449,13 +1465,13 @@ class ExprAggregate extends TExprAggregate, Expr {
 }
 
 /** An aggregate expression, such as `count` or `sum`. */
-class Aggregate extends TAggregate, Expr {
+class FullAggregate extends TFullAggregate, Aggregate {
   Generated::Aggregate agg;
   string kind;
   Generated::FullAggregateBody body;
 
-  Aggregate() {
-    this = TAggregate(agg) and
+  FullAggregate() {
+    this = TFullAggregate(agg) and
     kind = agg.getChild(0).(Generated::AggId).getValue() and
     body = agg.getChild(_)
   }
@@ -1464,7 +1480,9 @@ class Aggregate extends TAggregate, Expr {
    * Gets the kind of aggregate.
    * E.g. for `min(int i | foo(i))` the result is "foo".
    */
-  string getKind() { result = kind }
+  override string getKind() { result = kind }
+
+  override Generated::Aggregate getAggregate() { result = agg }
 
   /** Gets the ith declared argument of this quantifier. */
   VarDecl getArgument(int i) { toGenerated(result) = body.getChild(i) }
@@ -1494,18 +1512,18 @@ class Aggregate extends TAggregate, Expr {
     result = body.getOrderBys().getChild(i).getChild(1).(Generated::Direction).getValue()
   }
 
-  override string getAPrimaryQlClass() { result = "Aggregate[" + kind + "]" }
+  override string getAPrimaryQlClass() { kind != "rank" and result = "FullAggregate[" + kind + "]" }
 
   override Type getType() {
     exists(PrimitiveType prim | prim = result |
-      kind.regexpMatch("(strict)?count|sum|min|max|rank") and
+      kind.regexpMatch("(strict)?(count|sum|min|max|rank)") and
       result.getName() = "int"
       or
       kind.regexpMatch("(strict)?concat") and
       result.getName() = "string"
     )
     or
-    kind = ["any", "min", "max"] and
+    kind = ["any", "min", "max", "unique"] and
     not exists(getExpr(_)) and
     result = getArgument(0).getTypeExpr().getResolvedType()
     or
@@ -1532,14 +1550,14 @@ class Aggregate extends TAggregate, Expr {
  * A "rank" expression, such as `rank[4](int i | i = [5 .. 15] | i)`.
  */
 class Rank extends Aggregate {
-  Rank() { kind = "rank" }
+  Rank() { this.getKind() = "rank" }
 
   override string getAPrimaryQlClass() { result = "Rank" }
 
   /**
    * The `i` in `rank[i]( | | )`.
    */
-  Expr getRankExpr() { toGenerated(result) = agg.getChild(1) }
+  Expr getRankExpr() { toGenerated(result) = this.getAggregate().getChild(1) }
 
   override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
@@ -1704,6 +1722,8 @@ class ExprAnnotation extends TExprAnnotation, Expr {
    */
   Expr getExpression() { toGenerated(result) = expr_anno.getChild() }
 
+  override Type getType() { result = this.getExpression().getType() }
+
   override string getAPrimaryQlClass() { result = "ExprAnnotation" }
 
   override AstNode getAChild(string pred) {
@@ -1744,6 +1764,27 @@ class AddSubExpr extends TAddSubExpr, BinOpExpr {
   /** Gets the operator of the binary expression. */
   FunctionSymbol getOperator() { result = operator }
 
+  override PrimitiveType getType() {
+    // Both operands are the same type
+    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*()
+    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
+    )
+    or
+    // Coercion to `string`
+    result.getName() = "string" and
+    this.getAnOperand().getType() = result
+  }
+
   override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
@@ -1792,6 +1833,23 @@ class MulDivModExpr extends TMulDivModExpr, BinOpExpr {
   /** Gets the operator of the binary expression. */
   FunctionSymbol getOperator() { result = operator }
 
+  override PrimitiveType getType() {
+    // Both operands are of the same type
+    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*()
+    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
+    )
+  }
+
   override AstNode getAChild(string pred) {
     result = super.getAChild(pred)
     or
@@ -1846,6 +1904,8 @@ class Range extends TRange, Expr {
    */
   Expr getHighEndpoint() { toGenerated(result) = range.getUpper() }
 
+  override PrimitiveType getType() { result.getName() = "int" }
+
   override string getAPrimaryQlClass() { result = "Range" }
 
   override AstNode getAChild(string pred) {
@@ -1870,6 +1930,8 @@ class Set extends TSet, Expr {
    */
   Expr getElement(int i) { toGenerated(result) = set.getChild(i) }
 
+  override Type getType() { result = this.getElement(0).getType() }
+
   override string getAPrimaryQlClass() { result = "Set" }
 
   override AstNode getAChild(string pred) {
@@ -1891,6 +1953,8 @@ class UnaryExpr extends TUnaryExpr, Expr {
   /** Gets the operator of the unary expression as a string. */
   FunctionSymbol getOperator() { result = unaryexpr.getChild(0).toString() }
 
+  override Type getType() { result = this.getOperand().getType() }
+
   override string getAPrimaryQlClass() { result = "UnaryExpr" }
 
   override AstNode getAChild(string pred) {
@@ -1906,6 +1970,8 @@ class DontCare extends TDontCare, Expr {
 
   DontCare() { this = TDontCare(dontcare) }
 
+  override DontCareType getType() { any() }
+
   override string getAPrimaryQlClass() { result = "DontCare" }
 }
 

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

@@ -21,7 +21,9 @@ newtype TAstNode =
   TComparisonFormula(Generated::CompTerm comp) or
   TComparisonOp(Generated::Compop op) or
   TQuantifier(Generated::Quantified quant) or
-  TAggregate(Generated::Aggregate agg) { agg.getChild(_) instanceof Generated::FullAggregateBody } or
+  TFullAggregate(Generated::Aggregate agg) {
+    agg.getChild(_) instanceof Generated::FullAggregateBody
+  } or
   TExprAggregate(Generated::Aggregate agg) {
     agg.getChild(_) instanceof Generated::ExprAggregateBody
   } or
@@ -63,9 +65,11 @@ class TFormula =
 
 class TBinOpExpr = TAddSubExpr or TMulDivModExpr;
 
+class TAggregate = TFullAggregate or TExprAggregate;
+
 class TExpr =
-  TBinOpExpr or TLiteral or TAggregate or TExprAggregate or TIdentifier or TInlineCast or TCall or
-      TUnaryExpr or TExprAnnotation or TDontCare or TRange or TSet or TAsExpr or TSuper;
+  TBinOpExpr or TLiteral or TAggregate or TIdentifier or TInlineCast or TCall or TUnaryExpr or
+      TExprAnnotation or TDontCare or TRange or TSet or TAsExpr or TSuper;
 
 class TCall = TPredicateCall or TMemberCall or TNoneCall or TAnyCall;
 
@@ -77,7 +81,7 @@ private Generated::AstNode toGeneratedFormula(AST::AstNode n) {
   n = TComparisonFormula(result) or
   n = TComparisonOp(result) or
   n = TQuantifier(result) or
-  n = TAggregate(result) or
+  n = TFullAggregate(result) or
   n = TIdentifier(result) or
   n = TNegation(result) or
   n = TIfFormula(result) or
@@ -94,7 +98,7 @@ private Generated::AstNode toGeneratedExpr(AST::AstNode n) {
   n = TSet(result) or
   n = TExprAnnotation(result) or
   n = TLiteral(result) or
-  n = TAggregate(result) or
+  n = TFullAggregate(result) or
   n = TExprAggregate(result) or
   n = TIdentifier(result) or
   n = TUnaryExpr(result) or

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

@@ -258,12 +258,18 @@ private predicate definesModule(
 module ModConsistency {
   query predicate noResolve(Import imp) {
     not resolve(imp, _) and
-    not imp.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not imp.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate noResolveModuleExpr(ModuleExpr me) {
     not resolveModuleExpr(me, _) and
-    not me.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not me.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate multipleResolve(Import imp, int c, ContainerOrModule m) {

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

@@ -234,14 +234,20 @@ private class BuiltinMember extends BuiltinPredicate, TBuiltinMember {
 module PredConsistency {
   query predicate noResolvePredicateExpr(PredicateExpr pe) {
     not resolvePredicateExpr(pe, _) and
-    not pe.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not pe.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate noResolveCall(Call c) {
     not resolveCall(c, _) and
     not c instanceof NoneCall and
     not c instanceof AnyCall and
-    not c.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not c.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate multipleResolvePredicateExpr(PredicateExpr pe, int c, ClasslessPredicate p) {

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

@@ -33,7 +33,7 @@ class Type extends TType {
 
   /**
    * Gets a supertype of this type. This follows the user-visible type heirarchy,
-   * and doesn't include internal types like thecharacteristic and domain types of classes.
+   * and doesn't include internal types like the characteristic and domain types of classes.
    */
   Type getASuperType() { none() }
 
@@ -325,7 +325,10 @@ private predicate defines(FileOrModule m, string name, Type t, boolean public) {
 module TyConsistency {
   query predicate noResolve(TypeExpr te) {
     not resolveTypeExpr(te, _) and
-    not te.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not te.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate multipleResolve(TypeExpr te, int c, Type t) {
@@ -336,7 +339,10 @@ module TyConsistency {
 
   query predicate varDefNoType(VarDef def) {
     not exists(def.getType()) and
-    not def.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not def.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 
   query predicate exprNoType(Expr e) {
@@ -345,6 +351,10 @@ module TyConsistency {
       p = e.(Call).getTarget() and
       not exists(p.getReturnType())
     ) and
-    not e.getLocation().getFile().getAbsolutePath().regexpMatch(".*/(test|examples)/.*")
+    not e instanceof Formula and
+    not e.getLocation()
+        .getFile()
+        .getAbsolutePath()
+        .regexpMatch(".*/(test|examples|ql-training|recorded-call-graph-metrics)/.*")
   }
 }

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

@@ -2,7 +2,7 @@ import ql
 import codeql_ql.ast.internal.AstNodes
 
 private class TScope =
-  TClass or TAggregate or TExprAggregate or TQuantifier or TSelect or TPredicate or TNewTypeBranch;
+  TClass or TAggregate or TQuantifier or TSelect or TPredicate or TNewTypeBranch;
 
 /** A variable scope. */
 class VariableScope extends TScope, AstNode {
@@ -24,7 +24,7 @@ class VariableScope extends TScope, AstNode {
       or
       decl = this.(Select).getExpr(_).(AsExpr)
       or
-      decl = this.(Aggregate).getExpr(_).(AsExpr)
+      decl = this.(FullAggregate).getExpr(_).(AsExpr)
       or
       decl = this.(ExprAggregate).getExpr(_).(AsExpr)
       or

ql/test/printAst/printAst.expected

@@ -135,8 +135,8 @@ nodes
 | Foo.qll:22:3:22:16 | ComparisonFormula | semmle.order | 66 |
 | Foo.qll:22:5:22:5 | ComparisonOp | semmle.label | [ComparisonOp] ComparisonOp |
 | Foo.qll:22:5:22:5 | ComparisonOp | semmle.order | 68 |
-| Foo.qll:22:7:22:16 | Aggregate[any] | semmle.label | [Aggregate[any]] Aggregate[any] |
-| Foo.qll:22:7:22:16 | Aggregate[any] | semmle.order | 69 |
+| Foo.qll:22:7:22:16 | FullAggregate[any] | semmle.label | [FullAggregate[any]] FullAggregate[any] |
+| Foo.qll:22:7:22:16 | FullAggregate[any] | semmle.order | 69 |
 | Foo.qll:22:11:22:13 | TypeExpr | semmle.label | [TypeExpr] TypeExpr |
 | Foo.qll:22:11:22:13 | TypeExpr | semmle.order | 70 |
 | Foo.qll:22:11:22:15 | f | semmle.label | [VarDecl] f |
@@ -312,10 +312,10 @@ edges
 | Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:3:22:3 | f | semmle.order | 66 |
 | Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:5:22:5 | ComparisonOp | semmle.label | getOperator() |
 | Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:5:22:5 | ComparisonOp | semmle.order | 68 |
-| Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:7:22:16 | Aggregate[any] | semmle.label | getRightOperand() |
-| Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:7:22:16 | Aggregate[any] | semmle.order | 69 |
-| Foo.qll:22:7:22:16 | Aggregate[any] | Foo.qll:22:11:22:15 | f | semmle.label | getArgument(_) |
-| Foo.qll:22:7:22:16 | Aggregate[any] | Foo.qll:22:11:22:15 | f | semmle.order | 70 |
+| Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:7:22:16 | FullAggregate[any] | semmle.label | getRightOperand() |
+| Foo.qll:22:3:22:16 | ComparisonFormula | Foo.qll:22:7:22:16 | FullAggregate[any] | semmle.order | 69 |
+| Foo.qll:22:7:22:16 | FullAggregate[any] | Foo.qll:22:11:22:15 | f | semmle.label | getArgument(_) |
+| Foo.qll:22:7:22:16 | FullAggregate[any] | Foo.qll:22:11:22:15 | f | semmle.order | 70 |
 | Foo.qll:22:11:22:15 | f | Foo.qll:22:11:22:13 | TypeExpr | semmle.label | getTypeExpr() |
 | Foo.qll:22:11:22:15 | f | Foo.qll:22:11:22:13 | TypeExpr | semmle.order | 70 |
 | Foo.qll:24:3:24:23 | ComparisonFormula | Foo.qll:24:3:24:3 | Integer | semmle.label | getLeftOperand() |