Conversation
3328282 to
ab37605
Compare
There was a problem hiding this comment.
Pull request overview
This PR implements proper handling of the hashbang property in AST snapshots for test cases that include a hashbang (shebang). Oxc's AST has a non-standard Hashbang property, and this change populates it correctly instead of always setting it to null. This allows the repository to avoid skipping these test cases in ESTree conformance testing.
Changes:
- Removed the global
ast.hashbang = nullinitialization fromstringifyWithfunction - Added hashbang detection and property generation in both the TypeScript-ESLint and Test262 parsers
- Updated snapshot files to include proper
hashbangobjects withtype,value,start, andendproperties where files contain shebangs
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/json.js | Removed the automatic hashbang: null assignment since parsers now handle this property |
| src/typescript-eslint.js | Added logic to detect hashbangs, convert them to comments for parsing, then reconstruct the hashbang property from the first comment |
| src/test262.js | Added comment collection and hashbang property generation from the first comment when code starts with #! |
| tests/typescript/tests/cases/compiler/shebang.ts.md | Updated snapshot with correct hashbang object for /usr/bin/env node |
| tests/typescript/tests/cases/compiler/shebangBeforeReferences.ts.md | Updated snapshot with correct hashbang object for /usr/bin/env node |
| tests/typescript/tests/cases/compiler/emitBundleWithShebang1.ts.md | Updated snapshot with correct hashbang object for /usr/bin/env gjs |
| tests/typescript/tests/cases/compiler/emitBundleWithShebang2.ts.md | Updated snapshots (2 instances) with correct hashbang objects for /usr/bin/env gjs and /usr/bin/env js |
| tests/typescript/tests/cases/compiler/emitBundleWithShebangAndPrologueDirectives1.ts.md | Updated snapshot with correct hashbang object for /usr/bin/env gjs |
| tests/typescript/tests/cases/compiler/emitBundleWithShebangAndPrologueDirectives2.ts.md | Updated snapshots (2 instances) with correct hashbang object for /usr/bin/env gjs |
| tests/test262/test/language/comments/hashbang/module.json | Updated snapshot with correct hashbang object for empty hashbang |
| tests/test262/test/language/comments/hashbang/not-empty.json | Updated snapshot with correct hashbang object for hashbang with comment text |
| tests/test262/test/language/comments/hashbang/use-strict.json | Updated snapshot with correct hashbang object for "use strict" hashbang |
| tests/test262/test/language/comments/hashbang/line-terminator-carriage-return.json | Updated snapshot with correct hashbang object for hashbang ending with carriage return |
| tests/test262/test/language/comments/hashbang/line-terminator-line-separator.json | Updated snapshot with correct hashbang object for hashbang ending with line separator |
| tests/test262/test/language/comments/hashbang/line-terminator-paragraph-separator.json | Updated snapshot with correct hashbang object for hashbang ending with paragraph separator |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Add `hashbang` property to AST if file starts with a hashbang. | ||
| // This property is non-standard and exclusive to Oxc. | ||
| program.hashbang = hasHashbang ? { ...comments[0], type: "Hashbang" } : null; |
There was a problem hiding this comment.
Potential issue: Assuming that comments[0] is the hashbang comment may not be safe. If the code has leading whitespace or other comments before the hashbang is converted to a comment, this could fail or assign the wrong comment. Consider checking that comments[0].start === 0 to ensure it's actually the first thing in the file.
| program.hashbang = hasHashbang ? { ...comments[0], type: "Hashbang" } : null; | |
| let hashbangComment = null; | |
| if (hasHashbang && Array.isArray(comments) && comments.length > 0) { | |
| // Prefer a comment that starts at the very beginning of the file. | |
| const firstCommentAtStart = comments.find((comment) => comment.start === 0); | |
| const baseComment = firstCommentAtStart ?? comments[0]; | |
| hashbangComment = { ...baseComment, type: "Hashbang" }; | |
| } | |
| program.hashbang = hashbangComment; |
|
|
||
| // Add `hashbang` property to AST if file starts with a hashbang. | ||
| // This property is non-standard and exclusive to Oxc. | ||
| if (comments.length > 0 && code.startsWith("#!") && comments[0].type === "Line") { |
There was a problem hiding this comment.
Potential issue: The check comments.length > 0 && code.startsWith("#!") && comments[0].type === "Line" doesn't verify that comments[0] is actually the hashbang comment at position 0. If there's leading whitespace or other comments, comments[0] might not correspond to the hashbang. Consider adding a check that comments[0].start === 0 to ensure it's the first thing in the file.
| if (comments.length > 0 && code.startsWith("#!") && comments[0].type === "Line") { | |
| if ( | |
| comments.length > 0 && | |
| code.startsWith("#!") && | |
| comments[0].type === "Line" && | |
| comments[0].start === 0 | |
| ) { |
Oxc's AST on JS side has a non-standard `hashbang` property. Previously in ESTree conformance tests, we skipped files with a hashbang. Bump `estree-conformance` submodule to include oxc-project/estree-conformance#183 which adds `hashbang` property to AST in snapshots. We can now enable the tests which we previously skipped.

Add
hashbangproperty to AST for test cases which include one.Oxc's AST has a non-standard
Hashbangproperty. We already addedhashbang: nullto ASTs in snapshots. The PR fills in that property correctly where the file does have a hashbang. We now don't need to skip these test cases in ESTree conformance.(2nd attempt after #180)