Skip to content

Commit a8cc669

Browse files
committed
Ruby: Address review comments
1 parent 9004e82 commit a8cc669

File tree

2 files changed

+13
-11
lines changed

2 files changed

+13
-11
lines changed

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -208,12 +208,12 @@ private predicate selfInModule(SelfVariable self, Module m) {
208208
)
209209
}
210210

211-
/** Holds if `self` belongs to a method inside module `m`. */
211+
/** Holds if `self` belongs to method `method` inside module `m`. */
212212
pragma[nomagic]
213-
private predicate selfInMethod(SelfVariable self, Module m) {
214-
exists(Scope scope, ModuleBase encl |
215-
scope = self.getDeclaringScope() and
216-
encl = scope.(MethodBase).getEnclosingModule() and
213+
private predicate selfInMethod(SelfVariable self, MethodBase method, Module m) {
214+
exists(ModuleBase encl |
215+
method = self.getDeclaringScope() and
216+
encl = method.getEnclosingModule() and
217217
if encl instanceof SingletonClass
218218
then m = encl.getEnclosingModule().getModule()
219219
else m = encl.getModule()
@@ -354,7 +354,7 @@ private module Cached {
354354
// end
355355
// end
356356
// ```
357-
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), m)
357+
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), _, m)
358358
)
359359
)
360360
or
@@ -510,15 +510,17 @@ private DataFlow::Node trackInstance(Module tp, boolean exact, TypeTracker t) {
510510
selfInModule(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
511511
or
512512
// `self.new` inside a (singleton) method
513-
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), tp)
513+
selfInMethod(sourceNode.(SsaSelfDefinitionNode).getVariable(), _, tp)
514514
)
515515
or
516516
// `self` reference in method or top-level (but not in module, where instance
517517
// methods cannot be called; only singleton methods)
518518
result =
519519
any(SsaSelfDefinitionNode self |
520-
selfInMethod(self.getVariable(), tp) and
521-
exact = false
520+
exists(MethodBase m |
521+
selfInMethod(self.getVariable(), m, tp) and
522+
if m.getEnclosingModule() instanceof Toplevel then exact = true else exact = false
523+
)
522524
or
523525
selfInToplevel(self.getVariable(), tp) and
524526
exact = true
@@ -535,7 +537,7 @@ private DataFlow::Node trackInstance(Module tp, boolean exact, TypeTracker t) {
535537
selfInModule(result.(SsaSelfDefinitionNode).getVariable(), m)
536538
or
537539
// needed for e.g. `self.puts`
538-
selfInMethod(result.(SsaSelfDefinitionNode).getVariable(), m)
540+
selfInMethod(result.(SsaSelfDefinitionNode).getVariable(), any(SingletonMethod sm), m)
539541
)
540542
or
541543
// `in C => c then c.foo`

ruby/ql/test/library-tests/modules/callgraph.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ getTarget
1111
| calls.rb:28:5:28:15 | call to singleton_m | calls.rb:23:5:23:29 | singleton_m |
1212
| calls.rb:29:5:29:20 | call to singleton_m | calls.rb:23:5:23:29 | singleton_m |
1313
| calls.rb:33:1:33:13 | call to singleton_m | calls.rb:23:5:23:29 | singleton_m |
14-
| calls.rb:36:5:36:14 | call to instance_m | calls.rb:22:5:22:23 | instance_m |
1514
| calls.rb:40:5:40:13 | call to include | calls.rb:103:5:103:20 | include |
1615
| calls.rb:48:9:48:18 | call to instance_m | calls.rb:22:5:22:23 | instance_m |
1716
| calls.rb:49:9:49:23 | call to instance_m | calls.rb:22:5:22:23 | instance_m |
@@ -178,6 +177,7 @@ unresolvedCall
178177
| calls.rb:25:5:25:14 | call to instance_m |
179178
| calls.rb:26:5:26:19 | call to instance_m |
180179
| calls.rb:32:1:32:12 | call to instance_m |
180+
| calls.rb:36:5:36:14 | call to instance_m |
181181
| calls.rb:41:5:41:14 | call to instance_m |
182182
| calls.rb:42:5:42:19 | call to instance_m |
183183
| calls.rb:44:5:44:15 | call to singleton_m |

0 commit comments

Comments
 (0)