wip

Author: hvitved

ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll

@@ -114,7 +114,8 @@ predicate scopeDefinesParameterVariable(
   )
 }
 
-pragma[nomagic]
+bindingset[i]
+pragma[inline_late]
 private string variableNameInScope(Ruby::AstNode i, Scope::Range scope) {
   scope = scopeOf(i) and
   (
@@ -182,8 +183,12 @@ private module Input implements LocalNameBindingInputSig<Location> {
       |
         other order by other.getLocation().getStartLine(), other.getLocation().getStartColumn()
       ) and
-    not scopeDefinesParameterVariable(scope, name, _, _) and
-    not inherits(scope, name, _)
+    not scopeDefinesParameterVariable(scope, name, _, _)
+  }
+
+  predicate declInScopeIsUncertain(AstNode definingNode, string name, AstNode scope) {
+    declInScope(definingNode, name, scope) and
+    explicitAssignmentNode(definingNode, _)
   }
 
   predicate implicitDeclInScope(string name, AstNode scope) {
@@ -219,10 +224,20 @@ private module Input implements LocalNameBindingInputSig<Location> {
     n instanceof Ruby::Self and
     name = "self"
   }
+
+  bindingset[access, definingNode]
+  predicate isValidAccess(AstNode access, AstNode definingNode) {
+    not access.getLocation().strictlyBefore(definingNode.getLocation())
+  }
 }
 
 private import LocalNameBinding<Location, Input>
 
+pragma[nomagic]
+predicate access(Ruby::AstNode access, VariableReal variable) {
+  exists(Local l | variable = TLocalVariableReal(l) | access = l.getAnAccess())
+}
+
 cached
 private module Cached {
   cached
@@ -398,14 +413,6 @@ private module Cached {
     i = any(Ruby::ExpressionReferencePattern x).getValue()
   }
 
-  cached
-  predicate access(Ruby::AstNode access, VariableReal variable) {
-    exists(Local l | variable = TLocalVariableReal(l) |
-      access = l.getAnAccess() and
-      not access.getLocation().strictlyBefore(l.getDefiningNode().getLocation())
-    )
-  }
-
   private class Access extends Ruby::Token {
     Access() {
       access(this, _) or
@@ -456,29 +463,6 @@ private module Cached {
 
 import Cached
 
-/** Holds if this scope inherits `name` from an outer scope `outer`. */
-private predicate inherits(Scope::Range scope, string name, Scope::Range outer) {
-  (
-    scope instanceof Ruby::Block or
-    scope instanceof Ruby::DoBlock or
-    scope instanceof Ruby::Lambda
-  ) and
-  not scopeDefinesParameterVariable(scope, name, _, _) and
-  (
-    outer = scope.getOuterScope() and
-    (
-      scopeDefinesParameterVariable(outer, name, _, _)
-      or
-      exists(Ruby::AstNode i |
-        scopeAssigns(outer, name, i) and
-        i.getLocation().strictlyBefore(scope.getLocation())
-      )
-    )
-    or
-    inherits(scope.getOuterScope(), name, outer)
-  )
-}
-
 abstract class VariableImpl extends TVariable {
   abstract string getNameImpl();
 

shared/namebinding/codeql/namebinding/LocalNameBinding.qll

@@ -95,6 +95,19 @@ signature module LocalNameBindingInputSig<LocationSig Location> {
    */
   predicate declInScope(AstNode definingNode, string name, AstNode scope);
 
+  /**
+   * Holds if it is uncertain whether the reference to local declaration named
+   * `name` at `definingNode` in scope `scope` is a declaration or an access.
+   *
+   * For example, in Ruby variables are not explicitly declared, so we consider
+   * the first mention of a variable to be its declaration.
+   *
+   * This predicate must be a subset of `declInScope`.
+   */
+  default predicate declInScopeIsUncertain(AstNode definingNode, string name, AstNode scope) {
+    none()
+  }
+
   /**
    * Holds if a local declaration named `name` is implicitly in scope in the given `scope`.
    */
@@ -123,6 +136,12 @@ signature module LocalNameBindingInputSig<LocationSig Location> {
    * full control of scope resolution for for specific types of references.
    */
   default predicate lookupStartsAt(AstNode n, AstNode scope) { none() }
+
+  /**
+   * Holds if `access` is valid access of the local declared at `definingNode`.
+   */
+  bindingset[access, definingNode]
+  default predicate isValidAccess(AstNode access, AstNode definingNode) { any() }
 }
 
 /**
@@ -299,6 +318,12 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
       not lookupStartsAt(n, _) and
       lookup = getEnclosingScope(n)
     )
+    or
+    exists(Scope scope |
+      declInScopeIsUncertain(n, name, scope) and
+      not isTopScope(scope) and
+      lookup = getEnclosingScope(scope)
+    )
   }
 
   pragma[nomagic]
@@ -308,7 +333,11 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
     or
     exists(Scope mid |
       lookupInScope(name, lookup, mid) and
-      not declInScope(_, name, mid) and
+      not exists(AstNode definingNode |
+        declInScope(definingNode, name, mid) and
+        // allow to step through uncertain declarations
+        not declInScopeIsUncertain(definingNode, name, mid)
+      ) and
       not implicitDeclInScope(name, mid) and
       not isTopScope(mid) and
       scope = getEnclosingScope(mid)
@@ -352,7 +381,15 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
     private string name;
     private AstNode scope;
 
-    ExplicitLocal() { this = TExplicitLocal(definingNode, name, scope) }
+    cached
+    ExplicitLocal() {
+      CachedStage::ref() and
+      this = TExplicitLocal(definingNode, name, scope) and
+      // skip uncertain declarations that are in fact accesses
+      not exists(AstNode other |
+        accessStep(definingNode, TExplicitLocal(other, _, _)) and other != definingNode
+      )
+    }
 
     override AstNode getDefiningNode() { result = definingNode }
 
@@ -380,27 +417,47 @@ module LocalNameBinding<LocationSig Location, LocalNameBindingInputSig<Location>
   }
 
   pragma[nomagic]
-  private predicate resolveInScope(string name, Scope lookup, Local l) {
+  private predicate resolveInScope(string name, Scope lookup, TLocal 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) {
+  pragma[nomagic]
+  private predicate accessStep(AstNode access, TLocal l) {
     CachedStage::ref() and
     exists(Scope lookup, string name |
       accessCandInLookupScope(access, name, lookup) and
       resolveInScope(name, lookup, l)
+    |
+      l instanceof TImplicitLocal
+      or
+      l = TExplicitLocal(any(AstNode definingNode | isValidAccess(access, definingNode)), _, _)
+    )
+  }
+
+  pragma[nomagic]
+  private predicate accessSteps(AstNode access, Local l) {
+    accessStep(access, l)
+    or
+    exists(AstNode mid, string name, AstNode scope |
+      accessSteps(mid, l) and
+      accessStep(access, TExplicitLocal(mid, name, scope)) and
+      declInScopeIsUncertain(mid, name, scope)
     )
   }
 
   /** A local access. */
   final class LocalAccess extends AstNodeFinal {
     private Local l;
 
-    LocalAccess() { access(this, l) }
+    cached
+    LocalAccess() {
+      CachedStage::ref() and
+      accessSteps(this, l) and
+      accessCand(this, _)
+    }
 
     /** Gets the local entity being accessed. */
     Local getLocal() { result = l }