Skip to content

Commit 3590a2c

Browse files
aibaarsaibaars
authored
Merge pull request #164 from github/aibaars/fix-modules
Improve module/class resolution
2 parents 3b73d41 + 24bb11b commit 3590a2c

7 files changed

Lines changed: 93 additions & 26 deletions

File tree

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

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,13 @@ private TResolved resolveScopeExpr(ConstantReadAccess r) {
6363
qname =
6464
min(string qn, int p |
6565
isDefinedConstant(qn) and
66-
qn = resolveScopeExpr(r, p)
66+
qn = resolveScopeExpr(r, p) and
67+
// prevent classes/modules that contain/extend themselves
68+
not exists(ConstantWriteAccess w | qn = constantDefinition0(w) |
69+
r = w.getScopeExpr()
70+
or
71+
r = w.(ClassDeclaration).getSuperclassExpr()
72+
)
6773
|
6874
qn order by p
6975
)
@@ -100,18 +106,20 @@ private string resolveScopeExpr(ConstantReadAccess c, int priority) {
100106
or
101107
not exists(c.getScopeExpr()) and
102108
not c.hasGlobalScope() and
103-
exists(Namespace n |
104-
result = qualifiedModuleName(constantDefinition0(n), c.getName()) and
105-
n = enclosing(c.getEnclosingModule(), priority)
109+
(
110+
exists(Namespace n |
111+
result = qualifiedModuleName(constantDefinition0(n), c.getName()) and
112+
n = enclosing(c.getEnclosingModule(), priority)
113+
)
114+
or
115+
result =
116+
qualifiedModuleName(ancestors(qualifiedModuleName(c.getEnclosingModule()),
117+
priority - maxDepth()), c.getName())
118+
or
119+
result = c.getName() and
120+
priority = maxDepth() + 4 and
121+
qualifiedModuleName(c.getEnclosingModule()) != "BasicObject"
106122
)
107-
or
108-
result =
109-
qualifiedModuleName(ancestors(qualifiedModuleName(c.getEnclosingModule()), priority - maxDepth()),
110-
c.getName())
111-
or
112-
result = c.getName() and
113-
priority = maxDepth() + 4 and
114-
qualifiedModuleName(c.getEnclosingModule()) != "BasicObject"
115123
}
116124

117125
bindingset[qualifier, name]

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1406,6 +1406,17 @@ modules/modules.rb:
14061406
# 102| getArgument: [ConstantReadAccess] Test
14071407
# 103| getStmt: [ModuleDeclaration] Y
14081408
# 103| getScopeExpr: [ConstantReadAccess] Foo2
1409+
# 107| getStmt: [ModuleDeclaration] MM
1410+
# 108| getStmt: [ModuleDeclaration] MM
1411+
# 108| getScopeExpr: [ConstantReadAccess] MM
1412+
# 112| getStmt: [ClassDeclaration] YY
1413+
# 115| getStmt: [ModuleDeclaration] XX
1414+
# 116| getStmt: [ClassDeclaration] YY
1415+
# 116| getSuperclassExpr: [ConstantReadAccess] YY
1416+
# 120| getStmt: [ModuleDeclaration] Baz
1417+
# 120| getScopeExpr: [ConstantReadAccess] Bar
1418+
# 120| getScopeExpr: [ConstantReadAccess] Foo1
1419+
# 120| getScopeExpr: [ConstantReadAccess] Test
14091420
operations/operations.rb:
14101421
# 1| [Toplevel] operations.rb
14111422
# 3| getStmt: [AssignExpr] ... = ...

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ classes
1313
| modules.rb:66:5:67:7 | Bar | ClassDeclaration | Bar |
1414
| modules.rb:72:5:73:7 | Bar | ClassDeclaration | Bar |
1515
| modules.rb:78:5:79:7 | Bar | ClassDeclaration | Bar |
16+
| modules.rb:112:1:113:3 | YY | ClassDeclaration | YY |
17+
| modules.rb:116:7:117:9 | YY | ClassDeclaration | YY |
1618
classesWithNameScopeExprs
1719
| classes.rb:16:1:17:3 | MyClass | classes.rb:16:7:16:14 | MyModule |
1820
| modules.rb:66:5:67:7 | Bar | modules.rb:66:11:66:14 | Foo1 |
@@ -37,3 +39,4 @@ modulesInClasses
3739
classesWithASuperclass
3840
| classes.rb:7:1:8:3 | Bar | classes.rb:7:13:7:21 | BaseClass |
3941
| classes.rb:11:1:12:3 | Baz | classes.rb:11:13:11:32 | call to superclass_for |
42+
| modules.rb:116:7:117:9 | YY | modules.rb:116:18:116:19 | YY |

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

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ moduleBases
1111
| classes.rb:41:1:52:3 | class << ... | ClassDeclaration |
1212
| classes.rb:55:1:56:3 | MyClassInGlobalScope | ClassDeclaration |
1313
| modules.rb:1:1:2:3 | Empty | ModuleDeclaration |
14-
| modules.rb:1:1:105:4 | modules.rb | Toplevel |
14+
| modules.rb:1:1:122:1 | modules.rb | Toplevel |
1515
| modules.rb:4:1:24:3 | Foo | ModuleDeclaration |
1616
| modules.rb:5:3:14:5 | Bar | ModuleDeclaration |
1717
| modules.rb:6:5:7:7 | ClassInFooBar | ClassDeclaration |
@@ -38,6 +38,12 @@ moduleBases
3838
| modules.rb:97:3:98:5 | Z | ModuleDeclaration |
3939
| modules.rb:101:1:105:3 | PrependTest | ModuleDeclaration |
4040
| modules.rb:103:3:104:5 | Y | ModuleDeclaration |
41+
| modules.rb:107:1:110:3 | MM | ModuleDeclaration |
42+
| modules.rb:108:3:109:5 | MM | ModuleDeclaration |
43+
| modules.rb:112:1:113:3 | YY | ClassDeclaration |
44+
| modules.rb:115:1:118:3 | XX | ModuleDeclaration |
45+
| modules.rb:116:7:117:9 | YY | ClassDeclaration |
46+
| modules.rb:120:1:121:3 | Baz | ModuleDeclaration |
4147
| toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
4248
moduleBaseClasses
4349
| classes.rb:2:1:56:3 | classes.rb | classes.rb:3:1:4:3 | Foo |
@@ -47,13 +53,15 @@ moduleBaseClasses
4753
| classes.rb:2:1:56:3 | classes.rb | classes.rb:20:1:37:3 | Wibble |
4854
| classes.rb:2:1:56:3 | classes.rb | classes.rb:55:1:56:3 | MyClassInGlobalScope |
4955
| classes.rb:20:1:37:3 | Wibble | classes.rb:32:3:33:5 | ClassInWibble |
56+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:112:1:113:3 | YY |
5057
| modules.rb:4:1:24:3 | Foo | modules.rb:19:3:20:5 | ClassInFoo |
5158
| modules.rb:5:3:14:5 | Bar | modules.rb:6:5:7:7 | ClassInFooBar |
5259
| modules.rb:26:1:35:3 | Foo | modules.rb:30:3:31:5 | ClassInAnotherDefinitionOfFoo |
5360
| modules.rb:48:1:57:3 | Bar | modules.rb:49:3:50:5 | ClassInAnotherDefinitionOfFooBar |
5461
| modules.rb:65:3:68:5 | Foo1 | modules.rb:66:5:67:7 | Bar |
5562
| modules.rb:70:3:74:5 | Foo2 | modules.rb:72:5:73:7 | Bar |
5663
| modules.rb:76:3:80:5 | Foo3 | modules.rb:78:5:79:7 | Bar |
64+
| modules.rb:115:1:118:3 | XX | modules.rb:116:7:117:9 | YY |
5765
moduleBaseMethods
5866
| classes.rb:20:1:37:3 | Wibble | classes.rb:21:3:23:5 | method_a |
5967
| classes.rb:20:1:37:3 | Wibble | classes.rb:25:3:27:5 | method_b |
@@ -68,17 +76,20 @@ moduleBaseMethods
6876
moduleBaseModules
6977
| classes.rb:2:1:56:3 | classes.rb | classes.rb:15:1:15:20 | MyModule |
7078
| classes.rb:20:1:37:3 | Wibble | classes.rb:35:3:36:5 | ModuleInWibble |
71-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:1:1:2:3 | Empty |
72-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:4:1:24:3 | Foo |
73-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:26:1:35:3 | Foo |
74-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:37:1:46:3 | Bar |
75-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:48:1:57:3 | Bar |
76-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
77-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:63:1:81:3 | Test |
78-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:83:1:86:3 | Other |
79-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:88:1:93:3 | IncludeTest |
80-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:95:1:99:3 | IncludeTest2 |
81-
| modules.rb:1:1:105:4 | modules.rb | modules.rb:101:1:105:3 | PrependTest |
79+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:1:1:2:3 | Empty |
80+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:4:1:24:3 | Foo |
81+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:26:1:35:3 | Foo |
82+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:37:1:46:3 | Bar |
83+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:48:1:57:3 | Bar |
84+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:60:1:61:3 | MyModuleInGlobalScope |
85+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:63:1:81:3 | Test |
86+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:83:1:86:3 | Other |
87+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:88:1:93:3 | IncludeTest |
88+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:95:1:99:3 | IncludeTest2 |
89+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:101:1:105:3 | PrependTest |
90+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:107:1:110:3 | MM |
91+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:115:1:118:3 | XX |
92+
| modules.rb:1:1:122:1 | modules.rb | modules.rb:120:1:121:3 | Baz |
8293
| modules.rb:4:1:24:3 | Foo | modules.rb:5:3:14:5 | Bar |
8394
| modules.rb:63:1:81:3 | Test | modules.rb:65:3:68:5 | Foo1 |
8495
| modules.rb:63:1:81:3 | Test | modules.rb:70:3:74:5 | Foo2 |
@@ -88,3 +99,4 @@ moduleBaseModules
8899
| modules.rb:88:1:93:3 | IncludeTest | modules.rb:91:3:92:5 | Y |
89100
| modules.rb:95:1:99:3 | IncludeTest2 | modules.rb:97:3:98:5 | Z |
90101
| modules.rb:101:1:105:3 | PrependTest | modules.rb:103:3:104:5 | Y |
102+
| modules.rb:107:1:110:3 | MM | modules.rb:108:3:109:5 | MM |

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,17 @@ modules
2121
| modules.rb:97:3:98:5 | Z | ModuleDeclaration | Z |
2222
| modules.rb:101:1:105:3 | PrependTest | ModuleDeclaration | PrependTest |
2323
| modules.rb:103:3:104:5 | Y | ModuleDeclaration | Y |
24+
| modules.rb:107:1:110:3 | MM | ModuleDeclaration | MM |
25+
| modules.rb:108:3:109:5 | MM | ModuleDeclaration | MM |
26+
| modules.rb:115:1:118:3 | XX | ModuleDeclaration | XX |
27+
| modules.rb:120:1:121:3 | Baz | ModuleDeclaration | Baz |
2428
modulesWithScopeExprs
2529
| modules.rb:48:1:57:3 | Bar | modules.rb:48:8:48:10 | Foo |
2630
| modules.rb:91:3:92:5 | Y | modules.rb:91:10:91:13 | Foo1 |
2731
| modules.rb:97:3:98:5 | Z | modules.rb:97:10:97:13 | Foo1 |
2832
| modules.rb:103:3:104:5 | Y | modules.rb:103:10:103:13 | Foo2 |
33+
| modules.rb:108:3:109:5 | MM | modules.rb:108:10:108:11 | MM |
34+
| modules.rb:120:1:121:3 | Baz | modules.rb:120:8:120:22 | Bar |
2935
modulesWithGlobalNameScopeExprs
3036
| modules.rb:60:1:61:3 | MyModuleInGlobalScope |
3137
exprsInModules
@@ -66,6 +72,8 @@ exprsInModules
6672
| modules.rb:95:1:99:3 | IncludeTest2 | 1 | modules.rb:97:3:98:5 | Z | ModuleDeclaration |
6773
| modules.rb:101:1:105:3 | PrependTest | 0 | modules.rb:102:3:102:16 | call to prepend | MethodCall |
6874
| modules.rb:101:1:105:3 | PrependTest | 1 | modules.rb:103:3:104:5 | Y | ModuleDeclaration |
75+
| modules.rb:107:1:110:3 | MM | 0 | modules.rb:108:3:109:5 | MM | ModuleDeclaration |
76+
| modules.rb:115:1:118:3 | XX | 0 | modules.rb:116:7:117:9 | YY | ClassDeclaration |
6977
methodsInModules
7078
| modules.rb:4:1:24:3 | Foo | modules.rb:16:3:17:5 | method_in_foo | method_in_foo |
7179
| modules.rb:5:3:14:5 | Bar | modules.rb:9:5:10:7 | method_in_foo_bar | method_in_foo_bar |
@@ -81,6 +89,7 @@ classesInModules
8189
| modules.rb:65:3:68:5 | Foo1 | modules.rb:66:5:67:7 | Bar | Bar |
8290
| modules.rb:70:3:74:5 | Foo2 | modules.rb:72:5:73:7 | Bar | Bar |
8391
| modules.rb:76:3:80:5 | Foo3 | modules.rb:78:5:79:7 | Bar | Bar |
92+
| modules.rb:115:1:118:3 | XX | modules.rb:116:7:117:9 | YY | YY |
8493
modulesInModules
8594
| modules.rb:4:1:24:3 | Foo | modules.rb:5:3:14:5 | Bar | Bar |
8695
| modules.rb:63:1:81:3 | Test | modules.rb:65:3:68:5 | Foo1 | Foo1 |
@@ -91,6 +100,7 @@ modulesInModules
91100
| modules.rb:88:1:93:3 | IncludeTest | modules.rb:91:3:92:5 | Y | Y |
92101
| modules.rb:95:1:99:3 | IncludeTest2 | modules.rb:97:3:98:5 | Z | Z |
93102
| modules.rb:101:1:105:3 | PrependTest | modules.rb:103:3:104:5 | Y | Y |
103+
| modules.rb:107:1:110:3 | MM | modules.rb:108:3:109:5 | MM | MM |
94104
moduleTypes
95105
| classes.rb:2:1:56:3 | classes.rb | file://:0:0:0:0 | Object |
96106
| classes.rb:3:1:4:3 | Foo | modules.rb:4:1:24:3 | Foo |
@@ -103,7 +113,7 @@ moduleTypes
103113
| classes.rb:35:3:36:5 | ModuleInWibble | classes.rb:35:3:36:5 | Wibble::ModuleInWibble |
104114
| classes.rb:55:1:56:3 | MyClassInGlobalScope | classes.rb:55:1:56:3 | MyClassInGlobalScope |
105115
| modules.rb:1:1:2:3 | Empty | modules.rb:1:1:2:3 | Empty |
106-
| modules.rb:1:1:105:4 | modules.rb | file://:0:0:0:0 | Object |
116+
| modules.rb:1:1:122:1 | modules.rb | file://:0:0:0:0 | Object |
107117
| modules.rb:4:1:24:3 | Foo | modules.rb:4:1:24:3 | Foo |
108118
| modules.rb:5:3:14:5 | Bar | modules.rb:5:3:14:5 | Foo::Bar |
109119
| modules.rb:6:5:7:7 | ClassInFooBar | modules.rb:6:5:7:7 | Foo::Bar::ClassInFooBar |
@@ -130,4 +140,10 @@ moduleTypes
130140
| modules.rb:97:3:98:5 | Z | modules.rb:97:3:98:5 | Test::Foo1::Z |
131141
| modules.rb:101:1:105:3 | PrependTest | modules.rb:101:1:105:3 | PrependTest |
132142
| modules.rb:103:3:104:5 | Y | modules.rb:103:3:104:5 | Test::Foo2::Y |
143+
| modules.rb:107:1:110:3 | MM | modules.rb:107:1:110:3 | MM |
144+
| modules.rb:108:3:109:5 | MM | modules.rb:108:3:109:5 | MM::MM |
145+
| modules.rb:112:1:113:3 | YY | modules.rb:112:1:113:3 | YY |
146+
| modules.rb:115:1:118:3 | XX | modules.rb:115:1:118:3 | XX |
147+
| modules.rb:116:7:117:9 | YY | modules.rb:116:7:117:9 | XX::YY |
148+
| modules.rb:120:1:121:3 | Baz | modules.rb:120:1:121:3 | Test::Foo1::Bar::Baz |
133149
| toplevel.rb:1:1:5:23 | toplevel.rb | file://:0:0:0:0 | Object |

‎ql/test/library-tests/ast/modules/modules.rb‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,3 +103,20 @@ module PrependTest
103103
module Foo2::Y
104104
end
105105
end
106+
107+
module MM
108+
module MM::MM
109+
end
110+
end
111+
112+
class YY
113+
end
114+
115+
module XX
116+
class YY < YY
117+
end
118+
end
119+
120+
module Test::Foo1::Bar::Baz
121+
end
122+

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
toplevel
22
| classes.rb:2:1:56:3 | classes.rb | Toplevel |
3-
| modules.rb:1:1:105:4 | modules.rb | Toplevel |
3+
| modules.rb:1:1:122:1 | modules.rb | Toplevel |
44
| toplevel.rb:1:1:5:23 | toplevel.rb | Toplevel |
55
beginBlocks
66
| toplevel.rb:1:1:5:23 | toplevel.rb | 0 | toplevel.rb:5:1:5:22 | BEGIN { ... } |

0 commit comments

Comments
 (0)