ExplainFiles to handle duplicated files (microsoft#2617)

Author: sheetalkamat

internal/compiler/fileInclude.go

@@ -5,7 +5,6 @@ import (
 	"sync"
 
 	"github.com/microsoft/typescript-go/internal/ast"
-	"github.com/microsoft/typescript-go/internal/core"
 	"github.com/microsoft/typescript-go/internal/diagnostics"
 	"github.com/microsoft/typescript-go/internal/module"
 	"github.com/microsoft/typescript-go/internal/scanner"
@@ -23,8 +22,6 @@ const (
 	fileIncludeKindLibReferenceDirective
 
 	fileIncludeKindRootFile
-	fileIncludeKindSourceFromProjectReference
-	fileIncludeKindOutputFromProjectReference
 	fileIncludeKindLibFile
 	fileIncludeKindAutomaticTypeDirectiveFile
 )
@@ -192,15 +189,6 @@ func (r *FileIncludeReason) computeDiagnostic(program *Program, toFileName func(
 		} else {
 			return ast.NewCompilerDiagnostic(diagnostics.Root_file_specified_for_compilation)
 		}
-	case fileIncludeKindSourceFromProjectReference,
-		fileIncludeKindOutputFromProjectReference:
-		diag := core.IfElse(
-			r.kind == fileIncludeKindOutputFromProjectReference,
-			diagnostics.Output_from_referenced_project_0_included_because_module_is_specified_as_none,
-			diagnostics.Source_from_referenced_project_0_included_because_module_is_specified_as_none,
-		)
-		referencedResolvedRef := program.projectReferenceFileMapper.getResolvedProjectReferences()[r.asIndex()]
-		return ast.NewCompilerDiagnostic(diag, toFileName(referencedResolvedRef.ConfigName()))
 	case fileIncludeKindAutomaticTypeDirectiveFile:
 		data := r.asAutomaticTypeDirectiveFileData()
 		if program.Options().Types != nil {
@@ -288,16 +276,6 @@ func (r *FileIncludeReason) toRelatedInfo(program *Program) *ast.Diagnostic {
 				return tsoptions.CreateDiagnosticForNodeInSourceFile(config.ConfigFile.SourceFile, includeNode.AsNode(), diagnostics.File_is_matched_by_include_pattern_specified_here)
 			}
 		}
-	case fileIncludeKindSourceFromProjectReference,
-		fileIncludeKindOutputFromProjectReference:
-		return tsoptions.CreateDiagnosticAtReferenceSyntax(
-			config,
-			r.asIndex(),
-			core.IfElse(
-				r.kind == fileIncludeKindOutputFromProjectReference,
-				diagnostics.File_is_output_from_referenced_project_specified_here,
-				diagnostics.File_is_source_from_referenced_project_specified_here,
-			))
 	case fileIncludeKindAutomaticTypeDirectiveFile:
 		if program.Options().Types != nil {
 			data := r.asAutomaticTypeDirectiveFileData()

internal/compiler/fileloader.go

@@ -50,6 +50,24 @@ type fileLoader struct {
 	pathForLibFileResolutions collections.SyncMap[tspath.Path, *libResolution]
 }
 
+type redirectsFile struct {
+	// Index of file at which this redirect file needs to be iterated
+	index    int
+	fileName string
+	path     tspath.Path
+	target   tspath.Path
+}
+
+var _ ast.HasFileName = (*redirectsFile)(nil)
+
+func (r *redirectsFile) FileName() string {
+	return r.fileName
+}
+
+func (r *redirectsFile) Path() tspath.Path {
+	return r.path
+}
+
 type processedFiles struct {
 	resolver                      *module.Resolver
 	files                         []*ast.SourceFile
@@ -70,9 +88,9 @@ type processedFiles struct {
 	outputFileToProjectReferenceSource 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
-	// Any paths involved in deduplication, including canonical paths and redirected paths
-	deduplicatedPaths  collections.Set[tspath.Path]
-	finishedProcessing bool
+	// filesByPath for redirect files
+	redirectFilesByPath map[tspath.Path]*redirectsFile
+	finishedProcessing  bool
 }
 
 type jsxRuntimeImportSpecifier struct {

internal/compiler/filesparser.go

@@ -293,11 +293,11 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 	libFilesMap := make(map[tspath.Path]*LibFile, libFileCount)
 
 	var redirectTargetsMap map[tspath.Path][]string
-	var deduplicatedPaths collections.Set[tspath.Path]
-	var packageIdToCanonicalPath map[module.PackageId]tspath.Path
+	var redirectFilesByPath map[tspath.Path]*redirectsFile
+	var packageIdToSourceFile map[module.PackageId]*ast.SourceFile
 	if !loader.opts.Config.CompilerOptions().DeduplicatePackages.IsFalse() {
 		redirectTargetsMap = make(map[tspath.Path][]string)
-		packageIdToCanonicalPath = make(map[module.PackageId]tspath.Path)
+		packageIdToSourceFile = make(map[module.PackageId]*ast.SourceFile)
 	}
 
 	var collectFiles func(tasks []*parseTask, seen map[*parseTaskData]string)
@@ -349,22 +349,31 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 				loader.opts.Host.Trace(trace.Message, trace.Args...)
 			}
 
-			var existingCanonicalPath tspath.Path
-			if packageIdToCanonicalPath != nil && data.packageId.Name != "" {
-				if canonical, exists := packageIdToCanonicalPath[data.packageId]; exists {
-					redirectTargetsMap[canonical] = append(redirectTargetsMap[canonical], task.normalizedFilePath)
-					existingCanonicalPath = canonical
-					deduplicatedPaths.Add(task.path)
-					deduplicatedPaths.Add(canonical)
-				} else {
-					packageIdToCanonicalPath[data.packageId] = task.path
+			file := task.file
+			if packageIdToSourceFile != nil && data.packageId.Name != "" {
+				if packageIdFile, exists := packageIdToSourceFile[data.packageId]; exists {
+					redirectTargetsMap[packageIdFile.Path()] = append(redirectTargetsMap[packageIdFile.Path()], task.normalizedFilePath)
+					if redirectFilesByPath == nil {
+						redirectFilesByPath = make(map[tspath.Path]*redirectsFile, totalFileCount)
+					}
+					redirectFilesByPath[task.path] = &redirectsFile{
+						index:    len(files) + len(redirectFilesByPath),
+						fileName: task.normalizedFilePath,
+						path:     task.path,
+						target:   packageIdFile.Path(),
+					}
+					filesByPath[task.path] = packageIdFile
+					if data.lowestDepth > 0 {
+						sourceFilesFoundSearchingNodeModules.Add(task.path)
+					}
+					continue
+				} else if file != nil {
+					packageIdToSourceFile[data.packageId] = file
 				}
 			}
 
-			if existingCanonicalPath == "" {
-				if subTasks := task.subTasks; len(subTasks) > 0 {
-					collectFiles(subTasks, seen)
-				}
+			if subTasks := task.subTasks; len(subTasks) > 0 {
+				collectFiles(subTasks, seen)
 			}
 
 			// Exclude automatic type directive tasks from include reason processing,
@@ -381,10 +390,6 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 				typeResolutionsInFile[task.path] = task.typeResolutionsInFile
 				continue
 			}
-			file := task.file
-			if existingCanonicalPath != "" {
-				file = filesByPath[existingCanonicalPath]
-			}
 
 			path := task.path
 
@@ -401,7 +406,7 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 			if task.libFile != nil {
 				libFiles = append(libFiles, file)
 				libFilesMap[path] = task.libFile
-			} else if existingCanonicalPath == "" {
+			} else {
 				files = append(files, file)
 			}
 			filesByPath[path] = file
@@ -431,6 +436,9 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 	loader.sortLibs(libFiles)
 
 	allFiles := append(libFiles, files...)
+	for _, redirectFile := range redirectFilesByPath {
+		redirectFile.index += len(libFiles)
+	}
 
 	keys := slices.Collect(loader.pathForLibFileResolutions.Keys())
 	slices.Sort(keys)
@@ -461,7 +469,7 @@ func (w *filesParser) getProcessedFiles(loader *fileLoader) processedFiles {
 		includeProcessor:                     includeProcessor,
 		outputFileToProjectReferenceSource:   outputFileToProjectReferenceSource,
 		redirectTargetsMap:                   redirectTargetsMap,
-		deduplicatedPaths:                    deduplicatedPaths,
+		redirectFilesByPath:                  redirectFilesByPath,
 	}
 }
 

internal/compiler/includeprocessor.go

@@ -122,28 +122,38 @@ func (i *includeProcessor) getRelatedInfo(r *FileIncludeReason, program *Program
 
 func (i *includeProcessor) explainRedirectAndImpliedFormat(
 	program *Program,
-	file *ast.SourceFile,
+	filePath tspath.Path,
 	toFileName func(fileName string) string,
 ) []*ast.Diagnostic {
-	if existing, ok := i.redirectAndFileFormat.Load(file.Path()); ok {
+	if existing, ok := i.redirectAndFileFormat.Load(filePath); ok {
 		return existing
 	}
+	var file ast.HasFileName
+	var sourceFile *ast.SourceFile
+	redirectsFile := program.redirectFilesByPath[filePath]
+	if redirectsFile != nil {
+		file = redirectsFile
+	} else {
+		sourceFile = program.GetSourceFileByPath(filePath)
+		file = sourceFile
+	}
 	var result []*ast.Diagnostic
 	if source := program.GetSourceOfProjectReferenceIfOutputIncluded(file); source != file.FileName() {
 		result = append(result, ast.NewCompilerDiagnostic(
 			diagnostics.File_is_output_of_project_reference_source_0,
 			toFileName(source),
 		))
 	}
-	// !!! redirects
-	// if (file.redirectInfo) {
-	//     (result ??= []).push(chainDiagnosticMessages(
-	//         /*details*/ undefined,
-	//         Diagnostics.File_redirects_to_file_0,
-	//         toFileName(file.redirectInfo.redirectTarget, fileNameConvertor),
-	//     ));
-	// }
-	if ast.IsExternalOrCommonJSModule(file) {
+
+	if redirectsFile != nil {
+		targetFile := program.GetSourceFileByPath(redirectsFile.target)
+		result = append(result, ast.NewCompilerDiagnostic(
+			diagnostics.File_redirects_to_file_0,
+			toFileName(targetFile.FileName()),
+		))
+	}
+
+	if sourceFile != nil && ast.IsExternalOrCommonJSModule(sourceFile) {
 		metaData := program.GetSourceFileMetaData(file.Path())
 		switch program.GetImpliedNodeFormatForEmit(file) {
 		case core.ModuleKindESNext:
@@ -166,6 +176,6 @@ func (i *includeProcessor) explainRedirectAndImpliedFormat(
 		}
 	}
 
-	result, _ = i.redirectAndFileFormat.LoadOrStore(file.Path(), result)
+	result, _ = i.redirectAndFileFormat.LoadOrStore(filePath, result)
 	return result
 }

internal/compiler/processingDiagnostic.go

@@ -102,7 +102,7 @@ func (d *processingDiagnostic) createDiagnosticExplainingFile(program *Program)
 		for _, reason := range reasons {
 			processInclude(reason)
 		}
-		redirectInfo = program.includeProcessor.explainRedirectAndImpliedFormat(program, program.GetSourceFileByPath(diag.file), func(fileName string) string { return fileName })
+		redirectInfo = program.includeProcessor.explainRedirectAndImpliedFormat(program, diag.file, func(fileName string) string { return fileName })
 	}
 	if diag.diagnosticReason != nil {
 		processInclude(diag.diagnosticReason)

internal/compiler/program.go

@@ -241,18 +241,19 @@ func NewProgram(opts ProgramOptions) *Program {
 // Return an updated program for which it is known that only the file with the given path has changed.
 // In addition to a new program, return a boolean indicating whether the data of the old program was reused.
 func (p *Program) UpdateProgram(changedFilePath tspath.Path, newHost CompilerHost) (*Program, bool) {
-	oldFile := p.filesByPath[changedFilePath]
 	newOpts := p.opts
 	newOpts.Host = newHost
-	newFile := newHost.GetSourceFile(oldFile.ParseOptions())
-	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.
-	if p.deduplicatedPaths.Has(changedFilePath) {
-		// File is either a canonical file or a redirect target; either way, need full rebuild
+	if _, exists := p.redirectFilesByPath[changedFilePath]; exists {
+		return NewProgram(newOpts), false
+	}
+
+	oldFile := p.filesByPath[changedFilePath]
+	newFile := newHost.GetSourceFile(oldFile.ParseOptions())
+	if !canReplaceFileInProgram(oldFile, newFile) {
 		return NewProgram(newOpts), false
 	}
 	// TODO: reverify compiler options when config has changed?
@@ -1576,6 +1577,8 @@ func (p *Program) HasSameFileNames(other *Program) bool {
 	return maps.EqualFunc(p.filesByPath, other.filesByPath, func(a, b *ast.SourceFile) bool {
 		// checks for casing differences on case-insensitive file systems
 		return a.FileName() == b.FileName()
+	}) && maps.EqualFunc(p.redirectFilesByPath, other.redirectFilesByPath, func(a, b *redirectsFile) bool {
+		return a.FileName() == b.FileName()
 	})
 }
 
@@ -1599,15 +1602,40 @@ func (p *Program) ExplainFiles(w io.Writer, locale locale.Locale) {
 	toRelativeFileName := func(fileName string) string {
 		return tspath.GetRelativePathFromDirectory(p.GetCurrentDirectory(), fileName, p.comparePathsOptions)
 	}
-	for _, file := range p.GetSourceFiles() {
+	filesExplained := 0
+	explainFile := func(file ast.HasFileName) {
 		fmt.Fprintln(w, toRelativeFileName(file.FileName()))
 		for _, reason := range p.includeProcessor.fileIncludeReasons[file.Path()] {
 			fmt.Fprintln(w, "  ", reason.toDiagnostic(p, true).Localize(locale))
 		}
-		for _, diag := range p.includeProcessor.explainRedirectAndImpliedFormat(p, file, toRelativeFileName) {
+		for _, diag := range p.includeProcessor.explainRedirectAndImpliedFormat(p, file.Path(), toRelativeFileName) {
 			fmt.Fprintln(w, "  ", diag.Localize(locale))
 		}
+		filesExplained++
+	}
+
+	redirectFiles := slices.Collect(maps.Values(p.redirectFilesByPath))
+	slices.SortFunc(redirectFiles, func(a, b *redirectsFile) int {
+		return a.index - b.index
+	})
+
+	files := p.GetSourceFiles()
+	sourceFileIndex := 0
+	explainSourceFiles := func(endIndex int) {
+		for filesExplained < endIndex {
+			explainFile(files[sourceFileIndex])
+			sourceFileIndex++
+		}
 	}
+
+	for _, redirectFile := range redirectFiles {
+		// Explain all sourceFiles till we reach this redirectFile index
+		explainSourceFiles(redirectFile.index)
+		explainFile(redirectFile)
+	}
+
+	// Explain any remaining sourceFiles
+	explainSourceFiles(len(files) + len(redirectFiles))
 }
 
 func (p *Program) GetLibFileFromReference(ref *ast.FileReference) *ast.SourceFile {

internal/execute/tsctests/tsc_test.go

@@ -257,7 +257,6 @@ func TestTscComposite(t *testing.T) {
 			commandLineArgs: []string{"--composite", "false"},
 		},
 		{
-			// !!! sheetal null is not reflected in final options
 			subScenario: "when setting composite null on command line",
 			files: FileMap{
 				"/home/src/workspaces/project/src/main.ts": "export const x = 10;",
@@ -725,7 +724,6 @@ func TestTscDeclarationEmit(t *testing.T) {
 			commandLineArgs:  []string{"-p", "D:\\Work\\pkg1", "--explainFiles"},
 		},
 		{
-			// !!! sheetal redirected files not yet implemented
 			subScenario: "when same version is referenced through source and another symlinked package",
 			files: FileMap{
 				`/user/username/projects/myproject/plugin-two/index.d.ts`:                               pluginTwoDts(),
@@ -742,7 +740,6 @@ func TestTscDeclarationEmit(t *testing.T) {
 			commandLineArgs: []string{"-p", "plugin-one", "--explainFiles"},
 		},
 		{
-			// !!! sheetal redirected files not yet implemented
 			subScenario: "when same version is referenced through source and another symlinked package with indirect link",
 			files: FileMap{
 				`/user/username/projects/myproject/plugin-two/package.json`: stringtestutil.Dedent(`

testdata/baselines/reference/tsc/declarationEmit/when-same-version-is-referenced-through-source-and-another-symlinked-package-with-indirect-link.js

@@ -170,6 +170,9 @@ plugin-two/node_modules/typescript-fsa/index.d.ts
    Imported via "typescript-fsa" from file 'plugin-two/dist/commonjs/index.d.ts' with packageId 'typescript-fsa/index.d.ts@3.0.0-beta-2'
 plugin-two/dist/commonjs/index.d.ts
    Imported via "plugin-two" from file 'plugin-one/index.ts' with packageId 'plugin-two/dist/commonjs/index.d.ts@0.1.3'
+plugin-one/node_modules/typescript-fsa/index.d.ts
+   Imported via "typescript-fsa" from file 'plugin-one/index.ts' with packageId 'typescript-fsa/index.d.ts@3.0.0-beta-2'
+   File redirects to file 'plugin-two/node_modules/typescript-fsa/index.d.ts'
 plugin-one/index.ts
    Matched by default include pattern '**/*'
 //// [/home/src/tslibs/TS/Lib/lib.d.ts] *Lib*

testdata/baselines/reference/tsc/declarationEmit/when-same-version-is-referenced-through-source-and-another-symlinked-package.js

@@ -157,6 +157,9 @@ plugin-one/node_modules/typescript-fsa/index.d.ts
    Imported via "typescript-fsa" from file 'plugin-one/action.ts' with packageId 'typescript-fsa/index.d.ts@3.0.0-beta-2'
 plugin-one/action.ts
    Matched by default include pattern '**/*'
+plugin-two/node_modules/typescript-fsa/index.d.ts
+   Imported via "typescript-fsa" from file 'plugin-two/index.d.ts' with packageId 'typescript-fsa/index.d.ts@3.0.0-beta-2'
+   File redirects to file 'plugin-one/node_modules/typescript-fsa/index.d.ts'
 plugin-two/index.d.ts
    Imported via "plugin-two" from file 'plugin-one/index.ts'
 plugin-one/index.ts