Skip to content

fix(semantic): report TS18019 for abstract modifier with private identifier#18173

Merged
graphite-app[bot] merged 1 commit intomainfrom
fix/abstract-private-identifier
Jan 19, 2026
Merged

fix(semantic): report TS18019 for abstract modifier with private identifier#18173
graphite-app[bot] merged 1 commit intomainfrom
fix/abstract-private-identifier

Conversation

@Boshen
Copy link
Member

@Boshen Boshen commented Jan 18, 2026

Summary

  • Add semantic check for TypeScript error 18019: "'abstract' modifier cannot be used with a private identifier"
  • This error is reported when a class member (method or property) has both the abstract modifier and a private identifier key (e.g., abstract #foo)

Fixes 2 Babel conformance tests that expected this syntax error.

Test plan

  • cargo coverage -- parser passes
  • Verified TypeScript reports TS18019 for these cases

🤖 Generated with Claude Code

@Boshen Boshen requested a review from Dunqing as a code owner January 18, 2026 07:57
Copilot AI review requested due to automatic review settings January 18, 2026 07:57
@github-actions github-actions bot added A-semantic Area - Semantic C-bug Category - Bug labels Jan 18, 2026
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 adds semantic validation for TypeScript error TS18019, which prevents the use of the abstract modifier with private identifiers in class members. This addresses a gap in the semantic checker where abstract private members were previously accepted without error.

Changes:

  • Added new diagnostic function abstract_cannot_be_used_with_private_identifier with TS error code 18019
  • Implemented validation in check_method_definition to detect abstract methods with private identifiers
  • Implemented validation in new check_property_definition function to detect abstract properties with private identifiers
  • Integrated PropertyDefinition checking into the main semantic checker dispatch

Reviewed changes

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

Show a summary per file
File Description
crates/oxc_semantic/src/diagnostics.rs Adds new diagnostic function for TS18019 error
crates/oxc_semantic/src/checker/typescript.rs Implements checks for abstract modifier with private identifiers in both methods and properties
crates/oxc_semantic/src/checker/mod.rs Integrates PropertyDefinition into the semantic checker dispatch
tasks/coverage/snapshots/semantic_babel.snap Updates to show proper error reporting instead of scope mismatch
tasks/coverage/snapshots/parser_typescript.snap Adds expected error output for TypeScript conformance test
tasks/coverage/snapshots/parser_babel.snap Shows two previously failing tests now correctly report semantic errors

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

@Boshen Boshen force-pushed the fix/abstract-private-identifier branch from d3c5c3f to dfc4239 Compare January 18, 2026 15:13
@codspeed-hq
Copy link

codspeed-hq bot commented Jan 18, 2026

Merging this PR will not alter performance

✅ 42 untouched benchmarks
⏩ 3 skipped benchmarks1


Comparing fix/abstract-private-identifier (329a648) with main (c1c152b)

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.

@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026
Copy link
Member

Dunqing commented Jan 19, 2026

Merge activity

  • Jan 19, 1:25 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 19, 1:25 AM UTC: This pull request can not be added to the Graphite merge queue. Please try rebasing and resubmitting to merge when ready.
  • Jan 19, 1:25 AM UTC: Graphite disabled "merge when ready" on this PR due to: a merge conflict with the target branch; resolve the conflict and try again..
  • Jan 19, 1:25 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 19, 2:11 AM UTC: Dunqing added this pull request to the Graphite merge queue.
  • Jan 19, 2:12 AM UTC: The Graphite merge queue couldn't merge this PR because it had merge conflicts.
  • Jan 19, 7:22 AM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jan 19, 7:22 AM UTC: Boshen added this pull request to the Graphite merge queue.
  • Jan 19, 7:33 AM UTC: Merged by the Graphite merge queue.

@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026
@Boshen Boshen force-pushed the fix/abstract-private-identifier branch from dfc4239 to 329a648 Compare January 19, 2026 04:34
@Boshen Boshen added the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026
…tifier (#18173)

## Summary

- Add semantic check for TypeScript error 18019: "'abstract' modifier cannot be used with a private identifier"
- This error is reported when a class member (method or property) has both the `abstract` modifier and a private identifier key (e.g., `abstract #foo`)

Fixes 2 Babel conformance tests that expected this syntax error.

## Test plan

- `cargo coverage -- parser` passes
- Verified TypeScript reports TS18019 for these cases

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@graphite-app graphite-app bot force-pushed the fix/abstract-private-identifier branch from 329a648 to 1b199af Compare January 19, 2026 07:27
@graphite-app graphite-app bot merged commit 1b199af into main Jan 19, 2026
21 checks passed
@graphite-app graphite-app bot deleted the fix/abstract-private-identifier branch January 19, 2026 07:33
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Jan 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-semantic Area - Semantic C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants