Fix a few symbol printing / node builder bugs

[jakebailey]

Was looking at one thing, then one thing led to another, and I'm now deleting 6000+ baseline diffs (nearly half a million lines).

What's in this PR so far (in order of how I did it):

• I noticed in another PR that many symbol baselines are almost good, down to A vs ns.A or similar. This came down to the symbol baselines not supplying an enclosing declaration.
• I did the above, but it regressed a bunch of baselines. I traced that down to a module resolution host being set when it shouldn't. Fixing that fixed the regressions from the previous item, but caused more.
• Tracing that down led me to the fact that we weren't wrapping SymbolTrackers in quite the same way. Fixing that (I think) got it down to deleting even more baselines, but adding two more.

Two baselines were added I need to sort out:

--- old.spuriousCircularityOnTypeImport.types
+++ new.spuriousCircularityOnTypeImport.types
@@= skipped -16, +16 lines =@@
 };

 export declare const value2: {
->value2 : { sliceSelectors: <FuncMap extends import("./types").SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]>; }; }
+>value2 : { sliceSelectors: <FuncMap extends import("types").SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]>; }; }

     sliceSelectors: <FuncMap extends import('./types').SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]> };
->sliceSelectors : <FuncMap extends import("./types").SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]>; }
+>sliceSelectors : <FuncMap extends import("types").SelectorMap<FuncMap>>(selectorsBySlice: FuncMap) => { [P in keyof FuncMap]: Parameters<FuncMap[P]>; }
 >selectorsBySlice : FuncMap

 };	

--- old.expandoFunctionSymbolPropertyJs.types
+++ new.expandoFunctionSymbolPropertyJs.types
@@= skipped -20, +20 lines =@@
  * @returns {import("./types").TestSymb}
  */
 export function test() {
->test : () => import("./types").TestSymb
+>test : () => import("/types").TestSymb

   function inner() {}
 >inner : { (): void; [symb]: boolean; }

[jakebailey]

After some differential debugging, I've figured out that the two baseline regressions here are because we don't yet have node reuse in serializeReturnTypeForSignature. If I turn off that reuse in Strada, it behaves like this PR does, emitting:

[Copilot]

Pull Request Overview

This PR fixes several symbol printing and node builder bugs in the TypeScript compiler by addressing how symbol declarations are qualified and how module resolution hosts are used. The changes result in the deletion of over 6,000 baseline diffs (nearly half a million lines) by improving symbol name generation consistency.

• Fixed symbol baselines by properly supplying enclosing declarations, resolving differences between A vs b.A style symbol names
• Corrected module resolution host usage to prevent regressions
• Improved SymbolTracker wrapping to ensure consistent symbol name generation

[jakebailey]

I question this, yet it is what Strada did.

[jakebailey]

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.

[sheetalkamat]

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

[jakebailey]

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.

[jakebailey]

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

[jakebailey]

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.

[weswigham]

I did the above, but it regressed a bunch of baselines. I traced that down to a module resolution host being set when it shouldn't. Fixing that fixed the regressions from the previous item, but caused more.

Yeah, I noted that in the node builder PR - I left it like this because the module names in strada are all pretty clearly wrong in these baselines (weird baseUrl-y names without baseUrl set), so it's noticable an improvement. I'd hoped we could make the same always-pass-a-host change in strada to reduce the diff, instead, but this works, too, for now, I guess. We should make a note to revert to always providing a resolution host when we're no longer tracking strada diffs, though. Strada only even has the bug because it obscures that the TypeCheckerHost is a Program which is a valid ModuleSpecifierGenerationHost, leaving the nodebuilder making poor partial-host stand-ins in many cases where it could just be sending the checker host on as-is.

[jakebailey]

I could make the change in Strada, if that's preferred. Though I feel like merging this now would be better and we make both changes in tandem later.

I'll update the PR with a note.

[jakebailey]

Alternatively, I could just undo the host change, and increase the diff count even more, but they'd look prettier?

I think this affects LS features, so that could be impactful...

[jakebailey]

For reference: f1706c9

[jakebailey]

I'd be totally fine with the above commit on this PR if it feels nicer, sure I lose some of the diff deletion, but what I mainly cared about were the diffs like Symbol(A) -> Symbol(ns.A) and whatnot

[weswigham]

I think this affects LS features, so that could be impactful...

Last time I looked it wasn't that important for LS stuff - those endpoints are pretty good about getting a host passed down into the checker APIs. The baseliner is, afaik, singular in passing no host object in its' symbol tracker.

[jakebailey]

It does appear to matter, e.g. these two tests have their hovers "improved" by the linked commit: