Skip to content

fix(parser): report accessor definite assertion on token#23203

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/fix-accessor-definite-offset
Jun 10, 2026
Merged

fix(parser): report accessor definite assertion on token#23203
graphite-app[bot] merged 1 commit into
mainfrom
codex/fix-accessor-definite-offset

Conversation

@camc314

@camc314 camc314 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Pass the definite-assignment assertion token offset through the class accessor-property parser path.
  • Report invalid ambient and abstract accessor prop! diagnostics on the ! token, matching the span behavior used by variable declarators and normal class properties.
  • Add focused misc parser coverage for the accessor-specific invalid cases.

Rationale

This follows up #23170. That parser change already narrowed definite-assignment diagnostics for variables and normal class properties, but accessor properties still kept only a boolean. That meant accessor prop! could only report TS1255 using the full class element span. Carrying the offset into parse_class_accessor_property aligns accessor diagnostics with the other definite-assignment diagnostics and keeps the AST shape unchanged by storing only definite.is_some().

The misc fixture is needed because upstream TypeScript coverage exercises invalid definite assertions on normal fields, but not invalid accessor properties. Existing misc coverage only had a valid accessor y! pass case, so just coverage did not specifically protect the accessor diagnostic span this fixes.

@github-actions github-actions Bot added A-parser Area - Parser A-transformer Area - Transformer / Transpiler labels Jun 10, 2026
@camc314 camc314 force-pushed the codex/fix-accessor-definite-offset branch from 7442e2b to e9f1daf Compare June 10, 2026 07:21
@codspeed-hq

codspeed-hq Bot commented Jun 10, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 62 untouched benchmarks
⏩ 9 skipped benchmarks1


Comparing codex/fix-accessor-definite-offset (e9f1daf) with main (3de589a)

Open in CodSpeed

Footnotes

  1. 9 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 marked this pull request as ready for review June 10, 2026 07:41
Copilot AI review requested due to automatic review settings June 10, 2026 07:41
@camc314 camc314 removed the A-transformer Area - Transformer / Transpiler label Jun 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Aligns TypeScript definite-assignment assertion (!) diagnostics for class accessor properties with existing span behavior for variable declarators and normal class properties by reporting TS1255 on the ! token rather than the full class element span.

Changes:

  • Thread the definite-assignment assertion token start offset (Option<u32>) through the class accessor-property parsing path and emit TS1255 using a 1-character span on !.
  • Keep the AST shape unchanged by continuing to store only definite.is_some() on the accessor property node.
  • Add misc negative coverage for invalid accessor prop! in ambient and abstract contexts, and update the parser misc snapshot.

Reviewed changes

Copilot reviewed 1 out of 3 changed files in this pull request and generated no comments.

File Description
crates/oxc_parser/src/js/class.rs Carries the ! token start offset into parse_class_accessor_property and reports TS1255 on a sized span targeting the !.
tasks/coverage/misc/fail/oxc-23170.ts Adds focused failing TypeScript inputs covering ambient and abstract accessor-property definite assertions.
tasks/coverage/snapshots/parser_misc.snap Updates snapshot to include the two new TS1255 diagnostics and adjusts negative test counts accordingly.

@camc314 camc314 added the 0-merge Merge with Graphite Merge Queue label Jun 10, 2026

camc314 commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

## Summary
- Pass the definite-assignment assertion token offset through the class accessor-property parser path.
- Report invalid ambient and abstract `accessor prop!` diagnostics on the `!` token, matching the span behavior used by variable declarators and normal class properties.
- Add focused misc parser coverage for the accessor-specific invalid cases.

## Rationale
This follows up #23170. That parser change already narrowed definite-assignment diagnostics for variables and normal class properties, but accessor properties still kept only a boolean. That meant `accessor prop!` could only report TS1255 using the full class element span. Carrying the offset into `parse_class_accessor_property` aligns accessor diagnostics with the other definite-assignment diagnostics and keeps the AST shape unchanged by storing only `definite.is_some()`.

The misc fixture is needed because upstream TypeScript coverage exercises invalid definite assertions on normal fields, but not invalid accessor properties. Existing misc coverage only had a valid `accessor y!` pass case, so `just coverage` did not specifically protect the accessor diagnostic span this fixes.
@graphite-app graphite-app Bot force-pushed the codex/fix-accessor-definite-offset branch from e9f1daf to 8edd234 Compare June 10, 2026 07:46
@graphite-app graphite-app Bot merged commit 8edd234 into main Jun 10, 2026
30 checks passed
@graphite-app graphite-app Bot removed the 0-merge Merge with Graphite Merge Queue label Jun 10, 2026
@graphite-app graphite-app Bot deleted the codex/fix-accessor-definite-offset branch June 10, 2026 07:50
Boshen added a commit that referenced this pull request Jun 15, 2026
### 💥 BREAKING CHANGES

- 7a24911 codegen: [**BREAKING**] Borrow sourcemaps from codegen
(#23422) (Boshen)
- bb0ed44 transformer: [**BREAKING**] Disable styled-components
transpileTemplateLiterals by default (#23171) (Boshen)

### 🚀 Features

- 1490a0a linter/react: Implement react-compiler rule (#23202) (Boshen)
- 6c0bdf0 transformer/react-refresh: Support `module.property.useHook()`
(#23190) (Dunqing)
- 47991bd semantic: Report TS1228 for invalid type predicates (#23174)
(camc314)
- 1d3af58 parser: Add TS2398 parameter property diagnostic (#23216)
(camc314)
- 44313da semantic: Add `scope_is_descendant_of` api (#22313) (camc314)
- e5050c0 parser: Improve diagnostic for rest initializer (#23205)
(camc314)
- ec266bb transformer: Run React Compiler as a feature-gated transform
pass (#23201) (Boshen)
- e7374fe parser: Report error for `const` modifier on interface type
parameter (#23173) (camc314)
- a7c1c9b parser: Report ambient definite variable assertions (#23165)
(camc314)
- d169fcd parser: Report invalid class definite assertions (#23164)
(camc314)
- 00244d8 parser: Report definite property initializer errors (#23160)
(camc314)

### 🐛 Bug Fixes

- 52d0c31 transformer: Replace ambient dot defines (#23231) (camc314)
- 2c28748 transformer/class: Parent generated constructors to class
scope (#23222) (camc314)
- 8edd234 parser: Report accessor definite assertion on token (#23203)
(camc314)
- de38a3f react_compiler: Keep imports referenced only by a local
re-export (#23176) (Boshen)
- f5721c2 codegen: Preserve parentheses around `intrinsic` type
reference (#23156) (Boshen)
- e89f81d parser: Don't emit TS1477 for parenthesized instantiation
expression (#23147) (Boshen)
- 8a04149 parser: Reject module-referencing imports/exports in a
namespace body (#22829) (Boshen)

### ⚡ Performance

- 2783295 parser: Table-driven operator precedence lookup (#23346)
(Boshen)
- 231d5de parser: Single-match member expression dispatch (#23347)
(Boshen)
- e89729b codegen: Accept one-shot wrap closures (#23265) (camc314)
- a6c11fa parser: Force-inline read_non_decimal to fold per-digit number
dispatch (#23157) (Boshen)
- d74964c parser: Store class definite assertion offset (#23170)
(camc314)
- f0fda4d parser: Shrink-wrap cold diagnostic tails out of hot frames
(#23159) (Boshen)
- a082180 parser: Store definite assertion offset (#23167) (camc314)
- 534f9c6 oxc: Conditionally rebuild semantic in compiler pipeline
(#23153) (Boshen)
- b435c6a parser: Skip checkpoint for `infer T extends U` constraint in
disallow context (#23128) (Boshen)
- 7464dce parser: Peek instead of checkpoint/rewind for `export default`
modifier (#23124) (Boshen)
- 80a9a32 parser: Fast-path single-keyword TS declarations (#23083)
(Boshen)
- da1a6c6 diagnostics: Migrate to allocation-optimized oxc-miette
(#23094) (Boshen)
- b7b08ce parser: Peek once for the static modifier disambiguation
(#23079) (Boshen)
- e7e07a3 parser: Fold unary dispatch into a single match (#23076)
(Boshen)

### 📚 Documentation

- d241add semantic: Add `AGENTS.md` test guidance for agents (#23441)
(camc314)
- 026f1ae parser: Add `AGENTS.md` test guidance for agents (#23440)
(camc314)
- 09755ac transformer: Add `AGENTS.md` test guidance for agents (#23439)
(camc314)
- e6bdfd4 lexer: Correct reference link for `byte_handlers!` (#23313)
(Dunqing)
- 65b6d7a allocator: Fix memory leaks in `Arena` examples (#23257)
(overlookmotel)

Co-authored-by: Boshen <1430279+Boshen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-parser Area - Parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants