Skip to content

fix(linter/plugins): use correct ScriptKind for tokens#17185

Merged
overlookmotel merged 9 commits intooxc-project:mainfrom
wagenet:fix-parsing-type-bug
Jan 15, 2026
Merged

fix(linter/plugins): use correct ScriptKind for tokens#17185
overlookmotel merged 9 commits intooxc-project:mainfrom
wagenet:fix-parsing-type-bug

Conversation

@wagenet
Copy link
Contributor

@wagenet wagenet commented Dec 20, 2025

Summary

When parsing tokens for JS plugins, the TypeScript parser was always using ScriptKind.TSX regardless of file extension. This caused TypeScript to incorrectly parse generic arrow functions like <T>() => {} in .ts files, creating bogus JsxText tokens that overlap with comments.

The fix reads the JSX and TypeScript flags from the buffer metadata:

  • isJsx() reads the is_jsx flag from RawTransferMetadata
  • isTypescript() reads the is_ts flag from RawTransferMetadata

This ensures the ScriptKind reflects what Oxc actually parsed, not just what the file extension suggests.

The Bug

Using the wrong ScriptKind (e.g., TSX for a .ts file) causes TypeScript to parse generic arrow functions like <T>() => {} incorrectly, creating bogus JsxText tokens that overlap with comments. This manifests as errors like:

Error running JS plugin.
Error: Overlapping token/comments: last end: 2715, next start: 2689

Changes

  • Added is_jsx field to RawTransferMetadata struct (in both napi/parser and crates/oxc_linter)
  • Added IS_JSX_FLAG_POS constant to generated constants (via ast_tools)
  • Added isJsx() and isTypescript() helper functions in source_code.ts
  • Updated getScriptKind() in tokens_parse.ts to use buffer flags instead of file extension matching
  • Added snapshot test with generic_arrow.ts test file

Test plan

  • Added generic_arrow.ts test file that triggers the bug when parsed as TSX
  • Snapshot test verifies correct tokenization (no overlapping tokens error)
  • All oxlint tests pass

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings December 20, 2025 19:14
@github-actions github-actions bot added A-linter Area - Linter A-cli Area - CLI A-linter-plugins Area - Linter JS plugins C-bug Category - Bug labels Dec 20, 2025
Copy link
Contributor

Copilot AI left a 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 bug where the TypeScript parser always used ScriptKind.TSX regardless of file extension, causing generic arrow functions in .ts files to be incorrectly parsed and create overlapping JsxText tokens with comments.

Key Changes:

  • Added getScriptKind() function to determine correct TypeScript ScriptKind based on file extension
  • Modified parseTokens() to use extension-specific ScriptKind instead of hardcoded TSX

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
apps/oxlint/src-js/plugins/tokens_parse.ts Implements getScriptKind() function and updates parser to use correct ScriptKind
apps/oxlint/test/tokens_script_kind.test.ts Adds comprehensive test coverage for ScriptKind detection and regression tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wagenet
Copy link
Contributor Author

wagenet commented Dec 20, 2025

I'm not sure this is the very best solution, but it does address a real issue with incorrect parsing.

@camc314 camc314 changed the title fix(linter): use correct TypeScript ScriptKind based on file extension fix(linter/plugins): use correct ScriptKind for tokens based on file extension Dec 21, 2025
Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid this is not the right way to solve this. We need to get the "is it JSX" flag from the Program. It's not completely trivial, hence why I left it as a TODO.

@overlookmotel
Copy link
Member

overlookmotel commented Dec 21, 2025

I've marked it as "requested changes" not necessarily to request that you make the changes, but just to stop the PR getting merged in current state by accident.

If you want to tackle it, need to create a const with the offset of the byte for the JSX flag in SourceType in codegen, and then look up that byte in the buffer, to determine whether file is JSX or not. There are some other consts for similar things e.g. byte offset of source text length field.

I don't know if that's enough to go on...

@wagenet wagenet changed the title fix(linter/plugins): use correct ScriptKind for tokens based on file extension fix(linter/plugins): use correct ScriptKind for tokens Dec 21, 2025
@github-actions github-actions bot added A-parser Area - Parser A-ast-tools Area - AST tools labels Dec 21, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Dec 22, 2025

Merging this PR will not alter performance

✅ 42 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing wagenet:fix-parsing-type-bug (3cf7e06) with main (8ee6f80)2

Open in CodSpeed

Footnotes

  1. 3 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (2a397f8) during the generation of this report, so 8ee6f80 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much for working on this.

Actually - and sorry to change my mind - there's an easier way to do this which I hadn't spotted before.

RawTransferMetadata already contains an is_ts field. We should add an is_jsx field too.

For convoluted reasons, there are 2 copies of RawTransferMetadata in apps/oxlint and napi/parser. Both must be identical, so you'd need to add the field to both.

Then get ast_tools codegen to generate an IS_JSX_FLAG_POS constant, same as for IS_TS_FLAG_POS.


Please also run the following after making the change, and commit any change to the conformance snapshot:

cd apps/oxlint
pnpm run init-conformance
pnpm run build-conformance
pnpm run conformance

init-conformance only needs to be run once - it initializes the ESLint submodule. If you make changes after that, just run the last 2 commands again.

I don't know if this is covered by ESLint's conformance tests, but it'd be good to find out!

@wagenet wagenet force-pushed the fix-parsing-type-bug branch from ddb614f to bf2fbcb Compare December 27, 2025 03:11
@wagenet wagenet force-pushed the fix-parsing-type-bug branch from a69742a to 1a81946 Compare January 5, 2026 18:09
Copy link
Member

@overlookmotel overlookmotel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for addressing my feedback. And sorry for the delay - only got back to my desk today.

(oops I didn't mean to approve the changes)

@overlookmotel
Copy link
Member

I think CI fail is caused by an oddity in snapshot encoding. If you build oxlint with pnpm run build-test, and then rerun the tests with snapshot update, it should solve it.

@overlookmotel overlookmotel self-requested a review January 15, 2026 01:29
@overlookmotel
Copy link
Member

You've waited a long time for review, so I've taken the liberty of making the changes I requested myself. Will merge once CI passes.

@overlookmotel overlookmotel merged commit 7a0ca99 into oxc-project:main Jan 15, 2026
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ast-tools Area - AST tools A-cli Area - CLI A-linter Area - Linter A-linter-plugins Area - Linter JS plugins A-parser Area - Parser C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants