Skip to content

Commit 4964f86

Browse files
aibaarsaibaars
authored
Merge pull request github#12540 from aibaars/destructured-assign
Ruby: change evaluation order of destructured assignments
2 parents 6b26510 + 3b12ddf commit 4964f86

File tree

19 files changed

+404
-169
lines changed

19 files changed

+404
-169
lines changed

‎ruby/ql/lib/change-notes/2023-03-21-evaluation-order.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* Control flow graph: the evaluation order of scope expressions and receivers in multiple assignments has been adjusted to match the changes made in Ruby
5+
3.1 and 3.2.

‎ruby/ql/lib/codeql/ruby/ast/Constant.qll

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -235,55 +235,6 @@ class ConstantAccess extends Expr, TConstantAccess {
235235
}
236236
}
237237

238-
private class TokenConstantAccess extends ConstantAccess, TTokenConstantAccess {
239-
private Ruby::Constant g;
240-
241-
TokenConstantAccess() { this = TTokenConstantAccess(g) }
242-
243-
final override string getName() { result = g.getValue() }
244-
}
245-
246-
private class ScopeResolutionConstantAccess extends ConstantAccess, TScopeResolutionConstantAccess {
247-
private Ruby::ScopeResolution g;
248-
private Ruby::Constant constant;
249-
250-
ScopeResolutionConstantAccess() { this = TScopeResolutionConstantAccess(g, constant) }
251-
252-
final override string getName() { result = constant.getValue() }
253-
254-
final override Expr getScopeExpr() { toGenerated(result) = g.getScope() }
255-
256-
final override predicate hasGlobalScope() { not exists(g.getScope()) }
257-
}
258-
259-
private class ConstantReadAccessSynth extends ConstantAccess, TConstantReadAccessSynth {
260-
private string value;
261-
262-
ConstantReadAccessSynth() { this = TConstantReadAccessSynth(_, _, value) }
263-
264-
final override string getName() {
265-
if this.hasGlobalScope() then result = value.suffix(2) else result = value
266-
}
267-
268-
final override Expr getScopeExpr() { synthChild(this, 0, result) }
269-
270-
final override predicate hasGlobalScope() { value.matches("::%") }
271-
}
272-
273-
private class ConstantWriteAccessSynth extends ConstantAccess, TConstantWriteAccessSynth {
274-
private string value;
275-
276-
ConstantWriteAccessSynth() { this = TConstantWriteAccessSynth(_, _, value) }
277-
278-
final override string getName() {
279-
if this.hasGlobalScope() then result = value.suffix(2) else result = value
280-
}
281-
282-
final override Expr getScopeExpr() { synthChild(this, 0, result) }
283-
284-
final override predicate hasGlobalScope() { value.matches("::%") }
285-
}
286-
287238
/**
288239
* A use (read) of a constant.
289240
*

‎ruby/ql/lib/codeql/ruby/ast/internal/Constant.qll

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
private import codeql.ruby.AST
2+
private import codeql.ruby.ast.internal.AST
23
private import codeql.ruby.ast.internal.Literal
34
private import codeql.ruby.ast.internal.Module
5+
private import codeql.ruby.ast.internal.TreeSitter
46
private import codeql.ruby.controlflow.CfgNodes
57
private import codeql.ruby.dataflow.SSA
68
private import ExprNodes
@@ -559,3 +561,60 @@ private predicate isArrayExpr(Expr e, ArrayLiteralCfgNode arr) {
559561
// results if the source is a phi node.
560562
forex(ExprCfgNode n | n = e.getAControlFlowNode() | isArrayConstant(n, arr))
561563
}
564+
565+
private class TokenConstantAccess extends ConstantAccess, TTokenConstantAccess {
566+
private Ruby::Constant g;
567+
568+
TokenConstantAccess() { this = TTokenConstantAccess(g) }
569+
570+
final override string getName() { result = g.getValue() }
571+
}
572+
573+
/**
574+
* A constant access that has a scope resolution qualifier.
575+
*/
576+
class ScopeResolutionConstantAccess extends ConstantAccess, TScopeResolutionConstantAccess {
577+
private Ruby::ScopeResolution g;
578+
private Ruby::Constant constant;
579+
580+
ScopeResolutionConstantAccess() { this = TScopeResolutionConstantAccess(g, constant) }
581+
582+
/**
583+
* Gets the name of the constant.
584+
*/
585+
final override string getName() { result = constant.getValue() }
586+
587+
/** Gets the scope resolution expression. */
588+
final override Expr getScopeExpr() { toGenerated(result) = g.getScope() }
589+
590+
/** Holds if this constant access has a global scope. */
591+
final override predicate hasGlobalScope() { not exists(g.getScope()) }
592+
}
593+
594+
private class ConstantReadAccessSynth extends ConstantAccess, TConstantReadAccessSynth {
595+
private string value;
596+
597+
ConstantReadAccessSynth() { this = TConstantReadAccessSynth(_, _, value) }
598+
599+
final override string getName() {
600+
if this.hasGlobalScope() then result = value.suffix(2) else result = value
601+
}
602+
603+
final override Expr getScopeExpr() { synthChild(this, 0, result) }
604+
605+
final override predicate hasGlobalScope() { value.matches("::%") }
606+
}
607+
608+
private class ConstantWriteAccessSynth extends ConstantAccess, TConstantWriteAccessSynth {
609+
private string value;
610+
611+
ConstantWriteAccessSynth() { this = TConstantWriteAccessSynth(_, _, value) }
612+
613+
final override string getName() {
614+
if this.hasGlobalScope() then result = value.suffix(2) else result = value
615+
}
616+
617+
final override Expr getScopeExpr() { synthChild(this, 0, result) }
618+
619+
final override predicate hasGlobalScope() { value.matches("::%") }
620+
}

‎ruby/ql/lib/codeql/ruby/ast/internal/Expr.qll

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@ private import AST
44
private import TreeSitter
55

66
class StmtSequenceSynth extends StmtSequence, TStmtSequenceSynth {
7-
final override Stmt getStmt(int n) { synthChild(this, n, result) }
7+
final override Stmt getStmt(int n) {
8+
result = rank[n + 1](int i, Stmt s | synthChild(this, i, s) | s order by i)
9+
}
810

911
final override string toString() { result = "..." }
1012
}

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

Lines changed: 103 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
private import AST
44
private import TreeSitter
55
private import codeql.ruby.ast.internal.Call
6+
private import codeql.ruby.ast.internal.Constant
67
private import codeql.ruby.ast.internal.Expr
78
private import codeql.ruby.ast.internal.Variable
89
private import codeql.ruby.ast.internal.Pattern
@@ -950,6 +951,28 @@ private module DestructuredAssignDesugar {
950951
}
951952
}
952953

954+
abstract private class LhsWithReceiver extends Expr {
955+
abstract Expr getReceiver();
956+
957+
abstract SynthKind getSynthKind();
958+
}
959+
960+
private class LhsCall extends LhsWithReceiver instanceof MethodCall {
961+
final override Expr getReceiver() { result = MethodCall.super.getReceiver() }
962+
963+
final override SynthKind getSynthKind() {
964+
result = MethodCallKind(super.getMethodName(), false, super.getNumberOfArguments())
965+
}
966+
}
967+
968+
private class LhsScopedConstant extends LhsWithReceiver, ScopeResolutionConstantAccess {
969+
LhsScopedConstant() { exists(this.getScopeExpr()) }
970+
971+
final override Expr getReceiver() { result = this.getScopeExpr() }
972+
973+
final override SynthKind getSynthKind() { result = ConstantWriteAccessKind(this.getName()) }
974+
}
975+
953976
pragma[nomagic]
954977
private predicate destructuredAssignSynthesis(AstNode parent, int i, Child child) {
955978
exists(DestructuredAssignExpr tae |
@@ -958,14 +981,32 @@ private module DestructuredAssignDesugar {
958981
child = SynthChild(StmtSequenceKind())
959982
or
960983
exists(AstNode seq | seq = TStmtSequenceSynth(tae, -1) |
984+
exists(LhsWithReceiver mc, int j | mc = tae.getElement(j) |
985+
parent = seq and
986+
i = j and
987+
child = SynthChild(AssignExprKind())
988+
or
989+
exists(AstNode assign | assign = TAssignExprSynth(seq, j) |
990+
parent = assign and
991+
i = 0 and
992+
child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(tae, j)))
993+
or
994+
parent = assign and
995+
i = 1 and
996+
child = childRef(mc.getReceiver())
997+
)
998+
)
999+
or
9611000
parent = seq and
962-
i = 0 and
1001+
i = tae.getNumberOfElements() and
9631002
child = SynthChild(AssignExprKind())
9641003
or
965-
exists(AstNode assign | assign = TAssignExprSynth(seq, 0) |
1004+
exists(AstNode assign | assign = TAssignExprSynth(seq, tae.getNumberOfElements()) |
9661005
parent = assign and
9671006
i = 0 and
968-
child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(tae, 0)))
1007+
child =
1008+
SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(tae,
1009+
tae.getNumberOfElements())))
9691010
or
9701011
parent = assign and
9711012
i = 1 and
@@ -981,10 +1022,37 @@ private module DestructuredAssignDesugar {
9811022
restIndex = tae.getRestIndexOrNumberOfElements()
9821023
|
9831024
parent = seq and
984-
i = j + 1 and
1025+
i = j + 1 + tae.getNumberOfElements() and
9851026
child = SynthChild(AssignExprKind())
9861027
or
987-
exists(AstNode assign | assign = TAssignExprSynth(seq, j + 1) |
1028+
exists(AstNode assign |
1029+
assign = TAssignExprSynth(seq, j + 1 + tae.getNumberOfElements())
1030+
|
1031+
exists(LhsWithReceiver mc | mc = elem |
1032+
parent = assign and
1033+
i = 0 and
1034+
child = SynthChild(mc.getSynthKind())
1035+
or
1036+
exists(AstNode call | synthChild(assign, 0, call) |
1037+
parent = call and
1038+
i = 0 and
1039+
child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(tae, j)))
1040+
or
1041+
parent = call and
1042+
child = childRef(mc.(MethodCall).getArgument(i - 1))
1043+
)
1044+
)
1045+
or
1046+
(
1047+
elem instanceof VariableAccess
1048+
or
1049+
elem instanceof ConstantAccess and
1050+
not exists(Ruby::ScopeResolution g |
1051+
elem = TScopeResolutionConstantAccess(g, _) and exists(g.getScope())
1052+
)
1053+
or
1054+
elem instanceof DestructuredLhsExpr
1055+
) and
9881056
parent = assign and
9891057
i = 0 and
9901058
child = childRef(elem)
@@ -995,7 +1063,9 @@ private module DestructuredAssignDesugar {
9951063
or
9961064
parent = TMethodCallSynth(assign, 1, _, _, _) and
9971065
i = 0 and
998-
child = SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(tae, 0)))
1066+
child =
1067+
SynthChild(LocalVariableAccessSynthKind(TLocalVariableSynth(tae,
1068+
tae.getNumberOfElements())))
9991069
or
10001070
j < restIndex and
10011071
parent = TMethodCallSynth(assign, 1, _, _, _) and
@@ -1050,26 +1120,48 @@ private module DestructuredAssignDesugar {
10501120

10511121
final override predicate location(AstNode n, Location l) {
10521122
exists(DestructuredAssignExpr tae, StmtSequence seq | seq = tae.getDesugared() |
1053-
n = seq.getStmt(0) and
1123+
synthChild(seq, tae.getNumberOfElements(), n) and
10541124
hasLocation(tae.getRightOperand(), l)
10551125
or
1056-
exists(AstNode elem, int j |
1126+
exists(LhsWithReceiver elem, int j |
10571127
elem = tae.getElement(j) and
1058-
n = seq.getStmt(j + 1) and
1128+
synthChild(seq, j, n) and
1129+
hasLocation(elem.getReceiver(), l)
1130+
)
1131+
or
1132+
exists(AstNode elem, int j | elem = tae.getElement(j) |
1133+
synthChild(seq, j + 1 + tae.getNumberOfElements(), n) and
10591134
hasLocation(elem, l)
10601135
)
10611136
)
10621137
}
10631138

10641139
final override predicate localVariable(AstNode n, int i) {
1065-
n instanceof DestructuredAssignExpr and
1066-
i = 0
1140+
i = [0 .. n.(DestructuredAssignExpr).getNumberOfElements()]
1141+
}
1142+
1143+
final override predicate constantWriteAccess(string name) {
1144+
exists(DestructuredAssignExpr tae, LhsScopedConstant ca |
1145+
ca = tae.getElement(_) and
1146+
name = ca.getName()
1147+
)
10671148
}
10681149

10691150
final override predicate methodCall(string name, boolean setter, int arity) {
10701151
name = "[]" and
10711152
setter = false and
10721153
arity = 1
1154+
or
1155+
exists(DestructuredAssignExpr tae, MethodCall mc |
1156+
mc = tae.getElement(_) and
1157+
name = mc.getMethodName() and
1158+
setter = false and
1159+
arity = mc.getNumberOfArguments()
1160+
)
1161+
}
1162+
1163+
final override predicate excludeFromControlFlowTree(AstNode n) {
1164+
n = any(DestructuredAssignExpr tae).getElement(_).(LhsWithReceiver)
10731165
}
10741166
}
10751167
}

‎ruby/ql/test/library-tests/ast/Ast.expected

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2878,6 +2878,16 @@ operations/operations.rb:
28782878
# 103| getStmt: [AssignLogicalOrExpr] ... ||= ...
28792879
# 103| getAnOperand/getLeftOperand: [ConstantAssignment, ConstantReadAccess] CONSTANT4
28802880
# 103| getAnOperand/getRightOperand: [IntegerLiteral] 7
2881+
# 104| getStmt: [AssignExpr] ... = ...
2882+
# 104| getAnOperand/getLeftOperand: [DestructuredLhsExpr] (..., ...)
2883+
# 104| getElement: [ConstantAssignment] FOO
2884+
# 104| getElement: [ConstantAssignment] BAR
2885+
# 104| getElement: [ConstantAssignment] FOO
2886+
# 104| getScopeExpr: [LocalVariableAccess] foo
2887+
# 104| getAnOperand/getRightOperand: [ArrayLiteral] [...]
2888+
# 104| getElement: [IntegerLiteral] 1
2889+
# 104| getElement: [IntegerLiteral] 2
2890+
# 104| getElement: [IntegerLiteral] 3
28812891
params/params.rb:
28822892
# 1| [Toplevel] params.rb
28832893
# 4| getStmt: [Method] identifier_method_params

0 commit comments

Comments
 (0)