Merge pull request #22547 from asgerf/unified/local-variables-and-self

Unified: Introduce local variables and 'self' bindings

Author: asgerf

shared/namebinding/codeql/namebinding/LocalNameBinding.qll

@@ -337,54 +337,91 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
     )
   }
 
-  private predicate accessCandInLookupScope(AstNode n, string name, Scope lookup) {
-    accessCand(n, name) and
-    (
-      lookupStartsAt(n, lookup)
-      or
-      not lookupStartsAt(n, _) and
-      lookup = getEnclosingScope(n)
-    )
-  }
-
-  pragma[nomagic]
-  private predicate lookupInScope(string name, Scope lookup, Scope scope) {
-    accessCandInLookupScope(_, name, lookup) and
-    scope = lookup
-    or
-    exists(Scope mid |
-      lookupInScope(name, lookup, mid) and
-      not declInScope(name, mid) and
-      not isTopScope(mid) and
-      scope = getEnclosingScope(mid)
-    )
-  }
-
   private predicate declInScope(string name, AstNode scope) {
     declInScope(_, name, scope) or
     implicitDeclInScope(name, scope)
   }
 
+  signature predicate accessCandSig(AstNode n, string name);
+
   /**
-   * Holds if `name`, when resolved from `lookup`, may resolve to one of the uncertain members of `scope`.
+   * Allows resolution of access candidates.
+   *
+   * This is instantiated once by the local name binding library itself in order to populate `LocalAccess`.
+   * It can be instantiated further by the client, to resolve additional lookups at a later evaluation stage.
    */
-  pragma[nomagic]
-  private predicate lookupInUncertainScope(string name, Scope lookup, Scope scope) {
-    lookupInScope(name, lookup, scope) and
-    uncertainScope(scope) and
-    not declInScope(name, scope)
+  module ResolveAccesses<accessCandSig/2 accessCandInput> {
+    private predicate accessCandInLookupScope(AstNode n, string name, Scope lookup) {
+      accessCandInput(n, name) and
+      (
+        lookupStartsAt(n, lookup)
+        or
+        not lookupStartsAt(n, _) and
+        lookup = getEnclosingScope(n)
+      )
+    }
+
+    pragma[nomagic]
+    private predicate lookupInScope(string name, Scope lookup, Scope scope) {
+      accessCandInLookupScope(_, name, lookup) and
+      scope = lookup
+      or
+      exists(Scope mid |
+        lookupInScope(name, lookup, mid) and
+        not declInScope(name, mid) and
+        not isTopScope(mid) and
+        scope = getEnclosingScope(mid)
+      )
+    }
+
+    pragma[nomagic]
+    private predicate resolveInScope(string name, Scope lookup, Local l) {
+      exists(Scope scope | lookupInScope(name, lookup, scope) |
+        l = TExplicitLocal(_, name, scope) or
+        l = TImplicitLocal(name, scope)
+      )
+    }
+
+    /** Holds if `access` resolves to `l`. */
+    predicate access(AstNode access, Local l) {
+      exists(Scope lookup, string name |
+        accessCandInLookupScope(access, name, lookup) and
+        resolveInScope(name, lookup, l)
+      )
+    }
+
+    /**
+     * Holds if `name`, when resolved from `lookup`, may resolve to one of the uncertain members of `scope`.
+     */
+    pragma[nomagic]
+    private predicate lookupInUncertainScope(string name, Scope lookup, Scope scope) {
+      lookupInScope(name, lookup, scope) and
+      uncertainScope(scope) and
+      not declInScope(name, scope)
+    }
+
+    /**
+     * Gets an uncertain scope in which the `accessCand` pair may resolve.
+     */
+    AstNode getAnUncertainScope(AstNode access, string name) {
+      exists(Scope lookup |
+        accessCandInLookupScope(access, name, lookup) and
+        lookupInUncertainScope(name, lookup, result)
+      )
+    }
   }
 
-  /**
-   * Gets an uncertain scope in which the `accessCand` pair may resolve.
-   */
-  AstNode getAnUncertainScope(AstNode access, string name) {
-    exists(Scope lookup |
-      accessCandInLookupScope(access, name, lookup) and
-      lookupInUncertainScope(name, lookup, result)
-    )
+  private module DefaultAccesses = ResolveAccesses<accessCand/2>;
+
+  /** Holds if `access` resolves to `l`. */
+  cached
+  private predicate access(AstNode access, Local l) {
+    CachedStage::ref() and
+    DefaultAccesses::access(access, l)
   }
 
+  predicate getAnUncertainScope = DefaultAccesses::getAnUncertainScope/2;
+
   cached
   private newtype TLocal =
     TExplicitLocal(AstNode definingNode, string name, AstNode scope) {
@@ -447,23 +484,10 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
     override string getName() { result = name }
 
     override Location getLocation() { result = scope.getLocation() }
-  }
-
-  pragma[nomagic]
-  private predicate resolveInScope(string name, Scope lookup, Local l) {
-    exists(Scope scope | lookupInScope(name, lookup, scope) |
-      l = TExplicitLocal(_, name, scope) or
-      l = TImplicitLocal(name, scope)
-    )
-  }
 
-  cached
-  private predicate access(AstNode access, Local l) {
-    CachedStage::ref() and
-    exists(Scope lookup, string name |
-      accessCandInLookupScope(access, name, lookup) and
-      resolveInScope(name, lookup, l)
-    )
+    /** Holds if this variable has the given name and scope. */
+    pragma[nomagic]
+    predicate hasNameAndScope(string name_, AstNode scope_) { name = name_ and scope = scope_ }
   }
 
   /** A local access. */

unified/ql/lib/codeql/Definitions.qll

@@ -3,14 +3,14 @@
  */
 
 private import unified
-private import codeql.unified.internal.StaticNameBinding
+private import codeql.unified.internal.NameBinding
 
 /**
  * Holds if `reference` refers to `definition`.
  */
 cached
-predicate definitionOf(Identifier reference, NameDeclaration definition, string kind) {
+predicate definitionOf(Identifier reference, NameBinding definition, string kind) {
   definition = getStaticBindingTarget(reference) and
-  not reference instanceof NameDeclaration and
+  not reference instanceof NameBinding and
   kind = "name"
 }

unified/ql/lib/codeql/unified/internal/AnalysisQuality.qll

@@ -1,8 +1,6 @@
 private import unified
 private import codeql.util.ReportStats
-private import codeql.unified.internal.StaticNameBinding
-private import codeql.unified.internal.LocalNameBinding
-private import codeql.unified.internal.NameBindingPlugin
+private import codeql.unified.internal.NameBinding
 
 /** Stats about name nodes that static name binding could resolve. */
 module StaticNameResolutionStats implements EntityStatsSig {
@@ -44,15 +42,21 @@ module StaticNameResolutionStats implements EntityStatsSig {
         this = getIdentifierFromRef(ref) and
         not memberAccessDependsOnTypeInference(ref)
       ) and
-      not this instanceof NameDeclaration
+      not this instanceof NameBinding
     }
 
     NameBindingNode getTarget() {
-      (
-        result.asIdentifier() = getStaticBindingTarget(this)
-        or
-        result.isModuleScopeNode(_) and
-        result.(NamespaceNode).ref().isIdentifier(this)
+      result.asIdentifier() = getStaticBindingTarget(this)
+      or
+      result.isModuleScopeNode(_) and
+      result.(NamespaceNode).ref().isIdentifier(this)
+      or
+      // Resolving to an implicitly-declared local such as "self" should count as
+      // as a successfully resolved name
+      exists(LocalName implicitLocal |
+        implicitLocal = this.(LocalNameAccess).getLocalName() and
+        not exists(implicitLocal.getABinding()) and
+        result.isLocalName(implicitLocal)
       )
     }
 

unified/ql/lib/codeql/unified/internal/AstExtra.qll

@@ -3,6 +3,7 @@
  */
 
 private import unified
+private import codeql.unified.internal.NameBindingPlugin
 
 module Public {
   /** A short-circuiting logical AND expression. */
@@ -29,24 +30,14 @@ module Public {
    * Declaration of a local or top-level variable.
    */
   class LocalVariableDeclaration extends VariableDeclaration {
-    private Block block;
-
-    LocalVariableDeclaration() { this = block.getStmt(_) }
-
-    /** Gets the block in which this variable is declared. */
-    Block getDeclaringBlock() { result = block }
+    LocalVariableDeclaration() { not isStaticMember(this) and not isInstanceMember(this) }
   }
 
   /**
    * Declaration of a local or top-level function.
    */
   class LocalFunctionDeclaration extends FunctionDeclaration {
-    private Block block;
-
-    LocalFunctionDeclaration() { this = block.getStmt(_) }
-
-    /** Gets the block in which this function is declared. */
-    Block getDeclaringBlock() { result = block }
+    LocalFunctionDeclaration() { not isStaticMember(this) and not isInstanceMember(this) }
   }
 
   /**

unified/ql/lib/codeql/unified/internal/ControlFlowGraph.qll

@@ -39,14 +39,7 @@ private module Ast implements AstSig<Location> {
     not skipControlFlow(result)
   }
 
-  Callable getEnclosingCallable(AstNode node) {
-    exists(AstNode parent | parent = node.getParent() |
-      result = parent
-      or
-      not parent instanceof Callable and
-      result = getEnclosingCallable(parent)
-    )
-  }
+  Callable getEnclosingCallable(AstNode node) { result = node.getEnclosingCallable() }
 
   class Callable = U::Callable;
 

unified/ql/lib/codeql/unified/internal/FacadeAst.qll

@@ -23,12 +23,47 @@ module Unified {
       )
     }
 
-    /** Gets the nearest enclosing class declaration, possibly this node itself. */
+    /** Gets the nearest enclosing class declaration, if any. */
     ClassLikeDeclaration getEnclosingClass() {
-      result = this
-      or
-      not this instanceof ClassLikeDeclaration and
-      result = this.getParent().getEnclosingClass()
+      exists(AstNode parent | parent = this.getParent() |
+        result = parent
+        or
+        not parent instanceof ClassLikeDeclaration and
+        result = parent.getEnclosingClass()
+      )
+    }
+
+    private AstNode overrideEnclosingCallableParent() {
+      exists(FunctionExpr func |
+        // Capture declarations are evaluated as part of the outer context, and
+        // considered to be captured by the function expression.
+        this = func.getACaptureDeclaration() and
+        result = func.getParent()
+      )
+    }
+
+    /**
+     * Gets the nearest callable containing this AST node.
+     *
+     * If this node is itself a callable, this gets the outer callable, not the node itself.
+     *
+     * Note that the `TopLevel` is callable, so all nodes other than the `TopLevel` itself has an enclosing callable.
+     *
+     * In some cases this predicate skips overs the syntactically-enclosing callable in order to get the callable in which
+     * the AST is actually evaluated (such as for capture declarations in a function expression).
+     */
+    Callable getEnclosingCallable() {
+      exists(AstNode parent |
+        parent = this.overrideEnclosingCallableParent()
+        or
+        not exists(this.overrideEnclosingCallableParent()) and
+        parent = this.getParent()
+      |
+        result = parent
+        or
+        not parent instanceof Callable and
+        result = parent.getEnclosingCallable()
+      )
     }
 
     /** Gets the depth of this node in the AST. The root node has a depth of 0. */