Skip to content

Commit af428a1

Browse files
committed
Address comments
1 parent b0a97f9 commit af428a1

File tree

2 files changed

+19
-11
lines changed

2 files changed

+19
-11
lines changed

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,21 @@ class SetterMethodCall extends MethodCall, TMethodCallSynth {
145145
SetterMethodCall() { this = TMethodCallSynth(_, _, _, true, _) }
146146

147147
final override string getAPrimaryQlClass() { result = "SetterMethodCall" }
148+
149+
/**
150+
* Gets the name of the method being called without the trailing `=`. For example, in the following
151+
* two statements the target name is `value`:
152+
* ```rb
153+
* foo.value=(1)
154+
* foo.value = 1
155+
* ```
156+
*/
157+
final string getTargetName() {
158+
exists(string methodName |
159+
methodName = this.getMethodName() and
160+
result = methodName.prefix(methodName.length() - 1)
161+
)
162+
}
148163
}
149164

150165
/**

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPrivate.qll

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,7 @@ private module Cached {
352352
TFieldContent(string name) {
353353
name = any(InstanceVariable v).getName()
354354
or
355-
exists(SetterMethodCall c, string methodName |
356-
methodName = c.getMethodName() and
357-
name = "@" + methodName.prefix(methodName.length() - 1)
358-
)
355+
name = "@" + any(SetterMethodCall c).getTargetName()
359356
}
360357

361358
/**
@@ -835,14 +832,10 @@ predicate storeStep(Node node1, ContentSet c, Node node2) {
835832
// Attribute assignment, `receiver.property = value`
836833
node2.(PostUpdateNode).getPreUpdateNode().asExpr() =
837834
any(CfgNodes::ExprNodes::MethodCallCfgNode call |
838-
call.getExpr() instanceof SetterMethodCall and
839835
node1.asExpr() = call.getArgument(0) and
840836
call.getNumberOfArguments() = 1 and
841-
c.isSingleton(any(Content::FieldContent ct, string methodName |
842-
methodName = call.getExpr().getMethodName() and
843-
ct.getName() = "@" + methodName.prefix(methodName.length() - 1)
844-
|
845-
ct
837+
c.isSingleton(any(Content::FieldContent ct |
838+
ct.getName() = "@" + call.getExpr().(SetterMethodCall).getTargetName()
846839
))
847840
).getReceiver()
848841
or
@@ -884,7 +877,7 @@ predicate readStep(Node node1, ContentSet c, Node node2) {
884877
// Attribute read, `receiver.field`. Note that we do not check whether
885878
// the `field` method is really an attribute reader. This is probably fine
886879
// because the read step has only effect if there exists a matching store step
887-
// (instance variable assignmentor setter method call).
880+
// (instance variable assignment or setter method call).
888881
node2.asExpr() =
889882
any(CfgNodes::ExprNodes::MethodCallCfgNode call |
890883
node1.asExpr() = call.getReceiver() and

0 commit comments

Comments
 (0)