Skip to content

JS: Overhaul import resolution #19391

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
eb05996
Move getAChildContainer one scope up
asgerf Apr 24, 2025
2ce01bf
Add Folder::Resolve as a generalisation of Folder::Append
asgerf Apr 24, 2025
ec9d15b
JS: Make shared Folder module visible
asgerf Apr 24, 2025
8c0b0c4
JS: Ensure json files are extracted properly in tests
asgerf Apr 24, 2025
359525b
JS: Extract more tsconfig.json patterns
asgerf Apr 29, 2025
565cb43
JS: Add test
asgerf Apr 24, 2025
17aa522
JS: Add some helpers
asgerf Apr 24, 2025
ef32a03
JS: Extract from methods from PathString into a non-abstract base class
asgerf Apr 28, 2025
59e1cbc
JS: Add tsconfig class
asgerf Apr 24, 2025
bb91df8
JS: Add helper for doing path resolution with JS rules
asgerf Apr 24, 2025
f542956
JS: Add internal extension of PackageJson class
asgerf Apr 24, 2025
ed4864e
JS: Add two more helpers to FilePath class
asgerf Apr 28, 2025
6725cb5
JS: Implement import resolution
asgerf Apr 29, 2025
e4420f6
JS: Move babel-root-import test
asgerf Apr 28, 2025
d724874
JS: Implement babel-plugin-root-import as a PathMapping
asgerf Apr 28, 2025
a195d07
JS: Resolve Angular2 templateUrl with ResolveExpr instead of PathExpr
asgerf Apr 28, 2025
c293f03
JS: Remove a dependency on getImportedPath()
asgerf Apr 28, 2025
fe055ad
JS: Use PackageJsonEx instead of resolveMainModule
asgerf Apr 28, 2025
ed2a832
JS: Deprecate PathExpr and related classes
asgerf Apr 29, 2025
be5de9c
JS: Update test output
asgerf Apr 29, 2025
5de2c93
JS: Rename getTargetFile to getImportedFile and remove its deprecated…
asgerf Apr 29, 2025
70a5ec5
JS: Add package.json files in tests relying on node_modules
asgerf Apr 29, 2025
b0f73f1
JS: Update test output now that we import .d.ts files more liberally
asgerf Apr 29, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ private void setupFilters() {
patterns.add("**/*.view.json"); // SAP UI5
patterns.add("**/manifest.json");
patterns.add("**/package.json");
patterns.add("**/tsconfig*.json");
patterns.add("**/*tsconfig*.json");
patterns.add("**/codeql-javascript-*.json");

// include any explicitly specified extensions
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public Context(Label parent, int childIndex) {
private final boolean tolerateParseErrors;

public JSONExtractor(ExtractorConfig config) {
this.tolerateParseErrors = config.isTolerateParseErrors();
this.tolerateParseErrors = true;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,9 +301,14 @@ public void setupMatchers(ArgsParser ap) {
// only extract HTML and JS by default
addIncludesFor(includes, FileType.HTML);
addIncludesFor(includes, FileType.JS);
includes.add("**/.babelrc*.json");


// extract TypeScript if `--typescript` or `--typescript-full` was specified
if (getTypeScriptMode(ap) != TypeScriptMode.NONE) addIncludesFor(includes, FileType.TYPESCRIPT);
if (getTypeScriptMode(ap) != TypeScriptMode.NONE) {
addIncludesFor(includes, FileType.TYPESCRIPT);
includes.add("**/*tsconfig*.json");
}

// add explicit include patterns
for (String pattern : ap.getZeroOrMore(P_INCLUDE))
Expand Down
2 changes: 1 addition & 1 deletion javascript/ql/examples/snippets/importfrom.ql
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
import javascript

from ImportDeclaration id
where id.getImportedPath().getValue() = "react"
where id.getImportedPathString() = "react"
select id
2 changes: 1 addition & 1 deletion javascript/ql/lib/definitions.qll
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ private predicate importLookup(AstNode path, Module target, string kind) {
kind = "I" and
(
exists(Import i |
path = i.getImportedPath() and
path = i.getImportedPathExpr() and
target = i.getImportedModule()
)
or
Expand Down
28 changes: 18 additions & 10 deletions javascript/ql/lib/semmle/javascript/AMD.qll
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,14 @@ class AmdModuleDefinition extends CallExpr instanceof AmdModuleDefinition::Range
result = this.getArgument(1)
}

/** DEPRECATED. Use `getDependencyExpr` instead. */
deprecated PathExpr getDependency(int i) { result = this.getDependencyExpr(i) }

/** DEPRECATED. Use `getADependencyExpr` instead. */
deprecated PathExpr getADependency() { result = this.getADependencyExpr() }

/** Gets the `i`th dependency of this module definition. */
PathExpr getDependency(int i) {
Expr getDependencyExpr(int i) {
exists(Expr expr |
expr = this.getDependencies().getElement(i) and
not isPseudoDependency(expr.getStringValue()) and
Expand All @@ -71,8 +77,8 @@ class AmdModuleDefinition extends CallExpr instanceof AmdModuleDefinition::Range
}

/** Gets a dependency of this module definition. */
PathExpr getADependency() {
result = this.getDependency(_) or
Expr getADependencyExpr() {
result = this.getDependencyExpr(_) or
result = this.getARequireCall().getAnArgument()
}

Expand Down Expand Up @@ -233,7 +239,7 @@ private class AmdDependencyPath extends PathExprCandidate {
}

/** A constant path element appearing in an AMD dependency expression. */
private class ConstantAmdDependencyPathElement extends PathExpr, ConstantString {
deprecated private class ConstantAmdDependencyPathElement extends PathExpr, ConstantString {
ConstantAmdDependencyPathElement() { this = any(AmdDependencyPath amd).getAPart() }

override string getValue() { result = this.getStringValue() }
Expand Down Expand Up @@ -261,11 +267,13 @@ private predicate amdModuleTopLevel(AmdModuleDefinition def, TopLevel tl) {
* An AMD dependency, viewed as an import.
*/
private class AmdDependencyImport extends Import {
AmdDependencyImport() { this = any(AmdModuleDefinition def).getADependency() }
AmdDependencyImport() { this = any(AmdModuleDefinition def).getADependencyExpr() }

override Module getEnclosingModule() { this = result.(AmdModule).getDefine().getADependency() }
override Module getEnclosingModule() {
this = result.(AmdModule).getDefine().getADependencyExpr()
}

override PathExpr getImportedPath() { result = this }
override Expr getImportedPathExpr() { result = this }

/**
* Gets a file that looks like it might be the target of this import.
Expand All @@ -274,7 +282,7 @@ private class AmdDependencyImport extends Import {
* adding well-known JavaScript file extensions like `.js`.
*/
private File guessTarget() {
exists(PathString imported, string abspath, string dirname, string basename |
exists(FilePath imported, string abspath, string dirname, string basename |
this.targetCandidate(result, abspath, imported, dirname, basename)
|
abspath.regexpMatch(".*/\\Q" + imported + "\\E")
Expand All @@ -296,9 +304,9 @@ private class AmdDependencyImport extends Import {
* `dirname` and `basename` to the dirname and basename (respectively) of `imported`.
*/
private predicate targetCandidate(
File f, string abspath, PathString imported, string dirname, string basename
File f, string abspath, FilePath imported, string dirname, string basename
) {
imported = this.getImportedPath().getValue() and
imported = this.getImportedPathString() and
f.getStem() = imported.getStem() and
f.getAbsolutePath() = abspath and
dirname = imported.getDirName() and
Expand Down
29 changes: 10 additions & 19 deletions javascript/ql/lib/semmle/javascript/ES2015Modules.qll
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import javascript
private import semmle.javascript.internal.CachedStages
private import semmle.javascript.internal.paths.PathExprResolver

/**
* An ECMAScript 2015 module.
Expand Down Expand Up @@ -91,7 +92,12 @@ private predicate hasDefaultExport(ES2015Module mod) {
class ImportDeclaration extends Stmt, Import, @import_declaration {
override ES2015Module getEnclosingModule() { result = this.getTopLevel() }

override PathExpr getImportedPath() { result = this.getChildExpr(-1) }
/**
* INTERNAL USE ONLY. DO NOT USE.
*/
string getRawImportPath() { result = this.getChildExpr(-1).getStringValue() }

override Expr getImportedPathExpr() { result = this.getChildExpr(-1) }

/**
* Gets the object literal passed as part of the `with` (or `assert`) clause in this import declaration.
Expand Down Expand Up @@ -149,7 +155,7 @@ class ImportDeclaration extends Stmt, Import, @import_declaration {
}

/** A literal path expression appearing in an `import` declaration. */
private class LiteralImportPath extends PathExpr, ConstantString {
deprecated private class LiteralImportPath extends PathExpr, ConstantString {
LiteralImportPath() { exists(ImportDeclaration req | this = req.getChildExpr(-1)) }

override string getValue() { result = this.getStringValue() }
Expand Down Expand Up @@ -725,27 +731,12 @@ abstract class ReExportDeclaration extends ExportDeclaration {
cached
Module getReExportedModule() {
Stages::Imports::ref() and
result.getFile() = this.getEnclosingModule().resolve(this.getImportedPath())
or
result = this.resolveFromTypeRoot()
}

/**
* Gets a module in a `node_modules/@types/` folder that matches the imported module name.
*/
private Module resolveFromTypeRoot() {
result.getFile() =
min(TypeRootFolder typeRoot |
|
typeRoot.getModuleFile(this.getImportedPath().getStringValue())
order by
typeRoot.getSearchPriority(this.getFile().getParentContainer())
)
result.getFile() = ImportPathResolver::resolveExpr(this.getImportedPath())
}
}

/** A literal path expression appearing in a re-export declaration. */
private class LiteralReExportPath extends PathExpr, ConstantString {
deprecated private class LiteralReExportPath extends PathExpr, ConstantString {
LiteralReExportPath() { exists(ReExportDeclaration bred | this = bred.getImportedPath()) }

override string getValue() { result = this.getStringValue() }
Expand Down
4 changes: 2 additions & 2 deletions javascript/ql/lib/semmle/javascript/Expr.qll
Original file line number Diff line number Diff line change
Expand Up @@ -2821,7 +2821,7 @@ class DynamicImportExpr extends @dynamic_import, Expr, Import {
result = this.getSource().getFirstControlFlowNode()
}

override PathExpr getImportedPath() { result = this.getSource() }
override Expr getImportedPathExpr() { result = this.getSource() }

/**
* Gets the second "argument" to the import expression, that is, the `Y` in `import(X, Y)`.
Expand Down Expand Up @@ -2852,7 +2852,7 @@ class DynamicImportExpr extends @dynamic_import, Expr, Import {
}

/** A literal path expression appearing in a dynamic import. */
private class LiteralDynamicImportPath extends PathExpr, ConstantString {
deprecated private class LiteralDynamicImportPath extends PathExpr, ConstantString {
LiteralDynamicImportPath() {
exists(DynamicImportExpr di | this.getParentExpr*() = di.getSource())
}
Expand Down
15 changes: 15 additions & 0 deletions javascript/ql/lib/semmle/javascript/Files.qll
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ private module Impl = Make<FsInput>;

class Container = Impl::Container;

module Folder = Impl::Folder;

/** A folder. */
class Folder extends Container, Impl::Folder {
/** Gets the file or subfolder in this folder that has the given `name`, if any. */
Expand Down Expand Up @@ -73,6 +75,19 @@ class Folder extends Container, Impl::Folder {
)
}

/**
* Gets an implementation file and/or a typings file from this folder that has the given `stem`.
* This could be a single `.ts` file or a pair of `.js` and `.d.ts` files.
*/
File getJavaScriptFileOrTypings(string stem) {
exists(File jsFile | jsFile = this.getJavaScriptFile(stem) |
result = jsFile
or
not jsFile.getFileType().isTypeScript() and
result = this.getFile(stem + ".d.ts")
)
}

/** Gets a subfolder contained in this folder. */
Folder getASubFolder() { result = this.getAChildContainer() }
}
Expand Down
10 changes: 8 additions & 2 deletions javascript/ql/lib/semmle/javascript/HTML.qll
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ module HTML {
result = path.regexpCapture("file://(/.*)", 1)
or
not path.regexpMatch("(\\w+:)?//.*") and
result = this.getSourcePath().(ScriptSrcPath).resolve(this.getSearchRoot()).toString()
result = ResolveScriptSrc::resolve(this.getSearchRoot(), this.getSourcePath()).toString()
)
}

Expand Down Expand Up @@ -274,10 +274,16 @@ module HTML {
)
}

private module ResolverConfig implements Folder::ResolveSig {
predicate shouldResolve(Container base, string path) { scriptSrc(path, base) }
}

private module ResolveScriptSrc = Folder::Resolve<ResolverConfig>;

/**
* A path string arising from the `src` attribute of a `script` tag.
*/
private class ScriptSrcPath extends PathString {
deprecated private class ScriptSrcPath extends PathString {
ScriptSrcPath() { scriptSrc(this, _) }

override Folder getARootFolder() { scriptSrc(this, result) }
Expand Down
80 changes: 17 additions & 63 deletions javascript/ql/lib/semmle/javascript/Modules.qll
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import javascript
private import semmle.javascript.internal.CachedStages
private import semmle.javascript.internal.paths.PathExprResolver

/**
* A module, which may either be an ECMAScript 2015-style module,
Expand Down Expand Up @@ -68,7 +69,7 @@ abstract class Module extends TopLevel {
* This predicate is not part of the public API, it is only exposed to allow
* overriding by subclasses.
*/
predicate searchRoot(PathExpr path, Folder searchRoot, int priority) {
deprecated predicate searchRoot(PathExpr path, Folder searchRoot, int priority) {
path.getEnclosingModule() = this and
priority = 0 and
exists(string v | v = path.getValue() |
Expand All @@ -89,7 +90,7 @@ abstract class Module extends TopLevel {
* resolves to a folder containing a main module (such as `index.js`), then
* that file is the result.
*/
File resolve(PathExpr path) {
deprecated File resolve(PathExpr path) {
path.getEnclosingModule() = this and
(
// handle the case where the import path is complete
Expand Down Expand Up @@ -122,8 +123,14 @@ abstract class Import extends AstNode {
/** Gets the module in which this import appears. */
abstract Module getEnclosingModule();

/** DEPRECATED. Use `getImportedPathExpr` instead. */
deprecated PathExpr getImportedPath() { result = this.getImportedPathExpr() }

/** Gets the (unresolved) path that this import refers to. */
abstract PathExpr getImportedPath();
abstract Expr getImportedPathExpr();

/** Gets the imported path as a string. */
final string getImportedPathString() { result = this.getImportedPathExpr().getStringValue() }

/**
* Gets an externs module the path of this import resolves to.
Expand All @@ -132,45 +139,23 @@ abstract class Import extends AstNode {
* path is assumed to be a possible target of the import.
*/
Module resolveExternsImport() {
result.isExterns() and result.getName() = this.getImportedPath().getValue()
result.isExterns() and result.getName() = this.getImportedPathString()
}

/**
* Gets the module the path of this import resolves to.
*/
Module resolveImportedPath() {
result.getFile() = this.getEnclosingModule().resolve(this.getImportedPath())
}

/**
* Gets a module with a `@providesModule` JSDoc tag that matches
* the imported path.
*/
private Module resolveAsProvidedModule() {
exists(JSDocTag tag |
tag.getTitle() = "providesModule" and
tag.getParent().getComment().getTopLevel() = result and
tag.getDescription().trim() = this.getImportedPath().getValue()
)
}
Module resolveImportedPath() { result.getFile() = this.getImportedFile() }

/**
* Gets a module in a `node_modules/@types/` folder that matches the imported module name.
* Gets the module the path of this import resolves to.
*/
private Module resolveFromTypeRoot() {
result.getFile() =
min(TypeRootFolder typeRoot |
|
typeRoot.getModuleFile(this.getImportedPath().getValue())
order by
typeRoot.getSearchPriority(this.getFile().getParentContainer())
)
}
File getImportedFile() { result = ImportPathResolver::resolveExpr(this.getImportedPathExpr()) }

/**
* Gets the imported module, as determined by the TypeScript compiler, if any.
* DEPRECATED. Use `getImportedModule()` instead.
*/
private Module resolveFromTypeScriptSymbol() {
deprecated Module resolveFromTypeScriptSymbol() {
exists(CanonicalName symbol |
ast_node_symbol(this, symbol) and
ast_node_symbol(result, symbol)
Expand All @@ -190,42 +175,11 @@ abstract class Import extends AstNode {
Stages::Imports::ref() and
if exists(this.resolveExternsImport())
then result = this.resolveExternsImport()
else (
result = this.resolveAsProvidedModule() or
result = this.resolveImportedPath() or
result = this.resolveFromTypeRoot() or
result = this.resolveFromTypeScriptSymbol() or
result = resolveNeighbourPackage(this.getImportedPath().getValue())
)
else result = this.resolveImportedPath()
}

/**
* Gets the data flow node that the default import of this import is available at.
*/
abstract DataFlow::Node getImportedModuleNode();
}

/**
* Gets a module imported from another package in the same repository.
*
* No support for importing from folders inside the other package.
*/
private Module resolveNeighbourPackage(PathString importPath) {
exists(PackageJson json | importPath = json.getPackageName() and result = json.getMainModule())
or
exists(string package |
result.getFile().getParentContainer() = getPackageFolder(package) and
importPath = package + "/" + [result.getFile().getBaseName(), result.getFile().getStem()]
)
}

/**
* Gets the folder for a package that has name `package` according to a package.json file in the resulting folder.
*/
pragma[noinline]
private Folder getPackageFolder(string package) {
exists(PackageJson json |
json.getPackageName() = package and
result = json.getFile().getParentContainer()
)
}
Loading
Loading