From 2830d54146622dca53fdfcbdd2a53cf5eb439298 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 25 Jan 2026 17:33:58 +0000 Subject: [PATCH 1/2] Initial plan From d7946323497a7768740327f1897f2bf3ade0152c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 25 Jan 2026 18:23:48 +0000 Subject: [PATCH 2/2] Fix autocomplete incorrectly suggesting transitive files from monorepo packages When a package has an "exports" field in its package.json, only files explicitly exposed via exports should be importable. Previously, internal files that were transitively imported within a package were incorrectly suggested for auto-import, resulting in import statements that wouldn't resolve. The fix: 1. Added PackageHasExports field to ResolvedEntrypoints to track whether a package has an exports field 2. Added PackagesWithExports set to RegistryBucket to efficiently lookup which packages have exports 3. Modified GetModuleSpecifier to skip EndingChangeable entrypoints (files discovered by directory scanning rather than exports) when the package has an exports field 4. Added isInvalidPackageSpecifier to filter out bare package specifiers with subpaths that don't resolve to valid entrypoints Added test case TestAutoImportTransitiveLeak that reproduces the issue. Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com> --- .../tests/autoImportTransitiveLeak_test.go | 117 +++++++++++++++++ internal/ls/autoimport/registry.go | 24 ++-- internal/ls/autoimport/specifiers.go | 119 ++++++++++++++++-- internal/module/resolver.go | 6 +- 4 files changed, 250 insertions(+), 16 deletions(-) create mode 100644 internal/fourslash/tests/autoImportTransitiveLeak_test.go diff --git a/internal/fourslash/tests/autoImportTransitiveLeak_test.go b/internal/fourslash/tests/autoImportTransitiveLeak_test.go new file mode 100644 index 00000000000..2f317386289 --- /dev/null +++ b/internal/fourslash/tests/autoImportTransitiveLeak_test.go @@ -0,0 +1,117 @@ +package fourslash_test + +import ( + "testing" + + "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/lsp/lsproto" + "github.com/microsoft/typescript-go/internal/testutil" +) + +const TestAutoImportTransitiveLeakScenario = ` +// @Filename: /home/src/workspaces/project/tsconfig.base.json +{ + "compilerOptions": { + "module": "nodenext", + "moduleResolution": "nodenext", + "composite": true + } +} + +// @Filename: /home/src/workspaces/project/packages/foo/package.json +{ + "name": "@packages/foo", + "type": "module", + "exports": { + ".": { + "types": "./src/index.ts", + "default": "./dist/index.js" + } + }, + "imports": { + "#*": { + "types": "./src/*.ts", + "default": "./dist/*.js" + } + } +} + +// @Filename: /home/src/workspaces/project/packages/foo/tsconfig.json +{ "extends": "../../tsconfig.base.json" } + +// @Filename: /home/src/workspaces/project/packages/foo/src/internal/index.ts +export function fooInternal() { + console.log("foo"); +} + +// @Filename: /home/src/workspaces/project/packages/foo/src/index.ts +import { fooInternal } from "#internal/index" +export function foo() { + fooInternal(); +} + +// @Filename: /home/src/workspaces/project/packages/bar/package.json +{ + "name": "@packages/bar", + "type": "module", + "exports": { + ".": { + "types": "./src/index.ts", + "default": "./dist/index.js" + } + }, + "imports": { + "#*": { + "types": "./src/*.ts", + "default": "./dist/*.js" + } + }, + "dependencies": { + "@packages/foo": "*" + } +} + +// @Filename: /home/src/workspaces/project/packages/bar/tsconfig.json +{ "extends": "../../tsconfig.base.json" } + +// @Filename: /home/src/workspaces/project/packages/bar/src/index.ts +import { foo } from "@packages/foo" + +fo/*fooCompletion*/ + +// @Filename: /home/src/workspaces/project/package.json +{ "workspaces": ["packages/*"], "type": "module" } + +// @link: /home/src/workspaces/project/packages/bar -> /home/src/workspaces/project/node_modules/@packages/bar +// @link: /home/src/workspaces/project/packages/foo -> /home/src/workspaces/project/node_modules/@packages/foo +` + +func TestAutoImportTransitiveLeak(t *testing.T) { + t.Parallel() + + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, TestAutoImportTransitiveLeakScenario) + + defer done() + + f.VerifyCompletions(t, "fooCompletion", &fourslash.CompletionsExpectedList{ + IsIncomplete: false, + ItemDefaults: &fourslash.CompletionsExpectedItemDefaults{ + CommitCharacters: &DefaultCommitCharacters, + EditRange: Ignored, + }, + Items: &fourslash.CompletionsExpectedItems{ + Includes: []fourslash.CompletionsExpectedItem{ + &lsproto.CompletionItem{ + Label: "foo", + AdditionalTextEdits: fourslash.AnyTextEdits, + SortText: PtrTo(string(ls.SortTextLocationPriority)), + }, + }, + Excludes: []string{"fooInternal"}, + }, + }) +} diff --git a/internal/ls/autoimport/registry.go b/internal/ls/autoimport/registry.go index dc12754c339..acca451d8cd 100644 --- a/internal/ls/autoimport/registry.go +++ b/internal/ls/autoimport/registry.go @@ -127,7 +127,10 @@ type RegistryBucket struct { // file paths, and values describe the ways of importing the package that would resolve // to that file. Entrypoints map[tspath.Path][]*module.ResolvedEntrypoint - Index *Index[*Export] + // PackagesWithExports tracks which packages have an "exports" field in their package.json. + // This is used to filter out EndingChangeable entrypoints for packages that have exports. + PackagesWithExports *collections.Set[string] + Index *Index[*Export] } func newRegistryBucket() *RegistryBucket { @@ -148,6 +151,7 @@ func (b *RegistryBucket) Clone() *RegistryBucket { DependencyNames: b.DependencyNames, AmbientModuleNames: b.AmbientModuleNames, Entrypoints: b.Entrypoints, + PackagesWithExports: b.PackagesWithExports, Index: b.Index, } } @@ -1005,6 +1009,7 @@ type packageExtractionResult struct { packageFiles map[string]map[tspath.Path]string ambientModuleNames map[string][]string entrypoints []*module.ResolvedEntrypoints + packagesWithExports *collections.Set[string] projectReferencePackages *collections.Set[string] possibleFailedAmbientModuleLookupSources *collections.SyncMap[tspath.Path, *failedAmbientModuleLookupSource] possibleFailedAmbientModuleLookupTargets *collections.SyncSet[string] @@ -1026,6 +1031,7 @@ func (b *registryBuilder) extractPackages( exports: make(map[tspath.Path][]*Export), packageFiles: make(map[string]map[tspath.Path]string), ambientModuleNames: make(map[string][]string), + packagesWithExports: &collections.Set[string]{}, projectReferencePackages: &collections.Set[string]{}, possibleFailedAmbientModuleLookupSources: &collections.SyncMap[tspath.Path, *failedAmbientModuleLookupSource]{}, possibleFailedAmbientModuleLookupTargets: &collections.SyncSet[string]{}, @@ -1123,6 +1129,9 @@ func (b *registryBuilder) extractPackages( entrypointsMu.Lock() result.entrypoints = append(result.entrypoints, packageEntrypoints) + if packageEntrypoints.PackageHasExports { + result.packagesWithExports.Add(packageName) + } entrypointsMu.Unlock() aliasResolver := createAliasResolver(packageName, packageEntrypoints.Entrypoints, toSymlink, resolver) @@ -1211,12 +1220,13 @@ func (b *registryBuilder) buildNodeModulesBucket( result := &bucketBuildResult{ bucket: &RegistryBucket{ - Index: &Index[*Export]{}, - DependencyNames: dependencies, - PackageFiles: allPackageFiles, - AmbientModuleNames: extraction.ambientModuleNames, - Paths: paths, - Entrypoints: make(map[tspath.Path][]*module.ResolvedEntrypoint, len(extraction.exports)), + Index: &Index[*Export]{}, + DependencyNames: dependencies, + PackageFiles: allPackageFiles, + AmbientModuleNames: extraction.ambientModuleNames, + Paths: paths, + Entrypoints: make(map[tspath.Path][]*module.ResolvedEntrypoint, len(extraction.exports)), + PackagesWithExports: extraction.packagesWithExports, state: BucketState{ fileExcludePatterns: b.userPreferences.AutoImportFileExcludePatterns, }, diff --git a/internal/ls/autoimport/specifiers.go b/internal/ls/autoimport/specifiers.go index f53ad3cbb66..6d8a006d0f5 100644 --- a/internal/ls/autoimport/specifiers.go +++ b/internal/ls/autoimport/specifiers.go @@ -3,7 +3,9 @@ package autoimport import ( "strings" + "github.com/microsoft/typescript-go/internal/module" "github.com/microsoft/typescript-go/internal/modulespecifiers" + "github.com/microsoft/typescript-go/internal/tspath" ) func (v *View) GetModuleSpecifier( @@ -20,8 +22,26 @@ func (v *View) GetModuleSpecifier( } if export.NodeModulesDirectory != "" { - if entrypoints, ok := v.registry.nodeModules[export.NodeModulesDirectory].Entrypoints[export.Path]; ok { + bucket := v.registry.nodeModules[export.NodeModulesDirectory] + if entrypoints, ok := bucket.Entrypoints[export.Path]; ok { + // Get the package name from one of the entrypoints + var packageNameForExport string + if len(entrypoints) > 0 { + packageNameForExport, _ = splitPackageSpecifier(entrypoints[0].ModuleSpecifier) + } + + // Check if this package has exports using the precomputed set + packageHasExports := bucket.PackagesWithExports != nil && bucket.PackagesWithExports.Has(packageNameForExport) + for _, entrypoint := range entrypoints { + // Skip EndingChangeable entrypoints only when the package has exports. + // These files were discovered by reading the package directory directly + // (not from exports), so they should not be suggested for auto-import + // when the package has an exports field that doesn't expose them. + // When the package doesn't have exports, EndingChangeable files are valid. + if packageHasExports && entrypoint.Ending == module.EndingChangeable { + continue + } if entrypoint.IncludeConditions.IsSubsetOf(v.conditions) && !v.conditions.Intersects(entrypoint.ExcludeConditions) { specifier := modulespecifiers.ProcessEntrypointEnding( entrypoint, @@ -37,18 +57,18 @@ func (v *View) GetModuleSpecifier( } } } - return "", modulespecifiers.ResultKindNone } + // If the export is from a node_modules package but has no valid entrypoints, + // it cannot be imported (e.g., internal files not exposed via package.json exports). + return "", modulespecifiers.ResultKindNone } cache := v.registry.specifierCache[v.importingFile.Path()] - if export.NodeModulesDirectory == "" { - if specifier, ok := cache.Load(export.Path); ok { - if specifier == "" { - return "", modulespecifiers.ResultKindNone - } - return specifier, modulespecifiers.ResultKindRelative + if specifier, ok := cache.Load(export.Path); ok { + if specifier == "" { + return "", modulespecifiers.ResultKindNone } + return specifier, modulespecifiers.ResultKindRelative } specifiers, kind := modulespecifiers.GetModuleSpecifiersForFileWithInfo( @@ -67,9 +87,92 @@ func (v *View) GetModuleSpecifier( if strings.Contains(specifier, "/node_modules/") { continue } + // Check if this is a package specifier (starts with @ or doesn't start with ./ or ../) + // that points to a non-entrypoint file in a symlinked package + if v.isInvalidPackageSpecifier(specifier, export) { + continue + } cache.Store(export.Path, specifier) return specifier, kind } cache.Store(export.Path, "") return "", modulespecifiers.ResultKindNone } + +// isInvalidPackageSpecifier checks if a specifier is a bare package specifier +// (like @foo/bar/something or foo/something) that points to a file that is not +// a valid entrypoint in any node_modules bucket. This can happen when a symlinked +// monorepo package has internal files that are part of the program but should not +// be importable from outside the package via the package specifier. +func (v *View) isInvalidPackageSpecifier(specifier string, export *Export) bool { + // Only check bare specifiers (not relative paths) + if tspath.PathIsRelative(specifier) { + return false + } + + // Extract the package name from the specifier + // For scoped packages like @foo/bar/path, package name is @foo/bar + // For non-scoped packages like foo/path, package name is foo + packageName, subpath := splitPackageSpecifier(specifier) + if packageName == "" { + return false + } + + // If there's no subpath, this is a root import like "@foo/bar" or "foo" + // These should be allowed as they go through normal resolution + if subpath == "" { + return false + } + + // There's a subpath like "@foo/bar/something" - check if this is a valid entrypoint + // Check if the export's file is a valid entrypoint in any node_modules bucket + filePath := export.Path + for _, bucket := range v.registry.nodeModules { + if _, ok := bucket.Entrypoints[filePath]; ok { + // File is a valid entrypoint + return false + } + } + + // File is not a valid entrypoint, so this subpath specifier should not be generated + // unless it can be resolved via the package's exports field (which it can't, or + // it would have been handled by tryGetModuleNameFromExports in tryGetModuleNameAsNodeModule) + return true +} + +// splitPackageSpecifier splits a bare package specifier into package name and subpath. +// For "@scope/pkg/path", returns ("@scope/pkg", "path") +// For "pkg/path", returns ("pkg", "path") +// For "@scope/pkg" or "pkg", returns the package name and empty subpath +func splitPackageSpecifier(specifier string) (packageName string, subpath string) { + if specifier == "" { + return "", "" + } + + // Handle scoped packages (@scope/package) + if specifier[0] == '@' { + // Find the second slash (after @scope/package) + firstSlash := strings.Index(specifier, "/") + if firstSlash == -1 { + // Just "@scope" - not valid, but return as-is + return specifier, "" + } + secondSlash := strings.Index(specifier[firstSlash+1:], "/") + if secondSlash == -1 { + // "@scope/package" - no subpath + return specifier, "" + } + // "@scope/package/subpath" + packageEnd := firstSlash + 1 + secondSlash + return specifier[:packageEnd], specifier[packageEnd+1:] + } + + // Non-scoped package + firstSlash := strings.Index(specifier, "/") + if firstSlash == -1 { + // "package" - no subpath + return specifier, "" + } + // "package/subpath" + return specifier[:firstSlash], specifier[firstSlash+1:] +} diff --git a/internal/module/resolver.go b/internal/module/resolver.go index 297a2f63ac2..5291cd1b44e 100644 --- a/internal/module/resolver.go +++ b/internal/module/resolver.go @@ -2022,6 +2022,9 @@ func GetAutomaticTypeDirectiveNames(options *core.CompilerOptions, host Resoluti type ResolvedEntrypoints struct { Entrypoints []*ResolvedEntrypoint FailedLookupLocations []string + // PackageHasExports indicates whether the package has an "exports" field in its package.json. + // When true, only files explicitly exposed via exports should be importable. + PackageHasExports bool } type Ending int @@ -2068,10 +2071,11 @@ func (r *Resolver) GetEntrypointsFromPackageJsonInfo(packageJson *packagejson.In return &ResolvedEntrypoints{ Entrypoints: entrypoints, FailedLookupLocations: state.failedLookupLocations, + PackageHasExports: true, } } - result := &ResolvedEntrypoints{} + result := &ResolvedEntrypoints{PackageHasExports: false} mainResolution := state.loadNodeModuleFromDirectoryWorker( extensions, packageJson.PackageDirectory,