From d80677bcea65245e08dcad48898e8b0e1cb7829f Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 17 May 2016 10:40:59 -0700 Subject: [PATCH 1/9] Add class expressions to navigation bar --- src/services/navigationBar.ts | 7 +++++++ .../fourslash/navigationBarDeclarationExpressions.ts | 8 ++++++++ 2 files changed, 15 insertions(+) create mode 100644 tests/cases/fourslash/navigationBarDeclarationExpressions.ts diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 2cd93d3856c2f..e6c96446170c7 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -159,6 +159,7 @@ namespace ts.NavigationBar { for (const node of nodes) { switch (node.kind) { + case SyntaxKind.ClassExpression: case SyntaxKind.ClassDeclaration: topLevelNodes.push(node); for (const member of (node).members) { @@ -193,6 +194,11 @@ namespace ts.NavigationBar { addTopLevelNodes((functionDeclaration.body).statements, topLevelNodes); } break; + + default: + const childrens: Node[] = []; + forEachChild(node, child => { childrens.push(child) }); + addTopLevelNodes(childrens, topLevelNodes); } } } @@ -405,6 +411,7 @@ namespace ts.NavigationBar { case SyntaxKind.SourceFile: return createSourceFileItem(node); + case SyntaxKind.ClassExpression: case SyntaxKind.ClassDeclaration: return createClassItem(node); diff --git a/tests/cases/fourslash/navigationBarDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarDeclarationExpressions.ts new file mode 100644 index 0000000000000..f16b42e14bfef --- /dev/null +++ b/tests/cases/fourslash/navigationBarDeclarationExpressions.ts @@ -0,0 +1,8 @@ +/// + +////console.log(class A { b() {} }); + +debug.printNavigationBar(); +verify.navigationBarCount(2); +verify.navigationBarContains("A", "class"); +verify.navigationBarChildItem("A", "b", "method"); From 86cce79e07da64e17e7ff453468f30c1137911ba Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 17 May 2016 11:32:57 -0700 Subject: [PATCH 2/9] Fix sort order for toplevel declarations in nested expressions --- src/harness/fourslash.ts | 15 +++ src/services/navigationBar.ts | 97 +++++++++++-------- tests/cases/fourslash/fourslash.ts | 1 + .../navigationBarDeclarationExpressions.ts | 16 ++- 4 files changed, 84 insertions(+), 45 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index c237734e1df13..c277fb829ee86 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1989,6 +1989,17 @@ namespace FourSlash { return result; } + public verifyNavigationBarIndex(name: string, index: number) { + const items = this.languageService.getNavigationBarItems(this.activeFile.fileName); + if (!items[index]) { + this.raiseError(`verifyNavigationBarIndex failed - No item at index ${index}`); + } + const actual = items[index].text; + if (actual !== name) { + this.raiseError(`verifyNavigationBarIndex failed - Item at index ${index} is named ${actual} instead of ${name}.`); + } + } + public verifyNavigationBarContains(name: string, kind: string) { const items = this.languageService.getNavigationBarItems(this.activeFile.fileName); @@ -3047,6 +3058,10 @@ namespace FourSlashInterface { this.state.verifyNavigationBarCount(count); } + public navigationBarIndex(name: string, index: number) { + this.state.verifyNavigationBarIndex(name, index); + } + // TODO: figure out what to do with the unused arguments. public navigationBarContains( name: string, diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index e6c96446170c7..33c369845f31d 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -138,7 +138,13 @@ namespace ts.NavigationBar { } function sortNodes(nodes: Node[]): Node[] { - return nodes.slice(0).sort((n1: Declaration, n2: Declaration) => { + const sortedCopy = nodes.slice(0); + doSortNodes(sortedCopy); + return sortedCopy; + } + + function doSortNodes(nodes: Node[]): void { + nodes.sort((n1: Declaration, n2: Declaration) => { if (n1.name && n2.name) { return getPropertyNameForPropertyNameNode(n1.name).localeCompare(getPropertyNameForPropertyNameNode(n2.name)); } @@ -154,52 +160,63 @@ namespace ts.NavigationBar { }); } - function addTopLevelNodes(nodes: Node[], topLevelNodes: Node[]): void { - nodes = sortNodes(nodes); - + // Add nodes in a single "level" of top-level nodes (as in, methods in a class.) + // Nodes in a single "level" are sorted together. + function addTopLevelNodes(nodes: Node[], higherLevel: Node[]): void { + const thisLevel: Node[] = []; for (const node of nodes) { - switch (node.kind) { - case SyntaxKind.ClassExpression: - case SyntaxKind.ClassDeclaration: - topLevelNodes.push(node); - for (const member of (node).members) { - if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.Constructor) { - type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; - if ((member).body) { - // We do not include methods that does not have child functions in it, because of duplications. - if (hasNamedFunctionDeclarations(((member).body).statements)) { - topLevelNodes.push(member); - } - addTopLevelNodes(((member).body).statements, topLevelNodes); + addTopLevelNode(node, thisLevel); + } + doSortNodes(thisLevel); + + for (const node of thisLevel) { + higherLevel.push(node); + } + } + + function addTopLevelNode(node: Node, thisLevel: Node[]): void { + switch (node.kind) { + case SyntaxKind.ClassExpression: + case SyntaxKind.ClassDeclaration: + thisLevel.push(node); + for (const member of (node).members) { + if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.Constructor) { + type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; + if ((member).body) { + // We do not include methods that does not have child functions in it, because of duplications. + if (hasNamedFunctionDeclarations(((member).body).statements)) { + thisLevel.push(member); } + addTopLevelNodes(((member).body).statements, thisLevel); } } - break; - case SyntaxKind.EnumDeclaration: - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.TypeAliasDeclaration: - topLevelNodes.push(node); - break; + } + break; - case SyntaxKind.ModuleDeclaration: - let moduleDeclaration = node; - topLevelNodes.push(node); - addTopLevelNodes((getInnermostModule(moduleDeclaration).body).statements, topLevelNodes); - break; + case SyntaxKind.EnumDeclaration: + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.TypeAliasDeclaration: + thisLevel.push(node); + break; - case SyntaxKind.FunctionDeclaration: - let functionDeclaration = node; - if (isTopLevelFunctionDeclaration(functionDeclaration)) { - topLevelNodes.push(node); - addTopLevelNodes((functionDeclaration.body).statements, topLevelNodes); - } - break; + case SyntaxKind.ModuleDeclaration: + let moduleDeclaration = node; + thisLevel.push(node); + addTopLevelNodes((getInnermostModule(moduleDeclaration).body).statements, thisLevel); + break; - default: - const childrens: Node[] = []; - forEachChild(node, child => { childrens.push(child) }); - addTopLevelNodes(childrens, topLevelNodes); - } + case SyntaxKind.FunctionDeclaration: + let functionDeclaration = node; + if (isTopLevelFunctionDeclaration(functionDeclaration)) { + thisLevel.push(node); + addTopLevelNodes((functionDeclaration.body).statements, thisLevel); + } + break; + + default: + // Nodes within nested expressions are still sorted as if they were top-level, + // so in this case we recurse with `addTopLevelNode` rather than calling `addTopLevelNodes`. + forEachChild(node, child => addTopLevelNode(child, thisLevel)); } } diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index 08883f163f2c9..f012b010726a2 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -176,6 +176,7 @@ declare namespace FourSlashInterface { noDocCommentTemplate(): void; navigationBarCount(count: number): void; + navigationBarIndex(name: string, index: number): void; navigationBarContains(name: string, kind: string, fileName?: string, parentName?: string, isAdditionalSpan?: boolean, markerPosition?: number): void; navigationBarChildItem(parent: string, text: string, kind: string): void; navigationItemsListCount(count: number, searchValue: string, matchKind?: string): void; diff --git a/tests/cases/fourslash/navigationBarDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarDeclarationExpressions.ts index f16b42e14bfef..2032fcc940df5 100644 --- a/tests/cases/fourslash/navigationBarDeclarationExpressions.ts +++ b/tests/cases/fourslash/navigationBarDeclarationExpressions.ts @@ -1,8 +1,14 @@ /// -////console.log(class A { b() {} }); +////console.log(console.log(class Y {}, class X {}), console.log(class B {}, class A {})); +////console.log(class Cls { meth() {} }); -debug.printNavigationBar(); -verify.navigationBarCount(2); -verify.navigationBarContains("A", "class"); -verify.navigationBarChildItem("A", "b", "method"); +verify.navigationBarCount(6); +verify.navigationBarIndex("A", 0); +verify.navigationBarIndex("B", 1); +verify.navigationBarIndex("Cls", 2); +verify.navigationBarIndex("X", 3); +verify.navigationBarIndex("Y", 4); + +verify.navigationBarContains("Cls", "class"); +verify.navigationBarChildItem("Cls", "meth", "method"); From dda27f7587b3cc6da86022d217847e207c07637f Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 18 May 2016 06:44:18 -0700 Subject: [PATCH 3/9] Respond to PR comments --- src/services/navigationBar.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 33c369845f31d..fa5d6e25b8134 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -139,19 +139,21 @@ namespace ts.NavigationBar { function sortNodes(nodes: Node[]): Node[] { const sortedCopy = nodes.slice(0); - doSortNodes(sortedCopy); + sortNodesInPlace(sortedCopy); return sortedCopy; } - function doSortNodes(nodes: Node[]): void { - nodes.sort((n1: Declaration, n2: Declaration) => { - if (n1.name && n2.name) { - return getPropertyNameForPropertyNameNode(n1.name).localeCompare(getPropertyNameForPropertyNameNode(n2.name)); + function sortNodesInPlace(nodes: Node[]): void { + nodes.sort((n1, n2) => { + // Get the name if it exists. OK if node is not a declaration. + const name1 = ( n1).name, name2 = ( n2).name; + if (name1 && name2) { + return getPropertyNameForPropertyNameNode(name1).localeCompare(getPropertyNameForPropertyNameNode(name2)); } - else if (n1.name) { + else if (name1) { return 1; } - else if (n2.name) { + else if (name2) { return -1; } else { @@ -160,14 +162,14 @@ namespace ts.NavigationBar { }); } - // Add nodes in a single "level" of top-level nodes (as in, methods in a class.) + // Add nodes in a single "level" of top-level nodes (e.g. methods in a class.) // Nodes in a single "level" are sorted together. function addTopLevelNodes(nodes: Node[], higherLevel: Node[]): void { const thisLevel: Node[] = []; for (const node of nodes) { addTopLevelNode(node, thisLevel); } - doSortNodes(thisLevel); + sortNodesInPlace(thisLevel); for (const node of thisLevel) { higherLevel.push(node); @@ -183,7 +185,7 @@ namespace ts.NavigationBar { if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.Constructor) { type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; if ((member).body) { - // We do not include methods that does not have child functions in it, because of duplications. + // We do not include methods that do not have child functions in them, because of duplications. if (hasNamedFunctionDeclarations(((member).body).statements)) { thisLevel.push(member); } From c9ec628db7f6f9849859f4ec5f8fb4156ecc1206 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Thu, 19 May 2016 13:44:52 -0700 Subject: [PATCH 4/9] Fix algorithm and include function expressions. Previous algorithm would sort *after* adding to top-level nodes. This was broken because top-level nodes were simply all in a flat array, so this would cause sorting among unrelated elements. Now we collect all the nodes in a single logical level and sort them before adding them to topLevelNodes. --- src/harness/fourslash.ts | 17 +- src/services/navigationBar.ts | 280 ++++++++++-------- tests/cases/fourslash/fourslash.ts | 2 +- ...ationBarAnonymousDeclarationExpressions.ts | 42 +++ .../fourslash/navigationBarItemsFunctions.ts | 2 +- .../navigationBarItemsFunctionsBroken.ts | 2 +- .../navigationBarItemsFunctionsBroken2.ts | 2 +- ...rItemsItemsContainsNoAnonymousFunctions.ts | 44 --- .../navigationBarItemsMissingName1.ts | 16 - 9 files changed, 213 insertions(+), 194 deletions(-) create mode 100644 tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts delete mode 100644 tests/cases/fourslash/navigationBarItemsItemsContainsNoAnonymousFunctions.ts delete mode 100644 tests/cases/fourslash/navigationBarItemsMissingName1.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index c277fb829ee86..4fdbf10396574 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -2058,7 +2058,7 @@ namespace FourSlash { } } - public printNavigationBar() { + public printNavigationBar(showChildItems = false) { const items = this.languageService.getNavigationBarItems(this.activeFile.fileName); const length = items && items.length; @@ -2066,7 +2066,16 @@ namespace FourSlash { for (let i = 0; i < length; i++) { const item = items[i]; - Harness.IO.log(`name: ${item.text}, kind: ${item.kind}`); + const childrenText = showChildItems ? `, children: ${item.childItems.map(child => child.text)}` : ""; + Harness.IO.log(`${strRepeat(" ", item.indent)}name: ${item.text}, kind: ${item.kind}${childrenText}`); + } + + function strRepeat(str: string, times: number) { + let out = ""; + for (let i = 0; i < times; i++) { + out += str; + } + return out; } } @@ -3267,8 +3276,8 @@ namespace FourSlashInterface { this.state.printNavigationItems(searchValue); } - public printNavigationBar() { - this.state.printNavigationBar(); + public printNavigationBar(showChildItems = false) { + this.state.printNavigationBar(showChildItems); } public printReferences() { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index fa5d6e25b8134..e038e605098d0 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -32,9 +32,12 @@ namespace ts.NavigationBar { while (current.kind === SyntaxKind.ModuleDeclaration); // fall through + case SyntaxKind.ClassExpression: case SyntaxKind.ClassDeclaration: case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: indent++; } @@ -96,16 +99,22 @@ namespace ts.NavigationBar { break; } // Fall through + case SyntaxKind.ClassExpression: case SyntaxKind.ClassDeclaration: case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: case SyntaxKind.ModuleDeclaration: + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: case SyntaxKind.ImportEqualsDeclaration: case SyntaxKind.ImportSpecifier: case SyntaxKind.ExportSpecifier: childNodes.push(node); break; + + default: + forEachChild(node, visit); } } @@ -164,32 +173,62 @@ namespace ts.NavigationBar { // Add nodes in a single "level" of top-level nodes (e.g. methods in a class.) // Nodes in a single "level" are sorted together. - function addTopLevelNodes(nodes: Node[], higherLevel: Node[]): void { - const thisLevel: Node[] = []; - for (const node of nodes) { - addTopLevelNode(node, thisLevel); + // Returns whether any nodes were added. + function addTopLevelNodes(nodes: Node | Node[], topLevelNodes: Node[]) { + const decls = getNextLevelOfDeclarations(nodes); + // Sort each level of declarations together + sortNodesInPlace(decls); + for (const decl of decls) { + addTopLevelNode(decl, topLevelNodes); } - sortNodesInPlace(thisLevel); + } - for (const node of thisLevel) { - higherLevel.push(node); + // Gets all declarations contained within `nodes` and their sub-expressions, + // but not declarations within declarations. + function getNextLevelOfDeclarations(nodes: Node | Node[]): Node[] { + const result: Node[] = []; + function recur(node: Node): void { + switch (node.kind) { + case SyntaxKind.ClassExpression: + case SyntaxKind.ClassDeclaration: + case SyntaxKind.EnumDeclaration: + case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.TypeAliasDeclaration: + case SyntaxKind.ModuleDeclaration: + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + result.push(node); + // Its children are handled by addTopLevelNode later. + break; + default: + forEachChild(node, recur); + } } + if (nodes instanceof Array) { + for (const node of nodes) { + recur(node); + } + } + else { + recur(nodes); + } + return result; } - function addTopLevelNode(node: Node, thisLevel: Node[]): void { + function addTopLevelNode(node: Node, topLevelNodes: Node[]) { switch (node.kind) { case SyntaxKind.ClassExpression: case SyntaxKind.ClassDeclaration: - thisLevel.push(node); + topLevelNodes.push(node); for (const member of (node).members) { if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.Constructor) { type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; - if ((member).body) { - // We do not include methods that do not have child functions in them, because of duplications. - if (hasNamedFunctionDeclarations(((member).body).statements)) { - thisLevel.push(member); - } - addTopLevelNodes(((member).body).statements, thisLevel); + const decl = member; + if (decl.body) { + // Add node, but it will not become an item unless it has children later. + topLevelNodes.push(member); + addTopLevelNodes(decl.body.statements, topLevelNodes); } } } @@ -198,68 +237,29 @@ namespace ts.NavigationBar { case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: case SyntaxKind.TypeAliasDeclaration: - thisLevel.push(node); + topLevelNodes.push(node); break; case SyntaxKind.ModuleDeclaration: let moduleDeclaration = node; - thisLevel.push(node); - addTopLevelNodes((getInnermostModule(moduleDeclaration).body).statements, thisLevel); + topLevelNodes.push(node); + addTopLevelNodes((getInnermostModule(moduleDeclaration).body).statements, topLevelNodes); break; + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: - let functionDeclaration = node; - if (isTopLevelFunctionDeclaration(functionDeclaration)) { - thisLevel.push(node); - addTopLevelNodes((functionDeclaration.body).statements, thisLevel); + const decl = node; + if (decl.body) { + topLevelNodes.push(node); + addTopLevelNodes(decl.body, topLevelNodes); } break; default: - // Nodes within nested expressions are still sorted as if they were top-level, - // so in this case we recurse with `addTopLevelNode` rather than calling `addTopLevelNodes`. - forEachChild(node, child => addTopLevelNode(child, thisLevel)); - } - } - - function hasNamedFunctionDeclarations(nodes: NodeArray): boolean { - for (const s of nodes) { - if (s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text)) { - return true; - } + // There should be a case in addTopLevelNode for each case in getNextLevelOfDeclarations. + throw new Error("Unreachable"); } - return false; - } - - function isTopLevelFunctionDeclaration(functionDeclaration: FunctionLikeDeclaration): boolean { - if (functionDeclaration.kind === SyntaxKind.FunctionDeclaration) { - // A function declaration is 'top level' if it contains any function declarations - // within it. - if (functionDeclaration.body && functionDeclaration.body.kind === SyntaxKind.Block) { - // Proper function declarations can only have identifier names - if (hasNamedFunctionDeclarations((functionDeclaration.body).statements)) { - return true; - } - - // Or if it is not parented by another function. I.e all functions at module scope are 'top level'. - if (!isFunctionBlock(functionDeclaration.parent)) { - return true; - } - - // Or if it is nested inside class methods and constructors. - else { - // We have made sure that a grand parent node exists with 'isFunctionBlock()' above. - const grandParentKind = functionDeclaration.parent.parent.kind; - if (grandParentKind === SyntaxKind.MethodDeclaration || - grandParentKind === SyntaxKind.Constructor) { - - return true; - } - } - } - } - - return false; } function getItemsWorker(nodes: Node[], createItem: (n: Node) => ts.NavigationBarItem): ts.NavigationBarItem[] { @@ -271,6 +271,12 @@ namespace ts.NavigationBar { const item = createItem(child); if (item !== undefined) { if (item.text.length > 0) { + if (isDeclarationExpression(child)) { + // Never merge + items.push(item); + continue; + } + const key = item.text + "-" + item.kind + "-" + item.indent; const itemWithSameName = keyToItem[key]; @@ -352,8 +358,10 @@ namespace ts.NavigationBar { case SyntaxKind.PropertySignature: return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.functionElement); + return createItem(node, declarationExpressionName(node), ts.ScriptElementKind.functionElement); case SyntaxKind.VariableDeclaration: case SyntaxKind.BindingElement: @@ -447,6 +455,8 @@ namespace ts.NavigationBar { case SyntaxKind.ModuleDeclaration: return createModuleItem(node); + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: return createFunctionItem(node); @@ -490,18 +500,25 @@ namespace ts.NavigationBar { } function createFunctionItem(node: FunctionDeclaration): ts.NavigationBarItem { - if (node.body && node.body.kind === SyntaxKind.Block) { - const childItems = getItemsWorker(sortNodes((node.body).statements), createChildItem); - - return getNavigationBarItem(!node.name ? "default" : node.name.text, - ts.ScriptElementKind.functionElement, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); + const children = node.body ? getChildNodes([node.body]) : []; + const isTopLevel = node.parent.kind === SyntaxKind.ModuleBlock || node.parent.kind === SyntaxKind.SourceFile || + // Functions with declaration children (not including local variables) are worthy + children.some(child => child.kind !== SyntaxKind.VariableDeclaration && child.kind !== SyntaxKind.BindingElement) || + // Functions inside class methods are worthy + node.parent.parent.kind === SyntaxKind.MethodDeclaration || node.parent.parent.kind === SyntaxKind.Constructor; + if (!isTopLevel) { + return undefined; } - return undefined; + const childItems = getItemsWorker(children, createChildItem); + + return getNavigationBarItem( + declarationExpressionName(node), + ts.ScriptElementKind.functionElement, + getNodeModifiers(node), + [getNodeSpan(node)], + childItems, + getIndent(node)); } function createTypeAliasItem(node: TypeAliasDeclaration): ts.NavigationBarItem { @@ -514,28 +531,29 @@ namespace ts.NavigationBar { } function createMemberFunctionLikeItem(node: MethodDeclaration | ConstructorDeclaration): ts.NavigationBarItem { - if (node.body && node.body.kind === SyntaxKind.Block) { - const childItems = getItemsWorker(sortNodes((node.body).statements), createChildItem); - let scriptElementKind: string; - let memberFunctionName: string; - if (node.kind === SyntaxKind.MethodDeclaration) { - memberFunctionName = getPropertyNameForPropertyNameNode(node.name); - scriptElementKind = ts.ScriptElementKind.memberFunctionElement; - } - else { - memberFunctionName = "constructor"; - scriptElementKind = ts.ScriptElementKind.constructorImplementationElement; - } + const childItems = getItemsWorker(sortNodes(node.body.statements), createChildItem); + // Don't include member functions as top-level if they don't contain other items. + if (!childItems.length) { + return undefined; + } - return getNavigationBarItem(memberFunctionName, - scriptElementKind, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); + let scriptElementKind: string; + let memberFunctionName: string; + if (node.kind === SyntaxKind.MethodDeclaration) { + memberFunctionName = getPropertyNameForPropertyNameNode(node.name); + scriptElementKind = ts.ScriptElementKind.memberFunctionElement; + } + else { + memberFunctionName = "constructor"; + scriptElementKind = ts.ScriptElementKind.constructorImplementationElement; } - return undefined; + return getNavigationBarItem(memberFunctionName, + scriptElementKind, + getNodeModifiers(node), + [getNodeSpan(node)], + childItems, + getIndent(node)); } function createSourceFileItem(node: SourceFile): ts.NavigationBarItem { @@ -576,10 +594,8 @@ namespace ts.NavigationBar { childItems = getItemsWorker(sortNodes(nodes), createChildItem); } - const nodeName = !node.name ? "default" : node.name.text; - return getNavigationBarItem( - nodeName, + declarationExpressionName(node), ts.ScriptElementKind.classElement, getNodeModifiers(node), [getNodeSpan(node)], @@ -641,8 +657,6 @@ namespace ts.NavigationBar { } export function getJsNavigationBarItems(sourceFile: SourceFile, compilerOptions: CompilerOptions): NavigationBarItem[] { - const anonFnText = ""; - const anonClassText = ""; let indent = 0; const rootName = isExternalModule(sourceFile) ? @@ -672,8 +686,7 @@ namespace ts.NavigationBar { topItem = lastTop; indent--; - // If the last item added was an anonymous function expression, and it had no children, discard it. - if (newItem && newItem.text === anonFnText && newItem.childItems.length === 0) { + if (newItem && isAnonFn(newItem) && newItem.childItems.length === 0) { topItem.childItems.pop(); } } @@ -783,30 +796,7 @@ namespace ts.NavigationBar { } const fnExpr = node as FunctionExpression | ArrowFunction | ClassExpression; - let fnName: string; - if (fnExpr.name && getFullWidth(fnExpr.name) > 0) { - // The expression has an identifier, so use that as the name - fnName = declarationNameToString(fnExpr.name); - } - else { - // See if it is a var initializer. If so, use the var name. - if (fnExpr.parent.kind === SyntaxKind.VariableDeclaration) { - fnName = declarationNameToString((fnExpr.parent as VariableDeclaration).name); - } - // See if it is of the form " = function(){...}". If so, use the text from the left-hand side. - else if (fnExpr.parent.kind === SyntaxKind.BinaryExpression && - (fnExpr.parent as BinaryExpression).operatorToken.kind === SyntaxKind.EqualsToken) { - fnName = (fnExpr.parent as BinaryExpression).left.getText(); - } - // See if it is a property assignment, and if so use the property name - else if (fnExpr.parent.kind === SyntaxKind.PropertyAssignment && - (fnExpr.parent as PropertyAssignment).name) { - fnName = (fnExpr.parent as PropertyAssignment).name.getText(); - } - else { - fnName = node.kind === SyntaxKind.ClassExpression ? anonClassText : anonFnText; - } - } + const fnName = declarationExpressionName(fnExpr); const scriptKind = node.kind === SyntaxKind.ClassExpression ? ScriptElementKind.classElement : ScriptElementKind.functionElement; return getNavBarItem(fnName, scriptKind, [getNodeSpan(node)]); } @@ -838,4 +828,42 @@ namespace ts.NavigationBar { return sourceFileItem.childItems; } + + const anonFnText = ""; + const anonClassText = ""; + + // Get the name for a (possibly anonymous) class/function expression. + function declarationExpressionName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { + if (node.name && getFullWidth(node.name) > 0) { + return declarationNameToString(node.name); + } + // See if it is a var initializer. If so, use the var name. + else if (node.parent.kind === SyntaxKind.VariableDeclaration) { + return declarationNameToString((node.parent as VariableDeclaration).name); + } + // See if it is of the form " = function(){...}". If so, use the text from the left-hand side. + else if (node.parent.kind === SyntaxKind.BinaryExpression && + (node.parent as BinaryExpression).operatorToken.kind === SyntaxKind.EqualsToken) { + return (node.parent as BinaryExpression).left.getText(); + } + // See if it is a property assignment, and if so use the property name + else if (node.parent.kind === SyntaxKind.PropertyAssignment && (node.parent as PropertyAssignment).name) { + return (node.parent as PropertyAssignment).name.getText(); + } + // Default exports are named "default" + else if (node.flags & NodeFlags.Default) { + return "default"; + } + else { + return node.kind === SyntaxKind.ClassExpression ? anonClassText : anonFnText; + } + } + + function isAnonFn(item: NavigationBarItem): boolean { + return item.text === anonFnText; + } + + function isDeclarationExpression(node: Node) { + return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.ClassExpression; + } } diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index f012b010726a2..0e6c1ae205a78 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -238,7 +238,7 @@ declare namespace FourSlashInterface { printBreakpointAtCurrentLocation(): void; printNameOrDottedNameSpans(pos: number): void; printErrorList(): void; - printNavigationBar(): void; + printNavigationBar(showChildItems?: boolean): void; printNavigationItems(searchValue?: string): void; printScriptLexicalStructureItems(): void; printReferences(): void; diff --git a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts new file mode 100644 index 0000000000000..c7749606e77d5 --- /dev/null +++ b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts @@ -0,0 +1,42 @@ +/// + +////global.cls = class { }; +////(function() { +//// const x = () => { +//// // Presence of inner function causes x to be a top-level function. +//// function xx() {} +//// }; +//// const y = { +//// // This is not a top-level function (contains nothing, but shows up in childItems of its parent.) +//// foo: function() {} +//// }; +//// (function nest() { +//// function moreNest() {} +//// })(); +////})(); +////(function() { // Different anonymous functions are not merged +//// These will only show up as childItems. +//// function z() {} +//// console.log(function() {}) +////}) +////(function classes() { +//// // Classes show up in top-level regardless of whether they have names or inner declarations. +//// const cls2 = class { }; +//// console.log(class cls3 {}); +//// (class { }); +////}) + +verify.navigationBarCount(21); +verify.navigationBarIndex("", 0); +verify.navigationBarIndex("", 1); +verify.navigationBarIndex("x", 2); +verify.navigationBarIndex("nest", 3); +verify.navigationBarIndex("", 4); +verify.navigationBarIndex("global.cls", 5); +verify.navigationBarIndex("classes", 6); +verify.navigationBarIndex("cls2", 7); +verify.navigationBarIndex("", 8); +verify.navigationBarIndex("cls3", 9); + +verify.navigationBarContains("global.cls", "class"); + diff --git a/tests/cases/fourslash/navigationBarItemsFunctions.ts b/tests/cases/fourslash/navigationBarItemsFunctions.ts index 0948923c0461f..693d0d76561e9 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctions.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctions.ts @@ -20,4 +20,4 @@ test.markers().forEach((marker) => { verify.navigationBarContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.navigationBarCount(8); // 4 functions + global. Note: there are 8 because of the functions show up at the top level and as child items. +verify.navigationBarCount(11); // Includes many nested functions, with duplications as they appear both at top-level and as childItems diff --git a/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts b/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts index e8bf1d33fc3dc..9f7207a33d063 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts @@ -9,4 +9,4 @@ test.markers().forEach((marker) => { verify.navigationBarContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.navigationBarCount(3); // and 'f'. \ No newline at end of file +verify.navigationBarCount(4); // with child 'f' and 'f' with child ''. diff --git a/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts b/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts index 9576d4dd78964..904b74aa7dec5 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts @@ -10,4 +10,4 @@ test.markers().forEach((marker) => { verify.navigationBarContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); }); -verify.navigationBarCount(3); // and 'f' \ No newline at end of file +verify.navigationBarCount(5); // with children '' and 'f', and 'f' with child '' diff --git a/tests/cases/fourslash/navigationBarItemsItemsContainsNoAnonymousFunctions.ts b/tests/cases/fourslash/navigationBarItemsItemsContainsNoAnonymousFunctions.ts deleted file mode 100644 index dac6e96ab931d..0000000000000 --- a/tests/cases/fourslash/navigationBarItemsItemsContainsNoAnonymousFunctions.ts +++ /dev/null @@ -1,44 +0,0 @@ -/// -// @Filename: scriptLexicalStructureItemsContainsNoAnonymouseFunctions_0.ts -/////*file1*/ -////(function() { -//// // this should not be included -//// var x = 0; -//// -//// // this should not be included either -//// function foo() { -//// -//// } -////})(); -//// -// @Filename: scriptLexicalStructureItemsContainsNoAnonymouseFunctions_1.ts -/////*file2*/ -////var x = function() { -//// // this should not be included -//// var x = 0; -//// -//// // this should not be included either -//// function foo() { -////}; -//// -// @Filename: scriptLexicalStructureItemsContainsNoAnonymouseFunctions_2.ts -////// Named functions should still show up -/////*file3*/ -////function foo() { -////} -////function bar() { -////} - -goTo.marker("file1"); -verify.navigationBarCount(0); - -goTo.marker("file2"); -verify.navigationBarContains("", "module"); -verify.navigationBarContains("x", "var"); -verify.navigationBarCount(2); - -goTo.marker("file3"); -verify.navigationBarContains("", "module"); -verify.navigationBarContains("foo", "function"); -verify.navigationBarContains("bar", "function"); -verify.navigationBarCount(5); \ No newline at end of file diff --git a/tests/cases/fourslash/navigationBarItemsMissingName1.ts b/tests/cases/fourslash/navigationBarItemsMissingName1.ts deleted file mode 100644 index 98321039c5af9..0000000000000 --- a/tests/cases/fourslash/navigationBarItemsMissingName1.ts +++ /dev/null @@ -1,16 +0,0 @@ -////export function -/////** -//// * This is a class. -//// */ -////{| "itemName": "C", "kind": "class" |} class C { -//// {| "itemName": "foo", "kind": "method" |} foo() { -//// } -////} - - -test.markers().forEach((marker) => { - verify.navigationBarContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); -}); - -/// Only have two named elements. -verify.navigationBarCount(2); From 1d71bf7cec979cfc427548571c9e6d70231f8271 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 27 May 2016 06:21:17 -0700 Subject: [PATCH 5/9] Improve naming --- src/services/navigationBar.ts | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index e038e605098d0..37b2b5ed6ff82 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -271,7 +271,7 @@ namespace ts.NavigationBar { const item = createItem(child); if (item !== undefined) { if (item.text.length > 0) { - if (isDeclarationExpression(child)) { + if (isFunctionOrClassExpression(child)) { // Never merge items.push(item); continue; @@ -361,7 +361,7 @@ namespace ts.NavigationBar { case SyntaxKind.ArrowFunction: case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: - return createItem(node, declarationExpressionName(node), ts.ScriptElementKind.functionElement); + return createItem(node, getFunctionOrClassName(node), ts.ScriptElementKind.functionElement); case SyntaxKind.VariableDeclaration: case SyntaxKind.BindingElement: @@ -513,7 +513,7 @@ namespace ts.NavigationBar { const childItems = getItemsWorker(children, createChildItem); return getNavigationBarItem( - declarationExpressionName(node), + getFunctionOrClassName(node), ts.ScriptElementKind.functionElement, getNodeModifiers(node), [getNodeSpan(node)], @@ -595,7 +595,7 @@ namespace ts.NavigationBar { } return getNavigationBarItem( - declarationExpressionName(node), + getFunctionOrClassName(node), ts.ScriptElementKind.classElement, getNodeModifiers(node), [getNodeSpan(node)], @@ -686,7 +686,7 @@ namespace ts.NavigationBar { topItem = lastTop; indent--; - if (newItem && isAnonFn(newItem) && newItem.childItems.length === 0) { + if (newItem && isAnonymousFunction(newItem) && newItem.childItems.length === 0) { topItem.childItems.pop(); } } @@ -796,7 +796,7 @@ namespace ts.NavigationBar { } const fnExpr = node as FunctionExpression | ArrowFunction | ClassExpression; - const fnName = declarationExpressionName(fnExpr); + const fnName = getFunctionOrClassName(fnExpr); const scriptKind = node.kind === SyntaxKind.ClassExpression ? ScriptElementKind.classElement : ScriptElementKind.functionElement; return getNavBarItem(fnName, scriptKind, [getNodeSpan(node)]); } @@ -829,11 +829,11 @@ namespace ts.NavigationBar { return sourceFileItem.childItems; } - const anonFnText = ""; - const anonClassText = ""; + const anonymousFunctionText = ""; + const anonymousClassText = ""; - // Get the name for a (possibly anonymous) class/function expression. - function declarationExpressionName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { + /** Get the name for a (possibly anonymous) class/function expression. */ + function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { if (node.name && getFullWidth(node.name) > 0) { return declarationNameToString(node.name); } @@ -855,15 +855,15 @@ namespace ts.NavigationBar { return "default"; } else { - return node.kind === SyntaxKind.ClassExpression ? anonClassText : anonFnText; + return node.kind === SyntaxKind.ClassExpression ? anonymousClassText : anonymousFunctionText; } } - function isAnonFn(item: NavigationBarItem): boolean { - return item.text === anonFnText; + function isAnonymousFunction(item: NavigationBarItem): boolean { + return item.text === anonymousFunctionText; } - function isDeclarationExpression(node: Node) { + function isFunctionOrClassExpression(node: Node): boolean { return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.ClassExpression; } } From a7bedb1c99c42e655964ea6ee9f5335330e7f75f Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 27 May 2016 07:49:19 -0700 Subject: [PATCH 6/9] Update tests and fix bug where `const x = function() {}` was considered a const instead of a function --- src/services/navigationBar.ts | 33 ++++- tests/cases/fourslash/navbar_exportDefault.ts | 7 + ...ationBarAnonymousDeclarationExpressions.ts | 123 +++++++++++++++--- .../navigationBarDeclarationExpressions.ts | 62 +++++++++ .../fourslash/navigationBarItemsFunctions.ts | 14 ++ .../navigationBarItemsFunctionsBroken.ts | 6 + .../navigationBarItemsFunctionsBroken2.ts | 10 ++ .../fourslash/navigationBarItemsItems2.ts | 2 +- .../navigationBarItemsMissingName1.ts | 47 ++++--- .../navigationBarItemsMissingName2.ts | 2 +- 10 files changed, 262 insertions(+), 44 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index b9a560621e68d..2474b28c2c8b8 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -88,10 +88,17 @@ namespace ts.NavigationBar { case SyntaxKind.BindingElement: case SyntaxKind.VariableDeclaration: - if (isBindingPattern((node).name)) { - visit((node).name); + const decl = node; + if (isBindingPattern(decl.name)) { + visit(decl.name); break; } + // Don't include the const 'x' in `const x = function() {}`, just use the function. + if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { + visit(decl.initializer); + break; + } + // Fall through case SyntaxKind.ClassExpression: case SyntaxKind.ClassDeclaration: @@ -149,10 +156,9 @@ namespace ts.NavigationBar { function sortNodesInPlace(nodes: Node[]): void { nodes.sort((n1, n2) => { - // Get the name if it exists. OK if node is not a declaration. - const name1 = ( n1).name, name2 = ( n2).name; + const name1 = tryGetName(n1), name2 = tryGetName(n2); if (name1 && name2) { - return localeCompareFix(getPropertyNameForPropertyNameNode(name1), getPropertyNameForPropertyNameNode(name2)); + return localeCompareFix(name1, name2); } else if (name1) { return 1; @@ -848,6 +854,21 @@ namespace ts.NavigationBar { const anonymousFunctionText = ""; const anonymousClassText = ""; + function tryGetName(node: Node): string { + const decl = node; + if (decl.name) { + return getPropertyNameForPropertyNameNode(decl.name); + } + switch (node.kind) { + case SyntaxKind.FunctionExpression: + case SyntaxKind.ArrowFunction: + case SyntaxKind.ClassExpression: + return getFunctionOrClassName(node); + default: + return undefined; + } + } + /** Get the name for a (possibly anonymous) class/function expression. */ function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { if (node.name && getFullWidth(node.name) > 0) { @@ -871,7 +892,7 @@ namespace ts.NavigationBar { return "default"; } else { - return node.kind === SyntaxKind.ClassExpression ? anonymousClassText : anonymousFunctionText; + return isClassLike(node) ? anonymousClassText : anonymousFunctionText; } } diff --git a/tests/cases/fourslash/navbar_exportDefault.ts b/tests/cases/fourslash/navbar_exportDefault.ts index dc99cff7d649c..97dca2dc204d5 100644 --- a/tests/cases/fourslash/navbar_exportDefault.ts +++ b/tests/cases/fourslash/navbar_exportDefault.ts @@ -52,6 +52,13 @@ verify.navigationBar([ { "text": "\"c\"", "kind": "module", + "childItems": [ + { + "text": "default", + "kind": "function", + "kindModifiers": "export" + } + ] }, { "text": "default", diff --git a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts index c7749606e77d5..dbd01ec71b0b6 100644 --- a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts +++ b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts @@ -15,7 +15,7 @@ //// })(); ////})(); ////(function() { // Different anonymous functions are not merged -//// These will only show up as childItems. +//// // These will only show up as childItems. //// function z() {} //// console.log(function() {}) ////}) @@ -26,17 +26,110 @@ //// (class { }); ////}) -verify.navigationBarCount(21); -verify.navigationBarIndex("", 0); -verify.navigationBarIndex("", 1); -verify.navigationBarIndex("x", 2); -verify.navigationBarIndex("nest", 3); -verify.navigationBarIndex("", 4); -verify.navigationBarIndex("global.cls", 5); -verify.navigationBarIndex("classes", 6); -verify.navigationBarIndex("cls2", 7); -verify.navigationBarIndex("", 8); -verify.navigationBarIndex("cls3", 9); - -verify.navigationBarContains("global.cls", "class"); - +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "", + "kind": "function" + }, + { + "text": "", + "kind": "function" + }, + { + "text": "classes", + "kind": "function" + } + ] + }, + { + "text": "", + "kind": "function", + "childItems": [ + { + "text": "nest", + "kind": "function" + }, + { + "text": "x", + "kind": "function" + }, + { + "text": "y", + "kind": "const" + } + ], + "indent": 1 + }, + { + "text": "nest", + "kind": "function", + "childItems": [ + { + "text": "moreNest", + "kind": "function" + } + ], + "indent": 2 + }, + { + "text": "x", + "kind": "function", + "childItems": [ + { + "text": "xx", + "kind": "function" + } + ], + "indent": 2 + }, + { + "text": "", + "kind": "function", + "childItems": [ + { + "text": "", + "kind": "function" + }, + { + "text": "z", + "kind": "function" + } + ], + "indent": 1 + }, + { + "text": "classes", + "kind": "function", + "childItems": [ + { + "text": "cls3", + "kind": "class" + } + ], + "indent": 1 + }, + { + "text": "", + "kind": "class", + "indent": 2 + }, + { + "text": "cls2", + "kind": "class", + "indent": 2 + }, + { + "text": "cls3", + "kind": "class", + "indent": 2 + }, + { + "text": "global.cls", + "kind": "class", + "indent": 1 + } +]); diff --git a/tests/cases/fourslash/navigationBarDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarDeclarationExpressions.ts index 2032fcc940df5..4d037e42fa78e 100644 --- a/tests/cases/fourslash/navigationBarDeclarationExpressions.ts +++ b/tests/cases/fourslash/navigationBarDeclarationExpressions.ts @@ -3,6 +3,67 @@ ////console.log(console.log(class Y {}, class X {}), console.log(class B {}, class A {})); ////console.log(class Cls { meth() {} }); +verify.navigationBar( [ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "A", + "kind": "class" + }, + { + "text": "B", + "kind": "class" + }, + { + "text": "Cls", + "kind": "class" + }, + { + "text": "X", + "kind": "class" + }, + { + "text": "Y", + "kind": "class" + } + ] + }, + { + "text": "A", + "kind": "class", + "indent": 1 + }, + { + "text": "B", + "kind": "class", + "indent": 1 + }, + { + "text": "Cls", + "kind": "class", + "childItems": [ + { + "text": "meth", + "kind": "method" + } + ], + "indent": 1 + }, + { + "text": "X", + "kind": "class", + "indent": 1 + }, + { + "text": "Y", + "kind": "class", + "indent": 1 + } +]); + +/* verify.navigationBarCount(6); verify.navigationBarIndex("A", 0); verify.navigationBarIndex("B", 1); @@ -12,3 +73,4 @@ verify.navigationBarIndex("Y", 4); verify.navigationBarContains("Cls", "class"); verify.navigationBarChildItem("Cls", "meth", "method"); +*/ \ No newline at end of file diff --git a/tests/cases/fourslash/navigationBarItemsFunctions.ts b/tests/cases/fourslash/navigationBarItemsFunctions.ts index 91546462a2572..f5aa3fb681dd5 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctions.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctions.ts @@ -32,6 +32,12 @@ verify.navigationBar([ { "text": "baz", "kind": "function", + "childItems": [ + { + "text": "v", + "kind": "var" + } + ], "indent": 1 }, { @@ -41,6 +47,10 @@ verify.navigationBar([ { "text": "bar", "kind": "function" + }, + { + "text": "x", + "kind": "var" } ], "indent": 1 @@ -52,6 +62,10 @@ verify.navigationBar([ { "text": "biz", "kind": "function" + }, + { + "text": "y", + "kind": "var" } ], "indent": 2 diff --git a/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts b/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts index 96dcbb944ae07..2f17a5006ff5f 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts @@ -18,6 +18,12 @@ verify.navigationBar([ { "text": "f", "kind": "function", + "childItems": [ + { + "text": "", + "kind": "function" + } + ], "indent": 1 } ]); diff --git a/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts b/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts index 127bde8c9e71d..f907bdad06719 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctionsBroken2.ts @@ -10,6 +10,10 @@ verify.navigationBar([ "text": "", "kind": "module", "childItems": [ + { + "text": "", + "kind": "function" + }, { "text": "f", "kind": "function" @@ -19,6 +23,12 @@ verify.navigationBar([ { "text": "f", "kind": "function", + "childItems": [ + { + "text": "", + "kind": "function" + } + ], "indent": 1 } ]); diff --git a/tests/cases/fourslash/navigationBarItemsItems2.ts b/tests/cases/fourslash/navigationBarItemsItems2.ts index 762531d8778b5..c088271fc4857 100644 --- a/tests/cases/fourslash/navigationBarItemsItems2.ts +++ b/tests/cases/fourslash/navigationBarItemsItems2.ts @@ -19,7 +19,7 @@ verify.navigationBar([ ] }, { - "text": "default", + "text": "", "kind": "class", "kindModifiers": "export", "indent": 1 diff --git a/tests/cases/fourslash/navigationBarItemsMissingName1.ts b/tests/cases/fourslash/navigationBarItemsMissingName1.ts index af0e89683bcfe..84ee46e9cfad3 100644 --- a/tests/cases/fourslash/navigationBarItemsMissingName1.ts +++ b/tests/cases/fourslash/navigationBarItemsMissingName1.ts @@ -3,26 +3,31 @@ //// foo() {} ////} -verify.navigationBar([ - { - "text": "\"navigationBarItemsMissingName1\"", - "kind": "module", - "childItems": [ - { - "text": "C", - "kind": "class" - } - ] - }, - { +verify.navigationBar( [ + { + "text": "\"navigationBarItemsMissingName1\"", + "kind": "module", + "childItems": [ + { + "text": "", + "kind": "function", + "kindModifiers": "export" + }, + { "text": "C", - "kind": "class", - "childItems": [ - { - "text": "foo", - "kind": "method" - } - ], - "indent": 1 - } + "kind": "class" + } + ] + }, + { + "text": "C", + "kind": "class", + "childItems": [ + { + "text": "foo", + "kind": "method" + } + ], + "indent": 1 + } ]); diff --git a/tests/cases/fourslash/navigationBarItemsMissingName2.ts b/tests/cases/fourslash/navigationBarItemsMissingName2.ts index 2eda3b07855c7..89a16bc532c82 100644 --- a/tests/cases/fourslash/navigationBarItemsMissingName2.ts +++ b/tests/cases/fourslash/navigationBarItemsMissingName2.ts @@ -12,7 +12,7 @@ verify.navigationBar([ "kind": "module" }, { - "text": "default", + "text": "", "kind": "class", "childItems": [ { From 1fc181391bd1cc1e84ccf3e0e9f2e268ddaf8d1b Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 27 May 2016 09:36:08 -0700 Subject: [PATCH 7/9] Fix bug where class expression child item didn't get right name --- src/services/navigationBar.ts | 2 +- tests/cases/fourslash/navbar_exportDefault.ts | 9 ++++- ...ationBarAnonymousDeclarationExpressions.ts | 15 +++++++- .../fourslash/navigationBarItemsItems2.ts | 5 +++ .../navigationBarItemsMissingName2.ts | 34 +++++++++++-------- 5 files changed, 48 insertions(+), 17 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 2474b28c2c8b8..23343ec25485a 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -383,7 +383,7 @@ namespace ts.NavigationBar { case SyntaxKind.ClassDeclaration: case SyntaxKind.ClassExpression: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.classElement); + return createItem(node, getFunctionOrClassName(node), ts.ScriptElementKind.classElement); case SyntaxKind.ArrowFunction: case SyntaxKind.FunctionExpression: diff --git a/tests/cases/fourslash/navbar_exportDefault.ts b/tests/cases/fourslash/navbar_exportDefault.ts index 97dca2dc204d5..e547c9129a69d 100644 --- a/tests/cases/fourslash/navbar_exportDefault.ts +++ b/tests/cases/fourslash/navbar_exportDefault.ts @@ -16,7 +16,14 @@ goTo.file("a.ts"); verify.navigationBar([ { "text": "\"a\"", - "kind": "module" + "kind": "module", + "childItems": [ + { + "text": "default", + "kind": "class", + "kindModifiers": "export" + } + ] }, { "text": "default", diff --git a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts index dbd01ec71b0b6..8fe12ec94a284 100644 --- a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts +++ b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts @@ -42,6 +42,10 @@ verify.navigationBar([ { "text": "classes", "kind": "function" + }, + { + "text": "global.cls", + "kind": "class" } ] }, @@ -105,6 +109,14 @@ verify.navigationBar([ "text": "classes", "kind": "function", "childItems": [ + { + "text": "", + "kind": "class" + }, + { + "text": "cls2", + "kind": "class" + }, { "text": "cls3", "kind": "class" @@ -132,4 +144,5 @@ verify.navigationBar([ "kind": "class", "indent": 1 } -]); +] +); diff --git a/tests/cases/fourslash/navigationBarItemsItems2.ts b/tests/cases/fourslash/navigationBarItemsItems2.ts index c088271fc4857..6ab0827e8f959 100644 --- a/tests/cases/fourslash/navigationBarItemsItems2.ts +++ b/tests/cases/fourslash/navigationBarItemsItems2.ts @@ -12,6 +12,11 @@ verify.navigationBar([ "text": "\"navigationBarItemsItems2\"", "kind": "module", "childItems": [ + { + "text": "", + "kind": "class", + "kindModifiers": "export" + }, { "text": "A", "kind": "module" diff --git a/tests/cases/fourslash/navigationBarItemsMissingName2.ts b/tests/cases/fourslash/navigationBarItemsMissingName2.ts index 89a16bc532c82..c39185f9dd2d9 100644 --- a/tests/cases/fourslash/navigationBarItemsMissingName2.ts +++ b/tests/cases/fourslash/navigationBarItemsMissingName2.ts @@ -7,19 +7,25 @@ // Anonymous classes are still included. verify.navigationBar([ - { - "text": "", - "kind": "module" - }, - { + { + "text": "", + "kind": "module", + "childItems": [ + { "text": "", - "kind": "class", - "childItems": [ - { - "text": "foo", - "kind": "method" - } - ], - "indent": 1 - } + "kind": "class" + } + ] + }, + { + "text": "", + "kind": "class", + "childItems": [ + { + "text": "foo", + "kind": "method" + } + ], + "indent": 1 + } ]); From 37ed5e4c22532c69e57a6b69defac6e26233f2b6 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 24 May 2016 10:33:28 -0700 Subject: [PATCH 8/9] Refactor navigation bar, fixing many bugs along the way --- src/harness/fourslash.ts | 6 +- src/services/navigationBar.ts | 1106 ++++++----------- src/services/services.ts | 2 +- .../navbar_contains-no-duplicates.ts | 8 +- ...ationBarAnonymousDeclarationExpressions.ts | 3 +- .../fourslash/navigationBarGetterAndSetter.ts | 48 + .../navigationBarItemsFunctionsBroken.ts | 2 +- ...ionBarItemsInsideMethodsAndConstructors.ts | 12 +- .../navigationBarItemsMissingName1.ts | 2 +- .../navigationBarItemsMissingName2.ts | 40 +- .../fourslash/navigationBarItemsModules.ts | 192 +-- tests/cases/fourslash/navigationBarMerging.ts | 189 +++ .../cases/fourslash/navigationBarVariables.ts | 26 + 13 files changed, 746 insertions(+), 890 deletions(-) create mode 100644 tests/cases/fourslash/navigationBarGetterAndSetter.ts create mode 100644 tests/cases/fourslash/navigationBarMerging.ts create mode 100644 tests/cases/fourslash/navigationBarVariables.ts diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 87bcbb14fbe88..4a756ab3b1ecc 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1992,7 +1992,7 @@ namespace FourSlash { } } - public printNavigationBar(showChildItems = false) { + public printNavigationBar() { const items = this.languageService.getNavigationBarItems(this.activeFile.fileName); const length = items && items.length; @@ -3190,8 +3190,8 @@ namespace FourSlashInterface { this.state.printNavigationItems(searchValue); } - public printNavigationBar(showChildItems = false) { - this.state.printNavigationBar(showChildItems); + public printNavigationBar() { + this.state.printNavigationBar(); } public printReferences() { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 23343ec25485a..8906e54cddb99 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -2,86 +2,97 @@ /* @internal */ namespace ts.NavigationBar { - export function getNavigationBarItems(sourceFile: SourceFile, compilerOptions: CompilerOptions): ts.NavigationBarItem[] { - // TODO: Handle JS files differently in 'navbar' calls for now, but ideally we should unify - // the 'navbar' and 'navto' logic for TypeScript and JavaScript. - if (isSourceFileJavaScript(sourceFile)) { - return getJsNavigationBarItems(sourceFile, compilerOptions); - } - - return getItemsWorker(getTopLevelNodes(sourceFile), createTopLevelItem); - - function getIndent(node: Node): number { - let indent = 1; // Global node is the only one with indent 0. - - let current = node.parent; - while (current) { - switch (current.kind) { - case SyntaxKind.ModuleDeclaration: - // If we have a module declared as A.B.C, it is more "intuitive" - // to say it only has a single layer of depth - do { - current = current.parent; - } - while (current.kind === SyntaxKind.ModuleDeclaration); - - // fall through - case SyntaxKind.ClassExpression: - case SyntaxKind.ClassDeclaration: - case SyntaxKind.EnumDeclaration: - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - indent++; - } + export function getNavigationBarItems(sourceFile: SourceFile): NavigationBarItem[] { + const root = createNavNode(undefined, sourceFile); + return map(topLevelItems(root), convertToTopLevelItem); + } - current = current.parent; - } + /** + * Represents a navBar item and its children. + * The returned NavigationBarItem is more complicated and doesn't include 'parent', so we use these to do work before converting. + */ + interface NavNode { + node: Node; + additionalNodes?: Node[]; + parent: NavNode; // Missing for root decl + children: NavNode[]; + indent: number; // # of parents + } + function navKind(n: NavNode): SyntaxKind { + return n.node.kind; + } + function navModifiers(n: NavNode): string { + return getNodeModifiers(n.node); + } - return indent; + /** Creates a child node and adds it to parent. */ + function createNavNode(parent: NavNode, node: Node): NavNode { + // `item` is set during `convertToItem` + const item: NavNode = { node, parent, children: [], indent: parent ? parent.indent + 1 : 0 }; + if (parent) { + parent.children.push(item); } + addChildren(item); + return item; + } - function getChildNodes(nodes: Node[]): Node[] { - const childNodes: Node[] = []; - - function visit(node: Node) { + /** Traverse through parent.node's descendants and find declarations to add as parent's children. */ + function addChildren(parent: NavNode): void { + function recur(node: Node) { + if (isDeclaration(node)) { //TODO:PERF: get rid of this call, just use 1 switch statement. switch (node.kind) { - case SyntaxKind.VariableStatement: - forEach((node).declarationList.declarations, visit); + case SyntaxKind.Parameter: // Parameter properties handled by SyntaxKind.Constructor case + case SyntaxKind.TypeParameter: + case SyntaxKind.PropertyAssignment: + // Don't treat this as a declaration. + forEachChild(node, recur); break; - case SyntaxKind.ObjectBindingPattern: - case SyntaxKind.ArrayBindingPattern: - forEach((node).elements, visit); + + case SyntaxKind.Constructor: + // Get parameter properties, and treat them as being on the *same* level as the constructor, not under it. + const ctr = node; + createNavNode(parent, ctr); + for (const param of ctr.parameters) { + if (isParameterPropertyDeclaration(param)) { + createNavNode(parent, param); + } + } break; - case SyntaxKind.ExportDeclaration: - // Handle named exports case e.g.: - // export {a, b as B} from "mod"; - if ((node).exportClause) { - forEach((node).exportClause.elements, visit); + case SyntaxKind.MethodDeclaration: + case SyntaxKind.MethodSignature: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + case SyntaxKind.PropertyDeclaration: + case SyntaxKind.PropertySignature: + if (!hasDynamicName(( node))) { + createNavNode(parent, node); } break; - case SyntaxKind.ImportDeclaration: - let importClause = (node).importClause; - if (importClause) { - // Handle default import case e.g.: - // import d from "mod"; - if (importClause.name) { - childNodes.push(importClause); - } + case SyntaxKind.EnumMember: + if (!isComputedProperty(node)) { + createNavNode(parent, node); + } + break; + + case SyntaxKind.ImportClause: + let importClause = node; + // Handle default import case e.g.: + // import d from "mod"; + if (importClause.name) { + createNavNode(parent, importClause); + } - // Handle named bindings in imports e.g.: - // import * as NS from "mod"; - // import {a, b as B} from "mod"; - if (importClause.namedBindings) { - if (importClause.namedBindings.kind === SyntaxKind.NamespaceImport) { - childNodes.push(importClause.namedBindings); - } - else { - forEach((importClause.namedBindings).elements, visit); - } + // Handle named bindings in imports e.g.: + // import * as NS from "mod"; + // import {a, b as B} from "mod"; + if (importClause.namedBindings) { + if (importClause.namedBindings.kind === SyntaxKind.NamespaceImport) { + createNavNode(parent, importClause.namedBindings); + } + else { + forEach((importClause.namedBindings).elements, recur); } } break; @@ -89,770 +100,355 @@ namespace ts.NavigationBar { case SyntaxKind.BindingElement: case SyntaxKind.VariableDeclaration: const decl = node; - if (isBindingPattern(decl.name)) { - visit(decl.name); - break; + const name = decl.name; + if (isBindingPattern(name)) { + recur(name); } - // Don't include the const 'x' in `const x = function() {}`, just use the function. - if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { - visit(decl.initializer); - break; + else if (decl.initializer && isFunctionOrClassExpression(decl.initializer)) { + // For `const x = function() {}`, just use the function node, not the const. + recur(decl.initializer); + } + else { + createNavNode(parent, node); } - - // Fall through - case SyntaxKind.ClassExpression: - case SyntaxKind.ClassDeclaration: - case SyntaxKind.EnumDeclaration: - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.ModuleDeclaration: - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.ImportEqualsDeclaration: - case SyntaxKind.ImportSpecifier: - case SyntaxKind.ExportSpecifier: - case SyntaxKind.TypeAliasDeclaration: - childNodes.push(node); break; default: - forEachChild(node, visit); + createNavNode(parent, node); } } + else { + switch (node.kind) { + case SyntaxKind.CallSignature: + case SyntaxKind.ConstructSignature: + case SyntaxKind.IndexSignature: + case SyntaxKind.MethodSignature: + createNavNode(parent, node); + break; + default: + forEachChild(node, recur); + } + } + } - // for (let i = 0, n = nodes.length; i < n; i++) { - // let node = nodes[i]; - - // if (node.kind === SyntaxKind.ClassDeclaration || - // node.kind === SyntaxKind.EnumDeclaration || - // node.kind === SyntaxKind.InterfaceDeclaration || - // node.kind === SyntaxKind.ModuleDeclaration || - // node.kind === SyntaxKind.FunctionDeclaration) { - - // childNodes.push(node); - // } - // else if (node.kind === SyntaxKind.VariableStatement) { - // childNodes.push.apply(childNodes, (node).declarations); - // } - // } - forEach(nodes, visit); - return sortNodes(childNodes); + let parentNode = parent.node; + if (parentNode.kind === SyntaxKind.ModuleDeclaration) { + parentNode = getInteriorModule(parentNode); } + forEachChild(parentNode, recur); - function getTopLevelNodes(node: SourceFile): Node[] { - const topLevelNodes: Node[] = []; - topLevelNodes.push(node); + parent.children = mergeChildren(parent.children); + sortChildren(parent.children); + } + + //TODO: mutate input array, return void + /** Merge declarations of the same kind. */ + function mergeChildren(children: NavNode[]): NavNode[] { + const map: Map = {}; + return filter(children, child => { + const decl = child.node; + const name = decl.name && decl.name.getText(); + if (!name) + // Anonymous items are never merged. + return true; + + const itemsWithSameName = map[name]; + if (!itemsWithSameName) { + map[name] = [child]; + return true; + } + + for (const s of itemsWithSameName) { + if (shouldReallyMerge(s.node, child.node)) { + merge(s, child); + return false; + } + } + itemsWithSameName.push(child); + return true; + }); - addTopLevelNodes(node.statements, topLevelNodes); + /** a and b have the same name, but they may not be mergeable. */ + function shouldReallyMerge(a: Node, b: Node): boolean { + return a.kind === b.kind && (a.kind !== SyntaxKind.ModuleDeclaration || areSameModule(a, b)); - return topLevelNodes; + // We use 1 NavNode to represent 'A.B.C', but there are multiple source nodes. + // Only merge module nodes that have the same chain. Don't merge 'A.B.C' with 'A'! + function areSameModule(a: ModuleDeclaration, b: ModuleDeclaration): boolean { + if (a.body.kind !== b.body.kind) { + return false; + } + if (a.body.kind !== SyntaxKind.ModuleDeclaration) { + return true; + } + return areSameModule(a.body, b.body); + } } - function sortNodes(nodes: Node[]): Node[] { - const sortedCopy = nodes.slice(0); - sortNodesInPlace(sortedCopy); - return sortedCopy; + /** Merge source into target. Source should be thrown away after this is called. */ + function merge(target: NavNode, source: NavNode): void { + target.additionalNodes = target.additionalNodes || []; + target.additionalNodes.push(source.node); + if (source.additionalNodes) { + target.additionalNodes.push(...source.additionalNodes); + } + + //TODO:PERF + target.children = mergeChildren(target.children.concat(source.children)); + sortChildren(target.children); } + } - function sortNodesInPlace(nodes: Node[]): void { - nodes.sort((n1, n2) => { - const name1 = tryGetName(n1), name2 = tryGetName(n2); - if (name1 && name2) { - return localeCompareFix(name1, name2); - } - else if (name1) { - return 1; - } - else if (name2) { - return -1; - } - else { - return n1.kind - n2.kind; - } - }); + /** Recursively ensure that each NavNode's children are in sorted order. */ + function sortChildren(children: NavNode[]): void { + children.sort((child1, child2) => { + const name1 = tryGetName(child1.node), name2 = tryGetName(child2.node); + if (name1 && name2) { + const cmp = localeCompareFix(name1, name2); + return cmp !== 0 ? cmp : navKind(child1) - navKind(child2); + } + if (name1) { + return 1; + } + if (name2) { + return -1; + } + else { + return navKind(child1) - navKind(child2); + } // node 0.10 treats "a" as greater than "B". // For consistency, sort alphabetically, falling back to which is lower-case. - function localeCompareFix(a: string, b: string) { + function localeCompareFix(a: string, b: string): number { const cmp = a.toLowerCase().localeCompare(b.toLowerCase()); if (cmp !== 0) return cmp; // Return the *opposite* of the `<` operator, which works the same in node 0.10 and 6.0. return a < b ? 1 : a > b ? -1 : 0; } + }); + } + + function getItemName(node: Node): string { + if (node.kind === SyntaxKind.ModuleDeclaration) { + return getModuleName(node); } - // Add nodes in a single "level" of top-level nodes (e.g. methods in a class.) - // Nodes in a single "level" are sorted together. - // Returns whether any nodes were added. - function addTopLevelNodes(nodes: Node | Node[], topLevelNodes: Node[]) { - const decls = getNextLevelOfDeclarations(nodes); - // Sort each level of declarations together - sortNodesInPlace(decls); - for (const decl of decls) { - addTopLevelNode(decl, topLevelNodes); - } + const name = (node).name; + if (name) { + const text = name.getText(); + if (text.length > 0) + return text; } - // Gets all declarations contained within `nodes` and their sub-expressions, - // but not declarations within declarations. - function getNextLevelOfDeclarations(nodes: Node | Node[]): Node[] { - const result: Node[] = []; - function recur(node: Node): void { - switch (node.kind) { - case SyntaxKind.ClassExpression: - case SyntaxKind.ClassDeclaration: - case SyntaxKind.EnumDeclaration: - case SyntaxKind.InterfaceDeclaration: - case SyntaxKind.TypeAliasDeclaration: - case SyntaxKind.ModuleDeclaration: - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - result.push(node); - // Its children are handled by addTopLevelNode later. - break; - default: - forEachChild(node, recur); + switch (node.kind) { + case SyntaxKind.SourceFile: + const sourceFile = node; + return isExternalModule(sourceFile) + ? `"${escapeString(getBaseFileName(removeFileExtension(normalizePath(sourceFile.fileName))))}"` + : ""; + case SyntaxKind.ArrowFunction: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.FunctionExpression: + case SyntaxKind.ClassDeclaration: + case SyntaxKind.ClassExpression: + if (node.flags & NodeFlags.Default) { + return "default"; } - } - if (nodes instanceof Array) { - for (const node of nodes) { - recur(node); + return getFunctionOrClassName(node); + case SyntaxKind.Constructor: + return "constructor"; + case SyntaxKind.ConstructSignature: + return "new()"; + case SyntaxKind.CallSignature: + return "()"; + case SyntaxKind.IndexSignature: + return "[]"; + default: + Debug.fail(); + return ""; + } + } + + /** Flattens the NavNode tree to a list, keeping only the top-level items. */ + function topLevelItems(root: NavNode): NavNode[] { + const topLevel: NavNode[] = []; + function recur(item: NavNode) { + if (isTopLevel(item)) { + topLevel.push(item); + for (const child of item.children) { + recur(child); } } - else { - recur(nodes); - } - return result; } + recur(root); + return topLevel; - function addTopLevelNode(node: Node, topLevelNodes: Node[]) { - switch (node.kind) { - case SyntaxKind.ClassExpression: + function isTopLevel(item: NavNode): boolean { + switch (navKind(item)) { case SyntaxKind.ClassDeclaration: - topLevelNodes.push(node); - for (const member of (node).members) { - if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.Constructor) { - type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; - const decl = member; - if (decl.body) { - // Add node, but it will not become an item unless it has children later. - topLevelNodes.push(member); - addTopLevelNodes(decl.body.statements, topLevelNodes); - } - } - } - break; - + case SyntaxKind.ClassExpression: case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: + case SyntaxKind.ModuleDeclaration: + case SyntaxKind.SourceFile: case SyntaxKind.TypeAliasDeclaration: - topLevelNodes.push(node); - break; + return true; - case SyntaxKind.ModuleDeclaration: - let moduleDeclaration = node; - topLevelNodes.push(node); - addTopLevelNodes((getInnermostModule(moduleDeclaration).body).statements, topLevelNodes); - break; + case SyntaxKind.Constructor: + case SyntaxKind.MethodDeclaration: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + return hasSomeImportantChild(item); case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: - const decl = node; - if (decl.body) { - topLevelNodes.push(node); - addTopLevelNodes(decl.body, topLevelNodes); - } - break; + case SyntaxKind.FunctionExpression: + return isTopLevelFunctionDeclaration(item); default: - // There should be a case in addTopLevelNode for each case in getNextLevelOfDeclarations. - throw new Error("Unreachable"); - } - } - - function getItemsWorker(nodes: Node[], createItem: (n: Node) => ts.NavigationBarItem): ts.NavigationBarItem[] { - const items: ts.NavigationBarItem[] = []; - - const keyToItem: Map = {}; - - for (const child of nodes) { - const item = createItem(child); - if (item !== undefined) { - if (item.text.length > 0) { - if (isFunctionOrClassExpression(child)) { - // Never merge - items.push(item); - continue; - } - - const key = item.text + "-" + item.kind + "-" + item.indent; - - const itemWithSameName = keyToItem[key]; - if (itemWithSameName) { - // We had an item with the same name. Merge these items together. - merge(itemWithSameName, item); - } - else { - keyToItem[key] = item; - items.push(item); - } - } - } + return false; } - - return items; - } - - function merge(target: ts.NavigationBarItem, source: ts.NavigationBarItem) { - // First, add any spans in the source to the target. - addRange(target.spans, source.spans); - - if (source.childItems) { - if (!target.childItems) { - target.childItems = []; + function isTopLevelFunctionDeclaration(item: NavNode): boolean { + if (!(item.node).body) { + return false; } - // Next, recursively merge or add any children in the source as appropriate. - outer: - for (const sourceChild of source.childItems) { - for (const targetChild of target.childItems) { - if (targetChild.text === sourceChild.text && targetChild.kind === sourceChild.kind) { - // Found a match. merge them. - merge(targetChild, sourceChild); - continue outer; - } - } - - // Didn't find a match, just add this child to the list. - target.childItems.push(sourceChild); + switch (navKind(item.parent)) { + case SyntaxKind.ModuleBlock: + case SyntaxKind.SourceFile: + case SyntaxKind.MethodDeclaration: + case SyntaxKind.Constructor: + return true; + default: + return hasSomeImportantChild(item); } } - } - - function createChildItem(node: Node): ts.NavigationBarItem { - switch (node.kind) { - case SyntaxKind.Parameter: - if (isBindingPattern((node).name)) { - break; - } - if ((node.flags & NodeFlags.Modifier) === 0) { - return undefined; - } - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); - - case SyntaxKind.MethodDeclaration: - case SyntaxKind.MethodSignature: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberFunctionElement); - - case SyntaxKind.GetAccessor: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberGetAccessorElement); - - case SyntaxKind.SetAccessor: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberSetAccessorElement); - - case SyntaxKind.IndexSignature: - return createItem(node, "[]", ts.ScriptElementKind.indexSignatureElement); - - case SyntaxKind.EnumDeclaration: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.enumElement); - - case SyntaxKind.EnumMember: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); - - case SyntaxKind.ModuleDeclaration: - return createItem(node, getModuleName(node), ts.ScriptElementKind.moduleElement); - - case SyntaxKind.InterfaceDeclaration: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.interfaceElement); - - case SyntaxKind.TypeAliasDeclaration: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.typeElement); - - case SyntaxKind.CallSignature: - return createItem(node, "()", ts.ScriptElementKind.callSignatureElement); - - case SyntaxKind.ConstructSignature: - return createItem(node, "new()", ts.ScriptElementKind.constructSignatureElement); - - case SyntaxKind.PropertyDeclaration: - case SyntaxKind.PropertySignature: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.memberVariableElement); - - case SyntaxKind.ClassDeclaration: - case SyntaxKind.ClassExpression: - return createItem(node, getFunctionOrClassName(node), ts.ScriptElementKind.classElement); - - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - return createItem(node, getFunctionOrClassName(node), ts.ScriptElementKind.functionElement); - - case SyntaxKind.VariableDeclaration: - case SyntaxKind.BindingElement: - let variableDeclarationNode: Node; - let name: Node; - - if (node.kind === SyntaxKind.BindingElement) { - name = (node).name; - variableDeclarationNode = node; - // binding elements are added only for variable declarations - // bubble up to the containing variable declaration - while (variableDeclarationNode && variableDeclarationNode.kind !== SyntaxKind.VariableDeclaration) { - variableDeclarationNode = variableDeclarationNode.parent; - } - Debug.assert(variableDeclarationNode !== undefined); - } - else { - Debug.assert(!isBindingPattern((node).name)); - variableDeclarationNode = node; - name = (node).name; - } - - if (isConst(variableDeclarationNode)) { - return createItem(node, getTextOfNode(name), ts.ScriptElementKind.constElement); - } - else if (isLet(variableDeclarationNode)) { - return createItem(node, getTextOfNode(name), ts.ScriptElementKind.letElement); - } - else { - return createItem(node, getTextOfNode(name), ts.ScriptElementKind.variableElement); - } - - case SyntaxKind.Constructor: - return createItem(node, "constructor", ts.ScriptElementKind.constructorImplementationElement); - - case SyntaxKind.ExportSpecifier: - case SyntaxKind.ImportSpecifier: - case SyntaxKind.ImportEqualsDeclaration: - case SyntaxKind.ImportClause: - case SyntaxKind.NamespaceImport: - return createItem(node, getTextOfNode((node).name), ts.ScriptElementKind.alias); - } - - return undefined; - - function createItem(node: Node, name: string, scriptElementKind: string): NavigationBarItem { - return getNavigationBarItem(name, scriptElementKind, getNodeModifiers(node), [getNodeSpan(node)]); + function hasSomeImportantChild(item: NavNode) { + return forEach(item.children, child => { + const childKind = navKind(child); + return childKind !== SyntaxKind.VariableDeclaration && childKind !== SyntaxKind.BindingElement; + }); } } + } - function isEmpty(text: string) { - return !text || text.trim() === ""; - } - - function getNavigationBarItem(text: string, kind: string, kindModifiers: string, spans: TextSpan[], childItems: NavigationBarItem[] = [], indent = 0): NavigationBarItem { - if (isEmpty(text)) { - return undefined; - } - + function convertToTopLevelItem(n: NavNode): NavigationBarItem { + const spans = [getNodeSpan(n.node)]; + return { + text: getItemName(n.node), + kind: nodeKind(n.node), + kindModifiers: navModifiers(n), + spans, + childItems: map(n.children, convertToChildItem), + indent: n.indent, + bolded: false, + grayed: false + }; + + function convertToChildItem(n: NavNode): NavigationBarItem { + //TODO:PERF + const nodes = [n.node]; + if (n.additionalNodes) { + nodes.push(...n.additionalNodes); + } + const spans = map(nodes, getNodeSpan); return { - text, - kind, - kindModifiers, + text: getItemName(n.node), + kind: nodeKind(n.node), + kindModifiers: navModifiers(n), spans, - childItems, - indent, + childItems: [], + indent: 0, bolded: false, grayed: false }; } + } - function createTopLevelItem(node: Node): ts.NavigationBarItem { - switch (node.kind) { - case SyntaxKind.SourceFile: - return createSourceFileItem(node); - - case SyntaxKind.ClassExpression: - case SyntaxKind.ClassDeclaration: - return createClassItem(node); - - case SyntaxKind.MethodDeclaration: - case SyntaxKind.Constructor: - return createMemberFunctionLikeItem(node); - - case SyntaxKind.EnumDeclaration: - return createEnumItem(node); - - case SyntaxKind.InterfaceDeclaration: - return createInterfaceItem(node); - - case SyntaxKind.ModuleDeclaration: - return createModuleItem(node); - - case SyntaxKind.ArrowFunction: - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - return createFunctionItem(node); - - case SyntaxKind.TypeAliasDeclaration: - return createTypeAliasItem(node); - } - - return undefined; - - function createModuleItem(node: ModuleDeclaration): NavigationBarItem { - const moduleName = getModuleName(node); - - const childItems = getItemsWorker(getChildNodes((getInnermostModule(node).body).statements), createChildItem); - - return getNavigationBarItem(moduleName, - ts.ScriptElementKind.moduleElement, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); - } - - function createFunctionItem(node: FunctionDeclaration): ts.NavigationBarItem { - const children = node.body ? getChildNodes([node.body]) : []; - const isTopLevel = node.parent.kind === SyntaxKind.ModuleBlock || node.parent.kind === SyntaxKind.SourceFile || - // Functions with declaration children (not including local variables) are worthy - children.some(child => child.kind !== SyntaxKind.VariableDeclaration && child.kind !== SyntaxKind.BindingElement) || - // Functions inside class methods are worthy - node.parent.parent.kind === SyntaxKind.MethodDeclaration || node.parent.parent.kind === SyntaxKind.Constructor; - if (!isTopLevel) { - return undefined; + // TODO: file issue: we should just use getNodeKind. No reason why navigationBar and navigateTo should have different behaviors. + function nodeKind(node: Node): string { + switch (node.kind) { + case SyntaxKind.SourceFile: + return ScriptElementKind.moduleElement; + + case SyntaxKind.EnumMember: + return ScriptElementKind.memberVariableElement; + + case SyntaxKind.VariableDeclaration: + case SyntaxKind.BindingElement: + let variableDeclarationNode: Node; + let name: Node; + + if (node.kind === SyntaxKind.BindingElement) { + name = (node).name; + variableDeclarationNode = node; + // binding elements are added only for variable declarations + // bubble up to the containing variable declaration + while (variableDeclarationNode && variableDeclarationNode.kind !== SyntaxKind.VariableDeclaration) { + variableDeclarationNode = variableDeclarationNode.parent; + } + Debug.assert(variableDeclarationNode !== undefined); } - - const childItems = getItemsWorker(children, createChildItem); - - return getNavigationBarItem( - getFunctionOrClassName(node), - ts.ScriptElementKind.functionElement, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); - } - - function createTypeAliasItem(node: TypeAliasDeclaration): ts.NavigationBarItem { - return getNavigationBarItem(node.name.text, - ts.ScriptElementKind.typeElement, - getNodeModifiers(node), - [getNodeSpan(node)], - [], - getIndent(node)); - } - - function createMemberFunctionLikeItem(node: MethodDeclaration | ConstructorDeclaration): ts.NavigationBarItem { - const childItems = getItemsWorker(sortNodes(node.body.statements), createChildItem); - // Don't include member functions as top-level if they don't contain other items. - if (!childItems.length) { - return undefined; + else { + Debug.assert(!isBindingPattern((node).name)); + variableDeclarationNode = node; + name = (node).name; } - let scriptElementKind: string; - let memberFunctionName: string; - if (node.kind === SyntaxKind.MethodDeclaration) { - memberFunctionName = getPropertyNameForPropertyNameNode(node.name); - scriptElementKind = ts.ScriptElementKind.memberFunctionElement; + if (isConst(variableDeclarationNode)) { + return ts.ScriptElementKind.constElement; } - else { - memberFunctionName = "constructor"; - scriptElementKind = ts.ScriptElementKind.constructorImplementationElement; + else if (isLet(variableDeclarationNode)) { + return ts.ScriptElementKind.letElement; } - - return getNavigationBarItem(memberFunctionName, - scriptElementKind, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); - } - - function createSourceFileItem(node: SourceFile): ts.NavigationBarItem { - const childItems = getItemsWorker(getChildNodes(node.statements), createChildItem); - - const rootName = isExternalModule(node) - ? "\"" + escapeString(getBaseFileName(removeFileExtension(normalizePath(node.fileName)))) + "\"" - : ""; - - return getNavigationBarItem(rootName, - ts.ScriptElementKind.moduleElement, - ts.ScriptElementKindModifier.none, - [getNodeSpan(node)], - childItems); - } - - function createClassItem(node: ClassDeclaration): ts.NavigationBarItem { - let childItems: NavigationBarItem[]; - - if (node.members) { - const constructor = forEach(node.members, member => { - return member.kind === SyntaxKind.Constructor && member; - }); - - // Add the constructor parameters in as children of the class (for property parameters). - // Note that *all non-binding pattern named* parameters will be added to the nodes array, but parameters that - // are not properties will be filtered out later by createChildItem. - const nodes: Node[] = removeDynamicallyNamedProperties(node); - if (constructor) { - addRange(nodes, filter(constructor.parameters, p => !isBindingPattern(p.name))); - } - - childItems = getItemsWorker(sortNodes(nodes), createChildItem); + else { + return ts.ScriptElementKind.variableElement; } - return getNavigationBarItem( - getFunctionOrClassName(node), - ts.ScriptElementKind.classElement, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); - } - - function createEnumItem(node: EnumDeclaration): ts.NavigationBarItem { - const childItems = getItemsWorker(sortNodes(removeComputedProperties(node)), createChildItem); - return getNavigationBarItem( - node.name.text, - ts.ScriptElementKind.enumElement, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); - } - - function createInterfaceItem(node: InterfaceDeclaration): ts.NavigationBarItem { - const childItems = getItemsWorker(sortNodes(removeDynamicallyNamedProperties(node)), createChildItem); - return getNavigationBarItem( - node.name.text, - ts.ScriptElementKind.interfaceElement, - getNodeModifiers(node), - [getNodeSpan(node)], - childItems, - getIndent(node)); - } - } - - function getModuleName(moduleDeclaration: ModuleDeclaration): string { - // We want to maintain quotation marks. - if (isAmbientModule(moduleDeclaration)) { - return getTextOfNode(moduleDeclaration.name); - } - - // Otherwise, we need to aggregate each identifier to build up the qualified name. - const result: string[] = []; - - result.push(moduleDeclaration.name.text); - - while (moduleDeclaration.body && moduleDeclaration.body.kind === SyntaxKind.ModuleDeclaration) { - moduleDeclaration = moduleDeclaration.body; - - result.push(moduleDeclaration.name.text); - } - - return result.join("."); - } - - function removeComputedProperties(node: EnumDeclaration): Declaration[] { - return filter(node.members, member => member.name === undefined || member.name.kind !== SyntaxKind.ComputedPropertyName); - } - - /** - * Like removeComputedProperties, but retains the properties with well known symbol names - */ - function removeDynamicallyNamedProperties(node: ClassDeclaration | InterfaceDeclaration): Declaration[] { - return filter(node.members, member => !hasDynamicName(member)); - } - - function getInnermostModule(node: ModuleDeclaration): ModuleDeclaration { - while (node.body.kind === SyntaxKind.ModuleDeclaration) { - node = node.body; - } - - return node; - } - - function getNodeSpan(node: Node) { - return node.kind === SyntaxKind.SourceFile - ? createTextSpanFromBounds(node.getFullStart(), node.getEnd()) - : createTextSpanFromBounds(node.getStart(), node.getEnd()); - } + case SyntaxKind.ArrowFunction: + return ts.ScriptElementKind.functionElement; - function getTextOfNode(node: Node): string { - return getTextOfNodeFromSourceText(sourceFile.text, node); + default: + return getNodeKind(node); } } - export function getJsNavigationBarItems(sourceFile: SourceFile, compilerOptions: CompilerOptions): NavigationBarItem[] { - let indent = 0; - - const rootName = isExternalModule(sourceFile) ? - "\"" + escapeString(getBaseFileName(removeFileExtension(normalizePath(sourceFile.fileName)))) + "\"" - : ""; - - const sourceFileItem = getNavBarItem(rootName, ScriptElementKind.moduleElement, [getNodeSpan(sourceFile)]); - let topItem = sourceFileItem; - - // Walk the whole file, because we want to also find function expressions - which may be in variable initializer, - // call arguments, expressions, etc... - forEachChild(sourceFile, visitNode); - - function visitNode(node: Node) { - const newItem = createNavBarItem(node); - - if (newItem) { - topItem.childItems.push(newItem); - } - - // Add a level if traversing into a container - if (newItem && (isFunctionLike(node) || isClassLike(node))) { - const lastTop = topItem; - indent++; - topItem = newItem; - forEachChild(node, visitNode); - topItem = lastTop; - indent--; - - if (newItem && isAnonymousFunction(newItem) && newItem.childItems.length === 0) { - topItem.childItems.pop(); - } - } - else { - forEachChild(node, visitNode); - } + function getModuleName(moduleDeclaration: ModuleDeclaration): string { + // We want to maintain quotation marks. + if (isAmbientModule(moduleDeclaration)) { + return getTextOfNode(moduleDeclaration.name); } - function createNavBarItem(node: Node): NavigationBarItem { - switch (node.kind) { - case SyntaxKind.VariableDeclaration: - // Only add to the navbar if at the top-level of the file - // Note: "const" and "let" are also SyntaxKind.VariableDeclarations - if (node.parent/*VariableDeclarationList*/.parent/*VariableStatement*/ - .parent/*SourceFile*/.kind !== SyntaxKind.SourceFile) { - return undefined; - } - // If it is initialized with a function expression, handle it when we reach the function expression node - const varDecl = node as VariableDeclaration; - if (varDecl.initializer && (varDecl.initializer.kind === SyntaxKind.FunctionExpression || - varDecl.initializer.kind === SyntaxKind.ArrowFunction || - varDecl.initializer.kind === SyntaxKind.ClassExpression)) { - return undefined; - } - // Fall through - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.ClassDeclaration: - case SyntaxKind.Constructor: - case SyntaxKind.GetAccessor: - case SyntaxKind.SetAccessor: - // "export default function().." looks just like a regular function/class declaration, except with the 'default' flag - const name = node.flags && (node.flags & NodeFlags.Default) && !(node as (Declaration)).name ? "default" : - node.kind === SyntaxKind.Constructor ? "constructor" : - declarationNameToString((node as (Declaration)).name); - return getNavBarItem(name, getScriptKindForElementKind(node.kind), [getNodeSpan(node)]); - case SyntaxKind.FunctionExpression: - case SyntaxKind.ArrowFunction: - case SyntaxKind.ClassExpression: - return getDefineModuleItem(node) || getFunctionOrClassExpressionItem(node); - case SyntaxKind.MethodDeclaration: - const methodDecl = node as MethodDeclaration; - return getNavBarItem(declarationNameToString(methodDecl.name), - ScriptElementKind.memberFunctionElement, - [getNodeSpan(node)]); - case SyntaxKind.ExportAssignment: - // e.g. "export default " - return getNavBarItem("default", ScriptElementKind.variableElement, [getNodeSpan(node)]); - case SyntaxKind.ImportClause: // e.g. 'def' in: import def from 'mod' (in ImportDeclaration) - if (!(node as ImportClause).name) { - // No default import (this node is still a parent of named & namespace imports, which are handled below) - return undefined; - } - // fall through - case SyntaxKind.ImportSpecifier: // e.g. 'id' in: import {id} from 'mod' (in NamedImports, in ImportClause) - case SyntaxKind.NamespaceImport: // e.g. '* as ns' in: import * as ns from 'mod' (in ImportClause) - case SyntaxKind.ExportSpecifier: // e.g. 'a' or 'b' in: export {a, foo as b} from 'mod' - // Export specifiers are only interesting if they are reexports from another module, or renamed, else they are already globals - if (node.kind === SyntaxKind.ExportSpecifier) { - if (!(node.parent.parent as ExportDeclaration).moduleSpecifier && !(node as ExportSpecifier).propertyName) { - return undefined; - } - } - const decl = node as (ImportSpecifier | ImportClause | NamespaceImport | ExportSpecifier); - if (!decl.name) { - return undefined; - } - const declName = declarationNameToString(decl.name); - return getNavBarItem(declName, ScriptElementKind.constElement, [getNodeSpan(node)]); - default: - return undefined; - } - } + // Otherwise, we need to aggregate each identifier to build up the qualified name. + const result: string[] = []; - function getNavBarItem(text: string, kind: string, spans: TextSpan[], kindModifiers = ScriptElementKindModifier.none): NavigationBarItem { - return { - text, kind, kindModifiers, spans, childItems: [], indent, bolded: false, grayed: false - }; - } + result.push(moduleDeclaration.name.text); - function getDefineModuleItem(node: Node): NavigationBarItem { - if (node.kind !== SyntaxKind.FunctionExpression && node.kind !== SyntaxKind.ArrowFunction) { - return undefined; - } + while (moduleDeclaration.body && moduleDeclaration.body.kind === SyntaxKind.ModuleDeclaration) { + moduleDeclaration = moduleDeclaration.body; - // No match if this is not a call expression to an identifier named 'define' - if (node.parent.kind !== SyntaxKind.CallExpression) { - return undefined; - } - const callExpr = node.parent as CallExpression; - if (callExpr.expression.kind !== SyntaxKind.Identifier || callExpr.expression.getText() !== "define") { - return undefined; - } - - // Return a module of either the given text in the first argument, or of the source file path - let defaultName = node.getSourceFile().fileName; - if (callExpr.arguments[0].kind === SyntaxKind.StringLiteral) { - defaultName = ((callExpr.arguments[0]) as StringLiteral).text; - } - return getNavBarItem(defaultName, ScriptElementKind.moduleElement, [getNodeSpan(node.parent)]); - } - - function getFunctionOrClassExpressionItem(node: Node): NavigationBarItem { - if (node.kind !== SyntaxKind.FunctionExpression && - node.kind !== SyntaxKind.ArrowFunction && - node.kind !== SyntaxKind.ClassExpression) { - return undefined; - } - - const fnExpr = node as FunctionExpression | ArrowFunction | ClassExpression; - const fnName = getFunctionOrClassName(fnExpr); - const scriptKind = node.kind === SyntaxKind.ClassExpression ? ScriptElementKind.classElement : ScriptElementKind.functionElement; - return getNavBarItem(fnName, scriptKind, [getNodeSpan(node)]); + result.push(moduleDeclaration.name.text); } - function getNodeSpan(node: Node) { - return node.kind === SyntaxKind.SourceFile - ? createTextSpanFromBounds(node.getFullStart(), node.getEnd()) - : createTextSpanFromBounds(node.getStart(), node.getEnd()); - } + return result.join("."); + } - function getScriptKindForElementKind(kind: SyntaxKind) { - switch (kind) { - case SyntaxKind.VariableDeclaration: - return ScriptElementKind.variableElement; - case SyntaxKind.FunctionDeclaration: - return ScriptElementKind.functionElement; - case SyntaxKind.ClassDeclaration: - return ScriptElementKind.classElement; - case SyntaxKind.Constructor: - return ScriptElementKind.constructorImplementationElement; - case SyntaxKind.GetAccessor: - return ScriptElementKind.memberGetAccessorElement; - case SyntaxKind.SetAccessor: - return ScriptElementKind.memberSetAccessorElement; - default: - return "unknown"; - } - } + // For 'module A.B.C', we want to get the node for 'C'. + // We store 'A' as associated with a NavNode, and use getModuleName to traverse down again. + function getInteriorModule(decl: ModuleDeclaration): ModuleDeclaration { + return decl.body.kind === SyntaxKind.ModuleDeclaration ? getInteriorModule(decl.body) : decl; + } - return sourceFileItem.childItems; + function isComputedProperty(member: EnumMember): boolean { + return member.name === undefined || member.name.kind === SyntaxKind.ComputedPropertyName; } - const anonymousFunctionText = ""; - const anonymousClassText = ""; + function getNodeSpan(node: Node) { + return node.kind === SyntaxKind.SourceFile + ? createTextSpanFromBounds(node.getFullStart(), node.getEnd()) + : createTextSpanFromBounds(node.getStart(), node.getEnd()); + } function tryGetName(node: Node): string { const decl = node; @@ -869,7 +465,6 @@ namespace ts.NavigationBar { } } - /** Get the name for a (possibly anonymous) class/function expression. */ function getFunctionOrClassName(node: FunctionExpression | FunctionDeclaration | ArrowFunction | ClassLikeDeclaration): string { if (node.name && getFullWidth(node.name) > 0) { return declarationNameToString(node.name); @@ -896,11 +491,10 @@ namespace ts.NavigationBar { } } - function isAnonymousFunction(item: NavigationBarItem): boolean { - return item.text === anonymousFunctionText; - } - function isFunctionOrClassExpression(node: Node): boolean { return node.kind === SyntaxKind.FunctionExpression || node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.ClassExpression; } + + const anonymousFunctionText = ""; + const anonymousClassText = ""; } diff --git a/src/services/services.ts b/src/services/services.ts index 35fb0887b3b59..8c28c174f3519 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -6867,7 +6867,7 @@ namespace ts { function getNavigationBarItems(fileName: string): NavigationBarItem[] { const sourceFile = syntaxTreeCache.getCurrentSourceFile(fileName); - return NavigationBar.getNavigationBarItems(sourceFile, host.getCompilationSettings()); + return NavigationBar.getNavigationBarItems(sourceFile); } function getSemanticClassifications(fileName: string, span: TextSpan): ClassifiedSpan[] { diff --git a/tests/cases/fourslash/navbar_contains-no-duplicates.ts b/tests/cases/fourslash/navbar_contains-no-duplicates.ts index e259a7bef901f..6bae780a9996a 100644 --- a/tests/cases/fourslash/navbar_contains-no-duplicates.ts +++ b/tests/cases/fourslash/navbar_contains-no-duplicates.ts @@ -95,13 +95,13 @@ verify.navigationBar([ "kindModifiers": "export,declare" }, { - "text": "Test", - "kind": "class", + "text": "B", + "kind": "var", "kindModifiers": "export,declare" }, { - "text": "B", - "kind": "var", + "text": "Test", + "kind": "class", "kindModifiers": "export,declare" }, { diff --git a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts index 8fe12ec94a284..f15959812fc06 100644 --- a/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts +++ b/tests/cases/fourslash/navigationBarAnonymousDeclarationExpressions.ts @@ -144,5 +144,4 @@ verify.navigationBar([ "kind": "class", "indent": 1 } -] -); +]); diff --git a/tests/cases/fourslash/navigationBarGetterAndSetter.ts b/tests/cases/fourslash/navigationBarGetterAndSetter.ts new file mode 100644 index 0000000000000..48105e4e9af3a --- /dev/null +++ b/tests/cases/fourslash/navigationBarGetterAndSetter.ts @@ -0,0 +1,48 @@ +/// + +////class X { +//// get x() {} +//// set x(value) { +//// // Inner declaration should make the setter top-level. +//// function f() {} +//// } +////} + +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "X", + "kind": "class" + } + ] + }, + { + "text": "X", + "kind": "class", + "childItems": [ + { + "text": "x", + "kind": "getter" + }, + { + "text": "x", + "kind": "setter" + } + ], + "indent": 1 + }, + { + "text": "x", + "kind": "setter", + "childItems": [ + { + "text": "f", + "kind": "function" + } + ], + "indent": 2 + } +]); diff --git a/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts b/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts index 2f17a5006ff5f..391b71aac1014 100644 --- a/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts +++ b/tests/cases/fourslash/navigationBarItemsFunctionsBroken.ts @@ -1,7 +1,7 @@ /// ////function f() { -//// function; +//// function; // This is not included as a navigation item, but it causes 'f' to be considered top-level. ////} verify.navigationBar([ diff --git a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts index 28ec25c6f1dd2..3af3b351114b1 100644 --- a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts +++ b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts @@ -76,17 +76,17 @@ verify.navigationBar([ "kind": "property" } ], - "indent": 2 + "indent": 3 }, { "text": "LocalFunctionInConstructor", "kind": "function", - "indent": 2 + "indent": 3 }, { "text": "LocalInterfaceInConstrcutor", "kind": "interface", - "indent": 2 + "indent": 3 }, { "text": "method", @@ -116,7 +116,7 @@ verify.navigationBar([ "kind": "property" } ], - "indent": 2 + "indent": 3 }, { "text": "LocalFunctionInMethod", @@ -127,11 +127,11 @@ verify.navigationBar([ "kind": "function" } ], - "indent": 2 + "indent": 3 }, { "text": "LocalInterfaceInMethod", "kind": "interface", - "indent": 2 + "indent": 3 } ]); diff --git a/tests/cases/fourslash/navigationBarItemsMissingName1.ts b/tests/cases/fourslash/navigationBarItemsMissingName1.ts index 84ee46e9cfad3..2608100ba61b7 100644 --- a/tests/cases/fourslash/navigationBarItemsMissingName1.ts +++ b/tests/cases/fourslash/navigationBarItemsMissingName1.ts @@ -3,7 +3,7 @@ //// foo() {} ////} -verify.navigationBar( [ +verify.navigationBar([ { "text": "\"navigationBarItemsMissingName1\"", "kind": "module", diff --git a/tests/cases/fourslash/navigationBarItemsMissingName2.ts b/tests/cases/fourslash/navigationBarItemsMissingName2.ts index c39185f9dd2d9..a878c5be8455b 100644 --- a/tests/cases/fourslash/navigationBarItemsMissingName2.ts +++ b/tests/cases/fourslash/navigationBarItemsMissingName2.ts @@ -7,25 +7,25 @@ // Anonymous classes are still included. verify.navigationBar([ - { - "text": "", - "kind": "module", - "childItems": [ - { + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "", + "kind": "class" + } + ] + }, + { "text": "", - "kind": "class" - } - ] - }, - { - "text": "", - "kind": "class", - "childItems": [ - { - "text": "foo", - "kind": "method" - } - ], - "indent": 1 - } + "kind": "class", + "childItems": [ + { + "text": "foo", + "kind": "method" + } + ], + "indent": 1 + } ]); diff --git a/tests/cases/fourslash/navigationBarItemsModules.ts b/tests/cases/fourslash/navigationBarItemsModules.ts index 979f22084e587..0548e9838c22a 100644 --- a/tests/cases/fourslash/navigationBarItemsModules.ts +++ b/tests/cases/fourslash/navigationBarItemsModules.ts @@ -28,107 +28,107 @@ //The declarations of A.B.C.x do not get merged, so the 4 vars are independent. //The two 'A' modules, however, do get merged, so in reality we have 7 modules. verify.navigationBar([ - { - "text": "", - "kind": "module", - "childItems": [ - { + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "A.B.C", + "kind": "module" + }, + { + "text": "A.B", + "kind": "module" + }, + { + "text": "A", + "kind": "module" + }, + { + "text": "\"X.Y.Z\"", + "kind": "module", + "kindModifiers": "declare" + }, + { + "text": "'X2.Y2.Z2'", + "kind": "module", + "kindModifiers": "declare" + } + ] + }, + { "text": "A.B.C", - "kind": "module" - }, - { + "kind": "module", + "childItems": [ + { + "text": "x", + "kind": "var", + "kindModifiers": "export" + } + ], + "indent": 1 + }, + { "text": "A.B", - "kind": "module" - }, - { + "kind": "module", + "childItems": [ + { + "text": "y", + "kind": "var", + "kindModifiers": "export" + } + ], + "indent": 1 + }, + { "text": "A", - "kind": "module" - }, - { + "kind": "module", + "childItems": [ + { + "text": "B", + "kind": "module" + }, + { + "text": "z", + "kind": "var", + "kindModifiers": "export" + } + ], + "indent": 1 + }, + { + "text": "B", + "kind": "module", + "childItems": [ + { + "text": "C", + "kind": "module" + } + ], + "indent": 2 + }, + { + "text": "C", + "kind": "module", + "childItems": [ + { + "text": "x", + "kind": "var", + "kindModifiers": "declare" + } + ], + "indent": 3 + }, + { "text": "\"X.Y.Z\"", "kind": "module", - "kindModifiers": "declare" - }, - { + "kindModifiers": "declare", + "indent": 1 + }, + { "text": "'X2.Y2.Z2'", "kind": "module", - "kindModifiers": "declare" - } - ] - }, - { - "text": "A.B.C", - "kind": "module", - "childItems": [ - { - "text": "x", - "kind": "var", - "kindModifiers": "export" - } - ], - "indent": 1 - }, - { - "text": "A.B", - "kind": "module", - "childItems": [ - { - "text": "y", - "kind": "var", - "kindModifiers": "export" - } - ], - "indent": 1 - }, - { - "text": "A", - "kind": "module", - "childItems": [ - { - "text": "z", - "kind": "var", - "kindModifiers": "export" - }, - { - "text": "B", - "kind": "module" - } - ], - "indent": 1 - }, - { - "text": "B", - "kind": "module", - "childItems": [ - { - "text": "C", - "kind": "module" - } - ], - "indent": 2 - }, - { - "text": "C", - "kind": "module", - "childItems": [ - { - "text": "x", - "kind": "var", - "kindModifiers": "declare" - } - ], - "indent": 3 - }, - { - "text": "\"X.Y.Z\"", - "kind": "module", - "kindModifiers": "declare", - "indent": 1 - }, - { - "text": "'X2.Y2.Z2'", - "kind": "module", - "kindModifiers": "declare", - "indent": 1 - } + "kindModifiers": "declare", + "indent": 1 + } ]); diff --git a/tests/cases/fourslash/navigationBarMerging.ts b/tests/cases/fourslash/navigationBarMerging.ts new file mode 100644 index 0000000000000..192efe5db44b5 --- /dev/null +++ b/tests/cases/fourslash/navigationBarMerging.ts @@ -0,0 +1,189 @@ +/// + +// @Filename: file1.ts +////module a { +//// function foo() {} +////} +////module b { +//// function foo() {} +////} +////module a { +//// function bar() {} +////} + +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "a", + "kind": "module" + }, + { + "text": "b", + "kind": "module" + } + ] + }, + { + "text": "a", + "kind": "module", + "childItems": [ + { + "text": "bar", + "kind": "function" + }, + { + "text": "foo", + "kind": "function" + } + ], + "indent": 1 + }, + { + "text": "b", + "kind": "module", + "childItems": [ + { + "text": "foo", + "kind": "function" + } + ], + "indent": 1 + } +]); + +// Does not merge unlike declarations. +// @Filename: file2.ts +////module a {} +////function a() {} + +goTo.file("file2.ts"); +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "a", + "kind": "function" + }, + { + "text": "a", + "kind": "module" + } + ] + }, + { + "text": "a", + "kind": "function", + "indent": 1 + }, + { + "text": "a", + "kind": "module", + "indent": 1 + } +]); + +// Merges recursively +// @Filename: file3.ts +////module a { +//// interface A { +//// foo: number; +//// } +////} +////module a { +//// interface A { +//// bar: number; +//// } +////} + +goTo.file("file3.ts"); +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "a", + "kind": "module" + } + ] + }, + { + "text": "a", + "kind": "module", + "childItems": [ + { + "text": "A", + "kind": "interface" + } + ], + "indent": 1 + }, + { + "text": "A", + "kind": "interface", + "childItems": [ + { + "text": "bar", + "kind": "property" + }, + { + "text": "foo", + "kind": "property" + } + ], + "indent": 2 + } +]); + +// Does not merge 'module A' with 'module A.B' + +// @Filename: file4.ts +////module A { export var x; } +////module A.B { export var y; } + +goTo.file("file4.ts"); +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "A", + "kind": "module" + }, + { + "text": "A.B", + "kind": "module" + } + ] + }, + { + "text": "A", + "kind": "module", + "childItems": [ + { + "text": "x", + "kind": "var", + "kindModifiers": "export" + } + ], + "indent": 1 + }, + { + "text": "A.B", + "kind": "module", + "childItems": [ + { + "text": "y", + "kind": "var", + "kindModifiers": "export" + } + ], + "indent": 1 + } +]); diff --git a/tests/cases/fourslash/navigationBarVariables.ts b/tests/cases/fourslash/navigationBarVariables.ts new file mode 100644 index 0000000000000..e501d50d4c6f4 --- /dev/null +++ b/tests/cases/fourslash/navigationBarVariables.ts @@ -0,0 +1,26 @@ +/// + +////var x = 0; +////let y = 1; +////const z = 2; + +verify.navigationBar([ + { + "text": "", + "kind": "module", + "childItems": [ + { + "text": "x", + "kind": "var" + }, + { + "text": "y", + "kind": "let" + }, + { + "text": "z", + "kind": "const" + } + ] + } +]); From 7944d3393fca25430558cbcdf3de5af0b07bf82a Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 31 May 2016 06:15:24 -0700 Subject: [PATCH 9/9] Clean up and improve performance --- src/compiler/core.ts | 11 ++++++++++ src/services/navigationBar.ts | 40 +++++++++++++++++++---------------- 2 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 79b6249539dbc..ffd10c37b5d29 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -138,6 +138,17 @@ namespace ts { return result; } + export function filterMutate(array: T[], f: (x: T) => boolean): void { + let outIndex = 0; + for (const item of array) { + if (f(item)) { + array[outIndex] = item; + outIndex++; + } + } + array.length = outIndex; + } + export function map(array: T[], f: (x: T) => U): U[] { let result: U[]; if (array) { diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 8906e54cddb99..02c575d3d28a5 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -13,7 +13,7 @@ namespace ts.NavigationBar { */ interface NavNode { node: Node; - additionalNodes?: Node[]; + additionalNodes: Node[]; // May be missing parent: NavNode; // Missing for root decl children: NavNode[]; indent: number; // # of parents @@ -27,19 +27,25 @@ namespace ts.NavigationBar { /** Creates a child node and adds it to parent. */ function createNavNode(parent: NavNode, node: Node): NavNode { - // `item` is set during `convertToItem` - const item: NavNode = { node, parent, children: [], indent: parent ? parent.indent + 1 : 0 }; + const navNode: NavNode = { + node, + additionalNodes: undefined, + parent, + children: [], + indent: + parent ? parent.indent + 1 : 0 + }; if (parent) { - parent.children.push(item); + parent.children.push(navNode); } - addChildren(item); - return item; + addChildren(navNode); + return navNode; } /** Traverse through parent.node's descendants and find declarations to add as parent's children. */ function addChildren(parent: NavNode): void { function recur(node: Node) { - if (isDeclaration(node)) { //TODO:PERF: get rid of this call, just use 1 switch statement. + if (isDeclaration(node)) { switch (node.kind) { case SyntaxKind.Parameter: // Parameter properties handled by SyntaxKind.Constructor case case SyntaxKind.TypeParameter: @@ -119,10 +125,10 @@ namespace ts.NavigationBar { } else { switch (node.kind) { + // TODO: These should probably be added to isDeclaration case SyntaxKind.CallSignature: case SyntaxKind.ConstructSignature: case SyntaxKind.IndexSignature: - case SyntaxKind.MethodSignature: createNavNode(parent, node); break; default: @@ -137,24 +143,23 @@ namespace ts.NavigationBar { } forEachChild(parentNode, recur); - parent.children = mergeChildren(parent.children); + mergeChildren(parent.children); sortChildren(parent.children); } - //TODO: mutate input array, return void /** Merge declarations of the same kind. */ - function mergeChildren(children: NavNode[]): NavNode[] { - const map: Map = {}; - return filter(children, child => { + function mergeChildren(children: NavNode[]): void { + const nameToNavNodes: Map = {}; + filterMutate(children, child => { const decl = child.node; const name = decl.name && decl.name.getText(); if (!name) // Anonymous items are never merged. return true; - const itemsWithSameName = map[name]; + const itemsWithSameName = nameToNavNodes[name]; if (!itemsWithSameName) { - map[name] = [child]; + nameToNavNodes[name] = [child]; return true; } @@ -193,8 +198,8 @@ namespace ts.NavigationBar { target.additionalNodes.push(...source.additionalNodes); } - //TODO:PERF - target.children = mergeChildren(target.children.concat(source.children)); + target.children.push(...source.children); + mergeChildren(target.children); sortChildren(target.children); } } @@ -347,7 +352,6 @@ namespace ts.NavigationBar { }; function convertToChildItem(n: NavNode): NavigationBarItem { - //TODO:PERF const nodes = [n.node]; if (n.additionalNodes) { nodes.push(...n.additionalNodes);