Skip to content

Commit

Permalink
Merge pull request #15768 from asgerf/js/amd-pseudo-deps
Browse files Browse the repository at this point in the history
JS: Do not treat AMD pseudo-dependencies as imports
  • Loading branch information
asgerf authored Mar 13, 2024
2 parents adefdfd + 8533973 commit c5a02da
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 7 deletions.
23 changes: 17 additions & 6 deletions javascript/ql/lib/semmle/javascript/AMD.qll
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,13 @@ class AmdModuleDefinition extends CallExpr instanceof AmdModuleDefinition::Range
}

/** Gets the `i`th dependency of this module definition. */
PathExpr getDependency(int i) { result = this.getDependencies().getElement(i) }
PathExpr getDependency(int i) {
exists(Expr expr |
expr = this.getDependencies().getElement(i) and
not isPseudoDependency(expr.getStringValue()) and
result = expr
)
}

/** Gets a dependency of this module definition. */
PathExpr getADependency() {
Expand Down Expand Up @@ -102,9 +108,10 @@ class AmdModuleDefinition extends CallExpr instanceof AmdModuleDefinition::Range
/**
* Holds if `p` is the parameter corresponding to dependency `dep`.
*/
predicate dependencyParameter(PathExpr dep, Parameter p) {
predicate dependencyParameter(Expr dep, Parameter p) {
exists(int i |
dep = this.getDependency(i) and
// Note: to avoid spurious recursion, do not depend on PathExpr here
dep = this.getDependencies().getElement(i) and
p = this.getFactoryParameter(i)
)
}
Expand All @@ -122,9 +129,9 @@ class AmdModuleDefinition extends CallExpr instanceof AmdModuleDefinition::Range
* `dep1` and `dep2`.
*/
Parameter getDependencyParameter(string name) {
exists(PathExpr dep |
exists(Expr dep |
this.dependencyParameter(dep, result) and
dep.getValue() = name
name = dep.getStringValue()
)
}

Expand Down Expand Up @@ -202,11 +209,15 @@ class AmdModuleDefinition extends CallExpr instanceof AmdModuleDefinition::Range
}
}

private predicate isPseudoDependency(string s) { s = ["exports", "require", "module"] }

/** An AMD dependency, considered as a path expression. */
private class AmdDependencyPath extends PathExprCandidate {
AmdDependencyPath() {
exists(AmdModuleDefinition amd |
this = amd.getDependencies().getAnElement() or
this = amd.getDependencies().getAnElement() and
not isPseudoDependency(this.getStringValue())
or
this = amd.getARequireCall().getAnArgument()
)
}
Expand Down
1 change: 0 additions & 1 deletion javascript/ql/test/library-tests/AMD/tests.expected
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ amdModuleDefinition
| umd.js:4:9:4:43 | define( ... actory) | umd.js:1:18:1:24 | factory |
| umd.js:4:9:4:43 | define( ... actory) | umd.js:9:9:14:1 | functio ... };\\n} |
amdModuleDependencies
| tst2.js:1:1:3:2 | define( ... 42;\\n}) | tst2.js:1:9:1:17 | 'exports' |
| tst3.js:1:1:3:2 | define( ... 42;\\n}) | tst3.js:2:21:2:25 | './a' |
| tst4.js:1:1:11:2 | define( ... };\\n}) | tst4.js:2:9:2:14 | 'a.js' |
| tst4.js:1:1:11:2 | define( ... };\\n}) | tst4.js:3:9:3:13 | 'foo' |
Expand Down

0 comments on commit c5a02da

Please sign in to comment.