Tidy up navto and find-file-references (#48106)

They were using the same helpers as FAR and rename, but they actually behave differently.  Decoupling them helps avoid some unnecessary work and will make it easier to clean up FAR in the future.

Scoping NavTo results to a single project (rather than, e.g. all loaded projects) makes the behavior more logical and the implementation simpler.

Author: amcasey

src/server/session.ts

@@ -294,70 +294,35 @@ namespace ts.server {
         return deduplicate(outputs, equateValues);
     }
 
-    type CombineOutputResult<T> = { project: Project; result: readonly T[]; }[];
-    function combineOutputResultContains<T>(outputs: CombineOutputResult<T>, output: T, areEqual: (a: T, b: T) => boolean) {
-        return outputs.some(({ result }) => contains(result, output, areEqual));
-    }
-    function addToCombineOutputResult<T>(outputs: CombineOutputResult<T>, project: Project, result: readonly T[]) {
-        if (result.length) outputs.push({ project, result });
-    }
-
-    function combineProjectOutputFromEveryProject<T>(projectService: ProjectService, action: (project: Project) => readonly T[], areEqual: (a: T, b: T) => boolean): CombineOutputResult<T> {
-        const outputs: CombineOutputResult<T> = [];
-        projectService.loadAncestorProjectTree();
-        projectService.forEachEnabledProject(project => {
-            const theseOutputs = action(project);
-            addToCombineOutputResult(outputs, project, filter(theseOutputs, output => !combineOutputResultContains(outputs, output, areEqual)));
-        });
-        return outputs;
-    }
-
-    function flattenCombineOutputResult<T>(outputs: CombineOutputResult<T>): readonly T[] {
-        return flatMap(outputs, ({ result }) => result);
-    }
-
-    function combineProjectOutputWhileOpeningReferencedProjects<T>(
-        projects: Projects,
-        defaultProject: Project,
-        action: (project: Project) => readonly T[],
-        getLocation: (t: T) => DocumentPosition,
-        resultsEqual: (a: T, b: T) => boolean,
-    ): CombineOutputResult<T> {
-        const outputs: CombineOutputResult<T> = [];
-        combineProjectOutputWorker(
-            projects,
-            defaultProject,
-            /*initialLocation*/ undefined,
-            (project, _, tryAddToTodo) => {
-                const theseOutputs = action(project);
-                addToCombineOutputResult(outputs, project, filter(theseOutputs, output => !combineOutputResultContains(outputs, output, resultsEqual) && !tryAddToTodo(project, getLocation(output))));
-            },
-        );
-        return outputs;
-    }
+    interface ProjectNavigateToItems {
+        project: Project;
+        navigateToItems: readonly NavigateToItem[];
+    };
 
     function combineProjectOutputForRenameLocations(
         projects: Projects,
         defaultProject: Project,
         initialLocation: DocumentPosition,
         findInStrings: boolean,
         findInComments: boolean,
-        hostPreferences: UserPreferences
+        { providePrefixAndSuffixTextForRename }: UserPreferences
     ): readonly RenameLocation[] {
         const outputs: RenameLocation[] = [];
         combineProjectOutputWorker(
             projects,
             defaultProject,
             initialLocation,
             (project, location, tryAddToTodo) => {
-                for (const output of project.getLanguageService().findRenameLocations(location.fileName, location.pos, findInStrings, findInComments, hostPreferences.providePrefixAndSuffixTextForRename) || emptyArray) {
-                    if (!contains(outputs, output, documentSpansEqual) && !tryAddToTodo(project, documentSpanLocation(output))) {
-                        outputs.push(output);
+                const projectOutputs = project.getLanguageService().findRenameLocations(location.fileName, location.pos, findInStrings, findInComments, providePrefixAndSuffixTextForRename);
+                if (projectOutputs) {
+                    for (const output of projectOutputs) {
+                        if (!contains(outputs, output, documentSpansEqual) && !tryAddToTodo(project, documentSpanLocation(output))) {
+                            outputs.push(output);
+                        }
                     }
                 }
             },
         );
-
         return outputs;
     }
 
@@ -381,27 +346,30 @@ namespace ts.server {
             initialLocation,
             (project, location, getMappedLocation) => {
                 logger.info(`Finding references to ${location.fileName} position ${location.pos} in project ${project.getProjectName()}`);
-                for (const outputReferencedSymbol of project.getLanguageService().findReferences(location.fileName, location.pos) || emptyArray) {
-                    const mappedDefinitionFile = getMappedLocation(project, documentSpanLocation(outputReferencedSymbol.definition));
-                    const definition: ReferencedSymbolDefinitionInfo = mappedDefinitionFile === undefined ?
-                        outputReferencedSymbol.definition :
-                        {
-                            ...outputReferencedSymbol.definition,
-                            textSpan: createTextSpan(mappedDefinitionFile.pos, outputReferencedSymbol.definition.textSpan.length),
-                            fileName: mappedDefinitionFile.fileName,
-                            contextSpan: getMappedContextSpan(outputReferencedSymbol.definition, project)
-                        };
-
-                    let symbolToAddTo = find(outputs, o => documentSpansEqual(o.definition, definition));
-                    if (!symbolToAddTo) {
-                        symbolToAddTo = { definition, references: [] };
-                        outputs.push(symbolToAddTo);
-                    }
+                const projectOutputs = project.getLanguageService().findReferences(location.fileName, location.pos);
+                if (projectOutputs) {
+                    for (const referencedSymbol of projectOutputs) {
+                        const mappedDefinitionFile = getMappedLocation(project, documentSpanLocation(referencedSymbol.definition));
+                        const definition: ReferencedSymbolDefinitionInfo = mappedDefinitionFile === undefined ?
+                            referencedSymbol.definition :
+                            {
+                                ...referencedSymbol.definition,
+                                textSpan: createTextSpan(mappedDefinitionFile.pos, referencedSymbol.definition.textSpan.length),
+                                fileName: mappedDefinitionFile.fileName,
+                                contextSpan: getMappedContextSpan(referencedSymbol.definition, project)
+                            };
+
+                        let symbolToAddTo = find(outputs, o => documentSpansEqual(o.definition, definition));
+                        if (!symbolToAddTo) {
+                            symbolToAddTo = { definition, references: [] };
+                            outputs.push(symbolToAddTo);
+                        }
 
-                    for (const ref of outputReferencedSymbol.references) {
-                        // If it's in a mapped file, that is added to the todo list by `getMappedLocation`.
-                        if (!contains(symbolToAddTo.references, ref, documentSpansEqual) && !getMappedLocation(project, documentSpanLocation(ref))) {
-                            symbolToAddTo.references.push(ref);
+                        for (const ref of referencedSymbol.references) {
+                            // If it's in a mapped file, that is added to the todo list by `getMappedLocation`.
+                            if (!contains(symbolToAddTo.references, ref, documentSpansEqual) && !getMappedLocation(project, documentSpanLocation(ref))) {
+                                symbolToAddTo.references.push(ref);
+                            }
                         }
                     }
                 }
@@ -411,29 +379,6 @@ namespace ts.server {
         return outputs.filter(o => o.references.length !== 0);
     }
 
-    function combineProjectOutputForFileReferences(
-        projects: Projects,
-        defaultProject: Project,
-        fileName: string
-    ): readonly ReferenceEntry[] {
-        const outputs: ReferenceEntry[] = [];
-
-        combineProjectOutputWorker(
-            projects,
-            defaultProject,
-            /*initialLocation*/ undefined,
-            project => {
-                for (const referenceEntry of project.getLanguageService().getFileReferences(fileName) || emptyArray) {
-                    if (!contains(outputs, referenceEntry, documentSpansEqual)) {
-                        outputs.push(referenceEntry);
-                    }
-                }
-            },
-        );
-
-        return outputs;
-    }
-
     interface ProjectAndLocation<TLocation extends DocumentPosition | undefined> {
         readonly project: Project;
         readonly location: TLocation;
@@ -1610,11 +1555,22 @@ namespace ts.server {
 
         private getFileReferences(args: protocol.FileRequestArgs, simplifiedResult: boolean): protocol.FileReferencesResponseBody | readonly ReferenceEntry[] {
             const projects = this.getProjects(args);
-            const references = combineProjectOutputForFileReferences(
-                projects,
-                this.getDefaultProject(args),
-                args.file,
-            );
+            const fileName = args.file;
+
+            const references: ReferenceEntry[] = [];
+
+            forEachProjectInProjects(projects, /*path*/ undefined, project => {
+                if (project.getCancellationToken().isCancellationRequested()) return;
+
+                const projectOutputs = project.getLanguageService().getFileReferences(fileName);
+                if (projectOutputs) {
+                    for (const referenceEntry of projectOutputs) {
+                        if (!contains(references, referenceEntry, documentSpansEqual)) {
+                            references.push(referenceEntry);
+                        }
+                    }
+                }
+            });
 
             if (!simplifiedResult) return references;
             const refs = references.map(entry => referenceEntryToReferencesResponseItem(this.projectService, entry));
@@ -2092,10 +2048,10 @@ namespace ts.server {
         private getNavigateToItems(args: protocol.NavtoRequestArgs, simplifiedResult: boolean): readonly protocol.NavtoItem[] | readonly NavigateToItem[] {
             const full = this.getFullNavigateToItems(args);
             return !simplifiedResult ?
-                flattenCombineOutputResult(full) :
+                flatMap(full, ({ navigateToItems }) => navigateToItems) :
                 flatMap(
                     full,
-                    ({ project, result }) => result.map(navItem => {
+                    ({ project, navigateToItems }) => navigateToItems.map(navItem => {
                         const scriptInfo = project.getScriptInfo(navItem.fileName)!;
                         const bakedItem: protocol.NavtoItem = {
                             name: navItem.name,
@@ -2121,26 +2077,72 @@ namespace ts.server {
                 );
         }
 
-        private getFullNavigateToItems(args: protocol.NavtoRequestArgs): CombineOutputResult<NavigateToItem> {
+        private getFullNavigateToItems(args: protocol.NavtoRequestArgs): ProjectNavigateToItems[] {
             const { currentFileOnly, searchValue, maxResultCount, projectFileName } = args;
+
             if (currentFileOnly) {
                 Debug.assertIsDefined(args.file);
                 const { file, project } = this.getFileAndProject(args as protocol.FileRequestArgs);
-                return [{ project, result: project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, file) }];
-            }
-            else if (!args.file && !projectFileName) {
-                return combineProjectOutputFromEveryProject(
-                    this.projectService,
-                    project => project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*filename*/ undefined, /*excludeDts*/ project.isNonTsProject()),
-                    navigateToItemIsEqualTo);
-            }
-            const fileArgs = args as protocol.FileRequestArgs;
-            return combineProjectOutputWhileOpeningReferencedProjects<NavigateToItem>(
-                this.getProjects(fileArgs),
-                this.getDefaultProject(fileArgs),
-                project => project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*fileName*/ undefined, /*excludeDts*/ project.isNonTsProject()),
-                documentSpanLocation,
-                navigateToItemIsEqualTo);
+                return [{ project, navigateToItems: project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, file) }];
+            }
+
+            const outputs: ProjectNavigateToItems[] = [];
+
+            // This is effectively a hashset with `name` as the custom hash and `navigateToItemIsEqualTo` as the custom equals.
+            // `name` is a very cheap hash function, but we could incorporate other properties to reduce collisions.
+            const seenItems = new Map<string, NavigateToItem[]>(); // name to items with that name
+
+            if (!args.file && !projectFileName) {
+                // VS Code's `Go to symbol in workspaces` sends request like this
+
+                // TODO (https://github.com/microsoft/TypeScript/issues/47839)
+                // This appears to have been intended to search all projects but, in practice, it seems to only search
+                // those that are downstream from already-loaded projects.
+                // Filtering by !isSourceOfProjectReferenceRedirect is new, but seems appropriate and consistent with
+                // the case below.
+                this.projectService.loadAncestorProjectTree();
+                this.projectService.forEachEnabledProject(project => addItemsForProject(project));
+            }
+            else {
+                // VS's `Go to symbol` sends requests with just a project and doesn't want cascading since it will
+                // send a separate request for each project of interest
+
+                // TODO (https://github.com/microsoft/TypeScript/issues/47839)
+                // This doesn't really make sense unless it's a single project matching `projectFileName`
+                const projects = this.getProjects(args as protocol.FileRequestArgs);
+                forEachProjectInProjects(projects, /*path*/ undefined, project => addItemsForProject(project));
+            }
+
+            return outputs;
+
+            // Mutates `outputs`
+            function addItemsForProject(project: Project) {
+                const projectItems = project.getLanguageService().getNavigateToItems(searchValue, maxResultCount, /*filename*/ undefined, /*excludeDts*/ project.isNonTsProject());
+                const unseenItems = filter(projectItems, item => tryAddSeenItem(item) && !getMappedLocation(documentSpanLocation(item), project));
+                if (unseenItems.length) {
+                    outputs.push({ project, navigateToItems: unseenItems });
+                }
+            }
+
+            // Returns true if the item had not been seen before
+            // Mutates `seenItems`
+            function tryAddSeenItem(item: NavigateToItem) {
+                const name = item.name;
+                if (!seenItems.has(name)) {
+                    seenItems.set(name, [item]);
+                    return true;
+                }
+
+                const seen = seenItems.get(name)!;
+                for (const seenItem of seen) {
+                    if (navigateToItemIsEqualTo(seenItem, item)) {
+                        return false;
+                    }
+                }
+
+                seen.push(item);
+                return true;
+            }
 
             function navigateToItemIsEqualTo(a: NavigateToItem, b: NavigateToItem): boolean {
                 if (a === b) {
@@ -2255,14 +2257,29 @@ namespace ts.server {
             const newPath = toNormalizedPath(args.newFilePath);
             const formatOptions = this.getHostFormatOptions();
             const preferences = this.getHostPreferences();
-            const changes = flattenCombineOutputResult(
-                combineProjectOutputFromEveryProject(
-                    this.projectService,
-                    project => project.getLanguageService().getEditsForFileRename(oldPath, newPath, formatOptions, preferences),
-                    (a, b) => a.fileName === b.fileName
-                )
-            );
-            return simplifiedResult ? changes.map(c => this.mapTextChangeToCodeEdit(c)) : changes;
+
+
+            const seenFiles = new Set<string>();
+            const textChanges: FileTextChanges[] = [];
+            // TODO (https://github.com/microsoft/TypeScript/issues/47839)
+            // This appears to have been intended to search all projects but, in practice, it seems to only search
+            // those that are downstream from already-loaded projects.
+            this.projectService.loadAncestorProjectTree();
+            this.projectService.forEachEnabledProject(project => {
+                const projectTextChanges = project.getLanguageService().getEditsForFileRename(oldPath, newPath, formatOptions, preferences);
+                const projectFiles: string[] = [];
+                for (const textChange of projectTextChanges) {
+                    if (!seenFiles.has(textChange.fileName)) {
+                        textChanges.push(textChange);
+                        projectFiles.push(textChange.fileName);
+                    }
+                }
+                for (const file of projectFiles) {
+                    seenFiles.add(file);
+                }
+            });
+
+            return simplifiedResult ? textChanges.map(c => this.mapTextChangeToCodeEdit(c)) : textChanges;
         }
 
         private getCodeFixes(args: protocol.CodeFixRequestArgs, simplifiedResult: boolean): readonly protocol.CodeFixAction[] | readonly CodeFixAction[] | undefined {

src/testRunner/unittests/tsserver/declarationFileMaps.ts

@@ -286,6 +286,8 @@ namespace ts.projectSystem {
             const session = makeSampleProjects();
             const response = executeSessionRequest<protocol.NavtoRequest, protocol.NavtoResponse>(session, CommandNames.Navto, { file: userTs.path, searchValue: "fn" });
             assert.deepEqual<readonly protocol.NavtoItem[] | undefined>(response, [
+                // Keep the .d.ts file since the .ts file no longer exists
+                // (otherwise it would be treated as not in the project)
                 {
                     ...protocolFileSpanFromSubstring({
                         file: bDts,
@@ -308,20 +310,9 @@ namespace ts.projectSystem {
                     kind: ScriptElementKind.functionElement,
                     kindModifiers: "export",
                 },
-                {
-                    ...protocolFileSpanFromSubstring({
-                        file: aTs,
-                        text: "export function fnA() {}"
-                    }),
-                    name: "fnA",
-                    matchKind: "prefix",
-                    isCaseSensitive: true,
-                    kind: ScriptElementKind.functionElement,
-                    kindModifiers: "export",
-                },
             ]);
 
-            verifyATsConfigOriginalProject(session);
+            verifySingleInferredProject(session);
         });
 
         it("navigateToAll -- when neither file nor project is specified", () => {