Fix nil dereference crash during checker initialization when resolving array types#2970
Open
Andarist wants to merge 1 commit intomicrosoft:mainfrom
Open
Fix nil dereference crash during checker initialization when resolving array types#2970Andarist wants to merge 1 commit intomicrosoft:mainfrom
Andarist wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Andarist
commented
Mar 3, 2026
| @@ -23578,13 +23572,44 @@ func (c *Checker) getArrayOrTupleTargetType(node *ast.Node) *Type { | |||
| elementType := c.getArrayElementTypeNode(node) | |||
Contributor
Author
There was a problem hiding this comment.
I'm not 100% sure about this fix so I refrained myself from doing more changes to reduce the noise.
If the fix gets accepted, I think it would be best to update:
- all other raw
c.globalReadonlyArrayType/c.globalArrayTypeto use the introduced getters - likely do the same with other global types that are referenced directly and not through getters
I noticed that in Strada more types were retrieved through such caching getters but that pattern seems to be gone from Corsa so perhaps it's something you want to actively avoid.
That said, Strada doesn't use this pattern for those global array types and it's prone to the very same crash
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2953
Problem
During
initializeChecker, the first merge loop merges file locals into the global symbol table. When two files declaredeclare module 'mymod'— one re-exporting a namespace import (import * as foo from 'foo'; export { foo }) and another exporting a conflicting symbol —mergeSymbolresolves the alias chain. SinceesModuleInteropis always on in typescript-go, this triggersresolveESModuleSymbol→getTypeWithSyntheticDefaultImportType→getSpreadType, which deeply resolves the module's type, including anystring[]properties. This callsgetArrayOrTupleTargetType→createDeferredTypeReference, which dereferencesglobalArrayType— stillnilbecause we're in the merge loop, before global types are initialized.The related PR #21563 fixed a similar class of bug for global augmentations, but that occurs in a later phase of initialization.
Fix
Introduce lazy initialization for
globalArrayTypeandglobalReadonlyArrayTypeviagetGlobalArrayType()/getGlobalReadonlyArrayType()getters. These resolve the type on first access and cache it, so the crash path gets a valid type instead ofnil.When
Arraycomes from a single lib file (e.g.@lib: es5), its symbol is non-transient. If a global augmentation later adds members (e.g.interface Array<T> { customMethod(): T }),mergeSymbolwould clone theArraysymbol — creating a new identity disconnected from the already-cachedglobalArrayType. This causes augmented members to be invisible or have leaked type parameters.With multiple lib files (e.g.
@lib: es2015), this doesn't happen because the natural multi-file merge already makes theArraysymbol (and its type parameter members) transient before our lazy getter fires, so augmentation merges in-place into the same symbol identity.ensureGlobalSymbolTransientreplicates what the natural multi-file merge does for single-file libs: it clones the symbol to make it transient before any type resolution occurs on it. Crucially, it also clones the symbol's type parameter member symbols (e.g.TinArray<T>), ensuring that when a global augmentation later merges itsT, it merges in-place into the already-transientTsymbol — preserving the single type parameter identity that theInterfaceTypeuses for instantiation.