Skip to content
Draft
Show file tree
Hide file tree
Changes from all 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
117 changes: 117 additions & 0 deletions internal/fourslash/tests/autoImportTransitiveLeak_test.go
Original file line number Diff line number Diff line change
@@ -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"},
},
})
}
24 changes: 17 additions & 7 deletions internal/ls/autoimport/registry.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -148,6 +151,7 @@ func (b *RegistryBucket) Clone() *RegistryBucket {
DependencyNames: b.DependencyNames,
AmbientModuleNames: b.AmbientModuleNames,
Entrypoints: b.Entrypoints,
PackagesWithExports: b.PackagesWithExports,
Index: b.Index,
}
}
Expand Down Expand Up @@ -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]
Expand All @@ -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]{},
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
},
Expand Down
119 changes: 111 additions & 8 deletions internal/ls/autoimport/specifiers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand All @@ -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(
Expand All @@ -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:]
}
6 changes: 5 additions & 1 deletion internal/module/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
Loading