Skip to content

Commit efc1b7d

Browse files
author
Andy
authored
More thoroughly test navigateTo (microsoft#25239)
* More thoroughly test navigateTo * Fix microsoft#25233 and microsoft#25237 * Update API (microsoft#24966)
1 parent c3b81b3 commit efc1b7d

45 files changed

Lines changed: 642 additions & 494 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎src/compiler/utilities.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4866,14 +4866,9 @@ namespace ts {
48664866
return !!(node as NamedDeclaration).name; // A 'name' property should always be a DeclarationName.
48674867
}
48684868

4869-
export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
4869+
/** @internal */
4870+
export function getNonAssignedNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
48704871
switch (declaration.kind) {
4871-
case SyntaxKind.ClassExpression:
4872-
case SyntaxKind.FunctionExpression:
4873-
if (!(declaration as ClassExpression | FunctionExpression).name) {
4874-
return getAssignedName(declaration);
4875-
}
4876-
break;
48774872
case SyntaxKind.Identifier:
48784873
return declaration as Identifier;
48794874
case SyntaxKind.JSDocPropertyTag:
@@ -4906,6 +4901,12 @@ namespace ts {
49064901
return (declaration as NamedDeclaration).name;
49074902
}
49084903

4904+
export function getNameOfDeclaration(declaration: Declaration | Expression): DeclarationName | undefined {
4905+
if (declaration === undefined) return undefined;
4906+
return getNonAssignedNameOfDeclaration(declaration) ||
4907+
(isFunctionExpression(declaration) || isClassExpression(declaration) ? getAssignedName(declaration) : undefined);
4908+
}
4909+
49094910
function getAssignedName(node: Node): DeclarationName | undefined {
49104911
if (!node.parent) {
49114912
return undefined;

‎src/harness/client.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,9 @@ namespace ts.server {
224224
containerName: entry.containerName || "",
225225
containerKind: entry.containerKind || ScriptElementKind.unknown,
226226
kind: entry.kind,
227-
kindModifiers: entry.kindModifiers!, // TODO: GH#18217
228-
matchKind: entry.matchKind!, // TODO: GH#18217
229-
isCaseSensitive: entry.isCaseSensitive!, // TODO: GH#18217
227+
kindModifiers: entry.kindModifiers || "",
228+
matchKind: entry.matchKind as keyof typeof PatternMatchKind,
229+
isCaseSensitive: entry.isCaseSensitive,
230230
fileName: entry.file,
231231
textSpan: this.decodeSpan(entry),
232232
}));

‎src/harness/fourslash.ts‎

Lines changed: 33 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -2775,57 +2775,20 @@ Actual: ${stringify(fullActual)}`);
27752775
}
27762776
}
27772777

2778-
/*
2779-
Check number of navigationItems which match both searchValue and matchKind,
2780-
if a filename is passed in, limit the results to that file.
2781-
Report an error if expected value and actual value do not match.
2782-
*/
2783-
public verifyNavigationItemsCount(expected: number, searchValue: string, matchKind?: string, fileName?: string) {
2784-
const items = this.languageService.getNavigateToItems(searchValue, /*maxResultCount*/ undefined, fileName);
2785-
let actual = 0;
2786-
2787-
// Count only the match that match the same MatchKind
2788-
for (const item of items) {
2789-
if (!matchKind || item.matchKind === matchKind) {
2790-
actual++;
2791-
}
2792-
}
2793-
2794-
if (expected !== actual) {
2795-
this.raiseError(`verifyNavigationItemsCount failed - found: ${actual} navigation items, expected: ${expected}.`);
2796-
}
2797-
}
2798-
2799-
/*
2800-
Verify that returned navigationItems from getNavigateToItems have matched searchValue, matchKind, and kind.
2801-
Report an error if getNavigateToItems does not find any matched searchValue.
2802-
*/
2803-
public verifyNavigationItemsListContains(
2804-
name: string,
2805-
kind: string,
2806-
searchValue: string,
2807-
matchKind: string,
2808-
fileName?: string,
2809-
parentName?: string) {
2810-
const items = this.languageService.getNavigateToItems(searchValue);
2811-
2812-
if (!items || items.length === 0) {
2813-
this.raiseError("verifyNavigationItemsListContains failed - found 0 navigation items, expected at least one.");
2814-
}
2815-
2816-
for (const item of items) {
2817-
if (item && item.name === name && item.kind === kind &&
2818-
(matchKind === undefined || item.matchKind === matchKind) &&
2819-
(fileName === undefined || item.fileName === fileName) &&
2820-
(parentName === undefined || item.containerName === parentName)) {
2821-
return;
2822-
}
2823-
}
2824-
2825-
// if there was an explicit match kind specified, then it should be validated.
2826-
if (matchKind !== undefined) {
2827-
const missingItem = { name, kind, searchValue, matchKind, fileName, parentName };
2828-
this.raiseError(`verifyNavigationItemsListContains failed - could not find the item: ${stringify(missingItem)} in the returned list: (${stringify(items)})`);
2778+
public verifyNavigateTo(options: ReadonlyArray<FourSlashInterface.VerifyNavigateToOptions>): void {
2779+
for (const { pattern, expected, fileName } of options) {
2780+
const items = this.languageService.getNavigateToItems(pattern, /*maxResultCount*/ undefined, fileName);
2781+
this.assertObjectsEqual(items, expected.map((e): ts.NavigateToItem => ({
2782+
name: e.name,
2783+
kind: e.kind,
2784+
kindModifiers: e.kindModifiers || "",
2785+
matchKind: e.matchKind || "exact",
2786+
isCaseSensitive: e.isCaseSensitive === undefined ? true : e.isCaseSensitive,
2787+
fileName: e.range.fileName,
2788+
textSpan: ts.createTextSpanFromRange(e.range),
2789+
containerName: e.containerName || "",
2790+
containerKind: e.containerKind || ts.ScriptElementKind.unknown,
2791+
})));
28292792
}
28302793
}
28312794

@@ -4361,24 +4324,8 @@ namespace FourSlashInterface {
43614324
this.state.verifyNavigationTree(json, options);
43624325
}
43634326

4364-
public navigationItemsListCount(count: number, searchValue: string, matchKind?: string, fileName?: string) {
4365-
this.state.verifyNavigationItemsCount(count, searchValue, matchKind, fileName);
4366-
}
4367-
4368-
public navigationItemsListContains(
4369-
name: string,
4370-
kind: string,
4371-
searchValue: string,
4372-
matchKind: string,
4373-
fileName?: string,
4374-
parentName?: string) {
4375-
this.state.verifyNavigationItemsListContains(
4376-
name,
4377-
kind,
4378-
searchValue,
4379-
matchKind,
4380-
fileName,
4381-
parentName);
4327+
public navigateTo(...options: VerifyNavigateToOptions[]): void {
4328+
this.state.verifyNavigateTo(options);
43824329
}
43834330

43844331
public occurrencesAtPositionContains(range: FourSlash.Range, isWriteAccess?: boolean) {
@@ -4812,6 +4759,23 @@ namespace FourSlashInterface {
48124759
readonly tags?: ReadonlyArray<ts.JSDocTagInfo>;
48134760
}
48144761

4762+
export interface VerifyNavigateToOptions {
4763+
readonly pattern: string;
4764+
readonly fileName?: string;
4765+
readonly expected: ReadonlyArray<ExpectedNavigateToItem>;
4766+
}
4767+
4768+
export interface ExpectedNavigateToItem {
4769+
readonly name: string;
4770+
readonly kind: ts.ScriptElementKind;
4771+
readonly kindModifiers?: string;
4772+
readonly matchKind?: keyof typeof ts.PatternMatchKind;
4773+
readonly isCaseSensitive?: boolean;
4774+
readonly range: FourSlash.Range;
4775+
readonly containerName?: string;
4776+
readonly containerKind?: ts.ScriptElementKind;
4777+
}
4778+
48154779
export type ArrayOrSingle<T> = T | ReadonlyArray<T>;
48164780

48174781
export interface VerifyCompletionListContainsOptions extends ts.UserPreferences {

‎src/server/protocol.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2458,12 +2458,12 @@ namespace ts.server.protocol {
24582458
/**
24592459
* exact, substring, or prefix.
24602460
*/
2461-
matchKind?: string;
2461+
matchKind: string;
24622462

24632463
/**
24642464
* If this was a case sensitive or insensitive match.
24652465
*/
2466-
isCaseSensitive?: boolean;
2466+
isCaseSensitive: boolean;
24672467

24682468
/**
24692469
* Optional modifiers for the kind (such as 'public').

‎src/server/session.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1591,16 +1591,15 @@ namespace ts.server {
15911591
const bakedItem: protocol.NavtoItem = {
15921592
name: navItem.name,
15931593
kind: navItem.kind,
1594+
isCaseSensitive: navItem.isCaseSensitive,
1595+
matchKind: navItem.matchKind,
15941596
file: navItem.fileName,
15951597
start: scriptInfo.positionToLineOffset(navItem.textSpan.start),
15961598
end: scriptInfo.positionToLineOffset(textSpanEnd(navItem.textSpan))
15971599
};
15981600
if (navItem.kindModifiers && (navItem.kindModifiers !== "")) {
15991601
bakedItem.kindModifiers = navItem.kindModifiers;
16001602
}
1601-
if (navItem.matchKind !== "none") {
1602-
bakedItem.matchKind = navItem.matchKind;
1603-
}
16041603
if (navItem.containerName && (navItem.containerName.length > 0)) {
16051604
bakedItem.containerName = navItem.containerName;
16061605
}

‎src/services/navigateTo.ts‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
11
/* @internal */
22
namespace ts.NavigateTo {
33
interface RawNavigateToItem {
4-
name: string;
5-
fileName: string;
6-
matchKind: PatternMatchKind;
7-
isCaseSensitive: boolean;
8-
declaration: Declaration;
4+
readonly name: string;
5+
readonly fileName: string;
6+
readonly matchKind: PatternMatchKind;
7+
readonly isCaseSensitive: boolean;
8+
readonly declaration: Declaration;
99
}
1010

1111
export function getNavigateToItems(sourceFiles: ReadonlyArray<SourceFile>, checker: TypeChecker, cancellationToken: CancellationToken, searchValue: string, maxResultCount: number | undefined, excludeDtsFiles: boolean): NavigateToItem[] {
1212
const patternMatcher = createPatternMatcher(searchValue);
1313
if (!patternMatcher) return emptyArray;
14-
let rawItems: RawNavigateToItem[] = [];
14+
const rawItems: RawNavigateToItem[] = [];
1515

1616
// Search the declarations in all files and output matched NavigateToItem into array of NavigateToItem[]
1717
for (const sourceFile of sourceFiles) {
@@ -27,10 +27,7 @@ namespace ts.NavigateTo {
2727
}
2828

2929
rawItems.sort(compareNavigateToItems);
30-
if (maxResultCount !== undefined) {
31-
rawItems = rawItems.slice(0, maxResultCount);
32-
}
33-
return rawItems.map(createNavigateToItem);
30+
return (maxResultCount === undefined ? rawItems : rawItems.slice(0, maxResultCount)).map(createNavigateToItem);
3431
}
3532

3633
function getItemsFromNamedDeclaration(patternMatcher: PatternMatcher, name: string, declarations: ReadonlyArray<Declaration>, checker: TypeChecker, fileName: string, rawItems: Push<RawNavigateToItem>): void {
@@ -45,13 +42,13 @@ namespace ts.NavigateTo {
4542
if (!shouldKeepItem(declaration, checker)) continue;
4643

4744
if (patternMatcher.patternContainsDots) {
48-
const fullMatch = patternMatcher.getFullMatch(getContainers(declaration)!, name); // TODO: GH#18217
45+
// If the pattern has dots in it, then also see if the declaration container matches as well.
46+
const fullMatch = patternMatcher.getFullMatch(getContainers(declaration), name);
4947
if (fullMatch) {
5048
rawItems.push({ name, fileName, matchKind: fullMatch.kind, isCaseSensitive: fullMatch.isCaseSensitive, declaration });
5149
}
5250
}
5351
else {
54-
// If the pattern has dots in it, then also see if the declaration container matches as well.
5552
rawItems.push({ name, fileName, matchKind: match.kind, isCaseSensitive: match.isCaseSensitive, declaration });
5653
}
5754
}
@@ -62,7 +59,7 @@ namespace ts.NavigateTo {
6259
case SyntaxKind.ImportClause:
6360
case SyntaxKind.ImportSpecifier:
6461
case SyntaxKind.ImportEqualsDeclaration:
65-
const importer = checker.getSymbolAtLocation((declaration as ImportClause | ImportSpecifier | ImportEqualsDeclaration).name!)!;
62+
const importer = checker.getSymbolAtLocation((declaration as ImportClause | ImportSpecifier | ImportEqualsDeclaration).name!)!; // TODO: GH#18217
6663
const imported = checker.getAliasedSymbol(importer);
6764
return importer.escapedName !== imported.escapedName;
6865
default:
@@ -107,22 +104,22 @@ namespace ts.NavigateTo {
107104
return false;
108105
}
109106

110-
function getContainers(declaration: Declaration): string[] | undefined {
107+
function getContainers(declaration: Declaration): ReadonlyArray<string> {
111108
const containers: string[] = [];
112109

113110
// First, if we started with a computed property name, then add all but the last
114111
// portion into the container array.
115112
const name = getNameOfDeclaration(declaration);
116113
if (name && name.kind === SyntaxKind.ComputedPropertyName && !tryAddComputedPropertyName(name.expression, containers, /*includeLastPortion*/ false)) {
117-
return undefined;
114+
return emptyArray;
118115
}
119116

120117
// Now, walk up our containers, adding all their names to the container array.
121118
let container = getContainerNode(declaration);
122119

123120
while (container) {
124121
if (!tryAddSingleDeclarationName(container, containers)) {
125-
return undefined;
122+
return emptyArray;
126123
}
127124

128125
container = getContainerNode(container);
@@ -145,13 +142,13 @@ namespace ts.NavigateTo {
145142
name: rawItem.name,
146143
kind: getNodeKind(declaration),
147144
kindModifiers: getNodeModifiers(declaration),
148-
matchKind: PatternMatchKind[rawItem.matchKind],
145+
matchKind: PatternMatchKind[rawItem.matchKind] as keyof typeof PatternMatchKind,
149146
isCaseSensitive: rawItem.isCaseSensitive,
150147
fileName: rawItem.fileName,
151148
textSpan: createTextSpanFromNode(declaration),
152149
// TODO(jfreeman): What should be the containerName when the container has a computed name?
153150
containerName: containerName ? (<Identifier>containerName).text : "",
154-
containerKind: containerName ? getNodeKind(container!) : ScriptElementKind.unknown // TODO: GH#18217 Just use `container ? ...`
151+
containerKind: containerName ? getNodeKind(container!) : ScriptElementKind.unknown, // TODO: GH#18217 Just use `container ? ...`
155152
};
156153
}
157154
}

‎src/services/navigationBar.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ namespace ts.NavigationBar {
188188
// Handle default import case e.g.:
189189
// import d from "mod";
190190
if (importClause.name) {
191-
addLeafNode(importClause);
191+
addLeafNode(importClause.name);
192192
}
193193

194194
// Handle named bindings in imports e.g.:

‎src/services/patternMatcher.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,8 +124,6 @@ namespace ts {
124124
return undefined;
125125
}
126126

127-
candidateContainers = candidateContainers || [];
128-
129127
// -1 because the last part was checked against the name, and only the rest
130128
// of the parts are checked against the container.
131129
if (dotSeparatedSegments.length - 1 > candidateContainers.length) {

‎src/services/services.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -633,7 +633,7 @@ namespace ts {
633633
private computeNamedDeclarations(): Map<Declaration[]> {
634634
const result = createMultiMap<Declaration>();
635635

636-
forEachChild(this, visit);
636+
this.forEachChild(visit);
637637

638638
return result;
639639

@@ -653,7 +653,7 @@ namespace ts {
653653
}
654654

655655
function getDeclarationName(declaration: Declaration) {
656-
const name = getNameOfDeclaration(declaration);
656+
const name = getNonAssignedNameOfDeclaration(declaration);
657657
return name && (isComputedPropertyName(name) && isPropertyAccessExpression(name.expression) ? name.expression.name.text
658658
: isPropertyName(name) ? getNameFromPropertyName(name) : undefined);
659659
}
@@ -742,7 +742,7 @@ namespace ts {
742742
// Handle default import case e.g.:
743743
// import d from "mod";
744744
if (importClause.name) {
745-
addDeclaration(importClause);
745+
addDeclaration(importClause.name);
746746
}
747747

748748
// Handle named bindings in imports e.g.:

‎src/services/types.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ namespace ts {
7878
/* @internal */ scriptSnapshot: IScriptSnapshot | undefined;
7979
/* @internal */ nameTable: UnderscoreEscapedMap<number> | undefined;
8080

81-
/* @internal */ getNamedDeclarations(): Map<Declaration[]>;
81+
/* @internal */ getNamedDeclarations(): Map<ReadonlyArray<Declaration>>;
8282

8383
getLineAndCharacterOfPosition(pos: number): LineAndCharacter;
8484
getLineEndOfPosition(pos: number): number;
@@ -599,7 +599,7 @@ namespace ts {
599599
name: string;
600600
kind: ScriptElementKind;
601601
kindModifiers: string;
602-
matchKind: string; // TODO: keyof typeof PatternMatchKind; (https://github.com/Microsoft/TypeScript/issues/15102)
602+
matchKind: "exact" | "prefix" | "substring" | "camelCase";
603603
isCaseSensitive: boolean;
604604
fileName: string;
605605
textSpan: TextSpan;

0 commit comments

Comments
 (0)