Skip to content

Fix a few symbol printing / node builder bugs #1588

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 16, 2025
Merged

Fix a few symbol printing / node builder bugs #1588

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c5983d4
Fix some symbol stuff
jakebailey Aug 14, 2025
9526ffe
Update baselines
jakebailey Aug 14, 2025
52601e6
Fix more, except:
jakebailey Aug 15, 2025
a764eee
Wow I even fixed some fourslash tests
jakebailey Aug 15, 2025
8a0bacd
note
jakebailey Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
18 changes: 11 additions & 7 deletions internal/checker/nodebuilder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ import (
)

type NodeBuilder struct {
ctxStack []*NodeBuilderContext
host Host
impl *nodeBuilderImpl
ctxStack []*NodeBuilderContext
basicHost Host
impl *nodeBuilderImpl
}

// EmitContext implements NodeBuilderInterface.
Expand All @@ -32,10 +32,14 @@ func (b *NodeBuilder) enterContext(enclosingDeclaration *ast.Node, flags nodebui
enclosingSymbolTypes: make(map[ast.SymbolId]*Type),
remappedSymbolReferences: make(map[ast.SymbolId]*ast.Symbol),
}
if tracker == nil {
tracker = NewSymbolTrackerImpl(b.impl.ctx, nil, b.host)
b.impl.ctx.tracker = tracker
var moduleResolverHost Host
if tracker != nil {
moduleResolverHost = tracker.GetModuleSpecifierGenerationHost()
} else if internalFlags&nodebuilder.InternalFlagsDoNotIncludeSymbolChain != 0 {
moduleResolverHost = b.basicHost
}
tracker = NewSymbolTrackerImpl(b.impl.ctx, tracker, moduleResolverHost)
b.impl.ctx.tracker = tracker
}

func (b *NodeBuilder) popContext() {
Expand Down Expand Up @@ -173,7 +177,7 @@ func (b *NodeBuilder) TypeToTypeNode(typ *Type, enclosingDeclaration *ast.Node,

func NewNodeBuilder(ch *Checker, e *printer.EmitContext) *NodeBuilder {
impl := newNodeBuilderImpl(ch, e)
return &NodeBuilder{impl: impl, ctxStack: make([]*NodeBuilderContext, 0, 1), host: ch.program}
return &NodeBuilder{impl: impl, ctxStack: make([]*NodeBuilderContext, 0, 1), basicHost: ch.program}
}

func (c *Checker) getNodeBuilder() *NodeBuilder {
Expand Down
16 changes: 7 additions & 9 deletions internal/checker/symboltracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@ type SymbolTrackerImpl struct {
}

func NewSymbolTrackerImpl(context *NodeBuilderContext, tracker nodebuilder.SymbolTracker, tchost Host) *SymbolTrackerImpl {
var inner nodebuilder.SymbolTracker
if tracker != nil {
inner = tracker.GetInnerSymbolTracker()
if inner == nil {
inner = tracker
for {
t, ok := tracker.(*SymbolTrackerImpl)
if !ok {
break
}
tracker = t.inner
}
}

return &SymbolTrackerImpl{context, inner, false, tchost}
return &SymbolTrackerImpl{context, tracker, false, tchost}
}

func (this *SymbolTrackerImpl) GetModuleSpecifierGenerationHost() modulespecifiers.ModuleSpecifierGenerationHost {
Expand All @@ -32,10 +34,6 @@ func (this *SymbolTrackerImpl) GetModuleSpecifierGenerationHost() modulespecifie
return this.inner.GetModuleSpecifierGenerationHost()
}

func (this *SymbolTrackerImpl) GetInnerSymbolTracker() nodebuilder.SymbolTracker {
return this.inner
}

func (this *SymbolTrackerImpl) TrackSymbol(symbol *ast.Symbol, enclosingDeclaration *ast.Node, meaning ast.SymbolFlags) bool {
if !this.DisableTrackSymbol {
if this.inner != nil && this.inner.TrackSymbol(symbol, enclosingDeclaration, meaning) {
Expand Down
1 change: 0 additions & 1 deletion internal/nodebuilder/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
// TODO: previously all symboltracker methods were optional, but now they're required.
type SymbolTracker interface {
GetModuleSpecifierGenerationHost() modulespecifiers.ModuleSpecifierGenerationHost
GetInnerSymbolTracker() SymbolTracker

TrackSymbol(symbol *ast.Symbol, enclosingDeclaration *ast.Node, meaning ast.SymbolFlags) bool
ReportInaccessibleThisError()
Expand Down
5 changes: 0 additions & 5 deletions internal/transformers/declarations/tracker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"github.com/microsoft/typescript-go/internal/core"
"github.com/microsoft/typescript-go/internal/diagnostics"
"github.com/microsoft/typescript-go/internal/modulespecifiers"
"github.com/microsoft/typescript-go/internal/nodebuilder"
"github.com/microsoft/typescript-go/internal/printer"
"github.com/microsoft/typescript-go/internal/scanner"
)
Expand All @@ -23,10 +22,6 @@ func (s *SymbolTrackerImpl) GetModuleSpecifierGenerationHost() modulespecifiers.
return s.host
}

func (s *SymbolTrackerImpl) GetInnerSymbolTracker() nodebuilder.SymbolTracker {
return nil
}

// PopErrorFallbackNode implements checker.SymbolTracker.
func (s *SymbolTrackerImpl) PopErrorFallbackNode() {
s.fallbackStack = s.fallbackStack[:len(s.fallbackStack)-1]
Expand Down
4 changes: 2 additions & 2 deletions testdata/baselines/reference/compiler/declarationEmitAugmentationUsesCorrectSourceFile.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,12 @@ declare namespace Knex {
}

export = knex;
>knex : typeof import("knex")
>knex : typeof import("index.ts")

=== index.ts ===
import "knex";
declare module "knex" {
>"knex" : typeof import("knex")
>"knex" : typeof import("node_modules/knex/index")

namespace Knex {
>Knex : typeof Knex
Expand Down
4 changes: 2 additions & 2 deletions testdata/baselines/reference/compiler/symbolLinkDeclarationEmitModuleNamesRootDir.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ import { BindingKey } from '@loopback/context';
export const CONTROLLER_CLASS = BindingKey.create<ControllerClass>(null as any); // line in question
>CONTROLLER_CLASS : BindingKey<ControllerClass>
>BindingKey.create<ControllerClass>(null as any) : BindingKey<ControllerClass>
>BindingKey.create : <T extends import("@loopback/context").Constructor<any>>(ctor: T) => BindingKey<T>
>BindingKey.create : <T extends import("/monorepo/context/index").Constructor<any>>(ctor: T) => BindingKey<T>
Copy link
Member Author

Choose a reason for hiding this comment

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

I question this, yet it is what Strada did.

Copy link
Member Author

Choose a reason for hiding this comment

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

Theoretically, I think I could undo this sort of change by undoing what I've done to the resolution host code, but I don't think divergence is a great idea.

>BindingKey : typeof BindingKey
>create : <T extends import("@loopback/context").Constructor<any>>(ctor: T) => BindingKey<T>
>create : <T extends import("/monorepo/context/index").Constructor<any>>(ctor: T) => BindingKey<T>
>null as any : any

=== /monorepo/context/src/value-promise.d.ts ===
Expand Down
8 changes: 4 additions & 4 deletions ...rence/submodule/compiler/allowSyntheticDefaultImportsCanPaintCrossModuleDeclaration.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ export declare function styled(): Color;

=== file2.ts ===
import { styled } from "./file1";
>styled : () => import("./color").default
>styled : () => import("color").default

export const A = styled();
>A : import("./color").default
>styled() : import("./color").default
>styled : () => import("./color").default
>A : import("color").default
>styled() : import("color").default
>styled : () => import("color").default

16 changes: 0 additions & 16 deletions .../submodule/compiler/allowSyntheticDefaultImportsCanPaintCrossModuleDeclaration.types.diff
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/compiler/ambientExportDefaultErrors.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default 2 + 2;
>2 : 2

export as namespace Foo;
>Foo : typeof import("./foo")
>Foo : typeof import("foo")

=== foo2.d.ts ===
export = 2 + 2;
Expand Down
11 changes: 0 additions & 11 deletions testdata/baselines/reference/submodule/compiler/ambientExportDefaultErrors.types.diff
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/compiler/anonClassDeclarationEmitIsAnon.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function Timestamped<TBase extends Constructor>(Base: TBase) {
=== index.ts ===
import { wrapClass, Timestamped } from "./wrapClass";
>wrapClass : (param: any) => typeof Wrapped
>Timestamped : <TBase extends import("./wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase
>Timestamped : <TBase extends import("wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase

export default wrapClass(0);
>wrapClass(0) : typeof Wrapped
Expand All @@ -63,7 +63,7 @@ export class User {
export class TimestampedUser extends Timestamped(User) {
>TimestampedUser : TimestampedUser
>Timestamped(User) : Timestamped.(Anonymous class) & User
>Timestamped : <TBase extends import("./wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase
>Timestamped : <TBase extends import("wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase
>User : typeof User

constructor() {
Expand Down
4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/compiler/anonClassDeclarationEmitIsAnon.types.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import { wrapClass, Timestamped } from "./wrapClass";
>wrapClass : (param: any) => typeof Wrapped
->Timestamped : <TBase extends import("wrapClass").Constructor>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped<any>.(Anonymous class); } & TBase
+>Timestamped : <TBase extends import("./wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase
+>Timestamped : <TBase extends import("wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase

export default wrapClass(0);
>wrapClass(0) : typeof Wrapped
Expand All @@ -30,7 +30,7 @@
->Timestamped(User) : Timestamped<typeof User>.(Anonymous class) & User
->Timestamped : <TBase extends import("wrapClass").Constructor>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped<any>.(Anonymous class); } & TBase
+>Timestamped(User) : Timestamped.(Anonymous class) & User
+>Timestamped : <TBase extends import("./wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase
+>Timestamped : <TBase extends import("wrapClass").Constructor<{}>>(Base: TBase) => { new (...args: any[]): (Anonymous class); prototype: Timestamped.(Anonymous class); } & TBase
>User : typeof User

constructor() {
Expand Down
8 changes: 4 additions & 4 deletions testdata/baselines/reference/submodule/compiler/bundledDtsLateExportRenaming.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ export function f() {

=== nested/derived.ts ===
import { f } from "./base";
>f : () => import("./shared").B
>f : () => import("index").B

export function g() {
>g : () => import("./shared").B
>g : () => import("index").B

return f();
>f() : import("./shared").B
>f : () => import("./shared").B
>f() : import("index").B
>f : () => import("index").B
}

=== nested/index.ts ===
Expand Down
21 changes: 0 additions & 21 deletions testdata/baselines/reference/submodule/compiler/bundledDtsLateExportRenaming.types.diff
View file
Open in desktop

This file was deleted.

4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/compiler/checkJsdocTypeTagOnExportAssignment1.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ export default { c: false };

=== b.js ===
import a from "./a";
>a : import("./a").Foo
>a : import("a").Foo

a;
>a : import("./a").Foo
>a : import("a").Foo

4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/compiler/checkJsdocTypeTagOnExportAssignment2.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ export default { c: false };

=== c.js ===
import b from "./b";
>b : import("./a").Foo
>b : import("a").Foo
Copy link
Member

Choose a reason for hiding this comment

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

This seems incorrect no? the import there is "./b" -> "./a"

Copy link
Member Author

Choose a reason for hiding this comment

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

Shockingly. no; Strada has this:

//// [tests/cases/compiler/checkJsdocTypeTagOnExportAssignment2.ts] ////

=== checkJsdocTypeTagOnExportAssignment2.js ===

=== a.ts ===
export interface Foo {
    a: number;
>a : number
>  : ^^^^^^

    b: number;
>b : number
>  : ^^^^^^
}

=== b.js ===
/** @type {import("./a").Foo} */
export default { c: false };
>{ c: false } : { c: boolean; }
>             : ^^^^^^^^^^^^^^^
>c : boolean
>  : ^^^^^^^
>false : false
>      : ^^^^^

=== c.js ===
import b from "./b";
>b : import("a").Foo
>  : ^^^^^^^^^^^^^^^

b;
>b : import("a").Foo
>  : ^^^^^^^^^^^^^^^

This PR actually only removes .diff files, except the two that I noted above. Amazingly there are no instances besides those where we "regress" against our expectations.

Copy link
Member Author

Choose a reason for hiding this comment

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

More specifically, Foo is reexported through b and is actually declared in a.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why Strada omits the ./ is beyond me. Probably because the default node builder stuff doesn't use a module specifier host most of the time, which is something this PR tries to keep true.


b;
>b : import("./a").Foo
>b : import("a").Foo

4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/compiler/checkJsdocTypeTagOnExportAssignment5.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ export default { a: 1, b: 1 };

=== b.js ===
import a from "./a";
>a : import("./a").Foo
>a : import("a").Foo

a;
>a : import("./a").Foo
>a : import("a").Foo

4 changes: 2 additions & 2 deletions testdata/baselines/reference/submodule/compiler/checkJsdocTypeTagOnExportAssignment6.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ export default { a: 1, b: 1, c: 1 };

=== b.js ===
import a from "./a";
>a : import("./a").Foo
>a : import("a").Foo

a;
>a : import("./a").Foo
>a : import("a").Foo

6 changes: 3 additions & 3 deletions testdata/baselines/reference/submodule/compiler/checkMergedGlobalUMDSymbol.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import * as _three from './three';
>_three : typeof _three

export as namespace THREE;
>THREE : typeof import("./global")
>THREE : typeof import("global")

declare global {
>global : typeof global
Expand All @@ -25,6 +25,6 @@ declare global {

=== test.ts ===
const m = THREE
>m : typeof import("./three")
>THREE : typeof import("./three")
>m : typeof import("three")
>THREE : typeof import("three")

19 changes: 0 additions & 19 deletions testdata/baselines/reference/submodule/compiler/checkMergedGlobalUMDSymbol.types.diff
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/compiler/checkerInitializationCrash.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ declare namespace React {

=== /node_modules/preact/index.d.ts ===
export as namespace preact;
>preact : typeof import("preact")
>preact : typeof import("/node_modules/preact/index")

export interface VNode<P = {}> {}

Expand Down
10 changes: 0 additions & 10 deletions testdata/baselines/reference/submodule/compiler/checkerInitializationCrash.types.diff
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/compiler/commonJsUnusedLocals.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ const x = 0;

exports.y = 1;
>exports.y : Symbol(y, Decl(a.js, 0, 12))
>exports : Symbol("./a", Decl(a.js, 0, 0))
>exports : Symbol("/a", Decl(a.js, 0, 0))
>y : Symbol(y, Decl(a.js, 0, 12))

2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/compiler/commonJsUnusedLocals.symbols.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@
exports.y = 1;
>exports.y : Symbol(y, Decl(a.js, 0, 12))
->exports : Symbol(y, Decl(a.js, 0, 12))
+>exports : Symbol("./a", Decl(a.js, 0, 0))
+>exports : Symbol("/a", Decl(a.js, 0, 0))
>y : Symbol(y, Decl(a.js, 0, 12))
2 changes: 1 addition & 1 deletion testdata/baselines/reference/submodule/compiler/commonJsUnusedLocals.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const x = 0;
exports.y = 1;
>exports.y = 1 : 1
>exports.y : 1
>exports : typeof import("./a")
>exports : typeof import("/a")
>y : 1
>1 : 1

Loading
Loading