Improve desugaring of setter assignments

Author: hvitved

ql/src/codeql_ruby/ast/internal/Synthesis.qll

@@ -154,39 +154,73 @@ private module SetterDesugar {
    * desugars to
    *
    * ```rb
-   * x.foo=(y)
+   * x.foo=(__synth_0 = y);
+   * __synth_0;
    * ```
    */
   private class SetterMethodCallSynthesis extends Synthesis {
     final override predicate child(AstNode parent, int i, Child child, LocationOption l) {
       exists(AssignExpr ae, MethodCall mc | mc = ae.getLeftOperand() |
         parent = ae and
         i = -1 and
-        child = SynthChild(getCallKind(mc)) and
-        l = getSomeLocation(mc)
+        child = SynthChild(StmtSequenceKind()) and
+        l = NoneLocation()
         or
-        parent = getSynthChild(ae, -1) and
-        l = NoneLocation() and
-        (
+        exists(AstNode seq | seq = getSynthChild(ae, -1) |
+          parent = seq and
           i = 0 and
-          child = RealChild(mc.getReceiver())
-          or
-          child = RealChild(mc.getArgument(i - 1))
+          child = SynthChild(getCallKind(mc)) and
+          l = getSomeLocation(mc)
           or
-          i = mc.getNumberOfArguments() + 1 and
-          child = RealChild(ae.getRightOperand())
+          exists(AstNode call | call = getSynthChild(seq, 0) |
+            parent = call and
+            i = 0 and
+            child = RealChild(mc.getReceiver()) and
+            l = NoneLocation()
+            or
+            parent = call and
+            child = RealChild(mc.getArgument(i - 1)) and
+            l = NoneLocation()
+            or
+            l = getSomeLocation(mc) and
+            exists(int valueIndex | valueIndex = mc.getNumberOfArguments() + 1 |
+              parent = call and
+              i = valueIndex and
+              child = SynthChild(AssignExprKind())
+              or
+              parent = getSynthChild(call, valueIndex) and
+              (
+                i = 0 and
+                child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(ae, 0)))
+                or
+                i = 1 and
+                child = RealChild(ae.getRightOperand())
+              )
+            )
+            or
+            parent = call and
+            // special "number of arguments argument"; required to avoid non-monotonic recursion
+            i = -2 and
+            child = SynthChild(IntegerLiteralKind(mc.getNumberOfArguments() + 1)) and
+            l = NoneLocation()
+          )
           or
-          // special "number of arguments argument"; required to avoid non-monotonic recursion
-          i = -2 and
-          child = SynthChild(IntegerLiteralKind(mc.getNumberOfArguments() + 1)) and
-          l = NoneLocation()
+          parent = seq and
+          i = 1 and
+          child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(ae, 0))) and
+          l = getSomeLocation(mc)
         )
       )
     }
 
     final override predicate excludeFromControlFlowTree(AstNode n) {
       n.(MethodCall) = any(AssignExpr ae).getLeftOperand()
     }
+
+    final override predicate localVariable(AstNode n, int i) {
+      n.(AssignExpr).getLeftOperand() instanceof MethodCall and
+      i = 0
+    }
   }
 }
 

ql/test/library-tests/ast/AstDesugar.expected

@@ -11,16 +11,24 @@ calls/calls.rb:
 #   67|     getAnOperand/getLeftOperand: [LocalVariableAccess] var1
 #   67|     getAnOperand/getRightOperand: [MethodCall] call to bar
 #   67|       getReceiver: [ConstantReadAccess] X
-#  314| [SetterMethodCall] call to foo=
-#  314|   getReceiver: [Self] self
-#  314|   getNumberOfArguments: [IntegerLiteral] 1
-#  314|   getArgument: [IntegerLiteral] 10
-#  315| [ElementReference, SetterMethodCall] ...[...]
-#  315|   getReceiver: [MethodCall] call to foo
-#  315|     getReceiver: [Self] self
-#  315|   getNumberOfArguments: [IntegerLiteral] 2
-#  315|   getArgument: [IntegerLiteral] 0
-#  315|   getArgument: [IntegerLiteral] 10
+#  314| [StmtSequence] ...
+#  314|   getStmt: [SetterMethodCall] call to foo=
+#  314|     getReceiver: [Self] self
+#  314|     getNumberOfArguments: [IntegerLiteral] 1
+#  314|     getArgument: [AssignExpr] ... = ...
+#  314|       getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
+#  314|       getAnOperand/getRightOperand: [IntegerLiteral] 10
+#  314|   getStmt: [LocalVariableAccess] __synth__0
+#  315| [StmtSequence] ...
+#  315|   getStmt: [ElementReference, SetterMethodCall] ...[...]
+#  315|     getReceiver: [MethodCall] call to foo
+#  315|       getReceiver: [Self] self
+#  315|     getNumberOfArguments: [IntegerLiteral] 2
+#  315|     getArgument: [AssignExpr] ... = ...
+#  315|       getAnOperand/getLeftOperand: [LocalVariableAccess] __synth__0
+#  315|       getAnOperand/getRightOperand: [IntegerLiteral] 10
+#  315|     getArgument: [IntegerLiteral] 0
+#  315|   getStmt: [LocalVariableAccess] __synth__0
 #  318| [StmtSequence] ...
 #  318|   getStmt: [SetterMethodCall] call to count=
 #  318|     getNumberOfArguments: [IntegerLiteral] 2

ql/test/library-tests/ast/calls/calls.expected

@@ -31,10 +31,10 @@ callsWithArguments
 | calls.rb:293:5:293:33 | call to super | super | 0 | calls.rb:293:11:293:11 | 6 |
 | calls.rb:293:5:293:33 | call to super | super | 1 | calls.rb:293:14:293:14 | 7 |
 | calls.rb:311:1:311:7 | call to call | call | 0 | calls.rb:311:6:311:6 | 1 |
-| calls.rb:314:1:314:8 | call to foo= | foo= | 0 | calls.rb:314:12:314:13 | 10 |
+| calls.rb:314:1:314:8 | call to foo= | foo= | 0 | calls.rb:314:1:314:8 | ... = ... |
 | calls.rb:315:1:315:6 | ...[...] | []= | 0 | calls.rb:315:5:315:5 | 0 |
 | calls.rb:315:1:315:6 | ...[...] | []= | 0 | calls.rb:315:5:315:5 | 0 |
-| calls.rb:315:1:315:6 | ...[...] | []= | 1 | calls.rb:315:10:315:11 | 10 |
+| calls.rb:315:1:315:6 | ...[...] | []= | 1 | calls.rb:315:1:315:6 | ... = ... |
 | calls.rb:316:22:316:27 | ...[...] | []= | 0 | calls.rb:316:26:316:26 | 4 |
 | calls.rb:317:5:317:10 | ...[...] | []= | 0 | calls.rb:317:9:317:9 | 5 |
 | calls.rb:318:1:318:10 | call to count= | count= | 1 | calls.rb:318:12:318:13 | __synth__1 |

ql/test/library-tests/controlflow/graph/Cfg.expected

@@ -2127,17 +2127,29 @@ desugar.rb:
 #-----|  -> x
 
 #    6| call to foo
-#-----|  -> 1
+#-----|  -> __synth__0
 
 #    6| x
 #-----|  -> call to foo
 
-#    6| call to count=
+#    6| ...
 #-----|  -> exit m2 (normal)
 
-#    6| 1
+#    6| call to count=
+#-----|  -> __synth__0
+
+#    6| __synth__0
+#-----|  -> ...
+
+#    6| ... = ...
 #-----|  -> call to count=
 
+#    6| __synth__0
+#-----|  -> 1
+
+#    6| 1
+#-----|  -> ... = ...
+
 #    9| enter m3
 #-----|  -> x
 
@@ -2158,14 +2170,26 @@ desugar.rb:
 #   10| x
 #-----|  -> call to foo
 
-#   10| ...[...]
+#   10| ...
 #-----|  -> exit m3 (normal)
 
-#   10| 0
+#   10| ...[...]
+#-----|  -> __synth__0
+
+#   10| __synth__0
+#-----|  -> ...
+
+#   10| ... = ...
+#-----|  -> ...[...]
+
+#   10| __synth__0
 #-----|  -> 1
 
+#   10| 0
+#-----|  -> __synth__0
+
 #   10| 1
-#-----|  -> ...[...]
+#-----|  -> ... = ...
 
 #   13| enter m4
 #-----|  -> x