-
Notifications
You must be signed in to change notification settings - Fork 718
fix(parser): panic when parsing JSDoc @satisfies with export assignments #1647
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): panic when parsing JSDoc @satisfies with export assignments #1647
Conversation
…ments The parser was panicking when encountering JSDoc `@satisfies` tags on module.exports statements. The issue was that KindExportAssignment and KindJSExportAssignment were incorrectly grouped with nodes that have an Initializer() method, but they actually use Expression() instead. Also added missing support for KindCommonJSExport in the Initializer() and SetInitializer() methods. Reproduction: ```javascript /** * @Satisfies {import('../types.js').SupportVersionTraceMap} */ module.exports = { zlib: zlib, 'node:zlib': { ...zlib, [READ]: { supported: ['14.13.1', '12.20.0'] }, }, }; ```
a77e847
to
fd02030
Compare
To test, you'd want to make a compiler test in |
Adds a compiler test that verifies the parser doesn't panic when encountering JSDoc @Satisfies tags on module.exports statements. This test would have failed before the fix in the previous commit.
thanks as always @jakebailey 🙂 Before/after
|
testdata/baselines/reference/compiler/panicSatisfiesOnExportEqualsDeclaration.errors.txt
Outdated
Show resolved
Hide resolved
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 panic that occurred when encountering JSDoc @satisfies
tags on module.exports statements. The issue was caused by incorrect grouping of export assignment nodes with nodes that have an Initializer() method, when they actually use Expression() instead.
- Moves KindExportAssignment and KindJSExportAssignment from the Initializer() case to the Expression() case in the JSDoc satisfies parser logic
- Adds missing support for KindCommonJSExport in the Initializer() and SetInitializer() methods
- Includes a test case to verify the fix works correctly
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
testdata/tests/cases/compiler/panicSatisfiesOnExportEqualsDeclaration.ts | Test case demonstrating JSDoc @Satisfies on module.exports |
testdata/baselines/reference/compiler/panicSatisfiesOnExportEqualsDeclaration.types | Expected type output for the test case |
testdata/baselines/reference/compiler/panicSatisfiesOnExportEqualsDeclaration.symbols | Expected symbol output for the test case |
internal/parser/reparser.go | Fixes the parser logic by moving export assignments to the correct case |
internal/ast/ast.go | Adds missing KindCommonJSExport support in Initializer methods |
The parser was panicking when encountering JSDoc
@satisfies
tags on module.exports statements. The issue was that KindExportAssignment and KindJSExportAssignment were incorrectly grouped with nodes that have an Initializer() method, but they actually use Expression() instead.Also added missing support for KindCommonJSExport in the Initializer() and SetInitializer() methods.
Reproduction:
I couldn't actually work out how to write a test for this, so would appreciate some guidance. thanks