Skip to content

Commit d7cf5ef

Browse files
authored
Merge pull request #20647 from asgerf/js/type-resolution-cache
JS: Avoid magic and improve a join in type resolution
2 parents 7b32cd4 + c6577c8 commit d7cf5ef

File tree

2 files changed

+47
-4
lines changed

2 files changed

+47
-4
lines changed

javascript/ql/lib/semmle/javascript/internal/NameResolution.qll

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ module NameResolution {
7373
*
7474
* May also include some type-specific steps in cases where this is harmless when tracking values.
7575
*/
76+
pragma[nomagic]
7677
private predicate commonStep(Node node1, Node node2) {
7778
// Import paths are part of the graph and has an incoming edge from the imported module, if found.
7879
// This ensures we can also use the PathExpr as a source when working with external (unresolved) modules.
@@ -187,6 +188,7 @@ module NameResolution {
187188
/**
188189
* Holds if there is a read from `node1` to `node2` that accesses the member `name`.
189190
*/
191+
pragma[nomagic]
190192
predicate readStep(Node node1, string name, Node node2) {
191193
exists(QualifiedTypeAccess access |
192194
node1 = access.getQualifier() and
@@ -321,6 +323,7 @@ module NameResolution {
321323
/**
322324
* Gets the exported member of `mod` named `name`.
323325
*/
326+
pragma[nomagic]
324327
Node getModuleExport(ModuleLike mod, string name) {
325328
exists(ExportDeclaration exprt |
326329
mod = exprt.getContainer() and
@@ -362,6 +365,7 @@ module NameResolution {
362365
* Holds if `value` is stored in `target.prop`. Only needs to recognise assignments
363366
* that are also recognised by JSDoc tooling such as the Closure compiler.
364367
*/
368+
pragma[nomagic]
365369
private predicate storeToVariable(Expr value, string prop, LocalVariableLike target) {
366370
exists(AssignExpr assign |
367371
// target.name = value
@@ -374,6 +378,7 @@ module NameResolution {
374378
}
375379

376380
/** Steps that only apply for this configuration. */
381+
pragma[nomagic]
377382
private predicate specificStep(Node node1, Node node2) {
378383
exists(LexicalName var | S::isRelevantVariable(var) |
379384
node1.(LexicalDecl).getALexicalName() = var and
@@ -406,6 +411,7 @@ module NameResolution {
406411
/** Helps track flow from a particular set of source nodes. */
407412
module Track<nodeSig/1 isSource> {
408413
/** Gets the set of nodes reachable from `source`. */
414+
pragma[nomagic]
409415
Node track(Node source) {
410416
isSource(source) and
411417
result = source
@@ -419,6 +425,7 @@ module NameResolution {
419425
/** Helps track flow from a particular set of source nodes. */
420426
module TrackNode<AstNodeSig Source> {
421427
/** Gets the set of nodes reachable from `source`. */
428+
pragma[nomagic]
422429
Node track(Source source) {
423430
result = source
424431
or
@@ -482,6 +489,7 @@ module NameResolution {
482489
*
483490
* Unlike `trackModule`, this is intended to track uses of external packages.
484491
*/
492+
pragma[nomagic]
485493
predicate nodeRefersToModule(Node node, string mod, string qualifiedName) {
486494
exists(Expr path |
487495
path = any(Import imprt).getImportedPathExpr() or

javascript/ql/lib/semmle/javascript/internal/TypeResolution.qll

Lines changed: 39 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ module TypeResolution {
1212
* We track through underlying types as an approximate way to handle calls to a type
1313
* that is a union/intersection involving functions.
1414
*/
15+
pragma[nomagic]
1516
Node trackUnderlyingFunctionType(Function fun) {
1617
result = fun
1718
or
@@ -139,6 +140,28 @@ module TypeResolution {
139140
)
140141
}
141142

143+
/**
144+
* `ContentSet.getAReadContent` restricted to the content sets and contents relevant for type resolution.
145+
*/
146+
pragma[nomagic]
147+
private DataFlow::Content getAReadContentRestricted(DataFlow::ContentSet cs) {
148+
valueReadStep(_, cs, _) and
149+
result = cs.getAReadContent() and
150+
typeMember(_, result, _)
151+
}
152+
153+
/**
154+
* `valueReadStep` where the `ContentSet` has been mapped to the set of relevant read-contents.
155+
*/
156+
pragma[nomagic]
157+
private predicate valueReadStepOnContent(Node object, DataFlow::Content content, Node member) {
158+
exists(DataFlow::ContentSet contents |
159+
valueReadStep(object, contents, member) and
160+
content = getAReadContentRestricted(contents)
161+
)
162+
}
163+
164+
pragma[nomagic]
142165
predicate callTarget(InvokeExpr call, Function target) {
143166
exists(ClassDefinition cls |
144167
valueHasType(call.(NewExpr).getCallee(), trackClassValue(cls)) and
@@ -198,6 +221,7 @@ module TypeResolution {
198221
)
199222
}
200223

224+
pragma[nomagic]
201225
predicate contextualType(Node value, Node type) {
202226
exists(LocalVariableLike v |
203227
type = v.getADeclaration().getTypeAnnotation() and
@@ -239,6 +263,7 @@ module TypeResolution {
239263
/**
240264
* Holds if `value` has the given `type`.
241265
*/
266+
cached
242267
predicate valueHasType(Node value, Node type) {
243268
value.(BindingPattern).getTypeAnnotation() = type
244269
or
@@ -293,11 +318,18 @@ module TypeResolution {
293318
or
294319
exists(Node mid | valueHasType(mid, type) | ValueFlow::step(mid, value))
295320
or
296-
exists(Node mid, Node midType, DataFlow::ContentSet contents, Node host |
297-
valueReadStep(mid, contents, value) and
321+
exists(DataFlow::Content content, Node host |
322+
typeMemberHostRead(host, content, value) and
323+
typeMember(host, content, type)
324+
)
325+
}
326+
327+
pragma[nomagic]
328+
private predicate typeMemberHostRead(Node host, DataFlow::Content content, Node target) {
329+
exists(Node mid, Node midType |
330+
valueReadStepOnContent(mid, content, target) and
298331
valueHasType(mid, midType) and
299-
typeMemberHostReaches(host, midType) and
300-
typeMember(host, contents.getAReadContent(), type)
332+
typeMemberHostReaches(host, midType)
301333
)
302334
}
303335

@@ -309,6 +341,7 @@ module TypeResolution {
309341
* - a union type has the property if all its members have the property
310342
*/
311343
module TrackMustProp<nodeSig/1 directlyHasProperty> {
344+
pragma[nomagic]
312345
predicate hasProperty(Node node) {
313346
directlyHasProperty(node)
314347
or
@@ -341,6 +374,7 @@ module TypeResolution {
341374
}
342375

343376
module ValueHasProperty<nodeSig/1 typeHasProperty> {
377+
pragma[nomagic]
344378
predicate valueHasProperty(Node value) {
345379
exists(Node type |
346380
valueHasType(value, type) and
@@ -405,6 +439,7 @@ module TypeResolution {
405439
/**
406440
* Holds if `type` contains `string` or `any`, possibly wrapped in a promise.
407441
*/
442+
pragma[nomagic]
408443
predicate hasUnderlyingStringOrAnyType(Node type) {
409444
type.(TypeAnnotation).isStringy()
410445
or

0 commit comments

Comments
 (0)