Fixed an issue with type params being renamed incorrectly #1833

Andarist · 2025-10-06T12:47:56Z

This PR removes 5 .diffs, most (if not all) of the other ones that were changed here are a result of the lack of return type nodes reuse (here)

Andarist · 2025-10-06T12:49:50Z

internal/checker/nodebuilderscopes.go

+	context.hasCreatedTypeParameterSymbolList = oldTypeParameterSymbolList != nil
+	context.hasCreatedTypeParametersNamesLookups = oldTypeParameterNames != nil


why does it matter? because we don't want to override the saved clones of the data below with empty data here:

typescript-go/internal/checker/nodebuilderimpl.go

Lines 1308 to 1313 in 912f193

if !b.ctx.hasCreatedTypeParametersNamesLookups {

b.ctx.hasCreatedTypeParametersNamesLookups = true

b.ctx.typeParameterNames = make(map[TypeId]*ast.Identifier)

b.ctx.typeParameterNamesByText = make(map[string]struct{})

b.ctx.typeParameterNamesByTextNextNameCount = make(map[string]int)

}

jakebailey · 2025-10-06T16:48:35Z

testdata/baselines/reference/submodule/compiler/declarationEmitShadowingInferNotRenamed.js.diff

 };
 -export declare const createClient: <D extends (new (...args: any[]) => Client) | Record<string, new (...args: any[]) => Client>>(clientDef: D) => D extends new (...args: any[]) => infer C ? UpdatedClient<C> : { [K in keyof D]: D[K] extends new (...args: any[]) => infer C ? UpdatedClient<C> : never; };
-+export declare const createClient: <D extends Record<string, new (...args: any[]) => string> | (new (...args: any[]) => string)>(clientDef: D) => D extends new (...args: any[]) => infer C ? UpdatedClient<C> : { [K in keyof D]: D[K] extends new (...args: any[]) => infer C ? UpdatedClient<C> : never; };
+export declare const createClient: <D extends Record<string, new (...args: any[]) => string> | (new (...args: any[]) => string)>(clientDef: D) => D extends new (...args: any[]) => infer C ? UpdatedClient<C> : { [K in keyof D]: D[K] extends new (...args: any[]) => infer C_1 ? UpdatedClient<C_1> : never; };


This seems like a regression?

Based on my quick investigation into this, it will get fixed with return type nodes reuse

I can confirm that Strada generates the same thing if I only comment out what is not implemented in Corsa yet here

jakebailey · 2025-10-06T16:53:08Z

.../reference/submodule/compiler/mappedTypeGenericInstantiationPreservesHomomorphism.types.diff

 ->usePrivateType : <T_1 extends unknown[]>(...args: T_1) => T_1[any] extends infer T_2 ? { [K in keyof T_2]: T_2[K]; } : never
 +>usePrivateType(...args) : T_1[any] extends infer T ? { [K in keyof T]: T_1[any][K]; } : never
-+>usePrivateType : <T_1 extends unknown[]>(...args: T_1) => T_3[any] extends infer T_2 ? { [K in keyof T_2]: T_3[any][K]; } : never
+>usePrivateType : <T_1 extends unknown[]>(...args: T_1) => T_1[any] extends infer T_2 ? { [K in keyof T_2]: T_1[any][K]; } : never


These are so close to being correct. I wonder what tiny thing is causing this diff in general...

I'll look into this in a followup work

jakebailey

The results here seem correct but I don't know enough to say one way or the other for this particular func, so would like @weswigham to take a quick look.

weswigham

So hasCreatedTypeParameterSymbolList and hasCreatedTypeParametersNamesLookups are inversions of the field in strada (which says must instead of has). This logically tracks, since in strada when we clone a context we init the field to true, (which, since inverted, here, false).

Honestly, dunno why I conditionalized it on the field content in the first place - maybe I was trying to be cheeky and reuse empty lists across contexts? Seems wrong in any case.

* Port "Infer from annotated return type nodes before assigning contextual parameter types" (microsoft#1804) * Port "make exported destructured discriminated union narrowing work" (microsoft#1806) Co-authored-by: =?UTF-8?q?Michal=20Mar=C5=A1=C3=A1lek?= <MichalMarsalek@users.noreply.github.com> * Implement more of doc highlights / find all refs (microsoft#1796) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * Port "Provide string completions for `in` keyword checks" (microsoft#1803) * Port "Fix #61098" (microsoft#1810) Co-authored-by: Hans Brende <firedrake93@gmail.com> * Port "fix(61258): Renaming namespace with const enum doesn't update enum references" (microsoft#1811) Co-authored-by: "Oleksandr T." <oleksandr.tarasiuk@outlook.com> * Update dependencies (microsoft#1785) * Mark libReplacement default as false (microsoft#1812) * Port "Deemphasize old JSX transform" (microsoft#1809) Co-authored-by: "Sebastian \"Sebbie\" Silbermann" <sebastian.silbermann@vercel.com> * Port "fix(checker): report error when using bigint as enum key" (microsoft#1814) Co-authored-by: magic-akari <akari.ccino@gmail.com> * Port "explicitly disallow `using` in ambient contexts" (microsoft#1815) Co-authored-by: =?UTF-8?q?Ren=C3=A9?= <contact.9a5d6388@renegade334.me.uk> * Port 'Issue "'{0}' declarations can only be declared inside a block." for block-scoped variables in presence of parse errors' (microsoft#1816) * Port "Avoid resolving source prop type when the target is `unknown`/`any`" (microsoft#1817) * Port "Allow trailing commas after import attributes in `ImportType`" (microsoft#1818) * Port "Fix BigInt literal error in ambient contexts when targeting < ES2020" (microsoft#1819) * Port "Stop reassigning `.valueDeclaration` to avoid replacing earlier declarations with late ones" (microsoft#1813) * Port "Keep accessors as accessors in emitted anonymous class declarations" (microsoft#1822) * Port "Fixed crash in `hasVisibleDeclarations` related to binding elements" (microsoft#1820) * Port "Properly disallow `yield` in bodyless arrows" (microsoft#1825) * Port "Account for right operands & fix a weird error message for leftmost nullish literals in checkNullishCoalesceOperands" (microsoft#1805) Co-authored-by: Chiri Vulpes <github@chiri.works> * Remove JSDoc handling from the binder (microsoft#1827) * Port "`arguments` should not be allowed in class static block" (microsoft#1828) Co-authored-by: Zzzen <callmetsing@gmail.com> * Port "check usage before declaration for decorators" (microsoft#1829) Co-authored-by: Zzzen <callmetsing@gmail.com> * Fix nil pointer dereference in snapshotFS.GetFile for non-existent files (microsoft#1830) * Fix translation bug causing `unknown` infer-extends constraint to be emitted (microsoft#1831) * Port "Fixed nullish coalesce operator's precedence" (microsoft#1824) * Fixed an issue with type params being renamed incorrectly (microsoft#1833) * Port "Add support for `import defer` proposal" (microsoft#1826) Co-authored-by: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= <hello@nicr.dev> * Bump github/codeql-action from 3.30.5 to 3.30.6 in the github-actions group across 1 directory (microsoft#1832) Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Port "Preserve type parameter constraint in emitted mapped types while preserving their distributivity" (microsoft#1834) * disable create-cache ci job * merge fixup --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> Co-authored-by: =?UTF-8?q?Michal=20Mar=C5=A1=C3=A1lek?= <MichalMarsalek@users.noreply.github.com> Co-authored-by: Jake Bailey <5341706+jakebailey@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Hans Brende <firedrake93@gmail.com> Co-authored-by: "Oleksandr T." <oleksandr.tarasiuk@outlook.com> Co-authored-by: "Sebastian \"Sebbie\" Silbermann" <sebastian.silbermann@vercel.com> Co-authored-by: magic-akari <akari.ccino@gmail.com> Co-authored-by: =?UTF-8?q?Ren=C3=A9?= <contact.9a5d6388@renegade334.me.uk> Co-authored-by: Chiri Vulpes <github@chiri.works> Co-authored-by: Zzzen <callmetsing@gmail.com> Co-authored-by: John Xu <dyxushuai@gmail.com> Co-authored-by: =?UTF-8?q?Nicol=C3=B2=20Ribaudo?= <hello@nicr.dev> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

Fixed an issue with type params being renamed incorrectly

11c2d14

Andarist commented Oct 6, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main' into fix/type-param-renaming

ec7a962

jakebailey reviewed Oct 6, 2025

View reviewed changes

jakebailey requested a review from weswigham October 6, 2025 18:54

weswigham approved these changes Oct 6, 2025

View reviewed changes

jakebailey added this pull request to the merge queue Oct 6, 2025

Merged via the queue into microsoft:main with commit 28d95bf Oct 6, 2025
22 checks passed

Andarist deleted the fix/type-param-renaming branch October 6, 2025 20:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fixed an issue with type params being renamed incorrectly #1833

Fixed an issue with type params being renamed incorrectly #1833

Andarist commented Oct 6, 2025

Uh oh!

Andarist Oct 6, 2025

Uh oh!

jakebailey Oct 6, 2025

Uh oh!

Andarist Oct 6, 2025

Uh oh!

Andarist Oct 6, 2025

Uh oh!

jakebailey Oct 6, 2025

Uh oh!

Andarist Oct 6, 2025

Uh oh!

jakebailey left a comment

Uh oh!

weswigham left a comment

Uh oh!

Uh oh!

Uh oh!

		context.hasCreatedTypeParameterSymbolList = oldTypeParameterSymbolList != nil
		context.hasCreatedTypeParametersNamesLookups = oldTypeParameterNames != nil

	if !b.ctx.hasCreatedTypeParametersNamesLookups {
	b.ctx.hasCreatedTypeParametersNamesLookups = true
	b.ctx.typeParameterNames = make(map[TypeId]*ast.Identifier)
	b.ctx.typeParameterNamesByText = make(map[string]struct{})
	b.ctx.typeParameterNamesByTextNextNameCount = make(map[string]int)
	}

Fixed an issue with type params being renamed incorrectly #1833

Fixed an issue with type params being renamed incorrectly #1833

Conversation

Andarist commented Oct 6, 2025

Uh oh!

Andarist Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Andarist Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Andarist Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

Andarist Oct 6, 2025

Choose a reason for hiding this comment

Uh oh!

jakebailey left a comment

Choose a reason for hiding this comment

Uh oh!

weswigham left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!