Skip to content

feat(parser): improve errors for private ident outside class#19080

Closed
camc314 wants to merge 1 commit intoc/02-06-refactor_parser_semantic_move_private_ident_check_to_parserfrom
c/02-06-feat_parser_improve_errors_for_private_ident_outside_class
Closed

feat(parser): improve errors for private ident outside class#19080
camc314 wants to merge 1 commit intoc/02-06-refactor_parser_semantic_move_private_ident_check_to_parserfrom
c/02-06-feat_parser_improve_errors_for_private_ident_outside_class

Conversation

@camc314
Copy link
Contributor

@camc314 camc314 commented Feb 6, 2026

No description provided.

@camc314 camc314 marked this pull request as ready for review February 6, 2026 16:46
Copilot AI review requested due to automatic review settings February 6, 2026 16:46
@github-actions github-actions bot added A-parser Area - Parser C-enhancement Category - New feature or request labels Feb 6, 2026
@camc314 Graphite App
Copy link
Contributor Author

camc314 commented Feb 6, 2026 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

  • 0-merge - adds this PR to the back of the merge queue
  • hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

Copilot AI reviewed Feb 6, 2026
View reviewed changes
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

Updates the parser to emit a more specific diagnostic (TS18016) when encountering private identifiers (#foo) in places where they’re syntactically invalid (e.g., object literals, destructuring patterns, TS type members), and refreshes coverage snapshots accordingly.

Changes:

  • Add a TS18016 diagnostic for private identifiers used outside class bodies during property-name parsing.
  • Extend parse_property_name to optionally accept PrivateIdentifier tokens (gated by a boolean flag) and update call sites.
  • Update parser coverage snapshots (TypeScript, Babel fixtures, Test262) to reflect the new error output.

Reviewed changes

Copilot reviewed 5 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tasks/coverage/snapshots/parser_typescript.snap Snapshot updates to reflect TS18016 (and related diagnostics) for invalid private identifiers.
tasks/coverage/snapshots/parser_test262.snap Snapshot updates for Test262 negative cases now reporting TS18016 where applicable.
tasks/coverage/snapshots/parser_babel.snap Snapshot updates for Babel parser fixtures to reflect TS18016 output.
crates/oxc_parser/src/ts/types.rs Update TS type-member parsing to call the new parse_property_name(false) signature.
crates/oxc_parser/src/js/object.rs Add allow_private_identifier parameter, handle Kind::PrivateIdentifier, and update call sites.
crates/oxc_parser/src/js/class.rs Use parse_property_name(true) for class element names and keep private-name validations.
crates/oxc_parser/src/js/binding.rs Update destructuring binding property parsing to call parse_property_name(false).
crates/oxc_parser/src/diagnostics.rs Add new private_identifiers_not_allowed_outside_class_bodies TS(18016) diagnostic.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 6, 2026 •
edited
Loading

CodSpeed Performance Report

Merging this PR will improve performance by 69.78%

Comparing c/02-06-feat_parser_improve_errors_for_private_ident_outside_class (2a792c5) with c/02-06-refactor_parser_semantic_move_private_ident_check_to_parser (e3620ad)1

Summary

⚡ 4 improved benchmarks
✅ 43 untouched benchmarks
⏩ 3 skipped benchmarks2

Performance Changes

Mode Benchmark BASE HEAD Efficiency
⚡ Simulation mangler[RadixUIAdoptionSection.jsx] 14.2 µs 8.4 µs +69.78%
⚡ Simulation mangler[RadixUIAdoptionSection.jsx_keep_names] 19.9 µs 14.1 µs +41.32%
⚡ Simulation mangler[cal.com.tsx] 2.6 ms 2.2 ms +18.69%
⚡ Simulation mangler[binder.ts] 637 µs 539.1 µs +18.16%

Footnotes

  1. No successful run was found on c/02-06-refactor_parser_semantic_move_private_ident_check_to_parser (6851ec2) during the generation of this report, so 64af4d8 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

  2. 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. ↩

@camc314 camc314 force-pushed the c/02-06-feat_parser_improve_errors_for_private_ident_outside_class branch from fa7b65f to 4fecb4a Compare February 6, 2026 16:54
Boshen
Boshen reviewed Feb 6, 2026
View reviewed changes
tasks/coverage/snapshots/parser_test262.snap Outdated
╰────

× Unexpected token
× TS(18016): Private identifiers are not allowed outside class bodies.
Copy link
Member

@Boshen Boshen Feb 6, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

why do we have two errors, where one has a ts code. ecmascript syntax errors should not have a ts code.

Copy link
Contributor Author

@camc314 camc314 Feb 6, 2026

Choose a reason for hiding this comment

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

we were checking this both in the parser and in semantic - i've changed it see the upstack PR so that we only check this in the parser. this fixes the duplicate error diagnostic

@camc314 camc314 changed the base branch from main to graphite-base/19080 February 6, 2026 17:33
@camc314 camc314 force-pushed the c/02-06-feat_parser_improve_errors_for_private_ident_outside_class branch from 4fecb4a to 59a439a Compare February 6, 2026 17:33
@camc314 camc314 force-pushed the graphite-base/19080 branch from 7160485 to 13be494 Compare February 6, 2026 17:33
@camc314 camc314 changed the base branch from graphite-base/19080 to c/02-06-refactor_parser_semantic_move_private_ident_check_to_parser February 6, 2026 17:33
@camc314 camc314 mentioned this pull request Feb 6, 2026
Closed
@camc314 camc314 force-pushed the c/02-06-feat_parser_improve_errors_for_private_ident_outside_class branch 2 times, most recently from a2f6120 to ef013de Compare February 6, 2026 17:54
@camc314 camc314 force-pushed the c/02-06-refactor_parser_semantic_move_private_ident_check_to_parser branch 2 times, most recently from e3620ad to cd89ad2 Compare February 7, 2026 00:18
@camc314 camc314 force-pushed the c/02-06-feat_parser_improve_errors_for_private_ident_outside_class branch from ef013de to f7e7567 Compare February 7, 2026 00:18
@camc314 camc314 force-pushed the c/02-06-refactor_parser_semantic_move_private_ident_check_to_parser branch from cd89ad2 to 6851ec2 Compare February 7, 2026 20:41
@camc314 camc314 force-pushed the c/02-06-feat_parser_improve_errors_for_private_ident_outside_class branch from f7e7567 to 2a792c5 Compare February 7, 2026 20:41
@Boshen Boshen self-assigned this Feb 8, 2026
@Boshen Boshen closed this Feb 24, 2026
@Boshen Boshen deleted the c/02-06-feat_parser_improve_errors_for_private_ident_outside_class branch February 24, 2026 08:49
@Boshen Boshen mentioned this pull request Feb 24, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Boshen Boshen Boshen left review comments

Copilot code review Copilot Copilot left review comments

Assignees

@Boshen Boshen

Labels

A-parser Area - Parser C-enhancement Category - New feature or request

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@camc314 @Boshen