Skip to content

Conversation

@sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented Nov 8, 2025

Reserved keywords are allowed as an identifier in ambient contexts.

declare const private: number

This is allowed.

Copy link
Member Author

sapphi-red commented Nov 8, 2025


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.

@codspeed-hq
Copy link

codspeed-hq bot commented Nov 8, 2025

CodSpeed Performance Report

Merging #15495 will not alter performance

Comparing 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts (bace84d) with main (4fe3aac)

Summary

✅ 42 untouched
⏩ 3 skipped1

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.

@sapphi-red sapphi-red changed the base branch from 11-09-test_coverage_typescript_parse_as_module_if_export_is_present to graphite-base/15495 November 8, 2025 17:31
@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from 2812de7 to 28b3708 Compare November 8, 2025 17:31
@sapphi-red sapphi-red changed the base branch from graphite-base/15495 to 11-09-test_tasks_coverage_typescript_collect_more_error_files November 8, 2025 17:31
@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from 28b3708 to 75ad01b Compare November 8, 2025 17:38
@sapphi-red sapphi-red force-pushed the 11-09-test_tasks_coverage_typescript_collect_more_error_files branch from 6d6dd71 to a0895a4 Compare November 8, 2025 17:38
@sapphi-red
Copy link
Member Author

This probably should be solved in a different way...

@overlookmotel
Copy link
Member

This probably should be solved in a different way...

The "look up ancestors" part only applies to BindingIdentifiers, right? If so, I think you can look up the VariableDeclaration with Scoping::symbol_declaration (or maybe it's not built yet?)

@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from 75ad01b to e751988 Compare November 10, 2025 01:24
@sapphi-red sapphi-red force-pushed the 11-09-test_tasks_coverage_typescript_collect_more_error_files branch from a0895a4 to 103d98d Compare November 10, 2025 01:24
@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from e751988 to fc8846f Compare November 10, 2025 01:59
@sapphi-red sapphi-red force-pushed the 11-09-test_tasks_coverage_typescript_collect_more_error_files branch from 103d98d to c0ba508 Compare November 10, 2025 01:59
@graphite-app graphite-app bot force-pushed the 11-09-test_tasks_coverage_typescript_collect_more_error_files branch from c0ba508 to c8c7da7 Compare November 10, 2025 02:13
@graphite-app graphite-app bot force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from fc8846f to 0fb9c45 Compare November 10, 2025 02:13
@sapphi-red sapphi-red changed the base branch from 11-09-test_tasks_coverage_typescript_collect_more_error_files to graphite-base/15495 November 10, 2025 04:24
@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from 0fb9c45 to 8e2056d Compare November 10, 2025 04:24
@sapphi-red sapphi-red changed the base branch from graphite-base/15495 to 11-09-test_tasks_coverage_typescript_collect_more_error_files November 10, 2025 04:25
@graphite-app graphite-app bot changed the base branch from 11-09-test_tasks_coverage_typescript_collect_more_error_files to graphite-base/15495 November 10, 2025 04:36
@graphite-app graphite-app bot force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from 8e2056d to f0ce955 Compare November 10, 2025 04:43
@graphite-app graphite-app bot force-pushed the graphite-base/15495 branch from b3fb231 to ee035b4 Compare November 10, 2025 04:43
@graphite-app graphite-app bot changed the base branch from graphite-base/15495 to main November 10, 2025 04:43
@graphite-app graphite-app bot force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from f0ce955 to a1745a9 Compare November 10, 2025 04:44
@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch 2 times, most recently from 187bbc9 to ffda463 Compare November 10, 2025 13:26
@sapphi-red sapphi-red marked this pull request as ready for review November 10, 2025 15:35
@sapphi-red sapphi-red requested a review from Dunqing as a code owner November 10, 2025 15:35
Copilot AI review requested due to automatic review settings November 10, 2025 15:35
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 allows reserved keywords (like private, await) to be used as identifiers in TypeScript ambient contexts by relaxing keyword restrictions for declare variable declarations. The change fixes parsing of TypeScript declaration files where reserved keywords may appear as variable names.

  • Adds logic to check if a binding identifier is part of a declare variable declaration
  • Updates test coverage snapshots to reflect one additional passing test
  • Fixes the topLevelAwait.3.ts test case which previously had errors

Reviewed Changes

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

Show a summary per file
File Description
crates/oxc_semantic/src/checker/javascript.rs Adds new helper function and extends ambient context checks to include declare variable declarations
tasks/coverage/misc/pass/declare-let-private.ts New test case demonstrating private keyword usage in declare statement
tasks/coverage/snapshots/transformer_misc.snap Updated test counts (49→50)
tasks/coverage/snapshots/parser_misc.snap Updated test counts (49→50)
tasks/coverage/snapshots/formatter_misc.snap Updated test counts (49→50)
tasks/coverage/snapshots/codegen_misc.snap Updated test counts (49→50)
tasks/coverage/snapshots/semantic_misc.snap Updated test counts (49→50) and shows semantic error for new test
tasks/coverage/snapshots/semantic_typescript.snap Removes error for topLevelAwait.3.ts and updates pass count
tasks/coverage/snapshots/parser_typescript.snap Removes parsing error for topLevelAwait.3.ts and updates pass count

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

@Dunqing
Copy link
Member

Dunqing commented Nov 12, 2025

I think we can simplify it more by checking whether the binding is flagged with the Ambient symbol flag.

See Oxc Playground

@sapphi-red sapphi-red force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from ffda463 to a5af938 Compare November 15, 2025 14:31
@sapphi-red sapphi-red requested review from Dunqing and removed request for Dunqing November 15, 2025 14:36
@Dunqing Dunqing force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from a5af938 to 74feebf Compare November 18, 2025 06:01
@Dunqing Dunqing force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from 74feebf to bace84d Compare November 18, 2025 06:09
@Dunqing
Copy link
Member

Dunqing commented Nov 18, 2025

I tweaked a little bit to improve the identifier in a BindingIdentifier situation, which already has a symbol ID, so that we can avoid getting a symbol ID again from AstKind::BindingIdentifier. I hope you don't mind that I modified the PR directly without conversation because I think the PR has been left here for a long time, so I don't want to delay it anyway!

@Dunqing Dunqing added the 0-merge Merge with Graphite Merge Queue label Nov 18, 2025
Copy link
Member

Dunqing commented Nov 18, 2025

Merge activity

…#15495)

Reserved keywords are allowed as an identifier in ambient contexts.
```ts
declare const private: number
```
This is allowed.
@graphite-app graphite-app bot force-pushed the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch from bace84d to 2191ae9 Compare November 18, 2025 08:11
@graphite-app graphite-app bot merged commit 2191ae9 into main Nov 18, 2025
20 checks passed
@graphite-app graphite-app bot deleted the 11-09-fix_semantic_allow_reserved_keywords_in_typescript_ambient_contexts branch November 18, 2025 08:16
@graphite-app graphite-app bot removed the 0-merge Merge with Graphite Merge Queue label Nov 18, 2025
overlookmotel pushed a commit that referenced this pull request Nov 24, 2025
### 💥 BREAKING CHANGES

- cbb27fd ast: [**BREAKING**] Add `TSGlobalDeclaration` type (#15712)
(overlookmotel)

### 🚀 Features

- 0c1f82b linter/plugins: Add `tokens` property to `Program` (#16020)
(overlookmotel)
- 6cff132 span: Add `Span::merge_within` method (#15869) (sapphi-red)
- 102365d allocator/vec: Add `Vec::into_bump_slice` method (#15770)
(Dunqing)

### 🐛 Bug Fixes

- e2ca770 codegen: Add support for printing type arguments in new
expressions (#15963) (Ives van Hoorne)
- 2bd3cb6 apps, editors, napi: Fix `oxlint-disable` comments (#16014)
(overlookmotel)
- 622cb5e parser: Preserve legal comments with @preserve/@license when
preceded by other annotations (#15929) (copilot-swe-agent)
- 7c46a9e transformer/tagged-template-transform: Handle `\n` escape
sequences (#15830) (Dunqing)
- f386efc minifier: Avoid generating invalid spans (#15778) (sapphi-red)
- d4ff004 parser: Forbid invalid modifiers on `module` and `global`
(#15723) (overlookmotel)
- 2191ae9 semantic: Allow reserved keywords in typescript ambient
contexts (#15495) (sapphi-red)
- 7d1ebad isolated-declarations: Incorrect nested namespace output in
isolated declarations (#15800) (copilot-swe-agent)

### ⚡ Performance

- b4b0ed8 transformer/typescript: Reverse order of checks (#15722)
(overlookmotel)

### 📚 Documentation

- c81a331 data_structures: Doc comments on fields of `Stack` (#15793)
(overlookmotel)
- cfae31d allocator: Use `allocator` as var name in examples (#15781)
(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-semantic Area - Semantic C-bug Category - Bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants