-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Fix #62179: Restrict import attribute values to string literals #62638
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
base: main
Are you sure you want to change the base?
Fix #62179: Restrict import attribute values to string literals #62638
Conversation
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
1 similar comment
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
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 issue #62179 by restricting import attribute values to string literals only, preventing arbitrary JavaScript expressions from being used. The parser previously accepted any expression (functions, numbers, arrays, etc.) in import attributes, which violated the ECMAScript specification.
Key changes:
- Modified the parser to only accept string literals as import attribute values
- Added comprehensive test coverage for various invalid expression types
- Updated existing test baselines to reflect the stricter parser behavior
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/compiler/parser.ts | Changed parser to use parseLiteralLikeNode(SyntaxKind.StringLiteral) instead of parseAssignmentExpressionOrHigher() for import attribute values |
tests/cases/compiler/importTypeAttributesNonString.ts | New test file validating rejection of non-string expressions and acceptance of valid string literals |
tests/cases/fourslash/organizeImportsAttributes4.ts | Removed test case with numeric attribute value (now invalid) |
tests/baselines/reference/importTypeAttributesNonString.* | Baseline files for the new test showing expected errors for invalid attribute values |
tests/baselines/reference/importAttributes6(module=). | Updated baselines reflecting changed parser behavior producing syntax errors instead of type errors |
tests/baselines/reference/importAssertionNonstring.* | Updated baselines for import assertions with non-string values |
~~~~~~~~~~~ | ||
!!! error TS1005: ':' expected. | ||
~~~~~~ | ||
!!! error TS2880: Import assertions have been replaced by import attributes. Use 'with' instead of 'assert'. |
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.
The error messages in this test are much worse now
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.
I think they should be better now. Thank you for the review.
Summary
Fixed Issue #62179: Import type attributes were incorrectly accepting arbitrary JavaScript expressions instead of only string literals.
Changes Made:
- Changed parseAssignmentExpressionOrHigher() to parseLiteralLikeNode(SyntaxKind.StringLiteral)
- Now properly restricts import attribute values to string literals only
- Tests that non-string values (functions, numbers, arrays, objects, etc.) are properly rejected
- Verifies string literals still work correctly
- Fixed existing tests to reflect the new parser behavior
Fixes issue #62179