Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 69 additions & 0 deletions internal/fourslash/tests/completionNullExportCondition_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package fourslash_test

import (
"testing"

"github.com/microsoft/typescript-go/internal/core"
"github.com/microsoft/typescript-go/internal/fourslash"
. "github.com/microsoft/typescript-go/internal/fourslash/tests/util"
"github.com/microsoft/typescript-go/internal/ls"
"github.com/microsoft/typescript-go/internal/testutil"
)

// Regression test for panic when rehydrating cached export info fails.
// This can happen when:
// - Module resolution changes (e.g., null export conditions)
// - Symbols are removed/renamed in a module
// - Modules become unavailable
// The language service should handle this gracefully instead of panicking.
func TestCompletionNullExportCondition(t *testing.T) {
t.Parallel()
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
const content = `// @module: esnext
// @moduleResolution: node16
// @allowJs: true
// @noTypesAndSymbols: true
// @noEmit: true

// @Filename: /node_modules/dep/package.json
{
"name": "dep",
"version": "1.0.0",
"exports": {
".": {
"import": null,
"require": "./dist/index.js",
"types": "./dist/index.d.ts"
}
}
}

// @Filename: /node_modules/dep/dist/index.d.ts
export const sourceMapsEnabled = true;
export const someOtherExport = "value";

// @Filename: /node_modules/dep/dist/index.mjs
export const sourceMapsEnabled = true;
export const someOtherExport = "value";

// @Filename: /index.mts
// Trigger auto-import completion - should work without panicking
source/**/
`
f := fourslash.NewFourslash(t, nil /*capabilities*/, content)
// This should not panic when requesting completions
f.VerifyCompletions(t, "", &fourslash.CompletionsExpectedList{
UserPreferences: &ls.UserPreferences{
IncludeCompletionsForModuleExports: core.TSTrue,
IncludeCompletionsForImportStatements: core.TSTrue,
},
IsIncomplete: false,
ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{
CommitCharacters: &DefaultCommitCharacters,
EditRange: Ignored,
},
Items: &fourslash.CompletionsExpectedItems{
// The exact items don't matter - we're just testing it doesn't panic
},
})
}
43 changes: 30 additions & 13 deletions internal/ls/autoimports.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ func (e *exportInfoMap) get(importingFile tspath.Path, ch *checker.Checker, key
if e.usableByFileName != importingFile {
return nil
}
return core.Map(e.exportInfo.Get(key), func(info CachedSymbolExportInfo) *SymbolExportInfo { return e.rehydrateCachedInfo(ch, info) })
return core.MapFiltered(e.exportInfo.Get(key), func(info CachedSymbolExportInfo) (*SymbolExportInfo, bool) {
Copy link
Member

Choose a reason for hiding this comment

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

Just make these MapNonNil - every false condition is also nil anyway.

I don't know if tryRehydrateCachedInfo would be a better name to indicate that it might return nil now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in commit f4e7b5e. Changed to use MapNonNil and renamed the function to tryRehydrateCachedInfo to better indicate it may return nil.

return e.rehydrateCachedInfo(ch, info)
})
}

func (e *exportInfoMap) add(
Expand Down Expand Up @@ -227,12 +229,20 @@ func (e *exportInfoMap) search(
symbolName = info[0].capitalizedSymbolName
}
if matches(symbolName, info[0].targetFlags) {
rehydrated := core.Map(info, func(info CachedSymbolExportInfo) *SymbolExportInfo {
return e.rehydrateCachedInfo(ch, info)
})
filtered := core.FilterIndex(rehydrated, func(r *SymbolExportInfo, i int, _ []*SymbolExportInfo) bool {
return e.isNotShadowedByDeeperNodeModulesPackage(r, info[i].packageName)
})
// Rehydrate and filter in one pass to maintain correspondence between cached and rehydrated info
var rehydrated []*SymbolExportInfo
for i, cachedInfo := range info {
rehydratedInfo, ok := e.rehydrateCachedInfo(ch, cachedInfo)
if !ok {
// Symbol or module is no longer available, skip it
continue
}
if !e.isNotShadowedByDeeperNodeModulesPackage(rehydratedInfo, info[i].packageName) {
continue
}
rehydrated = append(rehydrated, rehydratedInfo)
}
filtered := rehydrated
if len(filtered) > 0 {
if res := action(filtered, symbolName, ambientModuleName != "", key); res != nil {
return res
Expand All @@ -254,7 +264,10 @@ func (e *exportInfoMap) isNotShadowedByDeeperNodeModulesPackage(info *SymbolExpo
return !ok || strings.HasPrefix(info.moduleFileName, packageDeepestNodeModulesPath)
}

func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymbolExportInfo) *SymbolExportInfo {
// rehydrateCachedInfo attempts to restore a SymbolExportInfo from cached data.
// Returns (info, true) if successful, or (nil, false) if the symbol or module
// can no longer be found (e.g., due to module resolution changes or removed exports).
func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymbolExportInfo) (*SymbolExportInfo, bool) {
if info.symbol != nil && info.moduleSymbol != nil {
return &SymbolExportInfo{
symbol: info.symbol,
Expand All @@ -263,7 +276,7 @@ func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymb
exportKind: info.exportKind,
targetFlags: info.targetFlags,
isFromPackageJson: info.isFromPackageJson,
}
}, true
}
cached := e.symbols[info.id]
cachedSymbol, cachedModuleSymbol := cached.symbol, cached.moduleSymbol
Expand All @@ -275,7 +288,7 @@ func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymb
exportKind: info.exportKind,
targetFlags: info.targetFlags,
isFromPackageJson: info.isFromPackageJson,
}
}, true
}

moduleSymbol := core.Coalesce(info.moduleSymbol, cachedModuleSymbol)
Expand All @@ -287,7 +300,9 @@ func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymb
}
}
if moduleSymbol == nil {
panic(fmt.Sprintf("Could not find module symbol for %s in exportInfoMap", info.moduleName))
// Module is no longer available - this can happen when module resolution changes
// or the module is removed. Just skip this cached entry.
return nil, false
}
symbol := core.Coalesce(info.symbol, cachedSymbol)
if symbol == nil {
Expand All @@ -299,7 +314,9 @@ func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymb
}

if symbol == nil {
panic(fmt.Sprintf("Could not find symbol '%s' by key '%s' in module %s", info.symbolName, info.symbolTableKey, moduleSymbol.Name))
// Symbol is no longer available in the module - this can happen when exports change
// or the symbol is removed/renamed. Just skip this cached entry.
return nil, false
}
e.symbols[info.id] = symbolExportEntry{symbol, moduleSymbol}
return &SymbolExportInfo{
Expand All @@ -309,7 +326,7 @@ func (e *exportInfoMap) rehydrateCachedInfo(ch *checker.Checker, info CachedSymb
info.exportKind,
info.targetFlags,
info.isFromPackageJson,
}
}, true
}

func getNamesForExportedSymbol(defaultExport *ast.Symbol, ch *checker.Checker, scriptTarget core.ScriptTarget) []string {
Expand Down