Skip to content

Commit a4de82d

Browse files
authored
Merge pull request #1185 from xiemaisi/js/improve-amd-imports
Approved by asger-semmle
2 parents 007cee8 + 8bb91bf commit a4de82d

33 files changed

+197
-50
lines changed

javascript/ql/src/Declarations/TooManyParameters.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,6 @@ where
1717
not f.inExternsFile() and
1818
f.getNumParameter() > 7 and
1919
// exclude AMD modules
20-
not exists(AMDModuleDefinition m | f = m.getFactoryNode().(DataFlow::FunctionNode).getAstNode())
20+
not exists(AmdModuleDefinition m | f = m.getFactoryNode().(DataFlow::FunctionNode).getAstNode())
2121
select f.(FirstLineOf),
2222
capitalize(f.describe()) + " has too many parameters (" + f.getNumParameter() + ")."

javascript/ql/src/semmle/javascript/AMD.qll

Lines changed: 83 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ import javascript
2323
* where the first argument is the module name, the second argument an
2424
* array of dependencies, and the third argument a factory method or object.
2525
*/
26-
class AMDModuleDefinition extends CallExpr {
27-
AMDModuleDefinition() {
26+
class AmdModuleDefinition extends CallExpr {
27+
AmdModuleDefinition() {
2828
getParent() instanceof ExprStmt and
2929
getCallee().(GlobalVarAccess).getName() = "define" and
3030
exists(int n | n = getNumArgument() |
@@ -153,7 +153,7 @@ class AMDModuleDefinition extends CallExpr {
153153
result = getModuleExpr().analyze().getAValue()
154154
or
155155
// explicit exports: anything assigned to `module.exports`
156-
exists(AbstractProperty moduleExports, AMDModule m |
156+
exists(AbstractProperty moduleExports, AmdModule m |
157157
this = m.getDefine() and
158158
moduleExports.getBase().(AbstractModuleObject).getModule() = m and
159159
moduleExports.getPropertyName() = "exports"
@@ -170,10 +170,15 @@ class AMDModuleDefinition extends CallExpr {
170170
}
171171
}
172172

173+
/**
174+
* DEPRECATED: Use `AmdModuleDefinition` instead.
175+
*/
176+
deprecated class AMDModuleDefinition = AmdModuleDefinition;
177+
173178
/** An AMD dependency, considered as a path expression. */
174179
private class AmdDependencyPath extends PathExprCandidate {
175180
AmdDependencyPath() {
176-
exists(AMDModuleDefinition amd |
181+
exists(AmdModuleDefinition amd |
177182
this = amd.getDependencies().getAnElement() or
178183
this = amd.getARequireCall().getAnArgument()
179184
)
@@ -191,21 +196,83 @@ private class ConstantAmdDependencyPathElement extends PathExprInModule, Constan
191196
* Holds if `def` is an AMD module definition in `tl` which is not
192197
* nested inside another module definition.
193198
*/
194-
private predicate amdModuleTopLevel(AMDModuleDefinition def, TopLevel tl) {
199+
private predicate amdModuleTopLevel(AmdModuleDefinition def, TopLevel tl) {
195200
def.getTopLevel() = tl and
196-
not def.getParent+() instanceof AMDModuleDefinition
201+
not def.getParent+() instanceof AmdModuleDefinition
202+
}
203+
204+
/**
205+
* An AMD dependency, viewed as an import.
206+
*/
207+
private class AmdDependencyImport extends Import {
208+
AmdDependencyImport() { this = any(AmdModuleDefinition def).getADependency() }
209+
210+
override Module getEnclosingModule() { this = result.(AmdModule).getDefine().getADependency() }
211+
212+
override PathExpr getImportedPath() { result = this }
213+
214+
/**
215+
* Gets a file that looks like it might be the target of this import.
216+
*
217+
* Specifically, we look for files whose absolute path ends with the imported path, possibly
218+
* adding well-known JavaScript file extensions like `.js`.
219+
*/
220+
private File guessTarget() {
221+
exists(PathString imported, string abspath, string dirname, string basename |
222+
targetCandidate(result, abspath, imported, dirname, basename)
223+
|
224+
abspath.regexpMatch(".*/\\Q" + imported + "\\E")
225+
or
226+
exists(Folder dir |
227+
// `dir` ends with the dirname of the imported path
228+
dir.getAbsolutePath().regexpMatch(".*/\\Q" + dirname + "\\E") or
229+
dirname = ""
230+
|
231+
result = dir.getJavaScriptFile(basename)
232+
)
233+
)
234+
}
235+
236+
/**
237+
* Holds if `f` is a file whose stem (that is, basename without extension) matches the imported path.
238+
*
239+
* Additionally, `abspath` is bound to the absolute path of `f`, `imported` to the imported path, and
240+
* `dirname` and `basename` to the dirname and basename (respectively) of `imported`.
241+
*/
242+
private predicate targetCandidate(
243+
File f, string abspath, PathString imported, string dirname, string basename
244+
) {
245+
imported = getImportedPath().getValue() and
246+
f.getStem() = imported.getStem() and
247+
f.getAbsolutePath() = abspath and
248+
dirname = imported.getDirName() and
249+
basename = imported.getBaseName()
250+
}
251+
252+
/**
253+
* Gets the module whose absolute path matches this import, if there is only a single such module.
254+
*/
255+
private Module resolveByAbsolutePath() {
256+
count(guessTarget()) = 1 and
257+
result.getFile() = guessTarget()
258+
}
259+
260+
override Module getImportedModule() {
261+
result = super.getImportedModule()
262+
or
263+
not exists(super.getImportedModule()) and
264+
result = resolveByAbsolutePath()
265+
}
197266
}
198267

199268
/**
200269
* An AMD-style module.
201270
*/
202-
class AMDModule extends Module {
203-
AMDModule() { strictcount(AMDModuleDefinition def | amdModuleTopLevel(def, this)) = 1 }
271+
class AmdModule extends Module {
272+
AmdModule() { strictcount(AmdModuleDefinition def | amdModuleTopLevel(def, this)) = 1 }
204273

205274
/** Gets the definition of this module. */
206-
AMDModuleDefinition getDefine() { amdModuleTopLevel(result, this) }
207-
208-
override Module getAnImportedModule() { result.getFile() = resolve(getDefine().getADependency()) }
275+
AmdModuleDefinition getDefine() { amdModuleTopLevel(result, this) }
209276

210277
override predicate exports(string name, ASTNode export) {
211278
exists(DataFlow::PropWrite pwn | export = pwn.getAstNode() |
@@ -214,3 +281,8 @@ class AMDModule extends Module {
214281
)
215282
}
216283
}
284+
285+
/**
286+
* DEPRECATED: Use `AmdModule` instead.
287+
*/
288+
deprecated class AMDModule = AmdModule;

javascript/ql/src/semmle/javascript/ES2015Modules.qll

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ class ES2015Module extends Module {
1919
/** Gets an export declaration in this module. */
2020
ExportDeclaration getAnExport() { result.getTopLevel() = this }
2121

22-
override Module getAnImportedModule() { result = getAnImport().getImportedModule() }
23-
2422
override predicate exports(string name, ASTNode export) {
2523
exists(ExportDeclaration ed | ed = getAnExport() and ed = export | ed.exportsAs(_, name))
2624
}

javascript/ql/src/semmle/javascript/Files.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ abstract class Container extends @container {
7575
* </table>
7676
*/
7777
string getBaseName() {
78-
result = getAbsolutePath().regexpCapture(".*/(([^/]*?)(?:\\.([^.]*))?)", 1)
78+
result = getAbsolutePath().regexpCapture(".*/(([^/]*?)(\\.([^.]*))?)", 1)
7979
}
8080

8181
/**
@@ -101,7 +101,7 @@ abstract class Container extends @container {
101101
* <tr><td>"/tmp/x.tar.gz"</td><td>"gz"</td></tr>
102102
* </table>
103103
*/
104-
string getExtension() { result = getAbsolutePath().regexpCapture(".*/([^/]*?)(\\.([^.]*))?", 3) }
104+
string getExtension() { result = getAbsolutePath().regexpCapture(".*/(([^/]*?)(\\.([^.]*))?)", 4) }
105105

106106
/**
107107
* Gets the stem of this container, that is, the prefix of its base name up to
@@ -120,7 +120,7 @@ abstract class Container extends @container {
120120
* <tr><td>"/tmp/x.tar.gz"</td><td>"x.tar"</td></tr>
121121
* </table>
122122
*/
123-
string getStem() { result = getAbsolutePath().regexpCapture(".*/([^/]*?)(?:\\.([^.]*))?", 1) }
123+
string getStem() { result = getAbsolutePath().regexpCapture(".*/(([^/]*?)(\\.([^.]*))?)", 2) }
124124

125125
/** Gets the parent container of this file or folder, if any. */
126126
Container getParentContainer() { containerparent(result, this) }

javascript/ql/src/semmle/javascript/Modules.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ abstract class Module extends TopLevel {
2121
Import getAnImport() { result.getTopLevel() = this }
2222

2323
/** Gets a module from which this module imports. */
24-
abstract Module getAnImportedModule();
24+
Module getAnImportedModule() { result = getAnImport().getImportedModule() }
2525

2626
/** Gets a symbol exported by this module. */
2727
string getAnExportedSymbol() { exports(result, _) }
@@ -92,8 +92,8 @@ abstract class Module extends TopLevel {
9292
}
9393

9494
/**
95-
* An import in a module, which may either be an ECMAScript 2015-style
96-
* `import` statement or a CommonJS-style `require` import.
95+
* An import in a module, which may be an ECMAScript 2015-style
96+
* `import` statement, a CommonJS-style `require` import, or an AMD dependency.
9797
*/
9898
abstract class Import extends ASTNode {
9999
/** Gets the module in which this import appears. */

javascript/ql/src/semmle/javascript/NodeJS.qll

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@ class NodeModule extends Module {
2121
/** Gets the scope induced by this module. */
2222
override ModuleScope getScope() { result.getScopeElement() = this }
2323

24-
/** Gets a module imported by this module. */
25-
override Module getAnImportedModule() { result = getAnImport().getImportedModule() }
26-
2724
/**
2825
* Gets an abstract value representing one or more values that may flow
2926
* into this module's `module.exports` property.

javascript/ql/src/semmle/javascript/Paths.qll

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,26 @@ private class ConsPath extends Path, TConsPath {
7676
override string toString() { result = pp(this) }
7777
}
7878

79+
/**
80+
* Gets a regular expression that can be used to parse slash-separated paths.
81+
*
82+
* The first capture group captures the dirname of the path, that is, everything
83+
* before the last slash, or the empty string if there isn't a slash.
84+
*
85+
* The second capture group captures the basename of the path, that is, everything
86+
* after the last slash, or the entire path if there isn't a slash.
87+
*
88+
* The third capture group captures the stem of the basename, that is, everything
89+
* before the last dot, or the entire basename if there isn't a dot.
90+
*
91+
* Finally, the fourth and fifth capture groups capture the extension of the basename,
92+
* that is, everything after the last dot. The fourth group includes the dot, the
93+
* fifth does not.
94+
*/
95+
private string pathRegex() {
96+
result = "(.*)(?:/|^)(([^/]*?)(\\.([^.]*))?)"
97+
}
98+
7999
/**
80100
* A string value that represents a (relative or absolute) file system path.
81101
*
@@ -98,7 +118,17 @@ abstract class PathString extends string {
98118
int getNumComponent() { result = count(int i | exists(getComponent(i))) }
99119

100120
/** Gets the base name of the folder or file this path refers to. */
101-
string getBaseName() { result = this.regexpCapture("(.*/|^)([^/]+)", 2) }
121+
string getBaseName() { result = this.regexpCapture(pathRegex(), 2) }
122+
123+
/**
124+
* Gets stem of the folder or file this path refers to, that is, the prefix of its base name
125+
* up to (but not including) the last dot character if there is one, or the entire
126+
* base name if there is not
127+
*/
128+
string getStem() { result = this.regexpCapture(pathRegex(), 3) }
129+
130+
/** Gets the path of the parent folder of the folder or file this path refers to. */
131+
string getDirName() { result = this.regexpCapture(pathRegex(), 1) }
102132

103133
/**
104134
* Gets the absolute path that the sub-path consisting of the first `n`

javascript/ql/src/semmle/javascript/dataflow/Nodes.qll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,12 @@ module ModuleImportNode {
472472
)
473473
or
474474
// declared AMD dependency
475-
exists(AMDModuleDefinition amd |
475+
exists(AmdModuleDefinition amd |
476476
this = DataFlow::parameterNode(amd.getDependencyParameter(path))
477477
)
478478
or
479479
// AMD require
480-
exists(AMDModuleDefinition amd, CallExpr req |
480+
exists(AmdModuleDefinition amd, CallExpr req |
481481
req = amd.getARequireCall() and
482482
this = DataFlow::valueNode(req) and
483483
path = req.getArgument(0).(ConstantString).getStringValue()

javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ private predicate mayDynamicallyComputeExports(Module m) {
6161
or
6262
// AMD modules can export arbitrary objects, so an import is essentially a property read
6363
// and hence must be considered indefinite
64-
m instanceof AMDModule
64+
m instanceof AmdModule
6565
or
6666
// `m` re-exports all exports of some other module that dynamically computes its exports
6767
exists(BulkReExportDeclaration rexp | rexp = m.(ES2015Module).getAnExport() |
@@ -229,7 +229,7 @@ class AnalyzedExternalModuleReference extends AnalyzedPropertyRead, DataFlow::Va
229229
* Flow analysis for AMD exports.
230230
*/
231231
private class AnalyzedAmdExport extends AnalyzedPropertyWrite, DataFlow::ValueNode {
232-
AMDModule amd;
232+
AmdModule amd;
233233

234234
AnalyzedAmdExport() { astNode = amd.getDefine().getModuleExpr() }
235235

@@ -248,7 +248,7 @@ private class AnalyzedAmdImport extends AnalyzedPropertyRead, DataFlow::Node {
248248
Module required;
249249

250250
AnalyzedAmdImport() {
251-
exists(AMDModule amd, PathExpr dep, Parameter p |
251+
exists(AmdModule amd, PathExpr dep, Parameter p |
252252
amd.getDefine().dependencyParameter(dep, p) and
253253
this = DataFlow::parameterNode(p) and
254254
required.getFile() = amd.resolve(dep)

javascript/ql/src/semmle/javascript/dataflow/internal/VariableTypeInference.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ private class AnalyzedAmdParameter extends AnalyzedVarDef {
180180
AbstractValue implicitInitVal;
181181

182182
AnalyzedAmdParameter() {
183-
exists(AMDModule m, AMDModuleDefinition mdef | mdef = m.getDefine() |
183+
exists(AmdModule m, AmdModuleDefinition mdef | mdef = m.getDefine() |
184184
this = mdef.getModuleParameter() and
185185
implicitInitVal = TAbstractModuleObject(m)
186186
or

0 commit comments

Comments
 (0)