Skip to content

Commit b82cbb0

Browse files
committed
JS: Do not restrict resolved imports to PathExpr
The return type of getImportedPath needs to change to Expr, which means we have to deprecate and rename it. Next commit updates the deprecated uses of the old predicate.
1 parent bc19473 commit b82cbb0

File tree

3 files changed

+18
-6
lines changed

3 files changed

+18
-6
lines changed

javascript/ql/lib/semmle/javascript/Modules.qll

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ abstract class Module extends TopLevel {
115115
}
116116
}
117117

118-
private predicate shouldResolveExpr(Expr e) { e = any(Import imprt).getImportedPath() }
118+
private predicate shouldResolveExpr(Expr e) { e = any(Import imprt).getImportedPathExpr() }
119119

120120
private module Resolver = ResolveExpr<shouldResolveExpr/1>;
121121

@@ -127,8 +127,13 @@ abstract class Import extends AstNode {
127127
/** Gets the module in which this import appears. */
128128
abstract Module getEnclosingModule();
129129

130+
/** DEPRECATED. Use `getImportedPathExpr` instead. */
131+
deprecated PathExpr getImportedPath() { none() }
132+
130133
/** Gets the (unresolved) path that this import refers to. */
131-
abstract PathExpr getImportedPath();
134+
abstract Expr getImportedPathExpr();
135+
136+
final string getImportedPathString() { result = this.getImportedPathExpr().getStringValue() }
132137

133138
/**
134139
* Gets an externs module the path of this import resolves to.
@@ -137,13 +142,18 @@ abstract class Import extends AstNode {
137142
* path is assumed to be a possible target of the import.
138143
*/
139144
Module resolveExternsImport() {
140-
result.isExterns() and result.getName() = this.getImportedPath().getValue()
145+
result.isExterns() and result.getName() = this.getImportedPathString()
141146
}
142147

143148
/**
144149
* Gets the module the path of this import resolves to.
145150
*/
146-
Module resolveImportedPath() { result.getFile() = Resolver::resolveExpr(this.getImportedPath()) }
151+
Module resolveImportedPath() { result.getFile() = this.getTargetFile() }
152+
153+
/**
154+
* Gets the module the path of this import resolves to.
155+
*/
156+
File getTargetFile() { result = Resolver::resolveExpr(this.getImportedPathExpr()) }
147157

148158
/**
149159
* DEPRECATED. Use `getImportedModule()` instead.

javascript/ql/test/library-tests/PathResolution/DirnameImports/main.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ require(__dirname + '/../import-packages.ts'); // $ importTarget=import-packages
77
require(__dirname + '/' + 'target.js'); // $ importTarget=DirnameImports/target.js
88

99
require(path.join(__dirname, 'target.js')); // $ importTarget=DirnameImports/target.js
10-
require(path.resolve(__dirname, 'target.js')); // $ MISSING: importTarget=DirnameImports/target.js // path.resolve is not a PathExpr
10+
require(path.resolve(__dirname, 'target.js')); // $ importTarget=DirnameImports/target.js
1111

1212
const subdir = 'nested';
1313
require(__dirname + '/' + subdir + '/target.js'); // $ importTarget=DirnameImports/nested/target.js
1414

15-
require(`${__dirname}/target.js`); // $ MISSING: importTarget=DirnameImports/target.js // TemplateLiteral is not a PathExpr
15+
require(`${__dirname}/target.js`); // $ importTarget=DirnameImports/target.js

javascript/ql/test/library-tests/PathResolution/test.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,9 @@ importTarget
6060
| DirnameImports/main.js:6:1:6:45 | require ... es.ts') | import-packages.ts |
6161
| DirnameImports/main.js:7:1:7:38 | require ... et.js') | DirnameImports/target.js |
6262
| DirnameImports/main.js:9:1:9:42 | require ... t.js')) | DirnameImports/target.js |
63+
| DirnameImports/main.js:10:1:10:45 | require ... t.js')) | DirnameImports/target.js |
6364
| DirnameImports/main.js:13:1:13:48 | require ... et.js') | DirnameImports/nested/target.js |
65+
| DirnameImports/main.js:15:1:15:33 | require ... et.js`) | DirnameImports/target.js |
6466
| Extended/src/main.ts:2:1:2:21 | import ... /file"; | Extended/lib/file.ts |
6567
| Extended/src/main.ts:3:1:3:24 | import ... le.ts"; | Extended/lib/file.ts |
6668
| Extended/src/main.ts:4:1:4:24 | import ... le.js"; | Extended/lib/file.ts |

0 commit comments

Comments
 (0)