Add package deduplication, --deduplicatePackages

internal/checker/checker.go

@@ -14867,7 +14867,7 @@ func (c *Checker) resolveExternalModule(location *ast.Node, moduleReference stri
 			return c.getMergedSymbol(sourceFile.Symbol)
 		}
 		if errorNode != nil && moduleNotFoundError != nil && !isSideEffectImport(errorNode) {
-			c.error(errorNode, diagnostics.File_0_is_not_a_module, sourceFile.FileName())
+			c.error(errorNode, diagnostics.File_0_is_not_a_module, resolvedModule.ResolvedFileName)
 		}
 		return nil
 	}

internal/compiler/fileloader.go

@@ -68,7 +68,14 @@ type processedFiles struct {
 	// if file was included using source file and its output is actually part of program
 	// this contains mapping from output to source file
 	outputFileToProjectReferenceSource map[tspath.Path]string
-	finishedProcessing                 bool
+	// Maps a source file path to the name of the package it was imported with
+	sourceFileToPackageName map[tspath.Path]string
+	// Key is a file path. Value is the list of files that redirect to it (same package, different install location)
+	redirectTargetsMap map[tspath.Path][]string
+	// Maps any path (canonical or redirect target) to its canonical path.
+	// Canonical paths map to themselves; redirect targets map to their canonical path.
+	deduplicatedPathMap map[tspath.Path]tspath.Path
+	finishedProcessing  bool
 }
 
 type jsxRuntimeImportSpecifier struct {

internal/compiler/filesparser.go

@@ -283,6 +283,17 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 	var sourceFilesFoundSearchingNodeModules collections.Set[tspath.Path]
 	libFilesMap := make(map[tspath.Path]*LibFile, libFileCount)
 
+	var sourceFileToPackageName map[tspath.Path]string
+	var redirectTargetsMap map[tspath.Path][]string
+	var deduplicatedPathMap map[tspath.Path]tspath.Path
+	var packageIdToCanonicalPath map[module.PackageId]tspath.Path
+	if !loader.opts.Config.CompilerOptions().DisablePackageDeduplication.IsTrue() {
+		sourceFileToPackageName = make(map[tspath.Path]string, totalFileCount)
+		redirectTargetsMap = make(map[tspath.Path][]string)
+		deduplicatedPathMap = make(map[tspath.Path]tspath.Path)
+		packageIdToCanonicalPath = make(map[module.PackageId]tspath.Path)
+	}
+
 	var collectFiles func(tasks []*parseTask, seen map[*parseTaskData]string)
 	collectFiles = func(tasks []*parseTask, seen map[*parseTaskData]string) {
 		for _, task := range tasks {
@@ -331,6 +342,36 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 			for _, trace := range task.resolutionsTrace {
 				loader.opts.Host.Trace(trace.Message, trace.Args...)
 			}
+
+			if packageIdToCanonicalPath != nil {
+				for _, resolution := range task.resolutionsInFile {
+					if !resolution.IsResolved() {
+						continue
+					}
+					pkgId := resolution.PackageId
+					if pkgId.Name == "" {
+						continue
+					}
+					resolvedPath := loader.toPath(resolution.ResolvedFileName)
+					packageName := pkgId.PackageName()
+
+					if canonical, exists := packageIdToCanonicalPath[pkgId]; exists {
+						if _, alreadyRecorded := sourceFileToPackageName[resolvedPath]; !alreadyRecorded {
+							sourceFileToPackageName[resolvedPath] = packageName
+							if resolvedPath != canonical {
+
+								deduplicatedPathMap[resolvedPath] = canonical
+								redirectTargetsMap[canonical] = append(redirectTargetsMap[canonical], resolution.ResolvedFileName)
+							}
+						}
+					} else {
+						packageIdToCanonicalPath[pkgId] = resolvedPath
+						sourceFileToPackageName[resolvedPath] = packageName
+						deduplicatedPathMap[resolvedPath] = resolvedPath
+					}
+				}
+			}
+
 			if subTasks := task.subTasks; len(subTasks) > 0 {
 				collectFiles(subTasks, seen)
 			}
@@ -408,6 +449,22 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 		}
 	}
 
+	if deduplicatedPathMap != nil {
+		for duplicatePath, canonicalPath := range deduplicatedPathMap {
+			if duplicatePath != canonicalPath {
+				if canonicalFile, ok := filesByPath[canonicalPath]; ok {
+					filesByPath[duplicatePath] = canonicalFile
+				}
+			}
+		}
+		allFiles = slices.DeleteFunc(allFiles, func(f *ast.SourceFile) bool {
+			if canonicalPath, ok := deduplicatedPathMap[f.Path()]; ok {
+				return f.Path() != canonicalPath
+			}
+			return false
+		})
+	}
+
 	return processedFiles{
 		finishedProcessing:                   true,
 		resolver:                             loader.resolver,
@@ -424,6 +481,9 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 		missingFiles:                         missingFiles,
 		includeProcessor:                     includeProcessor,
 		outputFileToProjectReferenceSource:   outputFileToProjectReferenceSource,
+		sourceFileToPackageName:              sourceFileToPackageName,
+		redirectTargetsMap:                   redirectTargetsMap,
+		deduplicatedPathMap:                  deduplicatedPathMap,
 	}
 }
 

internal/compiler/program.go

@@ -110,9 +110,10 @@ func (p *Program) GetPackageJsonInfo(pkgJsonPath string) *packagejson.InfoCacheE
 	return nil
 }
 
-// GetRedirectTargets implements checker.Program.
+// GetRedirectTargets returns the list of file paths that redirect to the given path.
+// These are files from the same package (same name@version) installed in different locations.
 func (p *Program) GetRedirectTargets(path tspath.Path) []string {
-	return nil // !!! TODO: project references support
+	return p.redirectTargetsMap[path]
 }
 
 // gets the original file that was included in program
@@ -241,6 +242,14 @@ func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHos
 	if !canReplaceFileInProgram(oldFile, newFile) {
 		return NewProgram(newOpts), false
 	}
+	// If this file is part of a package redirect group (same package installed in multiple
+	// node_modules locations), we need to rebuild the program because the redirect targets
+	// might need recalculation. A file is in a redirect group if it's either a canonical
+	// file that others redirect to, or if it redirects to another file.
+	if _, ok := p.deduplicatedPathMap[changedFilePath]; ok {
+		// File is either a canonical file or a redirect target; either way, need full rebuild
+		return NewProgram(newOpts), false
+	}
 	// TODO: reverify compiler options when config has changed?
 	result := &Program{
 		opts:                        newOpts,

internal/core/compileroptions.go

@@ -40,6 +40,7 @@ type CompilerOptions struct {
 	DisableSourceOfProjectReferenceRedirect   Tristate                                  `json:"disableSourceOfProjectReferenceRedirect,omitzero"`
 	DisableSolutionSearching                  Tristate                                  `json:"disableSolutionSearching,omitzero"`
 	DisableReferencedProjectLoad              Tristate                                  `json:"disableReferencedProjectLoad,omitzero"`
+	DisablePackageDeduplication               Tristate                                  `json:"disablePackageDeduplication,omitzero"`
 	ErasableSyntaxOnly                        Tristate                                  `json:"erasableSyntaxOnly,omitzero"`
 	ESModuleInterop                           Tristate                                  `json:"esModuleInterop,omitzero"`
 	ExactOptionalPropertyTypes                Tristate                                  `json:"exactOptionalPropertyTypes,omitzero"`

internal/diagnostics/extraDiagnosticMessages.json

@@ -86,5 +86,9 @@
     "Option '{0}' requires value to be greater than '{1}'.": {
         "category": "Error",
         "code": 5002
+    },
+    "Disable deduplication of packages with the same name and version.": {
+        "category": "Message",
+        "code": 100011
     }
 }

internal/fourslash/_scripts/failingTests.txt

@@ -246,7 +246,6 @@ TestContextuallyTypedFunctionExpressionGeneric1
 TestContextualTypingOfGenericCallSignatures2
 TestCrossFileQuickInfoExportedTypeDoesNotUseImportType
 TestDoubleUnderscoreCompletions
-TestDuplicatePackageServices
 TestEditJsdocType
 TestErrorsAfterResolvingVariableDeclOfMergedVariableAndClassDecl
 TestExportDefaultClass

internal/fourslash/_scripts/manualTests.txt

@@ -2,6 +2,7 @@ completionListInClosedFunction05
 completionsAtIncompleteObjectLiteralProperty
 completionsSelfDeclaring1
 completionsWithDeprecatedTag4
+duplicatePackageServices_fileChanges
 navigationBarFunctionPrototype
 navigationBarFunctionPrototype2
 navigationBarFunctionPrototype3
@@ -26,4 +27,4 @@ jsDocFunctionSignatures12
 outliningHintSpansForFunction
 getOutliningSpans
 outliningForNonCompleteInterfaceDeclaration
-incrementalParsingWithJsDoc
+incrementalParsingWithJsDoc

internal/fourslash/tests/manual/duplicatePackageServices_fileChanges_test.go

@@ -0,0 +1,68 @@
+package fourslash_test
+
+import (
+	"testing"
+
+	"github.com/microsoft/typescript-go/internal/fourslash"
+	"github.com/microsoft/typescript-go/internal/testutil"
+)
+
+func TestDuplicatePackageServices_fileChanges(t *testing.T) {
+	t.Parallel()
+
+	defer testutil.RecoverAndFail(t, "Panic on fourslash test")
+	const content = `// @noImplicitReferences: true
+// @Filename: /node_modules/a/index.d.ts
+import X from "x";
+export function a(x: X): void;
+// @Filename: /node_modules/a/node_modules/x/index.d.ts
+export default class /*defAX*/X {
+    private x: number;
+}
+// @Filename: /node_modules/a/node_modules/x/package.json
+{ "name": "x", "version": "1.2./*aVersionPatch*/3" }
+// @Filename: /node_modules/b/index.d.ts
+import X from "x";
+export const b: X;
+// @Filename: /node_modules/b/node_modules/x/index.d.ts
+export default class /*defBX*/X {
+    private x: number;
+}
+// @Filename: /node_modules/b/node_modules/x/package.json
+{ "name": "x", "version": "1.2./*bVersionPatch*/3" }
+// @Filename: /src/a.ts
+import { a } from "a";
+import { b } from "b";
+a(/*error*/b);`
+	f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
+	defer done()
+
+	f.GoToFile(t, "/src/a.ts")
+	f.VerifyNumberOfErrorsInCurrentFile(t, 0)
+
+	testChangeAndChangeBack := func(versionPatch string, def string) {
+		// Insert "4" after the version patch marker, changing version from 1.2.3 to 1.2.43
+		f.GoToMarker(t, versionPatch)
+		f.Insert(t, "4")
+
+		// Insert a space after the definition marker to trigger a recheck
+		f.GoToMarker(t, def)
+		f.Insert(t, " ")
+
+		// No longer have identical packageId, so we get errors.
+		f.VerifyErrorExistsAfterMarker(t, "error")
+
+		// Undo the changes
+		f.GoToMarker(t, versionPatch)
+		f.DeleteAtCaret(t, 1)
+		f.GoToMarker(t, def)
+		f.DeleteAtCaret(t, 1)
+
+		// Back to being identical.
+		f.GoToFile(t, "/src/a.ts")
+		f.VerifyNumberOfErrorsInCurrentFile(t, 0)
+	}
+
+	testChangeAndChangeBack("aVersionPatch", "defAX")
+	testChangeAndChangeBack("bVersionPatch", "defBX")
+}

internal/module/resolver.go

@@ -985,7 +985,7 @@ func (r *resolutionState) loadModuleFromSpecificNodeModulesDirectory(ext extensi
 			}
 
 			if fromDirectory := r.loadNodeModuleFromDirectoryWorker(ext, candidate, !nodeModulesDirectoryExists, packageInfo); !fromDirectory.shouldContinueSearching() {
-				fromDirectory.packageId = r.getPackageId(packageDirectory, packageInfo)
+				fromDirectory.packageId = r.getPackageId(fromDirectory.path, packageInfo)
 				return fromDirectory
 			}
 		}
@@ -994,12 +994,12 @@ func (r *resolutionState) loadModuleFromSpecificNodeModulesDirectory(ext extensi
 	loader := func(extensions extensions, candidate string, onlyRecordFailures bool) *resolved {
 		if rest != "" || !r.esmMode {
 			if fromFile := r.loadModuleFromFile(extensions, candidate, onlyRecordFailures); !fromFile.shouldContinueSearching() {
-				fromFile.packageId = r.getPackageId(packageDirectory, packageInfo)
+				fromFile.packageId = r.getPackageId(fromFile.path, packageInfo)
 				return fromFile
 			}
 		}
 		if fromDirectory := r.loadNodeModuleFromDirectoryWorker(extensions, candidate, onlyRecordFailures, packageInfo); !fromDirectory.shouldContinueSearching() {
-			fromDirectory.packageId = r.getPackageId(packageDirectory, packageInfo)
+			fromDirectory.packageId = r.getPackageId(fromDirectory.path, packageInfo)
 			return fromDirectory
 		}
 		// !!! this is ported exactly, but checking for null seems wrong?
@@ -1009,7 +1009,7 @@ func (r *resolutionState) loadModuleFromSpecificNodeModulesDirectory(ext extensi
 			// EsmMode disables index lookup in `loadNodeModuleFromDirectoryWorker` generally, however non-relative package resolutions still assume
 			// a default `index.js` entrypoint if no `main` or `exports` are present
 			if indexResult := r.loadModuleFromFile(extensions, tspath.CombinePaths(candidate, "index.js"), onlyRecordFailures); !indexResult.shouldContinueSearching() {
-				indexResult.packageId = r.getPackageId(packageDirectory, packageInfo)
+				indexResult.packageId = r.getPackageId(indexResult.path, packageInfo)
 				return indexResult
 			}
 		}

internal/tsoptions/declscompiler.go

@@ -185,6 +185,14 @@ var commonOptionsWithBuild = []*CommandLineOption{
 		DefaultValueDescription:  false,
 		// Not setting affectsSemanticDiagnostics or affectsBuildInfo because we dont want all diagnostics to go away, its handled in builder
 	},
+	{
+		Name:                    "disablePackageDeduplication",
+		Kind:                    CommandLineOptionTypeBoolean,
+		Category:                diagnostics.Type_Checking,
+		Description:             diagnostics.Disable_deduplication_of_packages_with_the_same_name_and_version,
+		DefaultValueDescription: false,
+		AffectsProgramStructure: true,
+	},
 	{
 		Name:                     "noEmit",
 		Kind:                     CommandLineOptionTypeBoolean,

internal/tsoptions/parsinghelpers.go

@@ -227,6 +227,8 @@ func parseCompilerOptions(key string, value any, allOptions *core.CompilerOption
 		allOptions.DisableSolutionSearching = ParseTristate(value)
 	case "disableReferencedProjectLoad":
 		allOptions.DisableReferencedProjectLoad = ParseTristate(value)
+	case "disablePackageDeduplication":
+		allOptions.DisablePackageDeduplication = ParseTristate(value)
 	case "declarationMap":
 		allOptions.DeclarationMap = ParseTristate(value)
 	case "declaration":

testdata/baselines/reference/compiler/disablePackageDeduplication(disablepackagededuplication=false).errors.txt

@@ -0,0 +1,48 @@
+/src/a.ts(5,3): error TS2345: Argument of type 'import("/node_modules/c/node_modules/x/index").default' is not assignable to parameter of type 'import("/node_modules/a/node_modules/x/index").default'.
+  Types have separate declarations of a private property 'x'.
+
+
+==== /src/a.ts (1 errors) ====
+    import { a } from "a";
+    import { b } from "b";
+    import { c } from "c";
+    a(b); // Works
+    a(c); // Error, these are from different versions of the library.
+      ~
+!!! error TS2345: Argument of type 'import("/node_modules/c/node_modules/x/index").default' is not assignable to parameter of type 'import("/node_modules/a/node_modules/x/index").default'.
+!!! error TS2345:   Types have separate declarations of a private property 'x'.
+
+==== /node_modules/a/index.d.ts (0 errors) ====
+    import X from "x";
+    export function a(x: X): void;
+
+==== /node_modules/a/node_modules/x/index.d.ts (0 errors) ====
+    export default class X {
+        private x: number;
+    }
+
+==== /node_modules/a/node_modules/x/package.json (0 errors) ====
+    { "name": "x", "version": "1.2.3" }
+
+==== /node_modules/b/index.d.ts (0 errors) ====
+    import X from "x";
+    export const b: X;
+
+==== /node_modules/b/node_modules/x/index.d.ts (0 errors) ====
+    content not parsed
+
+==== /node_modules/b/node_modules/x/package.json (0 errors) ====
+    { "name": "x", "version": "1.2.3" }
+
+==== /node_modules/c/index.d.ts (0 errors) ====
+    import X from "x";
+    export const c: X;
+
+==== /node_modules/c/node_modules/x/index.d.ts (0 errors) ====
+    export default class X {
+        private x: number;
+    }
+
+==== /node_modules/c/node_modules/x/package.json (0 errors) ====
+    { "name": "x", "version": "1.2.4" }
+