JS: Parse alternative import syntax - #22417
Open
MathiasVP wants to merge 2 commits into
Open
Conversation
…ny modern JS implementations, but may be supported by transpilers.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds parsing support for legacy module X from "..." syntax as namespace imports.
Changes:
- Detects and parses legacy module imports.
- Adds extraction input and expected TRAP output.
Show a summary per file
| File | Description |
|---|---|
Parser.java |
Implements legacy import parsing. |
import8.js |
Adds parser test input. |
import8.js.trap |
Adds expected extraction output. |
Review details
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Suppressed comments (2)
javascript/extractor/src/com/semmle/jcorn/Parser.java:2801
- This recognizes the declaration only after source-type detection has already classified the file.
JSExtractor.establishSourceTyperecognizes onlyimport,export, andgoog.module(JSExtractor.java:36-39,97-106), so a file containing only this syntax remains a script. The added TRAP fixture confirms that outcome: it has nois_module/is_es2015_moduletuple and bindsUtilsin the global scope, unlike the equivalent namespace import inimport5.js.trap. This breaks the promised equivalence and prevents the file from being represented as anES2015Module; extend source-type detection to recognize this legacy declaration too.
} else if (topLevel && this.isLegacyModuleImport()) {
if (!this.options.allowImportExportEverywhere() && !this.inModule)
this.raise(this.start, "Legacy module imports may appear only with 'sourceType: module'");
return this.parseLegacyModuleImport(startLoc);
javascript/extractor/src/com/semmle/jcorn/Parser.java:3616
- The namespace specifier is finished only after parsing
from, the source, and the semicolon, so its source range incorrectly spansUtils from "./utils";. The generated fixture shows this as location 1:8–1:28, whereas anImportNamespaceSpecifiershould cover only the syntactic specifier (the equivalent standard import covers* as foo). Finish this node immediately after parsinglocal, before consumingfrom.
List<ImportSpecifier> specifiers = new ArrayList<ImportSpecifier>();
specifiers.add(this.finishNode(new ImportNamespaceSpecifier(specifierLoc, local)));
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+121
to
+122
| private static final Pattern LEGACY_MODULE_IMPORT_TAIL = | ||
| Pattern.compile("\\s+[A-Za-z_$][A-Za-z0-9_$]*\\s+from\\s*['\"]"); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR implements support for ... "alternative module import syntax" in JS. The motivation can be seen in this old version of handlebars:
This is not valid JS, but we obviously still want to be able to create a CodeQL database for such projects. So this PR changes the JS parser so that it outputs TRAP equivalent to: