From d6485c9c8fd64afd26eafc7cac9a61061970887d Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Mon, 22 Feb 2016 05:37:07 +0800 Subject: [PATCH 1/7] Adds navigation bar items on methods and constructors --- src/services/navigationBar.ts | 23 ++++++++++- ...ionBarItemsInsideMethodsAndConstructors.ts | 38 +++++++++++++++++++ 2 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index f62c6cb170..24ca051985 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -154,6 +154,16 @@ namespace ts.NavigationBar { for (let node of nodes) { switch (node.kind) { case SyntaxKind.ClassDeclaration: + topLevelNodes.push(node); + forEach((node).members, (node) => { + if (node.kind === SyntaxKind.MethodDeclaration || + node.kind === SyntaxKind.Constructor) { + if ((node).body) { + addTopLevelNodes(((node).body).statements, topLevelNodes); + } + } + }); + break; case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: topLevelNodes.push(node); @@ -193,6 +203,15 @@ namespace ts.NavigationBar { if (!isFunctionBlock(functionDeclaration.parent)) { return true; } + else { + // Except for parent functions that are methods and constructors. + const grandParentKind = functionDeclaration.parent.parent.kind; + if (grandParentKind === SyntaxKind.MethodDeclaration || + grandParentKind === SyntaxKind.Constructor) { + + return true; + } + } } } @@ -407,7 +426,7 @@ namespace ts.NavigationBar { function createModuleItem(node: ModuleDeclaration): NavigationBarItem { let moduleName = getModuleName(node); - + let childItems = getItemsWorker(getChildNodes((getInnermostModule(node).body).statements), createChildItem); return getNavigationBarItem(moduleName, @@ -422,7 +441,7 @@ namespace ts.NavigationBar { if (node.body && node.body.kind === SyntaxKind.Block) { let childItems = getItemsWorker(sortNodes((node.body).statements), createChildItem); - return getNavigationBarItem(!node.name ? "default": node.name.text , + return getNavigationBarItem(!node.name ? "default": node.name.text, ts.ScriptElementKind.functionElement, getNodeModifiers(node), [getNodeSpan(node)], diff --git a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts new file mode 100644 index 0000000000..65e3384e61 --- /dev/null +++ b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts @@ -0,0 +1,38 @@ +/// + +////class Class { +//// constructor() { +//// {| "itemName": "LocalFunctionInConstructor", "kind": "function", "parentName": "Class"|}function LocalFunctionInConstructor() { +//// +//// } +//// +//// {| "itemName": "LocalInterfaceInConstrcutor", "kind": "interface", "parentName": "foo"|}interface LocalInterfaceInConstrcutor { +//// } +//// +//// enum LocalEnumInConstructor { +//// {| "itemName": "LocalEnumMemberInConstructor", "kind": "property", "parentName": "LocalEnumInConstructor"|}LocalEnumMemberInConstructor, +//// } +//// } +//// method() { +//// {| "itemName": "LocalFunctionInMethod", "kind": "function", "parentName": "foo"|}function LocalFunctionInMethod() { +//// {| "itemName": "LocalFunctionInLocalFunctionInMethod", "kind": "function", "parentName": "bar"|}function LocalFunctionInLocalFunctionInMethod() { +//// +//// } +//// } +//// +//// {| "itemName": "LocalInterfaceInMethod", "kind": "interface", "parentName": "foo"|}interface LocalInterfaceInMethod { +//// } +//// +//// enum LocalEnumInMethod { +//// {| "itemName": "LocalEnumMemberInMethod", "kind": "property", "parentName": "foo"|}LocalEnumMemberInMethod, +//// } +//// } +////} + +test.markers().forEach((marker) => { + verify.getScriptLexicalStructureListContains(marker.data.itemName, marker.data.kind, marker.fileName, marker.data.parentName); +}); + +// no other items +verify.getScriptLexicalStructureListCount(12); + From 1a9dadbc034b651e920d1adb7841a204b2fc0a9e Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Mon, 22 Feb 2016 05:39:19 +0800 Subject: [PATCH 2/7] Fixes typo --- src/services/navigationBar.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 24ca051985..ae3e61ba2c 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -198,13 +198,12 @@ namespace ts.NavigationBar { return true; } - // Or if it is not parented by another function. i.e all functions - // at module scope are 'top level'. + // Or if it is not parented by another function(except for parent functions that + // are methods and constructors). I.e all functions at module scope are 'top level'. if (!isFunctionBlock(functionDeclaration.parent)) { return true; } else { - // Except for parent functions that are methods and constructors. const grandParentKind = functionDeclaration.parent.parent.kind; if (grandParentKind === SyntaxKind.MethodDeclaration || grandParentKind === SyntaxKind.Constructor) { From 1b5b146152329e56e7ba0a298eb2cb77b0660afc Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Mon, 22 Feb 2016 05:42:32 +0800 Subject: [PATCH 3/7] Fixes if statement --- src/services/navigationBar.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index ae3e61ba2c..62a9769953 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -156,8 +156,7 @@ namespace ts.NavigationBar { case SyntaxKind.ClassDeclaration: topLevelNodes.push(node); forEach((node).members, (node) => { - if (node.kind === SyntaxKind.MethodDeclaration || - node.kind === SyntaxKind.Constructor) { + if (node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.Constructor) { if ((node).body) { addTopLevelNodes(((node).body).statements, topLevelNodes); } From 4d933f86ce190b1add76d54474700ed5c78863e7 Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Mon, 22 Feb 2016 11:19:38 +0800 Subject: [PATCH 4/7] Fixes method and constructor top-level --- src/services/navigationBar.ts | 30 +++++++++++++++++++ ...ionBarItemsInsideMethodsAndConstructors.ts | 1 - 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 62a9769953..44a7114169 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -158,6 +158,7 @@ namespace ts.NavigationBar { forEach((node).members, (node) => { if (node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.Constructor) { if ((node).body) { + topLevelNodes.push(node); addTopLevelNodes(((node).body).statements, topLevelNodes); } } @@ -386,6 +387,10 @@ namespace ts.NavigationBar { case SyntaxKind.ClassDeclaration: return createClassItem(node); + + case SyntaxKind.MethodDeclaration: + case SyntaxKind.Constructor: + return createMemberFunctionLikeItem(node); case SyntaxKind.EnumDeclaration: return createEnumItem(node); @@ -449,6 +454,31 @@ namespace ts.NavigationBar { return undefined; } + + function createMemberFunctionLikeItem(node: MethodDeclaration | ConstructorDeclaration) { + if (node.body && node.body.kind === SyntaxKind.Block) { + let 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; + } + + return getNavigationBarItem(memberFunctionName, + scriptElementKind, + getNodeModifiers(node), + [getNodeSpan(node)], + childItems, + getIndent(node)); + } + + return undefined; + } function createSourceFileItem(node: SourceFile): ts.NavigationBarItem { let childItems = getItemsWorker(getChildNodes(node.statements), createChildItem); diff --git a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts index 65e3384e61..538e864d10 100644 --- a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts +++ b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts @@ -35,4 +35,3 @@ test.markers().forEach((marker) => { // no other items verify.getScriptLexicalStructureListCount(12); - From fd2d28df0261b6b574cc415e9d47b02df21371eb Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Mon, 22 Feb 2016 12:38:14 +0800 Subject: [PATCH 5/7] Fixes new implementation --- src/services/navigationBar.ts | 23 +++++++++++++------ ...ionBarItemsInsideMethodsAndConstructors.ts | 7 +++++- 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index 44a7114169..b537a56e85 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -157,8 +157,12 @@ namespace ts.NavigationBar { topLevelNodes.push(node); forEach((node).members, (node) => { if (node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.Constructor) { - if ((node).body) { - topLevelNodes.push(node); + type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; + if ((node).body) { + // We do not include methods that does not have child functions in it, because of duplications. + if (hasNonAnonymousFunctionDeclarations(((node).body).statements)) { + topLevelNodes.push(node); + } addTopLevelNodes(((node).body).statements, topLevelNodes); } } @@ -186,14 +190,19 @@ namespace ts.NavigationBar { } } + function hasNonAnonymousFunctionDeclarations(nodes: NodeArray) { + if (forEach(nodes, s => s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text))) { + return true; + } + } + function isTopLevelFunctionDeclaration(functionDeclaration: FunctionLikeDeclaration) { 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 (forEach((functionDeclaration.body).statements, - s => s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text))) { + if (hasNonAnonymousFunctionDeclarations((functionDeclaration.body).statements)) { return true; } @@ -390,13 +399,13 @@ namespace ts.NavigationBar { case SyntaxKind.MethodDeclaration: case SyntaxKind.Constructor: - return createMemberFunctionLikeItem(node); + return createMemberFunctionLikeItem(node); case SyntaxKind.EnumDeclaration: return createEnumItem(node); case SyntaxKind.InterfaceDeclaration: - return createIterfaceItem(node); + return createInterfaceItem(node); case SyntaxKind.ModuleDeclaration: return createModuleItem(node); @@ -540,7 +549,7 @@ namespace ts.NavigationBar { getIndent(node)); } - function createIterfaceItem(node: InterfaceDeclaration): ts.NavigationBarItem { + function createInterfaceItem(node: InterfaceDeclaration): ts.NavigationBarItem { let childItems = getItemsWorker(sortNodes(removeDynamicallyNamedProperties(node)), createChildItem); return getNavigationBarItem( node.name.text, diff --git a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts index 538e864d10..4dcca43af3 100644 --- a/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts +++ b/tests/cases/fourslash/navigationBarItemsInsideMethodsAndConstructors.ts @@ -13,6 +13,7 @@ //// {| "itemName": "LocalEnumMemberInConstructor", "kind": "property", "parentName": "LocalEnumInConstructor"|}LocalEnumMemberInConstructor, //// } //// } +//// //// method() { //// {| "itemName": "LocalFunctionInMethod", "kind": "function", "parentName": "foo"|}function LocalFunctionInMethod() { //// {| "itemName": "LocalFunctionInLocalFunctionInMethod", "kind": "function", "parentName": "bar"|}function LocalFunctionInLocalFunctionInMethod() { @@ -27,6 +28,10 @@ //// {| "itemName": "LocalEnumMemberInMethod", "kind": "property", "parentName": "foo"|}LocalEnumMemberInMethod, //// } //// } +//// +//// emptyMethod() { // Non child functions method should not be duplicated +//// +//// } ////} test.markers().forEach((marker) => { @@ -34,4 +39,4 @@ test.markers().forEach((marker) => { }); // no other items -verify.getScriptLexicalStructureListCount(12); +verify.getScriptLexicalStructureListCount(17); From 99edce09bc1707ff6d41310a1174e85161355eac Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Thu, 3 Mar 2016 13:29:00 +0800 Subject: [PATCH 6/7] Fixes CR feedback --- src/services/navigationBar.ts | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index b537a56e85..2d06177e12 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -155,18 +155,18 @@ namespace ts.NavigationBar { switch (node.kind) { case SyntaxKind.ClassDeclaration: topLevelNodes.push(node); - forEach((node).members, (node) => { - if (node.kind === SyntaxKind.MethodDeclaration || node.kind === SyntaxKind.Constructor) { + for (const member of (node).members) { + if (member.kind === SyntaxKind.MethodDeclaration || member.kind === SyntaxKind.Constructor) { type FunctionLikeMember = MethodDeclaration | ConstructorDeclaration; - if ((node).body) { + if ((member).body) { // We do not include methods that does not have child functions in it, because of duplications. - if (hasNonAnonymousFunctionDeclarations(((node).body).statements)) { - topLevelNodes.push(node); + if (hasNamedFunctionDeclarations(((member).body).statements)) { + topLevelNodes.push(member); } - addTopLevelNodes(((node).body).statements, topLevelNodes); + addTopLevelNodes(((member).body).statements, topLevelNodes); } } - }); + } break; case SyntaxKind.EnumDeclaration: case SyntaxKind.InterfaceDeclaration: @@ -190,7 +190,7 @@ namespace ts.NavigationBar { } } - function hasNonAnonymousFunctionDeclarations(nodes: NodeArray) { + function hasNamedFunctionDeclarations(nodes: NodeArray) { if (forEach(nodes, s => s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text))) { return true; } @@ -202,12 +202,11 @@ namespace ts.NavigationBar { // within it. if (functionDeclaration.body && functionDeclaration.body.kind === SyntaxKind.Block) { // Proper function declarations can only have identifier names - if (hasNonAnonymousFunctionDeclarations((functionDeclaration.body).statements)) { - + if (hasNamedFunctionDeclarations((functionDeclaration.body).statements)) { return true; } - // Or if it is not parented by another function(except for parent functions that + // Or if it is not parented by another function (except for parent functions that // are methods and constructors). I.e all functions at module scope are 'top level'. if (!isFunctionBlock(functionDeclaration.parent)) { return true; From 86b6b6c21bb4fce138888feefe465407f20c3a2a Mon Sep 17 00:00:00 2001 From: Tingan Ho Date: Sat, 26 Mar 2016 17:57:33 +0800 Subject: [PATCH 7/7] Addresses CR feedback --- src/services/navigationBar.ts | 37 ++++++++++++++++++++--------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/src/services/navigationBar.ts b/src/services/navigationBar.ts index b8cada29ce..e9ed70b84c 100644 --- a/src/services/navigationBar.ts +++ b/src/services/navigationBar.ts @@ -130,7 +130,7 @@ namespace ts.NavigationBar { return topLevelNodes; } - + function sortNodes(nodes: Node[]): Node[] { return nodes.slice(0).sort((n1: Declaration, n2: Declaration) => { if (n1.name && n2.name) { @@ -147,7 +147,7 @@ namespace ts.NavigationBar { } }); } - + function addTopLevelNodes(nodes: Node[], topLevelNodes: Node[]): void { nodes = sortNodes(nodes); @@ -190,28 +190,33 @@ namespace ts.NavigationBar { } } - function hasNamedFunctionDeclarations(nodes: NodeArray) { - if (forEach(nodes, s => s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text))) { - return true; + function hasNamedFunctionDeclarations(nodes: NodeArray): boolean { + for (let s of nodes) { + if (s.kind === SyntaxKind.FunctionDeclaration && !isEmpty((s).name.text)) { + return true; + } } + return false; } - function isTopLevelFunctionDeclaration(functionDeclaration: FunctionLikeDeclaration) { + function isTopLevelFunctionDeclaration(functionDeclaration: FunctionLikeDeclaration): boolean { if (functionDeclaration.kind === SyntaxKind.FunctionDeclaration) { - // A function declaration is 'top level' if it contains any function declarations - // within it. + // 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 (except for parent functions that - // are methods and constructors). I.e all functions at module scope are 'top level'. + // 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) { @@ -224,7 +229,7 @@ namespace ts.NavigationBar { return false; } - + function getItemsWorker(nodes: Node[], createItem: (n: Node) => ts.NavigationBarItem): ts.NavigationBarItem[] { let items: ts.NavigationBarItem[] = []; @@ -425,12 +430,12 @@ namespace ts.NavigationBar { let 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("."); } @@ -448,7 +453,7 @@ namespace ts.NavigationBar { getIndent(node)); } - function createFunctionItem(node: FunctionDeclaration) { + function createFunctionItem(node: FunctionDeclaration): ts.NavigationBarItem { if (node.body && node.body.kind === SyntaxKind.Block) { let childItems = getItemsWorker(sortNodes((node.body).statements), createChildItem); @@ -463,7 +468,7 @@ namespace ts.NavigationBar { return undefined; } - function createMemberFunctionLikeItem(node: MethodDeclaration | ConstructorDeclaration) { + function createMemberFunctionLikeItem(node: MethodDeclaration | ConstructorDeclaration): ts.NavigationBarItem { if (node.body && node.body.kind === SyntaxKind.Block) { let childItems = getItemsWorker(sortNodes((node.body).statements), createChildItem); let scriptElementKind: string; @@ -589,4 +594,4 @@ namespace ts.NavigationBar { return getTextOfNodeFromSourceText(sourceFile.text, node); } } -} \ No newline at end of file +}