Skip to content

Commit 1264951

Browse files
author
Andy
authored
navigation tree / bar: Set span of anonymous function to span of VariableDeclaration containing it (#18575)
* navigation tree / bar: Set span of anonymous function to span of VariableDeclaration containing it * Add back `isFunctionOrClassExpression`
1 parent 0ae42ea commit 1264951

6 files changed

+112
-60
lines changed

‎src/harness/fourslash.ts

Lines changed: 25 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -2571,39 +2571,36 @@ namespace FourSlash {
25712571
}
25722572
}
25732573

2574-
public verifyNavigationBar(json: any) {
2575-
const items = this.languageService.getNavigationBarItems(this.activeFile.fileName);
2576-
if (JSON.stringify(items, replacer) !== JSON.stringify(json)) {
2577-
this.raiseError(`verifyNavigationBar failed - expected: ${stringify(json)}, got: ${stringify(items, replacer)}`);
2574+
public verifyNavigationBar(json: any, options: { checkSpans?: boolean } | undefined) {
2575+
this.verifyNavigationTreeOrBar(json, this.languageService.getNavigationBarItems(this.activeFile.fileName), "Bar", options);
2576+
}
2577+
2578+
public verifyNavigationTree(json: any, options: { checkSpans?: boolean } | undefined) {
2579+
this.verifyNavigationTreeOrBar(json, this.languageService.getNavigationTree(this.activeFile.fileName), "Tree", options);
2580+
}
2581+
2582+
private verifyNavigationTreeOrBar(json: any, tree: any, name: "Tree" | "Bar", options: { checkSpans?: boolean } | undefined) {
2583+
if (JSON.stringify(tree, replacer) !== JSON.stringify(json)) {
2584+
this.raiseError(`verifyNavigation${name} failed - expected: ${stringify(json)}, got: ${stringify(tree, replacer)}`);
25782585
}
25792586

2580-
// Make the data easier to read.
25812587
function replacer(key: string, value: any) {
25822588
switch (key) {
25832589
case "spans":
2584-
// We won't ever check this.
2585-
return undefined;
2590+
return options && options.checkSpans ? value : undefined;
2591+
case "start":
2592+
case "length":
2593+
// Never omit the values in a span, even if they are 0.
2594+
return value;
25862595
case "childItems":
2587-
return value.length === 0 ? undefined : value;
2596+
return !value || value.length === 0 ? undefined : value;
25882597
default:
25892598
// Omit falsy values, those are presumed to be the default.
25902599
return value || undefined;
25912600
}
25922601
}
25932602
}
25942603

2595-
public verifyNavigationTree(json: any) {
2596-
const tree = this.languageService.getNavigationTree(this.activeFile.fileName);
2597-
if (JSON.stringify(tree, replacer) !== JSON.stringify(json)) {
2598-
this.raiseError(`verifyNavigationTree failed - expected: ${stringify(json)}, got: ${stringify(tree, replacer)}`);
2599-
}
2600-
2601-
function replacer(key: string, value: any) {
2602-
// Don't check "spans", and omit falsy values.
2603-
return key === "spans" ? undefined : (value || undefined);
2604-
}
2605-
}
2606-
26072604
public printNavigationItems(searchValue: string) {
26082605
const items = this.languageService.getNavigateToItems(searchValue);
26092606
Harness.IO.log(`NavigationItems list (${items.length} items)`);
@@ -3533,6 +3530,10 @@ namespace FourSlashInterface {
35333530
return this.state.getRanges();
35343531
}
35353532

3533+
public spans(): ts.TextSpan[] {
3534+
return this.ranges().map(r => ts.createTextSpan(r.start, r.end - r.start));
3535+
}
3536+
35363537
public rangesByText(): ts.Map<FourSlash.Range[]> {
35373538
return this.state.rangesByText();
35383539
}
@@ -3966,12 +3967,12 @@ namespace FourSlashInterface {
39663967
this.state.verifyImportFixAtPosition(expectedTextArray, errorCode);
39673968
}
39683969

3969-
public navigationBar(json: any) {
3970-
this.state.verifyNavigationBar(json);
3970+
public navigationBar(json: any, options?: { checkSpans?: boolean }) {
3971+
this.state.verifyNavigationBar(json, options);
39713972
}
39723973

3973-
public navigationTree(json: any) {
3974-
this.state.verifyNavigationTree(json);
3974+
public navigationTree(json: any, options?: { checkSpans?: boolean }) {
3975+
this.state.verifyNavigationTree(json, options);
39753976
}
39763977

39773978
public navigationItemsListCount(count: number, searchValue: string, matchKind?: string, fileName?: string) {

‎src/services/navigationBar.ts

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -209,17 +209,24 @@ namespace ts.NavigationBar {
209209

210210
case SyntaxKind.BindingElement:
211211
case SyntaxKind.VariableDeclaration:
212-
const decl = <VariableDeclaration>node;
213-
const name = decl.name;
212+
const { name, initializer } = <VariableDeclaration | BindingElement>node;
214213
if (isBindingPattern(name)) {
215214
addChildrenRecursively(name);
216215
}
217-
else if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) {
218-
// For `const x = function() {}`, just use the function node, not the const.
219-
addChildrenRecursively(decl.initializer);
216+
else if (initializer && isFunctionOrClassExpression(initializer)) {
217+
if (initializer.name) {
218+
// Don't add a node for the VariableDeclaration, just for the initializer.
219+
addChildrenRecursively(initializer);
220+
}
221+
else {
222+
// Add a node for the VariableDeclaration, but not for the initializer.
223+
startNode(node);
224+
forEachChild(initializer, addChildrenRecursively);
225+
endNode();
226+
}
220227
}
221228
else {
222-
addNodeWithRecursiveChild(decl, decl.initializer);
229+
addNodeWithRecursiveChild(node, initializer);
223230
}
224231
break;
225232

@@ -644,7 +651,14 @@ namespace ts.NavigationBar {
644651
}
645652
}
646653

647-
function isFunctionOrClassExpression(node: Node): boolean {
648-
return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.ClassExpression;
654+
function isFunctionOrClassExpression(node: Node): node is ArrowFunction | FunctionExpression | ClassExpression {
655+
switch (node.kind) {
656+
case SyntaxKind.ArrowFunction:
657+
case SyntaxKind.FunctionExpression:
658+
case SyntaxKind.ClassExpression:
659+
return true;
660+
default:
661+
return false;
662+
}
649663
}
650664
}

‎tests/cases/fourslash/fourslash.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ declare namespace FourSlashInterface {
114114
markerNames(): string[];
115115
marker(name?: string): Marker;
116116
ranges(): Range[];
117+
spans(): Array<{ start: number, length: number }>;
117118
rangesByText(): ts.Map<Range[]>;
118119
markerByName(s: string): Marker;
119120
symbolsInScope(range: Range): any[];
@@ -250,8 +251,8 @@ declare namespace FourSlashInterface {
250251
fileAfterApplyingRefactorAtMarker(markerName: string, expectedContent: string, refactorNameToApply: string, formattingOptions?: FormatCodeOptions): void;
251252
importFixAtPosition(expectedTextArray: string[], errorCode?: number): void;
252253

253-
navigationBar(json: any): void;
254-
navigationTree(json: any): void;
254+
navigationBar(json: any, options?: { checkSpans?: boolean }): void;
255+
navigationTree(json: any, options?: { checkSpans?: boolean }): void;
255256
navigationItemsListCount(count: number, searchValue: string, matchKind?: string, fileName?: string): void;
256257
navigationItemsListContains(name: string, kind: string, searchValue: string, matchKind: string, fileName?: string, parentName?: string): void;
257258
occurrencesAtPositionContains(range: Range, isWriteAccess?: boolean): void;

‎tests/cases/fourslash/navigationBarAnonymousClassAndFunctionExpressions.ts

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ verify.navigationTree({
4646
},
4747
{
4848
"text": "x",
49-
"kind": "function",
49+
"kind": "const",
5050
"childItems": [
5151
{
5252
"text": "xx",
@@ -90,7 +90,7 @@ verify.navigationTree({
9090
},
9191
{
9292
"text": "cls2",
93-
"kind": "class"
93+
"kind": "const"
9494
},
9595
{
9696
"text": "cls3",
@@ -138,7 +138,7 @@ verify.navigationBar([
138138
},
139139
{
140140
"text": "x",
141-
"kind": "function"
141+
"kind": "const"
142142
},
143143
{
144144
"text": "y",
@@ -160,7 +160,7 @@ verify.navigationBar([
160160
},
161161
{
162162
"text": "x",
163-
"kind": "function",
163+
"kind": "const",
164164
"childItems": [
165165
{
166166
"text": "xx",
@@ -205,7 +205,7 @@ verify.navigationBar([
205205
},
206206
{
207207
"text": "cls2",
208-
"kind": "class"
208+
"kind": "const"
209209
},
210210
{
211211
"text": "cls3",
@@ -219,11 +219,6 @@ verify.navigationBar([
219219
"kind": "class",
220220
"indent": 2
221221
},
222-
{
223-
"text": "cls2",
224-
"kind": "class",
225-
"indent": 2
226-
},
227222
{
228223
"text": "cls3",
229224
"kind": "class",

‎tests/cases/fourslash/navigationBarInitializerSpans.ts

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
////const [|x = () => 0|];
4+
////const f = [|function f() {}|];
5+
6+
const [s0, s1] = test.spans();
7+
const sGlobal = { start: 0, length: 45 };
8+
9+
verify.navigationTree({
10+
text: "<global>",
11+
kind: "script",
12+
spans: [sGlobal],
13+
childItems: [
14+
{
15+
text: "f",
16+
kind: "function",
17+
spans: [s1],
18+
},
19+
{
20+
text: "x",
21+
kind: "const",
22+
spans: [s0],
23+
},
24+
]
25+
}, { checkSpans: true });
26+
27+
verify.navigationBar([
28+
{
29+
text: "<global>",
30+
kind: "script",
31+
spans: [sGlobal],
32+
childItems: [
33+
{
34+
text: "f",
35+
kind: "function",
36+
spans: [s1],
37+
},
38+
{
39+
text: "x",
40+
kind: "const",
41+
spans: [s0],
42+
},
43+
],
44+
},
45+
{
46+
text: "f",
47+
kind: "function",
48+
spans: [s1],
49+
indent: 1,
50+
},
51+
], { checkSpans: true });

‎tests/cases/fourslash/navigationBarItemsNamedArrowFunctions.ts

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@ verify.navigationBar([
1717
},
1818
{
1919
"text": "func",
20-
"kind": "function"
20+
"kind": "const",
21+
"kindModifiers": "export",
2122
},
2223
{
2324
"text": "func2",
24-
"kind": "function"
25+
"kind": "const",
26+
"kindModifiers": "export",
2527
},
2628
{
2729
"text": "value",
@@ -35,18 +37,6 @@ verify.navigationBar([
3537
"kind": "function",
3638
"kindModifiers": "export",
3739
"indent": 1
38-
},
39-
{
40-
"text": "func",
41-
"kind": "function",
42-
"kindModifiers": "export",
43-
"indent": 1
44-
},
45-
{
46-
"text": "func2",
47-
"kind": "function",
48-
"kindModifiers": "export",
49-
"indent": 1
5040
}
5141
]);
5242

@@ -61,12 +51,12 @@ verify.navigationTree({
6151
},
6252
{
6353
"text": "func",
64-
"kind": "function",
54+
"kind": "const",
6555
"kindModifiers": "export"
6656
},
6757
{
6858
"text": "func2",
69-
"kind": "function",
59+
"kind": "const",
7060
"kindModifiers": "export"
7161
},
7262
{

0 commit comments

Comments
 (0)