Skip to content

Commit 168e67d

Browse files
committed
deduplicate string constantQualifiedName(ConstantWriteAccess) as string ConstantWriteAccess#getQualifiedName
1 parent 5b38e06 commit 168e67d

File tree

5 files changed

+48
-40
lines changed

5 files changed

+48
-40
lines changed

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,35 @@ class ConstantWriteAccess extends ConstantAccess {
166166
}
167167

168168
override string getAPrimaryQlClass() { result = "ConstantWriteAccess" }
169+
170+
/**
171+
* Gets the fully qualified name for this constant, based on the context in
172+
* which it is defined.
173+
*
174+
* For example, given
175+
* ```rb
176+
* module Foo
177+
* module Bar
178+
* class Baz
179+
* end
180+
* end
181+
* CONST_A = "a"
182+
* end
183+
* ```
184+
*
185+
* the constant `Baz` has the fully qualified name `Foo::Bar::Baz`, and
186+
* `CONST_A` has the fully qualified name `Foo::CONST_A`.
187+
*/
188+
string getQualifiedName() {
189+
/* get the qualified name for the parent module, then append w */
190+
exists(ConstantWriteAccess parent | parent = this.getEnclosingModule() |
191+
result = parent.getQualifiedName() + "::" + this.getName()
192+
)
193+
or
194+
/* base case - there's no parent module */
195+
not exists(ConstantWriteAccess parent | parent = this.getEnclosingModule()) and
196+
result = this.getName()
197+
}
169198
}
170199

171200
/**

ql/lib/codeql/ruby/frameworks/ActiveRecord.qll

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -216,18 +216,6 @@ class ActiveRecordSqlExecutionRange extends SqlExecution::Range {
216216

217217
// TODO: model `ActiveRecord` sanitizers
218218
// https://api.rubyonrails.org/classes/ActiveRecord/Sanitization/ClassMethods.html
219-
// TODO: factor this out
220-
private string constantQualifiedName(ConstantWriteAccess w) {
221-
/* get the qualified name for the parent module, then append w */
222-
exists(ConstantWriteAccess parent | parent = w.getEnclosingModule() |
223-
result = constantQualifiedName(parent) + "::" + w.getName()
224-
)
225-
or
226-
/* base case - there's no parent module */
227-
not exists(ConstantWriteAccess parent | parent = w.getEnclosingModule()) and
228-
result = w.getName()
229-
}
230-
231219
/**
232220
* A node that may evaluate to one or more `ActiveRecordModelClass` instances.
233221
*/
@@ -290,7 +278,7 @@ private class ActiveRecordModelFinderCall extends ActiveRecordModelInstantiation
290278
ActiveRecordModelFinderCall() {
291279
call = this.asExpr().getExpr() and
292280
recv = getUltimateReceiver(call) and
293-
resolveConstant(recv) = constantQualifiedName(cls) and
281+
resolveConstant(recv) = cls.getQualifiedName() and
294282
call.getMethodName() = finderMethodName()
295283
}
296284

ql/src/queries/analysis/Definitions.ql

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -65,32 +65,6 @@ newtype DefLoc =
6565
not exists(MethodBase m | m.getAChild+() = write)
6666
}
6767

68-
/**
69-
* Gets the fully qualified name for a constant, based on the context in which it is defined.
70-
*
71-
* For example, given
72-
* ```ruby
73-
* module Foo
74-
* module Bar
75-
* class Baz
76-
* end
77-
* end
78-
* end
79-
* ```
80-
*
81-
* the constant `Baz` has the fully qualified name `Foo::Bar::Baz`.
82-
*/
83-
string constantQualifiedName(ConstantWriteAccess w) {
84-
/* get the qualified name for the parent module, then append w */
85-
exists(ConstantWriteAccess parent | parent = w.getEnclosingModule() |
86-
result = constantQualifiedName(parent) + "::" + w.getName()
87-
)
88-
or
89-
/* base case - there's no parent module */
90-
not exists(ConstantWriteAccess parent | parent = w.getEnclosingModule()) and
91-
result = w.getName()
92-
}
93-
9468
/**
9569
* Gets the constant write that defines the given constant.
9670
* Modules often don't have a unique definition, as they are opened multiple times in different
@@ -100,7 +74,7 @@ string constantQualifiedName(ConstantWriteAccess w) {
10074
ConstantWriteAccess definitionOf(ConstantReadAccess r) {
10175
result =
10276
min(ConstantWriteAccess w |
103-
constantQualifiedName(w) = resolveConstant(r)
77+
w.getQualifiedName() = resolveConstant(r)
10478
|
10579
w order by w.getLocation().toString()
10680
)

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,3 +55,16 @@ constantValue
5555
| constants.rb:39:6:39:31 | MAX_SIZE | constants.rb:37:30:37:33 | 1024 |
5656
| constants.rb:41:6:41:13 | GREETING | constants.rb:17:12:17:64 | ... + ... |
5757
| constants.rb:42:6:42:15 | GREETING | constants.rb:17:12:17:64 | ... + ... |
58+
constantWriteAccessQualifiedName
59+
| constants.rb:1:1:15:3 | ModuleA | ModuleA |
60+
| constants.rb:2:5:4:7 | ClassA | ModuleA::ClassA |
61+
| constants.rb:3:9:3:15 | CONST_A | ModuleA::ClassA::CONST_A |
62+
| constants.rb:6:5:6:11 | CONST_B | ModuleA::CONST_B |
63+
| constants.rb:8:5:14:7 | ModuleB | ModuleA::ModuleB |
64+
| constants.rb:9:9:10:11 | ClassB | ModuleA::ModuleB::ClassB |
65+
| constants.rb:12:9:13:11 | ClassC | ModuleA::ModuleB::ClassC |
66+
| constants.rb:17:1:17:8 | GREETING | GREETING |
67+
| constants.rb:20:5:20:9 | Names | Names |
68+
| constants.rb:31:1:32:3 | ClassD | ClassD |
69+
| constants.rb:34:1:35:3 | ModuleC | ModuleC |
70+
| constants.rb:37:1:37:26 | MAX_SIZE | MAX_SIZE |

ql/test/library-tests/ast/constants/constants.ql

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,7 @@ query Expr getConst(Module m, string name) { result = M::ExposedForTestingOnly::
1616
query Expr lookupConst(Module m, string name) { result = M::lookupConst(m, name) }
1717

1818
query predicate constantValue(ConstantReadAccess a, Expr e) { e = a.getValue() }
19+
20+
query predicate constantWriteAccessQualifiedName(ConstantWriteAccess w, string qualifiedName) {
21+
w.getQualifiedName() = qualifiedName
22+
}

0 commit comments

Comments
 (0)