Skip to content

Commit 754bfdd

Browse files
committed
Ignore include/prepend statements in blocks
Include and prepend statements are rarely used in block in normal code and when used in normal code they tend to be in blocks that are passed to methods like `module_eval` which is a builtin method that evaluates a block in the context of some other module (typically created with Module.new). We currently don't attempt to track such "dynamically" constructed modules, and ignoring such modules and the `module_eval` calls on them seems fine for now. Another, much more frequent use of include/prepend statements in blocks is in Rspec.describe and Rspec.context method calls in tests. Rspec also evaluates those blocks in the context of some special Rspec class. Precisely tracking such calls during the initial construction of the module/class hierarchy would be really hard and there would be little benefit because the interesting modules and classes of an application are not defined in test files.
1 parent 280fe73 commit 754bfdd

2 files changed

Lines changed: 22 additions & 2 deletions

File tree

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

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
private import codeql.Locations
2+
private import codeql_ruby.AST
23
private import codeql_ruby.ast.Call
34
private import codeql_ruby.ast.Constant
45
private import codeql_ruby.ast.Expr
@@ -169,7 +170,7 @@ private class IncludeOrPrependCall extends MethodCall {
169170
string getTarget() {
170171
result = resolveScopeExpr(this.getReceiver(), _)
171172
or
172-
result = qualifiedModuleName(this.getEnclosingModule()) and
173+
result = qualifiedModuleName(enclosingModule(this)) and
173174
(
174175
this.getReceiver() instanceof Self
175176
or
@@ -178,6 +179,25 @@ private class IncludeOrPrependCall extends MethodCall {
178179
}
179180
}
180181

182+
/**
183+
* A variant of AstNode::getEnclosingModule that excludes
184+
* results that are enclosed in a block. This is a bit wrong because
185+
* it could lead to false negatives. However, `include` statements in
186+
* blocks are very rare in normal code. The majority of cases are in calls
187+
* to methods like `module_eval` and `Rspec.describe` / `Rspec.context`. These
188+
* methods evaluate the block in the context of some other module/class instead of
189+
* the enclosing one.
190+
*/
191+
private ModuleBase enclosingModule(AstNode node) {
192+
exists(AstNode parent | parent = node.getParent() |
193+
result = parent
194+
or
195+
not parent instanceof ModuleBase and
196+
not parent instanceof Block and
197+
result = enclosingModule(parent)
198+
)
199+
}
200+
181201
private string prepends(string qname) {
182202
exists(IncludeOrPrependCall m |
183203
m.getMethodName() = "prepend" and

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ moduleTypes
125125
| modules.rb:83:1:86:3 | Other | modules.rb:83:1:86:3 | Other |
126126
| modules.rb:84:3:85:5 | Foo1 | modules.rb:84:3:85:5 | Other::Foo1 |
127127
| modules.rb:88:1:93:3 | IncludeTest | modules.rb:88:1:93:3 | IncludeTest |
128-
| modules.rb:91:3:92:5 | Y | modules.rb:91:3:92:5 | Other::Foo1::Y |
128+
| modules.rb:91:3:92:5 | Y | modules.rb:91:3:92:5 | Test::Foo1::Y |
129129
| modules.rb:95:1:99:3 | IncludeTest2 | modules.rb:95:1:99:3 | IncludeTest2 |
130130
| modules.rb:97:3:98:5 | Z | modules.rb:97:3:98:5 | Test::Foo1::Z |
131131
| modules.rb:101:1:105:3 | PrependTest | modules.rb:101:1:105:3 | PrependTest |

0 commit comments

Comments
 (0)