Skip to content

Commit f564687

Browse files
CopilotDunqing
andauthored
fix(isolated-declarations): preserve variable declarations when type with same name is referenced in implements clause (#16328)
- [x] Understand the issue: variable declarations with shadowed type names are dropped when the type is referenced in implements clause - [x] Identify root cause: `add_reference` and `resolve_references` were overwriting flags instead of combining them - [x] Implement fix: use bitwise OR to combine `KindFlags` when adding references and merging scopes - [x] Add test case to fixtures (extended shadowed.ts with issue #10996 test case) - [x] Run tests and update snapshots - [x] Run code review - [x] Run CodeQL security check (no issues found) - [x] Verify fix manually with original issue example - [x] Reverted .gitignore change per reviewer feedback ## Summary This PR fixes issue #10996 where variable declarations with shadowed type names were incorrectly dropped when the type was referenced in a class `implements` clause. ### Problem When a variable name is shadowed by a type alias and that type is referenced in `implements`, the variable declaration was missing from `.d.ts` output. ### Root Cause `ScopeTree` used `HashMap::insert()` which overwrites reference flags. When a name had both `KindFlags::Value` (from `typeof Test2`) and `KindFlags::Type` (from `implements Thing<Test2>`), only the last one was preserved. ### Fix Changed `add_reference()` and `resolve_references()` to combine flags using bitwise OR (`|=`) instead of overwriting. <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[isolatedDeclarations] variable emit is dropped if the variable is shadowed as a type name and the type is part of a class implements interface</issue_title> > <issue_description># Repo > > ```ts > export interface Thing<T> {} > > /** WORKS */ > > const Test1 = 'test1' as const; > export type Test1 = typeof Test1; > export class Class1 { > readonly id: 'test1' = Test1; > } > > /** DOES NOT WORK */ > > const Test2 = 'test2' as const; > export type Test2 = typeof Test2; > export class Class2 implements Thing<Test2> { > readonly id: 'test2' = Test2; > } > ``` > > # Expected > > ```ts > export interface Thing<T> { > } > /** WORKS */ > declare const Test1: "test1"; > export type Test1 = typeof Test1; > export declare class Class1 { > readonly id: 'test1'; > } > /** DOES NOT WORK */ > declare const Test2: "test2"; > export type Test2 = typeof Test2; > export declare class Class2 implements Thing<Test2> { > readonly id: 'test2'; > } > export {}; > ``` > [ts playground](https://www.typescriptlang.org/play/?#code/KYDwDg9gTgLgBASwHY2FAZgQwMbDgFQAtkBzAHnwD44BvAXwCgGB6AKlbgHUB5AJQGkAynFbMm2CEgDO8fMBkBGOAF44AclSK1cTFLgTpMANxxmzODGJ6SwGHuABbBDFQATBqEiwLATzB45RRVffwh0AnkYBSMPcGh4bAAbXT0AYWSpKSUaBjg4KGBMV0lEn0RXAC51TSjtVUComMYWdjgAEW4AUWEAOW58Lj5+ETEGAxkImQAmYI1Iqe1dfUkZEzM4Yvk4JAh4G3hHZzdYr3gYPwD54PPQ8IapmM94-Qy015mEBzBEx2AUPSIpAo82oOTyBSKJTKCEq1XmdUmMAeDGaQA) > > # Actual > > ```ts > export interface Thing<T> {} > /** WORKS */ > declare const Test1: "test1"; > export type Test1 = typeof Test1; > export declare class Class1 { > readonly id: "test1"; > } > export type Test2 = typeof Test2; > export declare class Class2 implements Thing<Test2> { > readonly id: "test2"; > } > export {}; > ``` > [oxc playground](https://playground.oxc.rs/#eNp1VE1PGzEQ/SuWL5UQVSBSJRRKpQqIVJUSSqJyycXZnU0MXntrzxKiKP+9z5u1idr0ktjz+ebN825lIUeS3hrnWWjL5CtVkJittF1+nn0R293czu3g5EQ8TR6/T8XJIN4LZwOLGQU+F1fiA8fDB6GC6ByXYjAQvNJBLImDoFozUzm3fRfeNGjQ58aLq/bXyxxSGBWCuI6/52I7t0J4UqWzZiN0OcoNr3JeRnkzuZ2K+8msg/sP2mFCO/wLbekoCOs4Iv4/4Jh+AHh4FPBQ6LoxVJPF7D2PMRhcHh8EWPaDxHrdIPJUOjnaSt/a+GewFzli39KprJyvFdB5OaqUCbCwVzZEc7bo4IzCADcEWF6xxpgpf73S6Nlgxzm8VnZp3q+FqxtPARm9IRSugXufHzb1wpl0K6plH7Y7lY3yIeLaQk5MNqAtpMXhDeMoY9z6kbj1dtJy0CWNW1tEZKlU7En+lR6UR26ydnm/Lr5Z9hoVi+wIBNisi1vvne+twBCp6jDgfEDVVraBZmrxPhSrxZMueSVHw1NJtpxUd9piSmkqwG3QjXv/xRm6YYuGfraO33l6Dm/TI+bf8fbgXYNeUoWPlqikEjWxJw14y2tX1yr6jIEVc+g8LGZZp/nhX6tNQMjCq+KFeIqlIT0FJ6uqaY+87+8Wz1Twk1cNaiRSY6MO61cGkYsWCMkfpIGtLKM9X6w8nkL8NITh2fknFACDN1QhZ+x8J/SxJlPmheAhkNdR9cpAeA66y4uBF08qW38Qq1IxSOi3dlywWxkAtsHuyVsFzSWohStpSd3TsHHHWY7PoXT4nPVsWrznrlayGFqm2K5K0nkUS3oEmNw1d/RKuegLUXMPlpN2ulTo0ZkxtBkzXskvXDjgUtu4g+jKpx3MZUdfNOfTLtojzWmm3R88D/Yg) > > Note the missing emit of `declare const Test2: "test2";`</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> - Fixes #16314 <!-- START COPILOT CODING AGENT TIPS --> --- ✨ Let Copilot coding agent [set things up for you](https://github.com/oxc-project/oxc/issues/new?title=✨+Set+up+Copilot+instructions&body=Configure%20instructions%20for%20this%20repository%20as%20documented%20in%20%5BBest%20practices%20for%20Copilot%20coding%20agent%20in%20your%20repository%5D%28https://gh.io/copilot-coding-agent-tips%29%2E%0A%0A%3COnboard%20this%20repo%3E&assignees=copilot) — coding agent works faster and does higher quality work when set up for your repo. --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Dunqing <[email protected]>
1 parent a07eec1 commit f564687

File tree

3 files changed

+37
-3
lines changed

3 files changed

+37
-3
lines changed

crates/oxc_isolated_declarations/src/scope.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ impl<'a> ScopeTree<'a> {
6666

6767
fn add_reference(&mut self, name: Atom<'a>, flags: KindFlags) {
6868
let scope = self.levels.last_mut().unwrap();
69-
scope.references.insert(name, flags);
69+
scope.references.entry(name).and_modify(|f| *f |= flags).or_insert(flags);
7070
}
7171

7272
/// Resolve references in the current scope, and propagate unresolved ones.
@@ -84,7 +84,10 @@ impl<'a> ScopeTree<'a> {
8484
});
8585

8686
// Merge unresolved references to the parent scope.
87-
self.levels.last_mut().unwrap().references.extend(current_references);
87+
let parent_scope = self.levels.last_mut().unwrap();
88+
for (name, flags) in current_references {
89+
parent_scope.references.entry(name).and_modify(|f| *f |= flags).or_insert(flags);
90+
}
8891
}
8992
}
9093

crates/oxc_isolated_declarations/tests/fixtures/shadowed.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,21 @@ type Module = () => void;
1313
namespace Module {
1414
export const x = 1;
1515
}
16-
export type ModuleType = Module;
16+
export type ModuleType = Module;
17+
18+
// https://github.com/oxc-project/oxc/issues/10996
19+
// Variable emit should not be dropped when variable is shadowed as a type name
20+
// and the type is part of a class implements interface
21+
export interface Thing<T> {}
22+
23+
const Test1 = 'test1' as const;
24+
export type Test1 = typeof Test1;
25+
export class Class1 {
26+
readonly id: 'test1' = Test1;
27+
}
28+
29+
const Test2 = 'test2' as const;
30+
export type Test2 = typeof Test2;
31+
export class Class2 implements Thing<Test2> {
32+
readonly id: 'test2' = Test2;
33+
}

crates/oxc_isolated_declarations/tests/snapshots/shadowed.snap

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,4 +17,18 @@ declare namespace Module {
1717
const x = 1;
1818
}
1919
export type ModuleType = Module;
20+
// https://github.com/oxc-project/oxc/issues/10996
21+
// Variable emit should not be dropped when variable is shadowed as a type name
22+
// and the type is part of a class implements interface
23+
export interface Thing<T> {}
24+
declare const Test1: "test1";
25+
export type Test1 = typeof Test1;
26+
export declare class Class1 {
27+
readonly id: "test1";
28+
}
29+
declare const Test2: "test2";
30+
export type Test2 = typeof Test2;
31+
export declare class Class2 implements Thing<Test2> {
32+
readonly id: "test2";
33+
}
2034
export {};

0 commit comments

Comments
 (0)