Ruby: Handle synthesized scopes

ruby/ql/lib/codeql/ruby/AST.qll

@@ -18,6 +18,27 @@ private import ast.internal.Scope
 private import ast.internal.Synthesis
 private import ast.internal.TreeSitter
 
+cached
+private module Cached {
+  cached
+  ModuleBase getEnclosingModule(Scope s) {
+    result = s
+    or
+    not s instanceof ModuleBase and result = getEnclosingModule(s.getOuterScope())
+  }
+
+  cached
+  MethodBase getEnclosingMethod(Scope s) {
+    result = s
+    or
+    not s instanceof MethodBase and
+    not s instanceof ModuleBase and
+    result = getEnclosingMethod(s.getOuterScope())
+  }
+}
+
+private import Cached
+
 /**
  * A node in the abstract syntax tree. This class is the base class for all Ruby
  * program elements.
@@ -39,20 +60,10 @@ class AstNode extends TAstNode {
   final string getPrimaryQlClasses() { result = concat(this.getAPrimaryQlClass(), ",") }
 
   /** Gets the enclosing module, if any. */
-  ModuleBase getEnclosingModule() {
-    exists(Scope::Range s |
-      s = scopeOf(toGeneratedInclSynth(this)) and
-      toGeneratedInclSynth(result) = s.getEnclosingModule()
-    )
-  }
+  ModuleBase getEnclosingModule() { result = getEnclosingModule(scopeOfInclSynth(this)) }
 
   /** Gets the enclosing method, if any. */
-  MethodBase getEnclosingMethod() {
-    exists(Scope::Range s |
-      s = scopeOf(toGeneratedInclSynth(this)) and
-      toGeneratedInclSynth(result) = s.getEnclosingMethod()
-    )
-  }
+  MethodBase getEnclosingMethod() { result = getEnclosingMethod(scopeOfInclSynth(this)) }
 
   /** Gets a textual representation of this node. */
   cached

ruby/ql/lib/codeql/ruby/ast/Method.qll

@@ -183,8 +183,6 @@ class Block extends Callable, StmtSequence, Scope, TBlock {
     or
     result = StmtSequence.super.getAChild(pred)
   }
-
-  override string getAPrimaryQlClass() { result = "Block" }
 }
 
 /** A block enclosed within `do` and `end`. */
@@ -215,16 +213,6 @@ class DoBlock extends Block, BodyStmt, TDoBlock {
  * ```
  */
 class BraceBlock extends Block, TBraceBlock {
-  private Ruby::Block g;
-
-  BraceBlock() { this = TBraceBlock(g) }
-
-  final override Parameter getParameter(int n) {
-    toGenerated(result) = g.getParameters().getChild(n)
-  }
-
-  final override Stmt getStmt(int i) { toGenerated(result) = g.getChild(i) }
-
   final override string toString() { result = "{ ... }" }
 
   final override string getAPrimaryQlClass() { result = "BraceBlock" }

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

@@ -93,8 +93,8 @@ private module Cached {
     } or
     TBlockArgument(Ruby::BlockArgument g) or
     TBlockParameter(Ruby::BlockParameter g) or
-    TBlockSynth(AST::AstNode parent, int i) { mkSynthChild(BlockKind(), parent, i) } or
-    TBraceBlock(Ruby::Block g) { not g.getParent() instanceof Ruby::Lambda } or
+    TBraceBlockSynth(AST::AstNode parent, int i) { mkSynthChild(BraceBlockKind(), parent, i) } or
+    TBraceBlockReal(Ruby::Block g) { not g.getParent() instanceof Ruby::Lambda } or
     TBreakStmt(Ruby::Break g) or
     TCaseEqExpr(Ruby::Binary g) { g instanceof @ruby_binary_equalequalequal } or
     TCaseExpr(Ruby::Case g) or
@@ -362,7 +362,7 @@ private module Cached {
     n = TBitwiseXorExprReal(result) or
     n = TBlockArgument(result) or
     n = TBlockParameter(result) or
-    n = TBraceBlock(result) or
+    n = TBraceBlockReal(result) or
     n = TBreakStmt(result) or
     n = TCaseEqExpr(result) or
     n = TCaseExpr(result) or
@@ -490,7 +490,7 @@ private module Cached {
     or
     result = TBitwiseXorExprSynth(parent, i)
     or
-    result = TBlockSynth(parent, i)
+    result = TBraceBlockSynth(parent, i)
     or
     result = TClassVariableAccessSynth(parent, i, _)
     or
@@ -645,7 +645,9 @@ class TCallable = TMethodBase or TLambda or TBlock;
 
 class TMethodBase = TMethod or TSingletonMethod;
 
-class TBlock = TDoBlock or TBraceBlock or TBlockSynth;
+class TBraceBlock = TBraceBlockReal or TBraceBlockSynth;
+
+class TBlock = TDoBlock or TBraceBlock;
 
 class TModuleBase = TToplevel or TNamespace or TSingletonClass;
 

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

@@ -2,19 +2,27 @@ private import codeql.ruby.AST
 private import AST
 private import TreeSitter
 
+class BraceBlockReal extends BraceBlock, TBraceBlockReal {
+  private Ruby::Block g;
+
+  BraceBlockReal() { this = TBraceBlockReal(g) }
+
+  final override Parameter getParameter(int n) {
+    toGenerated(result) = g.getParameters().getChild(n)
+  }
+
+  final override Stmt getStmt(int i) { toGenerated(result) = g.getChild(i) }
+}
+
 /**
  * A synthesized block, such as the block synthesized from the body of
  * a `for` loop.
  */
-class SynthBlock extends Block, TBlockSynth {
-  SynthBlock() { this = TBlockSynth(_, _) }
-
+class BraceBlockSynth extends BraceBlock, TBraceBlockSynth {
   final override Parameter getParameter(int n) { synthChild(this, n, result) }
 
   final override Stmt getStmt(int i) {
     i >= 0 and
     synthChild(this, i + this.getNumberOfParameters(), result)
   }
-
-  final override string toString() { result = "{ ... }" }
 }

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

@@ -19,8 +19,6 @@ private class TModuleLike = TToplevel or TModuleDeclaration or TClassDeclaration
 
 private class TScopeReal = TMethodBase or TModuleLike or TDoBlock or TLambda or TBraceBlock;
 
-private class TScopeSynth = TBlockSynth;
-
 module Scope {
   class TypeRange = Callable::TypeRange or ModuleBase::TypeRange or @ruby_end_block;
 
@@ -108,35 +106,74 @@ Ruby::HeredocBody getHereDocBody(Ruby::HeredocBeginning g) {
   )
 }
 
+private Ruby::AstNode specialParentOf(Ruby::AstNode n) {
+  n =
+    [
+      result.(Ruby::Module).getName(), result.(Ruby::Class).getName(),
+      result.(Ruby::Class).getSuperclass(), result.(Ruby::SingletonClass).getValue(),
+      result.(Ruby::Method).getName(), result.(Ruby::SingletonMethod).getName(),
+      result.(Ruby::SingletonMethod).getObject()
+    ]
+}
+
 private Ruby::AstNode parentOf(Ruby::AstNode n) {
   n = getHereDocBody(result)
   or
-  exists(Ruby::AstNode parent | parent = n.getParent() |
-    if
-      n =
-        [
-          parent.(Ruby::Module).getName(), parent.(Ruby::Class).getName(),
-          parent.(Ruby::Class).getSuperclass(), parent.(Ruby::SingletonClass).getValue(),
-          parent.(Ruby::Method).getName(), parent.(Ruby::SingletonMethod).getName(),
-          parent.(Ruby::SingletonMethod).getObject()
-        ]
-    then result = parent.getParent()
-    else result = parent
-  )
+  result = specialParentOf(n).getParent()
+  or
+  not exists(specialParentOf(n)) and
+  result = n.getParent()
 }
 
-/** Gets the enclosing scope of a node */
-cached
-Scope::Range scopeOf(Ruby::AstNode n) {
-  exists(Ruby::AstNode p | p = parentOf(n) |
-    p = result
-    or
-    not p instanceof Scope::Range and result = scopeOf(p)
-  )
+private AstNode specialParentOfInclSynth(AstNode n) {
+  n =
+    [
+      result.(Namespace).getScopeExpr(), result.(ClassDeclaration).getSuperclassExpr(),
+      result.(SingletonMethod).getObject()
+    ]
 }
 
+private AstNode parentOfInclSynth(AstNode n) {
+  (
+    result = specialParentOfInclSynth(n).getParent()
+    or
+    not exists(specialParentOfInclSynth(n)) and
+    result = n.getParent()
+  ) and
+  (synthChild(_, _, n) implies synthChild(result, _, n))
+}
+
+cached
+private module Cached {
+  /** Gets the enclosing scope of a node */
+  cached
+  Scope::Range scopeOf(Ruby::AstNode n) {
+    exists(Ruby::AstNode p | p = parentOf(n) |
+      p = result
+      or
+      not p instanceof Scope::Range and result = scopeOf(p)
+    )
+  }
+
+  /**
+   * Gets the enclosing scope of a node. Unlike `scopeOf`, this predicate
+   * operates on the external AST API, and therefore takes synthesized nodes
+   * and synthesized scopes into account.
+   */
+  cached
+  Scope scopeOfInclSynth(AstNode n) {
+    exists(AstNode p | p = parentOfInclSynth(n) |
+      p = result
+      or
+      not p instanceof Scope and result = scopeOfInclSynth(p)
+    )
+  }
+}
+
+import Cached
+
 abstract class ScopeImpl extends AstNode, TScopeType {
-  abstract Scope getOuterScopeImpl();
+  final Scope getOuterScopeImpl() { result = scopeOfInclSynth(this) }
 
   abstract Variable getAVariableImpl();
 
@@ -151,8 +188,6 @@ private class ScopeRealImpl extends ScopeImpl, TScopeReal {
 
   ScopeRealImpl() { range = toGenerated(this) }
 
-  override Scope getOuterScopeImpl() { toGenerated(result) = range.getOuterScope() }
-
   override Variable getAVariableImpl() { result.getDeclaringScope() = this }
 }
 
@@ -161,17 +196,15 @@ private class ScopeRealImpl extends ScopeImpl, TScopeReal {
 // in practice there is not a real nested scope created, so variables that
 // may appear to be local to the loop body (e.g. the iteration variable) are
 // scoped to the outer scope rather than the loop body.
-private class ScopeSynthImpl extends ScopeImpl, TScopeSynth {
-  ScopeSynthImpl() { this = TBlockSynth(_, _) }
-
-  override Scope getOuterScopeImpl() { scopeOf(toGeneratedInclSynth(this)) = toGenerated(result) }
+private class ScopeSynthImpl extends ScopeImpl, TBraceBlockSynth {
+  ScopeSynthImpl() { this = TBraceBlockSynth(_, _) }
 
   override Variable getAVariableImpl() {
     // Synthesized variables introduced as parameters to this scope
     // As this variable is also synthetic, it is genuinely local to this scope.
-    exists(SimpleParameter p | p = TSimpleParameterSynth(this, _) |
-      p.getVariable() = result and
-      exists(TLocalVariableAccessSynth(p, _, result))
+    exists(SimpleParameterSynthImpl p |
+      p = TSimpleParameterSynth(this, _) and
+      p.getVariableImpl() = result
     )
   }
 }

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

@@ -15,7 +15,7 @@ newtype SynthKind =
   BitwiseAndExprKind() or
   BitwiseOrExprKind() or
   BitwiseXorExprKind() or
-  BlockKind() or
+  BraceBlockKind() or
   ClassVariableAccessKind(ClassVariable v) or
   DivExprKind() or
   ExponentExprKind() or
@@ -834,6 +834,7 @@ private module ArrayLiteralDesugar {
  * scoped to the synthesized block.
  */
 private module ForLoopDesugar {
+  pragma[nomagic]
   private predicate forLoopSynthesis(AstNode parent, int i, Child child) {
     exists(ForExpr for |
       // each call
@@ -849,9 +850,9 @@ private module ForLoopDesugar {
         or
         parent = eachCall and
         i = -2 and
-        child = SynthChild(BlockKind())
+        child = SynthChild(BraceBlockKind())
         or
-        exists(Block block | block = TBlockSynth(eachCall, -2) |
+        exists(Block block | block = TBraceBlockSynth(eachCall, -2) |
           // block params
           parent = block and
           i = 0 and

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

@@ -327,7 +327,7 @@ private module Cached {
 
   cached
   predicate isCapturedAccess(LocalVariableAccess access) {
-    toGenerated(access.getVariable().getDeclaringScope()) != scopeOf(toGenerated(access))
+    access.getVariable().getDeclaringScope() != access.getCfgScope()
   }
 
   cached
@@ -524,7 +524,7 @@ private class LocalVariableAccessReal extends LocalVariableAccessImpl, TLocalVar
   final override string toString() { result = g.getValue() }
 }
 
-private class LocalVariableAccessSynth extends LocalVariableAccessImpl, TLocalVariableAccessSynth {
+class LocalVariableAccessSynth extends LocalVariableAccessImpl, TLocalVariableAccessSynth {
   private LocalVariable v;
 
   LocalVariableAccessSynth() { this = TLocalVariableAccessSynth(_, _, v) }

ruby/ql/lib/codeql/ruby/controlflow/internal/ControlFlowGraphImpl.qll

@@ -1050,7 +1050,7 @@ private module Cached {
   cached
   CfgScope getCfgScopeImpl(AstNode n) {
     forceCachingInSameStage() and
-    result = parent*(ASTInternal::fromGenerated(scopeOf(ASTInternal::toGeneratedInclSynth(n))))
+    result = parent*(scopeOfInclSynth(n))
   }
 
   cached