More thoroughly test navigateTo (#25239)

* More thoroughly test navigateTo

* Fix #25233 and #25237

* Update API (#24966)

Author: Andy

src/compiler/utilities.ts

@@ -4866,14 +4866,9 @@ namespace ts {
         return !!(node as NamedDeclaration).name; // A 'name' property should always be a DeclarationName.
     }
 
-    export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
+    /** @internal */
+    export function getNonAssignedNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
         switch (declaration.kind) {
-            case SyntaxKind.ClassExpression:
-            case SyntaxKind.FunctionExpression:
-                if (!(declaration as ClassExpression | FunctionExpression).name) {
-                    return getAssignedName(declaration);
-                }
-                break;
             case SyntaxKind.Identifier:
                 return declaration as Identifier;
             case SyntaxKind.JSDocPropertyTag:
@@ -4906,6 +4901,12 @@ namespace ts {
         return (declaration as NamedDeclaration).name;
     }
 
+    export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
+        if (declaration === undefined) return undefined;
+        return getNonAssignedNameOfDeclaration(declaration) ||
+            (isFunctionExpression(declaration) || isClassExpression(declaration) ? getAssignedName(declaration) : undefined);
+    }
+
     function getAssignedName(node: Node): DeclarationName | undefined {
         if (!node.parent) {
             return undefined;

src/harness/client.ts

@@ -224,9 +224,9 @@ namespace ts.server {
                 containerName: entry.containerName || "",
                 containerKind: entry.containerKind || ScriptElementKind.unknown,
                 kind: entry.kind,
-                kindModifiers: entry.kindModifiers!, // TODO: GH#18217
-                matchKind: entry.matchKind!, // TODO: GH#18217
-                isCaseSensitive: entry.isCaseSensitive!, // TODO: GH#18217
+                kindModifiers: entry.kindModifiers || "",
+                matchKind: entry.matchKind as keyof typeof PatternMatchKind,
+                isCaseSensitive: entry.isCaseSensitive,
                 fileName: entry.file,
                 textSpan: this.decodeSpan(entry),
             }));

src/harness/fourslash.ts

@@ -2775,57 +2775,20 @@ Actual: ${stringify(fullActual)}`);
             }
         }
 
-        /*
-            Check number of navigationItems which match both searchValue and matchKind,
-            if a filename is passed in, limit the results to that file.
-            Report an error if expected value and actual value do not match.
-        */
-        public verifyNavigationItemsCount(expected: number, searchValue: string, matchKind?: string, fileName?: string) {
-            const items = this.languageService.getNavigateToItems(searchValue, /*maxResultCount*/ undefined, fileName);
-            let actual = 0;
-
-            // Count only the match that match the same MatchKind
-            for (const item of items) {
-                if (!matchKind || item.matchKind === matchKind) {
-                    actual++;
-                }
-            }
-
-            if (expected !== actual) {
-                this.raiseError(`verifyNavigationItemsCount failed - found: ${actual} navigation items, expected: ${expected}.`);
-            }
-        }
-
-        /*
-            Verify that returned navigationItems from getNavigateToItems have matched searchValue, matchKind, and kind.
-            Report an error if getNavigateToItems does not find any matched searchValue.
-        */
-        public verifyNavigationItemsListContains(
-            name: string,
-            kind: string,
-            searchValue: string,
-            matchKind: string,
-            fileName?: string,
-            parentName?: string) {
-            const items = this.languageService.getNavigateToItems(searchValue);
-
-            if (!items || items.length === 0) {
-                this.raiseError("verifyNavigationItemsListContains failed - found 0 navigation items, expected at least one.");
-            }
-
-            for (const item of items) {
-                if (item && item.name === name && item.kind === kind &&
-                    (matchKind === undefined || item.matchKind === matchKind) &&
-                    (fileName === undefined || item.fileName === fileName) &&
-                    (parentName === undefined || item.containerName === parentName)) {
-                    return;
-                }
-            }
-
-            // if there was an explicit match kind specified, then it should be validated.
-            if (matchKind !== undefined) {
-                const missingItem = { name, kind, searchValue, matchKind, fileName, parentName };
-                this.raiseError(`verifyNavigationItemsListContains failed - could not find the item: ${stringify(missingItem)} in the returned list: (${stringify(items)})`);
+        public verifyNavigateTo(options: ReadonlyArray<FourSlashInterface.VerifyNavigateToOptions>): void {
+            for (const { pattern, expected, fileName } of options) {
+                const items = this.languageService.getNavigateToItems(pattern, /*maxResultCount*/ undefined, fileName);
+                this.assertObjectsEqual(items, expected.map((e): ts.NavigateToItem => ({
+                    name: e.name,
+                    kind: e.kind,
+                    kindModifiers: e.kindModifiers || "",
+                    matchKind: e.matchKind || "exact",
+                    isCaseSensitive: e.isCaseSensitive === undefined ? true : e.isCaseSensitive,
+                    fileName: e.range.fileName,
+                    textSpan: ts.createTextSpanFromRange(e.range),
+                    containerName: e.containerName || "",
+                    containerKind: e.containerKind || ts.ScriptElementKind.unknown,
+                })));
             }
         }
 
@@ -4361,24 +4324,8 @@ namespace FourSlashInterface {
             this.state.verifyNavigationTree(json, options);
         }
 
-        public navigationItemsListCount(count: number, searchValue: string, matchKind?: string, fileName?: string) {
-            this.state.verifyNavigationItemsCount(count, searchValue, matchKind, fileName);
-        }
-
-        public navigationItemsListContains(
-            name: string,
-            kind: string,
-            searchValue: string,
-            matchKind: string,
-            fileName?: string,
-            parentName?: string) {
-            this.state.verifyNavigationItemsListContains(
-                name,
-                kind,
-                searchValue,
-                matchKind,
-                fileName,
-                parentName);
+        public navigateTo(...options: VerifyNavigateToOptions[]): void {
+            this.state.verifyNavigateTo(options);
         }
 
         public occurrencesAtPositionContains(range: FourSlash.Range, isWriteAccess?: boolean) {
@@ -4812,6 +4759,23 @@ namespace FourSlashInterface {
         readonly tags?: ReadonlyArray<ts.JSDocTagInfo>;
     }
 
+    export interface VerifyNavigateToOptions {
+        readonly pattern: string;
+        readonly fileName?: string;
+        readonly expected: ReadonlyArray<ExpectedNavigateToItem>;
+    }
+
+    export interface ExpectedNavigateToItem {
+        readonly name: string;
+        readonly kind: ts.ScriptElementKind;
+        readonly kindModifiers?: string;
+        readonly matchKind?: keyof typeof ts.PatternMatchKind;
+        readonly isCaseSensitive?: boolean;
+        readonly range: FourSlash.Range;
+        readonly containerName?: string;
+        readonly containerKind?: ts.ScriptElementKind;
+    }
+
     export type ArrayOrSingle<T> = T | ReadonlyArray<T>;
 
     export interface VerifyCompletionListContainsOptions extends ts.UserPreferences {

src/server/protocol.ts

@@ -2458,12 +2458,12 @@ namespace ts.server.protocol {
         /**
          * exact, substring, or prefix.
          */
-        matchKind?: string;
+        matchKind: string;
 
         /**
          * If this was a case sensitive or insensitive match.
          */
-        isCaseSensitive?: boolean;
+        isCaseSensitive: boolean;
 
         /**
          * Optional modifiers for the kind (such as 'public').

src/server/session.ts

@@ -1591,16 +1591,15 @@ namespace ts.server {
                             const bakedItem: protocol.NavtoItem = {
                                 name: navItem.name,
                                 kind: navItem.kind,
+                                isCaseSensitive: navItem.isCaseSensitive,
+                                matchKind: navItem.matchKind,
                                 file: navItem.fileName,
                                 start: scriptInfo.positionToLineOffset(navItem.textSpan.start),
                                 end: scriptInfo.positionToLineOffset(textSpanEnd(navItem.textSpan))
                             };
                             if (navItem.kindModifiers && (navItem.kindModifiers !== "")) {
                                 bakedItem.kindModifiers = navItem.kindModifiers;
                             }
-                            if (navItem.matchKind !== "none") {
-                                bakedItem.matchKind = navItem.matchKind;
-                            }
                             if (navItem.containerName && (navItem.containerName.length > 0)) {
                                 bakedItem.containerName = navItem.containerName;
                             }

src/services/navigateTo.ts

@@ -1,17 +1,17 @@
 /* @internal */
 namespace ts.NavigateTo {
     interface RawNavigateToItem {
-        name: string;
-        fileName: string;
-        matchKind: PatternMatchKind;
-        isCaseSensitive: boolean;
-        declaration: Declaration;
+        readonly name: string;
+        readonly fileName: string;
+        readonly matchKind: PatternMatchKind;
+        readonly isCaseSensitive: boolean;
+        readonly declaration: Declaration;
     }
 
     export function getNavigateToItems(sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cancellationToken: CancellationToken, searchValue: string, maxResultCount: number | undefined, excludeDtsFiles: boolean): NavigateToItem[] {
         const patternMatcher = createPatternMatcher(searchValue);
         if (!patternMatcher) return emptyArray;
-        let rawItems: RawNavigateToItem[] = [];
+        const rawItems: RawNavigateToItem[] = [];
 
         // Search the declarations in all files and output matched NavigateToItem into array of NavigateToItem[]
         for (const sourceFile of sourceFiles) {
@@ -27,10 +27,7 @@ namespace ts.NavigateTo {
         }
 
         rawItems.sort(compareNavigateToItems);
-        if (maxResultCount !== undefined) {
-            rawItems = rawItems.slice(0, maxResultCount);
-        }
-        return rawItems.map(createNavigateToItem);
+        return (maxResultCount === undefined ? rawItems : rawItems.slice(0, maxResultCount)).map(createNavigateToItem);
     }
 
     function getItemsFromNamedDeclaration(patternMatcher: PatternMatcher, name: string, declarations: ReadonlyArray<Declaration>, checker: TypeChecker, fileName: string, rawItems: Push<RawNavigateToItem>): void {
@@ -45,13 +42,13 @@ namespace ts.NavigateTo {
             if (!shouldKeepItem(declaration, checker)) continue;
 
             if (patternMatcher.patternContainsDots) {
-                const fullMatch = patternMatcher.getFullMatch(getContainers(declaration)!, name); // TODO: GH#18217
+                // If the pattern has dots in it, then also see if the declaration container matches as well.
+                const fullMatch = patternMatcher.getFullMatch(getContainers(declaration), name);
                 if (fullMatch) {
                     rawItems.push({ name, fileName, matchKind: fullMatch.kind, isCaseSensitive: fullMatch.isCaseSensitive, declaration });
                 }
             }
             else {
-                // If the pattern has dots in it, then also see if the declaration container matches as well.
                 rawItems.push({ name, fileName, matchKind: match.kind, isCaseSensitive: match.isCaseSensitive, declaration });
             }
         }
@@ -62,7 +59,7 @@ namespace ts.NavigateTo {
             case SyntaxKind.ImportClause:
             case SyntaxKind.ImportSpecifier:
             case SyntaxKind.ImportEqualsDeclaration:
-                const importer = checker.getSymbolAtLocation((declaration as ImportClause | ImportSpecifier | ImportEqualsDeclaration).name!)!;
+                const importer = checker.getSymbolAtLocation((declaration as ImportClause | ImportSpecifier | ImportEqualsDeclaration).name!)!; // TODO: GH#18217
                 const imported = checker.getAliasedSymbol(importer);
                 return importer.escapedName !== imported.escapedName;
             default:
@@ -107,22 +104,22 @@ namespace ts.NavigateTo {
         return false;
     }
 
-    function getContainers(declaration: Declaration): string[] | undefined {
+    function getContainers(declaration: Declaration): ReadonlyArray<string> {
         const containers: string[] = [];
 
         // First, if we started with a computed property name, then add all but the last
         // portion into the container array.
         const name = getNameOfDeclaration(declaration);
         if (name && name.kind === SyntaxKind.ComputedPropertyName && !tryAddComputedPropertyName(name.expression, containers, /*includeLastPortion*/ false)) {
-            return undefined;
+            return emptyArray;
         }
 
         // Now, walk up our containers, adding all their names to the container array.
         let container = getContainerNode(declaration);
 
         while (container) {
             if (!tryAddSingleDeclarationName(container, containers)) {
-                return undefined;
+                return emptyArray;
             }
 
             container = getContainerNode(container);
@@ -145,13 +142,13 @@ namespace ts.NavigateTo {
             name: rawItem.name,
             kind: getNodeKind(declaration),
             kindModifiers: getNodeModifiers(declaration),
-            matchKind: PatternMatchKind[rawItem.matchKind],
+            matchKind: PatternMatchKind[rawItem.matchKind] as keyof typeof PatternMatchKind,
             isCaseSensitive: rawItem.isCaseSensitive,
             fileName: rawItem.fileName,
             textSpan: createTextSpanFromNode(declaration),
             // TODO(jfreeman): What should be the containerName when the container has a computed name?
             containerName: containerName ? (<Identifier>containerName).text : "",
-            containerKind: containerName ? getNodeKind(container!) : ScriptElementKind.unknown // TODO: GH#18217 Just use `container ? ...`
+            containerKind: containerName ? getNodeKind(container!) : ScriptElementKind.unknown, // TODO: GH#18217 Just use `container ? ...`
         };
     }
 }

src/services/navigationBar.ts

@@ -188,7 +188,7 @@ namespace ts.NavigationBar {
                 // Handle default import case e.g.:
                 //    import d from "mod";
                 if (importClause.name) {
-                    addLeafNode(importClause);
+                    addLeafNode(importClause.name);
                 }
 
                 // Handle named bindings in imports e.g.:

src/services/patternMatcher.ts

@@ -124,8 +124,6 @@ namespace ts {
             return undefined;
         }
 
-        candidateContainers = candidateContainers || [];
-
         // -1 because the last part was checked against the name, and only the rest
         // of the parts are checked against the container.
         if (dotSeparatedSegments.length - 1 > candidateContainers.length) {

src/services/services.ts

@@ -633,7 +633,7 @@ namespace ts {
         private computeNamedDeclarations(): Map<Declaration[]> {
             const result = createMultiMap<Declaration>();
 
-            forEachChild(this, visit);
+            this.forEachChild(visit);
 
             return result;
 
@@ -653,7 +653,7 @@ namespace ts {
             }
 
             function getDeclarationName(declaration: Declaration) {
-                const name = getNameOfDeclaration(declaration);
+                const name = getNonAssignedNameOfDeclaration(declaration);
                 return name && (isComputedPropertyName(name) && isPropertyAccessExpression(name.expression) ? name.expression.name.text
                     : isPropertyName(name) ? getNameFromPropertyName(name) : undefined);
             }
@@ -742,7 +742,7 @@ namespace ts {
                             // Handle default import case e.g.:
                             //    import d from "mod";
                             if (importClause.name) {
-                                addDeclaration(importClause);
+                                addDeclaration(importClause.name);
                             }
 
                             // Handle named bindings in imports e.g.:

src/services/types.ts

@@ -78,7 +78,7 @@ namespace ts {
         /* @internal */ scriptSnapshot: IScriptSnapshot | undefined;
         /* @internal */ nameTable: UnderscoreEscapedMap<number> | undefined;
 
-        /* @internal */ getNamedDeclarations(): Map<Declaration[]>;
+        /* @internal */ getNamedDeclarations(): Map<ReadonlyArray<Declaration>>;
 
         getLineAndCharacterOfPosition(pos: number): LineAndCharacter;
         getLineEndOfPosition(pos: number): number;
@@ -599,7 +599,7 @@ namespace ts {
         name: string;
         kind: ScriptElementKind;
         kindModifiers: string;
-        matchKind: string; // TODO: keyof typeof PatternMatchKind; (https://github.com/Microsoft/TypeScript/issues/15102)
+        matchKind: "exact" | "prefix" | "substring" | "camelCase";
         isCaseSensitive: boolean;
         fileName: string;
         textSpan: TextSpan;

src/services/utilities.ts

@@ -347,7 +347,6 @@ namespace ts {
             case SyntaxKind.Parameter: return hasModifier(node, ModifierFlags.ParameterPropertyModifier) ? ScriptElementKind.memberVariableElement : ScriptElementKind.parameterElement;
             case SyntaxKind.ImportEqualsDeclaration:
             case SyntaxKind.ImportSpecifier:
-            case SyntaxKind.ImportClause:
             case SyntaxKind.ExportSpecifier:
             case SyntaxKind.NamespaceImport:
                 return ScriptElementKind.alias;
@@ -375,6 +374,8 @@ namespace ts {
                         return ScriptElementKind.unknown;
                     }
                 }
+            case SyntaxKind.Identifier:
+                return isImportClause(node.parent) ? ScriptElementKind.alias : ScriptElementKind.unknown;
             default:
                 return ScriptElementKind.unknown;
         }