-
-
Notifications
You must be signed in to change notification settings - Fork 723
fix(parser): reject import something 'source'
#15746
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
fix(parser): reject import something 'source'
#15746
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a parser bug where invalid import syntax import something 'source' (missing the from keyword) was incorrectly accepted. According to the ECMAScript specification, import declarations must include the from keyword between the import specifier and the module source string.
Key changes:
- Added validation to reject import statements with a default specifier followed directly by a string literal without the
fromkeyword - Added comprehensive test coverage for various invalid import patterns
- Updated snapshot file to reflect the new expected error messages
Reviewed Changes
Copilot reviewed 1 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/oxc_parser/src/js/module.rs | Added check to reject import something 'source' syntax by expecting from keyword when default specifier is followed by a string |
| tasks/coverage/misc/pass/import-from-from.js | Test case verifying valid edge case import from from 'module' where from is used as an identifier |
| tasks/coverage/misc/fail/import-without-from.js | Test case for invalid import unknown 'module' syntax |
| tasks/coverage/misc/fail/import-type-without-from.ts | Test case for invalid import type 'module' syntax |
| tasks/coverage/misc/fail/import-source-without-from.js | Test case for invalid import source 'module' syntax |
| tasks/coverage/misc/fail/import-from-str.js | Test case for invalid import from 'module' syntax |
| tasks/coverage/misc/fail/import-defer-without-from.js | Test case for invalid import defer 'module' syntax |
| tasks/coverage/snapshots/parser_misc.snap | Updated snapshot with expected error messages for all new failing test cases |
##Icopilot Suggestions
No issues found
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CodSpeed Performance ReportMerging #15746 will not alter performanceComparing Summary
Footnotes |
|
@sapphi-red |
ff9bfc2 to
671f3cb
Compare
|
Oops, my bad, I forgot to run them. I've updated them now. |
Merge activity
|
```js import something 'source' ``` was not rejected but this should be rejected. https://tc39.es/ecma262/2025/multipage/ecmascript-language-scripts-and-modules.html#prod-ImportDeclaration
671f3cb to
d60ca81
Compare
### 💥 BREAKING CHANGES - ea51b0b napi: [**BREAKING**] Standardize function naming with sync suffixes (#15661) (Boshen) ### 🚀 Features - 77efb76 parser: Improve error message for invalid switch clauses (#15728) (sapphi-red) - 5691727 parser: Improve `import source` `from` error message (#15727) (sapphi-red) - b7404bc parser: Improve error message for missing function body (#15726) (sapphi-red) - 71c2fb0 parser: Improve error message when JSX is found while not enabled (#15725) (sapphi-red) - 56e7e44 minifier: Disable removal of unnecessary `use strict` directives for DCE (#15691) (sapphi-red) - 8a61cfd allocator, ast: Introduce `UnstableAddress` trait (#15700) (overlookmotel) - f5ce55a napi: Export all options using wildcard exports (Boshen) - 68703b9 minifier: Rotate binary expressions to remove parentheses (#15473) (sapphi-red) ### 🐛 Bug Fixes - c023ba6 semantic: Do not duplicate statements in temp `Vec` when binding `TSModuleDeclaration`s (#15724) (overlookmotel) - d60ca81 parser: Reject `import something 'source'` (#15746) (sapphi-red) - e0728fa ast: Exclude comment end position from `is_inside_comment` check (#15753) (camc314) - 9f54a36 semantic: Error on `\00` in strict mode (#15743) (sapphi-red) - 440a977 ast: Include rest properties when using `get_binding_identifiers` (#15710) (camc314) ### ⚡ Performance - 1f09d3c parser: Faster checking for invalid modifiers (#15717) (overlookmotel) - d8d4e31 ast: Use loop instead of recursion in `TSModuleDeclarationBody::as_module_block_mut` (#15713) (overlookmotel) ### 📚 Documentation - e033d50 ast: Clarify behavior of `TSModuleDeclaration::has_use_strict_directive` (#15730) (overlookmotel) - 9eda70f allocator: Improve docs for `Address` methods (#15697) (overlookmotel)
### 💥 BREAKING CHANGES - ea51b0b napi: [**BREAKING**] Standardize function naming with sync suffixes (#15661) (Boshen) ### 🚀 Features - 77efb76 parser: Improve error message for invalid switch clauses (#15728) (sapphi-red) - 5691727 parser: Improve `import source` `from` error message (#15727) (sapphi-red) - b7404bc parser: Improve error message for missing function body (#15726) (sapphi-red) - 71c2fb0 parser: Improve error message when JSX is found while not enabled (#15725) (sapphi-red) - 56e7e44 minifier: Disable removal of unnecessary `use strict` directives for DCE (#15691) (sapphi-red) - 8a61cfd allocator, ast: Introduce `UnstableAddress` trait (#15700) (overlookmotel) - f5ce55a napi: Export all options using wildcard exports (Boshen) - 68703b9 minifier: Rotate binary expressions to remove parentheses (#15473) (sapphi-red) ### 🐛 Bug Fixes - c023ba6 semantic: Do not duplicate statements in temp `Vec` when binding `TSModuleDeclaration`s (#15724) (overlookmotel) - d60ca81 parser: Reject `import something 'source'` (#15746) (sapphi-red) - e0728fa ast: Exclude comment end position from `is_inside_comment` check (#15753) (camc314) - 9f54a36 semantic: Error on `\00` in strict mode (#15743) (sapphi-red) - 440a977 ast: Include rest properties when using `get_binding_identifiers` (#15710) (camc314) ### ⚡ Performance - 1f09d3c parser: Faster checking for invalid modifiers (#15717) (overlookmotel) - d8d4e31 ast: Use loop instead of recursion in `TSModuleDeclarationBody::as_module_block_mut` (#15713) (overlookmotel) ### 📚 Documentation - e033d50 ast: Clarify behavior of `TSModuleDeclaration::has_use_strict_directive` (#15730) (overlookmotel) - 9eda70f allocator: Improve docs for `Address` methods (#15697) (overlookmotel)

was not rejected but this should be rejected.
https://tc39.es/ecma262/2025/multipage/ecmascript-language-scripts-and-modules.html#prod-ImportDeclaration