From c3196805897689f056476c0064a82de123e37ee8 Mon Sep 17 00:00:00 2001
From: Tom Hvitved <hvitved@github.com>
Date: Fri, 29 May 2026 09:59:26 +0200
Subject: [PATCH] Ruby: Fix bug in `implicitAssignmentNode`

---
 .../lib/codeql/ruby/ast/internal/Variable.qll | 24 ++++++++++---------
 .../test/library-tests/variables/ssa.expected |  2 ++
 .../variables/varaccess.expected              |  4 +---
 .../library-tests/variables/variable.expected |  1 -
 4 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll b/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll
index 6e92b54c246..5ff48191534 100644
--- a/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll
+++ b/ruby/ql/lib/codeql/ruby/ast/internal/Variable.qll
@@ -11,6 +11,17 @@ private import codeql.ruby.ast.internal.Pattern
 private import codeql.ruby.ast.internal.Scope
 private import codeql.ruby.ast.internal.Synthesis
 
+private Ruby::AstNode getAssignmentParent(Ruby::AstNode n) {
+  result = n.getParent() and
+  (
+    result instanceof Ruby::DestructuredLeftAssignment
+    or
+    result instanceof Ruby::LeftAssignmentList
+    or
+    result instanceof Ruby::RestAssignment
+  )
+}
+
 /**
  * Holds if `n` is in the left-hand-side of an explicit assignment `assignment`.
  */
@@ -19,16 +30,7 @@ predicate explicitAssignmentNode(Ruby::AstNode n, Ruby::AstNode assignment) {
   or
   n = assignment.(Ruby::OperatorAssignment).getLeft()
   or
-  exists(Ruby::AstNode parent |
-    parent = n.getParent() and
-    explicitAssignmentNode(parent, assignment)
-  |
-    parent instanceof Ruby::DestructuredLeftAssignment
-    or
-    parent instanceof Ruby::LeftAssignmentList
-    or
-    parent instanceof Ruby::RestAssignment
-  )
+  explicitAssignmentNode(getAssignmentParent(n), assignment)
 }
 
 /** Holds if `n` is inside an implicit assignment. */
@@ -49,7 +51,7 @@ predicate implicitAssignmentNode(Ruby::AstNode n) {
   or
   n = any(Ruby::For for).getPattern()
   or
-  implicitAssignmentNode(n.getParent())
+  implicitAssignmentNode(getAssignmentParent(n))
 }
 
 /** Holds if `n` is inside a parameter. */
diff --git a/ruby/ql/test/library-tests/variables/ssa.expected b/ruby/ql/test/library-tests/variables/ssa.expected
index 7808d18dbbe..69222157b05 100644
--- a/ruby/ql/test/library-tests/variables/ssa.expected
+++ b/ruby/ql/test/library-tests/variables/ssa.expected
@@ -321,6 +321,7 @@ read
 | scopes.rb:80:3:82:5 | self (name=) | scopes.rb:80:3:82:5 | self | scopes.rb:81:5:81:9 | self |
 | scopes.rb:80:13:80:17 | value | scopes.rb:80:13:80:17 | value | scopes.rb:81:13:81:17 | value |
 | scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self |
+| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:86:13:86:16 | self |
 | scopes.rb:84:11:84:13 | msg | scopes.rb:84:11:84:13 | msg | scopes.rb:85:11:85:13 | msg |
 | ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self |
 | ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self |
@@ -592,6 +593,7 @@ adjacentReads
 | scopes.rb:51:1:64:3 | self (ExceptionVariable) | scopes.rb:51:1:64:3 | self | scopes.rb:59:5:59:21 | self | scopes.rb:61:5:61:10 | self |
 | scopes.rb:51:1:64:3 | self (ExceptionVariable) | scopes.rb:51:1:64:3 | self | scopes.rb:61:5:61:10 | self | scopes.rb:63:3:63:8 | self |
 | scopes.rb:60:25:60:25 | x | scopes.rb:55:3:55:3 | x | scopes.rb:61:10:61:10 | x | scopes.rb:63:8:63:8 | x |
+| scopes.rb:84:3:88:5 | self (foo) | scopes.rb:84:3:88:5 | self | scopes.rb:85:5:85:13 | self | scopes.rb:86:13:86:16 | self |
 | ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:3:3:3:8 | self | ssa.rb:4:3:4:12 | self |
 | ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self | ssa.rb:7:5:7:10 | self |
 | ssa.rb:1:1:16:3 | self (m) | ssa.rb:1:1:16:3 | self | ssa.rb:4:3:4:12 | self | ssa.rb:11:5:11:10 | self |
diff --git a/ruby/ql/test/library-tests/variables/varaccess.expected b/ruby/ql/test/library-tests/variables/varaccess.expected
index 1cd9882283c..22f37fda64c 100644
--- a/ruby/ql/test/library-tests/variables/varaccess.expected
+++ b/ruby/ql/test/library-tests/variables/varaccess.expected
@@ -226,7 +226,6 @@ variableAccess
 | scopes.rb:85:5:85:13 | self | scopes.rb:84:3:88:5 | self | scopes.rb:84:3:88:5 | foo |
 | scopes.rb:85:11:85:13 | msg | scopes.rb:84:11:84:13 | msg | scopes.rb:84:3:88:5 | foo |
 | scopes.rb:86:13:86:16 | self | scopes.rb:84:3:88:5 | self | scopes.rb:84:3:88:5 | foo |
-| scopes.rb:86:18:86:21 | name | scopes.rb:86:18:86:21 | name | scopes.rb:84:3:88:5 | foo |
 | ssa.rb:1:7:1:7 | b | ssa.rb:1:7:1:7 | b | ssa.rb:1:1:16:3 | m |
 | ssa.rb:2:3:2:3 | i | ssa.rb:2:3:2:3 | i | ssa.rb:1:1:16:3 | m |
 | ssa.rb:3:3:3:8 | self | ssa.rb:1:1:16:3 | self | ssa.rb:1:1:16:3 | m |
@@ -436,8 +435,6 @@ implicitWrite
 | scopes.rb:69:15:69:15 | x |
 | scopes.rb:80:13:80:17 | value |
 | scopes.rb:84:11:84:13 | msg |
-| scopes.rb:86:13:86:16 | self |
-| scopes.rb:86:18:86:21 | name |
 | ssa.rb:1:7:1:7 | b |
 | ssa.rb:18:8:18:8 | x |
 | ssa.rb:25:8:25:15 | elements |
@@ -606,6 +603,7 @@ readAccess
 | scopes.rb:81:13:81:17 | value |
 | scopes.rb:85:5:85:13 | self |
 | scopes.rb:85:11:85:13 | msg |
+| scopes.rb:86:13:86:16 | self |
 | ssa.rb:3:3:3:8 | self |
 | ssa.rb:3:8:3:8 | i |
 | ssa.rb:4:3:4:12 | self |
diff --git a/ruby/ql/test/library-tests/variables/variable.expected b/ruby/ql/test/library-tests/variables/variable.expected
index e0873d046d3..32e4c87bb93 100644
--- a/ruby/ql/test/library-tests/variables/variable.expected
+++ b/ruby/ql/test/library-tests/variables/variable.expected
@@ -138,7 +138,6 @@
 | scopes.rb:80:13:80:17 | value |
 | scopes.rb:84:3:88:5 | self |
 | scopes.rb:84:11:84:13 | msg |
-| scopes.rb:86:18:86:21 | name |
 | ssa.rb:1:1:16:3 | self |
 | ssa.rb:1:1:103:3 | self |
 | ssa.rb:1:7:1:7 | b |