QL: Merge pull request #19 from github/more-variables

More variable resolution

Author: hvitved

ql/src/codeql_ql/ast/Ast.qll

@@ -2,7 +2,7 @@ import ql
 private import codeql_ql.ast.internal.AstNodes
 private import codeql_ql.ast.internal.Module
 private import codeql_ql.ast.internal.Predicate
-private import codeql_ql.ast.internal.Type
+import codeql_ql.ast.internal.Type
 private import codeql_ql.ast.internal.Variable
 
 bindingset[name, i]
@@ -256,18 +256,28 @@ class CharPred extends TCharPred, Predicate {
   override AstNode getAChild(string pred_name) { pred_name = "getBody" and result = this.getBody() }
 }
 
+/**
+ * A variable definition. This is either a variable declaration or
+ * an `as` expression.
+ */
+class VarDef extends TVarDef, AstNode {
+  /** Gets the name of the declared variable. */
+  string getName() { none() }
+
+  override string getAPrimaryQlClass() { result = "VarDef" }
+
+  override string toString() { result = this.getName() }
+}
+
 /**
  * A variable declaration, with a type and a name.
  */
-class VarDecl extends TVarDecl, AstNode {
+class VarDecl extends TVarDecl, VarDef {
   Generated::VarDecl var;
 
   VarDecl() { this = TVarDecl(var) }
 
-  /**
-   * Gets the name for this variable declaration.
-   */
-  string getName() { result = var.getChild(1).(Generated::VarName).getChild().getValue() }
+  override string getName() { result = var.getChild(1).(Generated::VarName).getChild().getValue() }
 
   override string getAPrimaryQlClass() { result = "VarDecl" }
 
@@ -407,6 +417,8 @@ class ModuleMember extends TModuleMember, AstNode {
 class Declaration extends TDeclaration, AstNode {
   /** Gets the name of this declaration. */
   string getName() { none() }
+
+  final override string toString() { result = this.getName() }
 }
 
 /** An entity that can be declared in a module. */
@@ -1088,43 +1100,55 @@ class ExprAggregate extends TExprAggregate, Expr {
 /** An aggregate expression, such as `count` or `sum`. */
 class Aggregate extends TAggregate, Expr {
   Generated::Aggregate agg;
-  Generated::FullAggregateBody body;
   string kind;
 
   Aggregate() {
     this = TAggregate(agg) and
-    kind = agg.getChild(0).(Generated::AggId).getValue() and
-    body = agg.getChild(_)
+    kind = agg.getChild(0).(Generated::AggId).getValue()
   }
 
+  private Generated::FullAggregateBody getBody1() { result = agg.getChild(_) }
+
+  private Generated::ExprAggregateBody getBody2() { result = agg.getChild(_) }
+
   string getKind() { result = kind }
 
   /** Gets the ith declared argument of this quantifier. */
-  VarDecl getArgument(int i) { toGenerated(result) = body.getChild(i) }
+  VarDecl getArgument(int i) { toGenerated(result) = this.getBody1().getChild(i) }
 
   /** Gets an argument of this quantifier. */
   VarDecl getAnArgument() { result = this.getArgument(_) }
 
   /**
    * Gets the formula restricting the range of this quantifier, if any.
    */
-  Formula getRange() { toGenerated(result) = body.getGuard() }
+  Formula getRange() { toGenerated(result) = this.getBody1().getGuard() }
 
   /**
    * Gets the ith "as" expression of this aggregate, if any.
    */
-  AsExpr getAsExpr(int i) { toGenerated(result) = body.getAsExprs().getChild(i) }
+  AsExpr getAsExpr(int i) {
+    toGenerated(result) = [this.getBody1().getAsExprs(), this.getBody2().getAsExprs()].getChild(i)
+  }
 
   /**
    * Gets the ith "order by" expression of this aggregate, if any.
    */
-  Expr getOrderBy(int i) { toGenerated(result) = body.getOrderBys().getChild(i).getChild(0) }
+  Expr getOrderBy(int i) {
+    toGenerated(result) =
+      [this.getBody1().getOrderBys(), this.getBody2().getOrderBys()].getChild(i).getChild(0)
+  }
 
   /**
    * Gets the direction (ascending or descending) of the ith "order by" expression of this aggregate.
    */
   string getOrderbyDirection(int i) {
-    result = body.getOrderBys().getChild(i).getChild(1).(Generated::Direction).getValue()
+    result =
+      [this.getBody1().getOrderBys(), this.getBody2().getOrderBys()]
+          .getChild(i)
+          .getChild(1)
+          .(Generated::Direction)
+          .getValue()
   }
 
   override string getAPrimaryQlClass() { result = "Aggregate[" + kind + "]" }
@@ -1159,13 +1183,15 @@ class Rank extends Aggregate {
 /**
  * An "as" expression, such as `foo as bar`.
  */
-class AsExpr extends TAsExpr, AstNode {
+class AsExpr extends TAsExpr, VarDef {
   Generated::AsExpr asExpr;
 
   AsExpr() { this = TAsExpr(asExpr) }
 
   override string getAPrimaryQlClass() { result = "AsExpr" }
 
+  final override string getName() { result = this.getAsName() }
+
   /**
    * Gets the name the inner expression gets "saved" under, if it exists.
    * For example this is `bar` in the expression `foo as bar`.
@@ -1189,30 +1215,73 @@ class AsExpr extends TAsExpr, AstNode {
   }
 
   override AstNode getAChild(string pred) { pred = "getInnerExpr" and result = this.getInnerExpr() }
+
+  override string toString() {
+    result = this.getName()
+    or
+    not exists(this.getName()) and
+    result = "AsExpr"
+  }
 }
 
 class Identifier extends TIdentifier, Expr {
   Generated::Variable id;
 
   Identifier() { this = TIdentifier(id) }
 
-  string getName() { result = id.getChild().(Generated::VarName).getChild().getValue() }
+  string getName() { none() }
+
+  final override string toString() { result = this.getName() }
 
   override string getAPrimaryQlClass() { result = "Identifier" }
 }
 
 /** An access to a variable. */
 class VarAccess extends Identifier {
-  private VarDecl decl;
+  private VarDef decl;
 
   VarAccess() { resolveVariable(this, decl) }
 
   /** Gets the accessed variable. */
-  VarDecl getDeclaration() { result = decl }
+  VarDef getDeclaration() { result = decl }
+
+  override string getName() { result = id.getChild().(Generated::VarName).getChild().getValue() }
 
   override string getAPrimaryQlClass() { result = "VarAccess" }
 }
 
+/** An access to a field. */
+class FieldAccess extends Identifier {
+  private VarDecl decl;
+
+  FieldAccess() { resolveField(this, decl) }
+
+  /** Gets the accessed field. */
+  VarDecl getDeclaration() { result = decl }
+
+  override string getName() { result = id.getChild().(Generated::VarName).getChild().getValue() }
+
+  override string getAPrimaryQlClass() { result = "FieldAccess" }
+}
+
+/** An access to `this`. */
+class ThisAccess extends Identifier {
+  ThisAccess() { any(Generated::This t).getParent() = id }
+
+  override string getName() { result = "this" }
+
+  override string getAPrimaryQlClass() { result = "ThisAccess" }
+}
+
+/** An access to `result`. */
+class ResultAccess extends Identifier {
+  ResultAccess() { any(Generated::Result r).getParent() = id }
+
+  override string getName() { result = "result" }
+
+  override string getAPrimaryQlClass() { result = "ResultAccess" }
+}
+
 /** A `not` formula. */
 class Negation extends TNegation, Formula {
   Generated::Negation neg;

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

@@ -22,7 +22,10 @@ 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
+  TAggregate(Generated::Aggregate agg) {
+    agg.getChild(_) instanceof Generated::FullAggregateBody or
+    agg.getChild(_) instanceof Generated::ExprAggregateBody
+  } or
   TExprAggregate(Generated::Aggregate agg) {
     agg.getChild(_) instanceof Generated::ExprAggregateBody
   } or
@@ -161,3 +164,5 @@ class TDeclaration = TTypeDeclaration or TModuleDeclaration;
 class TTypeDeclaration = TClass or TNewType or TNewTypeBranch;
 
 class TModuleDeclaration = TClasslessPredicate or TModule or TClass or TNewType;
+
+class TVarDef = TVarDecl or TAsExpr;

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

@@ -10,14 +10,33 @@ class VariableScope extends TScope, AstNode {
   VariableScope getOuterScope() { result = scopeOf(this) }
 
   /** Gets a variable declared directly in this scope. */
-  VarDecl getADeclaration() { result.getParent() = this }
+  VarDef getADefinition() { result.getParent() = this }
 
-  /** Holds if this scope contains declaration `decl`, either directly or inherited. */
-  predicate contains(VarDecl decl) {
-    decl = this.getADeclaration()
+  /** Holds if this scope contains variable `decl`, either directly or inherited. */
+  predicate containsVar(VarDef decl) {
+    not this instanceof Class and
+    decl = this.getADefinition()
     or
-    this.getOuterScope().contains(decl) and
-    not this.getADeclaration().getName() = decl.getName()
+    decl = this.(Select).getAsExpr(_)
+    or
+    decl = this.(Aggregate).getAsExpr(_)
+    or
+    this.getOuterScope().containsVar(decl) and
+    not this.getADefinition().getName() = decl.getName()
+  }
+
+  /** Holds if this scope contains field `decl`, either directly or inherited. */
+  predicate containsField(VarDef decl) {
+    decl = this.(Class).getAField()
+    or
+    this.getOuterScope().containsField(decl) and
+    not this.getADefinition().getName() = decl.getName()
+    or
+    exists(VariableScope sup |
+      sup = this.(Class).getASuperType().getResolvedType().(ClassType).getDeclaration() and
+      sup.containsField(decl) and
+      not this.(Class).getAField().getName() = decl.getName()
+    )
   }
 }
 
@@ -28,14 +47,31 @@ private AstNode parent(AstNode child) {
 
 VariableScope scopeOf(AstNode n) { result = parent*(n.getParent()) }
 
-predicate resolveVariable(Identifier i, VarDecl decl) {
-  scopeOf(i).contains(decl) and
-  decl.getName() = i.getName()
+private string getName(Identifier i) {
+  exists(Generated::Variable v |
+    i = TIdentifier(v) and
+    result = v.getChild().(Generated::VarName).getChild().getValue()
+  )
+}
+
+predicate resolveVariable(Identifier i, VarDef decl) {
+  scopeOf(i).containsVar(decl) and
+  decl.getName() = getName(i)
+}
+
+predicate resolveField(Identifier i, VarDef decl) {
+  scopeOf(i).containsField(decl) and
+  decl.getName() = getName(i)
 }
 
 module VarConsistency {
-  query predicate multipleVarDecls(VarAccess v, VarDecl decl) {
+  query predicate multipleVarDefs(VarAccess v, VarDef decl) {
     decl = v.getDeclaration() and
     strictcount(v.getDeclaration()) > 1
   }
+
+  query predicate multipleFieldDefs(FieldAccess f, VarDef decl) {
+    decl = f.getDeclaration() and
+    strictcount(f.getDeclaration()) > 1
+  }
 }