From 5d15cd22b1623b2f60460ac662c1506c8aa881ed Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 26 Aug 2014 11:38:39 -0700 Subject: [PATCH 01/13] Beginning special casing for getOccurrencesAtPosition. --- src/services/services.ts | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 06e67415ea..f158af5f07 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1933,7 +1933,9 @@ module ts { current = child; continue outer; } - if (child.end > position) break; + if (child.end > position) { + break; + } } return current; } @@ -2160,13 +2162,22 @@ module ts { return undefined; } - if (node.kind !== SyntaxKind.Identifier && - !isLiteralNameOfPropertyDeclarationOrIndexAccess(node) && - !isNameOfExternalModuleImportOrDeclaration(node)) { - return undefined; + if (node.kind === SyntaxKind.Identifier || + isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || + isNameOfExternalModuleImportOrDeclaration(node)) { + return getReferencesForNode(node, [sourceFile]); } - return getReferencesForNode(node, [sourceFile]); + switch (node.kind) { + } + + return undefined; + + function keywordsToReferenceEntries(keywords: Node[]): ReferenceEntry[]{ + return keywords.map(keyword => + new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(keyword.getStart(), keyword.end), /* isWriteAccess */ false) + ); + } } function getReferencesAtPosition(filename: string, position: number): ReferenceEntry[] { @@ -2284,13 +2295,13 @@ module ts { var container = getContainerNode(declarations[i]); if (scope && scope !== container) { - // Diffrent declarations have diffrent containers, bail out + // Different declarations have different containers, bail out return undefined; } if (container.kind === SyntaxKind.SourceFile && !isExternalModule(container)) { // This is a global variable and not an external module, any declaration defined - // withen this scope is visible outside the file + // within this scope is visible outside the file return undefined; } From 558be4ea22be84cdbed99e563b84359e0c49c718 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 26 Aug 2014 11:48:31 -0700 Subject: [PATCH 02/13] Implemented getOccsAtPos for try-catch-finally. --- src/services/services.ts | 20 ++++++++++++++ .../getOccurrencesTryCatchFinally.ts | 27 +++++++++++++++++++ .../getOccurrencesTryCatchFinally2.ts | 27 +++++++++++++++++++ .../getOccurrencesTryCatchFinally3.ts | 27 +++++++++++++++++++ 4 files changed, 101 insertions(+) create mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinally.ts create mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinally2.ts create mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinally3.ts diff --git a/src/services/services.ts b/src/services/services.ts index f158af5f07..663c3903c2 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2169,10 +2169,30 @@ module ts { } switch (node.kind) { + case SyntaxKind.TryKeyword: + case SyntaxKind.CatchKeyword: + case SyntaxKind.FinallyKeyword: + return getTryCatchFinallyOccurrences(node.parent.parent); } return undefined; + function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { + var keywords: Node[] = []; + + keywords.push(tryStatement.getFirstToken()) + + if (tryStatement.catchBlock) { + keywords.push(tryStatement.catchBlock.getFirstToken()); + } + + if (tryStatement.finallyBlock) { + keywords.push(tryStatement.finallyBlock.getFirstToken()); + } + + return keywordsToReferenceEntries(keywords); + } + function keywordsToReferenceEntries(keywords: Node[]): ReferenceEntry[]{ return keywords.map(keyword => new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(keyword.getStart(), keyword.end), /* isWriteAccess */ false) diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinally.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinally.ts new file mode 100644 index 0000000000..9037b6f802 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesTryCatchFinally.ts @@ -0,0 +1,27 @@ +/// + +/////*1*/[|try|] { +//// try { +//// } +//// catch (x) { +//// } +//// +//// try { +//// } +//// finally { +//// } +////} +////[|cat/*2*/ch|] (e) { +////} +////[|fina/*3*/lly|] { +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(3); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +} diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinally2.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinally2.ts new file mode 100644 index 0000000000..5d71b133c2 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesTryCatchFinally2.ts @@ -0,0 +1,27 @@ +/// + +////try { +//// [|t/*1*/r/*2*/y|] { +//// } +//// [|c/*3*/atch|] (x) { +//// } +//// +//// try { +//// } +//// finally { +//// } +////} +////catch (e) { +////} +////finally { +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(2); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +} \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinally3.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinally3.ts new file mode 100644 index 0000000000..f2a7936936 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesTryCatchFinally3.ts @@ -0,0 +1,27 @@ +/// + +////try { +//// try { +//// } +//// catch (x) { +//// } +//// +//// [|t/*1*/r/*2*/y|] { +//// } +//// [|finall/*3*/y|] { +//// } +////} +////catch (e) { +////} +////finally { +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(2); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +} \ No newline at end of file From 1f77198c4c4072a7552f9e860eac4c1584272dad Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 26 Aug 2014 12:27:54 -0700 Subject: [PATCH 03/13] Made getOccs more resilient. --- src/compiler/parser.ts | 33 ++++++++++++++++----------------- src/services/services.ts | 20 +++++++++++++++----- 2 files changed, 31 insertions(+), 22 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 4e1b737b40..1e957cd2ab 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -483,6 +483,22 @@ module ts { nodeIsNestedInLabel(label: Identifier, requireIterationStatement: boolean, stopAtFunctionBoundary: boolean): ControlBlockContext; } + export function isKeyword(token: SyntaxKind): boolean { + return SyntaxKind.FirstKeyword <= token && token <= SyntaxKind.LastKeyword; + } + + export function isModifier(token: SyntaxKind): boolean { + switch (token) { + case SyntaxKind.PublicKeyword: + case SyntaxKind.PrivateKeyword: + case SyntaxKind.StaticKeyword: + case SyntaxKind.ExportKeyword: + case SyntaxKind.DeclareKeyword: + return true; + } + return false; + } + export function createSourceFile(filename: string, sourceText: string, languageVersion: ScriptTarget, version: string, isOpen: boolean = false): SourceFile { var file: SourceFile; var scanner: Scanner; @@ -853,23 +869,6 @@ module ts { return parseIdentifierName(); } - - function isKeyword(token: SyntaxKind): boolean { - return SyntaxKind.FirstKeyword <= token && token <= SyntaxKind.LastKeyword; - } - - function isModifier(token: SyntaxKind): boolean { - switch (token) { - case SyntaxKind.PublicKeyword: - case SyntaxKind.PrivateKeyword: - case SyntaxKind.StaticKeyword: - case SyntaxKind.ExportKeyword: - case SyntaxKind.DeclareKeyword: - return true; - } - return false; - } - function parseContextualModifier(t: SyntaxKind): boolean { return token === t && tryParse(() => { nextToken(); diff --git a/src/services/services.ts b/src/services/services.ts index 663c3903c2..a8935c463a 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2172,27 +2172,37 @@ module ts { case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: - return getTryCatchFinallyOccurrences(node.parent.parent); + return getTryCatchFinallyOccurrences((node.parent && node.parent.parent)); } return undefined; - function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { + function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[]{ + if (!tryStatement || tryStatement.kind !== SyntaxKind.TryStatement) { + return undefined; + } + var keywords: Node[] = []; - keywords.push(tryStatement.getFirstToken()) + pushIfKeyword(keywords, tryStatement.getFirstToken()); if (tryStatement.catchBlock) { - keywords.push(tryStatement.catchBlock.getFirstToken()); + pushIfKeyword(keywords, tryStatement.catchBlock.getFirstToken()); } if (tryStatement.finallyBlock) { - keywords.push(tryStatement.finallyBlock.getFirstToken()); + pushIfKeyword(keywords, tryStatement.finallyBlock.getFirstToken()); } return keywordsToReferenceEntries(keywords); } + function pushIfKeyword(keywordList: Node[], token: Node) { + if (token && isKeyword(token.kind)) { + keywordList.push(token); + } + } + function keywordsToReferenceEntries(keywords: Node[]): ReferenceEntry[]{ return keywords.map(keyword => new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(keyword.getStart(), keyword.end), /* isWriteAccess */ false) From 8ab4df0e256cf76df8e698e60f4b9abe2fdeaef7 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 26 Aug 2014 14:18:09 -0700 Subject: [PATCH 04/13] Added tests. --- .../getOccurrencesTryCatchFinallyBroken.ts | 55 +++++++++++++++++++ .../getOccurrencesTryCatchFinallyNegatives.ts | 23 ++++++++ 2 files changed, 78 insertions(+) create mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinallyBroken.ts create mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinallyBroken.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinallyBroken.ts new file mode 100644 index 0000000000..c866ed9639 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesTryCatchFinallyBroken.ts @@ -0,0 +1,55 @@ +/// + +////t /*1*/ry { +//// t/*2*/ry { +//// } +//// ctch (x) { +//// } +//// +//// tr { +//// } +//// fin/*3*/ally { +//// } +////} +////c/*4*/atch (e) { +////} +////f/*5*/inally { +////} +//// +////// Missing catch variable +////t/*6*/ry { +////} +////catc/*7*/h { +////} +/////*8*/finally { +////} +//// +////// Missing try entirely +////cat/*9*/ch (x) { +////} +////final/*10*/ly { +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + + switch (i) { + case 1: + case 2: + case 3: + verify.occurrencesAtPositionCount(1); + break; + case 4: + case 5: + case 9: + case 10: + verify.occurrencesAtPositionCount(2); + break; + case 6: + case 7: + case 8: + verify.occurrencesAtPositionCount(3); + break; + } +} \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts new file mode 100644 index 0000000000..d5889ff84e --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts @@ -0,0 +1,23 @@ +/// + +////try/*1*/ { +//// try/*2*/ { +//// } +//// catch/*3*/ (x) { +//// } +//// +//// try/*4*/ { +//// } +//// finally/*5*/ {/*8*/ +//// } +////} +////catch/*6*/ (e) { +////} +////finally/*7*/ { +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(0); +} \ No newline at end of file From 88f37e579cc495feb6ee7a2f6d593695bf8bd038 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 26 Aug 2014 17:24:11 -0700 Subject: [PATCH 05/13] Support for switch/case/default/break in getOccs --- src/services/services.ts | 86 +++++++++++++++++-- .../getOccurrencesSwitchCaseDefault.ts | 29 +++++++ .../getOccurrencesSwitchCaseDefault2.ts | 29 +++++++ .../getOccurrencesSwitchCaseDefaultBroken.ts | 47 ++++++++++ ...etOccurrencesSwitchCaseDefaultNegatives.ts | 25 ++++++ 5 files changed, 210 insertions(+), 6 deletions(-) create mode 100644 tests/cases/fourslash/getOccurrencesSwitchCaseDefault.ts create mode 100644 tests/cases/fourslash/getOccurrencesSwitchCaseDefault2.ts create mode 100644 tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts create mode 100644 tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts diff --git a/src/services/services.ts b/src/services/services.ts index a8935c463a..0be021a32a 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2162,22 +2162,35 @@ module ts { return undefined; } - if (node.kind === SyntaxKind.Identifier || - isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || - isNameOfExternalModuleImportOrDeclaration(node)) { + if (node.kind === SyntaxKind.Identifier || isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || isNameOfExternalModuleImportOrDeclaration(node)) { return getReferencesForNode(node, [sourceFile]); } + var result: ReferenceEntry[]; + + // Each of these helper functions bails out if the node is undefined, + // which is why you'll see much of this'node.parent && node.parent.parent' pattern. switch (node.kind) { case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: - return getTryCatchFinallyOccurrences((node.parent && node.parent.parent)); + result = getTryCatchFinallyOccurrences((node.parent && node.parent.parent)); + break; + case SyntaxKind.SwitchKeyword: + result = getSwitchCaseDefaultOccurrences(node.parent); + break; + case SyntaxKind.CaseKeyword: + case SyntaxKind.DefaultKeyword: + result = getSwitchCaseDefaultOccurrences((node.parent && node.parent.parent)); + break; + case SyntaxKind.BreakKeyword: + result = getBreakStatementOccurences(node.parent); + } - return undefined; + return result; - function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[]{ + function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { if (!tryStatement || tryStatement.kind !== SyntaxKind.TryStatement) { return undefined; } @@ -2197,6 +2210,67 @@ module ts { return keywordsToReferenceEntries(keywords); } + function getSwitchCaseDefaultOccurrences(switchStatement: SwitchStatement) { + if (!switchStatement || switchStatement.kind !== SyntaxKind.SwitchStatement) { + return undefined; + } + + var keywords: Node[] = []; + + pushIfKeyword(keywords, switchStatement.getFirstToken()); + + // Go through each clause in the switch statement, collecting the clause keywords. + switchStatement.clauses.forEach(clause => { + pushIfKeyword(keywords, clause.getFirstToken()); + + // For each clause, also recursively traverse the statements where we can find analogous breaks. + forEachChild(clause, function aggregateBreakKeywords(node: Node): void { + switch (node.kind) { + case SyntaxKind.BreakStatement: + // If the break statement has a label, cannot be part of + if (!(node).label) { + pushIfKeyword(keywords, node.getFirstToken()); + } + // Fall through + case SyntaxKind.ForStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.DoStatement: + case SyntaxKind.WhileStatement: + case SyntaxKind.SwitchStatement: + return; + } + + forEachChild(node, aggregateBreakKeywords); + }); + }); + + return keywordsToReferenceEntries(keywords); + } + + function getBreakStatementOccurences(breakStatement: BreakOrContinueStatement): ReferenceEntry[]{ + if (!breakStatement || breakStatement.kind !== SyntaxKind.BreakStatement) { + return undefined; + } + + // TODO (drosen): Deal with labeled statements. + if (breakStatement.label) { + return undefined; + } + + for (var owner = node.parent; owner; owner = owner.parent) { + switch (owner.kind) { + case SyntaxKind.ForStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.DoStatement: + case SyntaxKind.WhileStatement: + // TODO (drosen): Handle loops! + return undefined; + case SyntaxKind.SwitchStatement: + return getSwitchCaseDefaultOccurrences(owner); + } + } + } + function pushIfKeyword(keywordList: Node[], token: Node) { if (token && isKeyword(token.kind)) { keywordList.push(token); diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefault.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefault.ts new file mode 100644 index 0000000000..a3d51e642d --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSwitchCaseDefault.ts @@ -0,0 +1,29 @@ +/// + +////[|sw/*1*/itch|] (10) { +//// [|/*2*/case|] 1: +//// [|cas/*3*/e|] 2: +//// [|c/*4*/ase|] 4: +//// [|c/*5*/ase|] 8: +//// foo: switch (20) { +//// case 1: +//// case 2: +//// break; +//// default: +//// break foo; +//// } +//// [|cas/*6*/e|] 0xBEEF: +//// [|defa/*7*/ult|]: +//// [|bre/*9*/ak|]; +//// [|/*8*/case|] 16: +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(9); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +} diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefault2.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefault2.ts new file mode 100644 index 0000000000..a8c91f530c --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSwitchCaseDefault2.ts @@ -0,0 +1,29 @@ +/// + +////switch (10) { +//// case 1: +//// case 2: +//// case 4: +//// case 8: +//// foo: [|swi/*1*/tch|] (20) { +//// [|/*2*/case|] 1: +//// [|cas/*3*/e|] 2: +//// [|b/*4*/reak|]; +//// [|defaul/*5*/t|]: +//// break foo; +//// } +//// case 0xBEEF: +//// default: +//// break; +//// case 16: +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(5); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +} diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts new file mode 100644 index 0000000000..9ce6d1ffb8 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts @@ -0,0 +1,47 @@ +/// + +////swi/*1*/tch(10) { +//// case 1: +//// case 2: +//// c/*2*/ase 4: +//// case 8: +//// case 0xBEEF: +//// de/*4*/fult: +//// break; +//// /*5*/cas 16: +//// c/*3*/ase 12: +////} + +////sw/*6*/itch (10) { +//// de/*7*/fault +//// case 1: +//// case 2 + +//// c/*8*/ose 4: +//// case 8: +//// case 0xBEEF: +//// bre/*9*/ak; +//// case 16: +////} + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + + switch (i) { + case 1: + case 2: + case 3: + verify.occurrencesAtPositionCount(8); + break; + case 4: + case 5: + case 8: + verify.occurrencesAtPositionCount(1); + break; + case 6: + case 7: + case 9: + verify.occurrencesAtPositionCount(8); + break; + } +} \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts new file mode 100644 index 0000000000..cfb70b0c11 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts @@ -0,0 +1,25 @@ +/// + +////switch/*1*/ (10) { +//// case/*2*/ 1: +//// case/*3*/ 2: +//// case/*4*/ 4: +//// case/*5*/ 8: +//// foo: switch/*6*/ (20) { +//// case/*7*/ 1: +//// case/*8*/ 2: +//// break/*9*/; +//// default/*10*/: +//// break foo; +//// } +//// case/*11*/ 0xBEEF: +//// default/*12*/: +//// break/*13*/; +//// case 16/*14*/: +////} + + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + verify.occurrencesAtPositionCount(0); +} From 3825c9b542751aa8bd7bd6362da8cd856ee0304d Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 12:12:45 -0700 Subject: [PATCH 06/13] Handled function boundaries. --- src/services/services.ts | 13 +++++++++++-- .../getOccurrencesSwitchCaseDefaultBroken.ts | 14 ++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 0be021a32a..5a76b81eed 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2169,7 +2169,7 @@ module ts { var result: ReferenceEntry[]; // Each of these helper functions bails out if the node is undefined, - // which is why you'll see much of this'node.parent && node.parent.parent' pattern. + // which is why you'll see much of this 'node.parent && node.parent.parent' pattern. switch (node.kind) { case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: @@ -2237,6 +2237,9 @@ module ts { case SyntaxKind.DoStatement: case SyntaxKind.WhileStatement: case SyntaxKind.SwitchStatement: + case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ArrowFunction: return; } @@ -2256,7 +2259,7 @@ module ts { if (breakStatement.label) { return undefined; } - + for (var owner = node.parent; owner; owner = owner.parent) { switch (owner.kind) { case SyntaxKind.ForStatement: @@ -2265,10 +2268,16 @@ module ts { case SyntaxKind.WhileStatement: // TODO (drosen): Handle loops! return undefined; + case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ArrowFunction: + return undefined; case SyntaxKind.SwitchStatement: return getSwitchCaseDefaultOccurrences(owner); } } + + return undefined; } function pushIfKeyword(keywordList: Node[], token: Node) { diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts index 9ce6d1ffb8..018280a403 100644 --- a/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts +++ b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultBroken.ts @@ -10,18 +10,23 @@ //// break; //// /*5*/cas 16: //// c/*3*/ase 12: +//// function f() { +//// br/*11*/eak; +//// /*12*/break; +//// } ////} - +//// ////sw/*6*/itch (10) { //// de/*7*/fault //// case 1: //// case 2 - +//// //// c/*8*/ose 4: //// case 8: //// case 0xBEEF: //// bre/*9*/ak; //// case 16: +//// () => bre/*10*/ak; ////} for (var i = 1; i <= test.markers().length; i++) { @@ -43,5 +48,10 @@ for (var i = 1; i <= test.markers().length; i++) { case 9: verify.occurrencesAtPositionCount(8); break; + case 10: + case 11: + case 12: + verify.occurrencesAtPositionCount(0); + break; } } \ No newline at end of file From 232e51383f135144cf53713886f52d07a2c0d9d1 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 16:07:46 -0700 Subject: [PATCH 07/13] Moved null-guards to appropriate places, added helpers. --- src/services/services.ts | 68 +++++++++++++++++++++++++--------------- 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 5a76b81eed..5ec359724c 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2174,62 +2174,62 @@ module ts { case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: - result = getTryCatchFinallyOccurrences((node.parent && node.parent.parent)); + if (hasKind(parent(parent(node)), SyntaxKind.TryStatement)) { + result = getTryCatchFinallyOccurrences((node.parent.parent)); + } break; case SyntaxKind.SwitchKeyword: - result = getSwitchCaseDefaultOccurrences(node.parent); + if (hasKind(node.parent, SyntaxKind.SwitchStatement)) { + result = getSwitchCaseDefaultOccurrences(node.parent); + } break; case SyntaxKind.CaseKeyword: case SyntaxKind.DefaultKeyword: - result = getSwitchCaseDefaultOccurrences((node.parent && node.parent.parent)); + if (hasKind(parent(parent(node)), SyntaxKind.SwitchStatement)) { + result = getSwitchCaseDefaultOccurrences((node.parent.parent)); + } break; case SyntaxKind.BreakKeyword: - result = getBreakStatementOccurences(node.parent); - + if (hasKind(node.parent, SyntaxKind.BreakStatement)) { + result = getBreakStatementOccurences(node.parent); + } + break; } return result; function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { - if (!tryStatement || tryStatement.kind !== SyntaxKind.TryStatement) { - return undefined; - } - var keywords: Node[] = []; - pushIfKeyword(keywords, tryStatement.getFirstToken()); + pushIfKeyword(keywords, tryStatement.getFirstToken(), SyntaxKind.TryKeyword); if (tryStatement.catchBlock) { - pushIfKeyword(keywords, tryStatement.catchBlock.getFirstToken()); + pushIfKeyword(keywords, tryStatement.catchBlock.getFirstToken(), SyntaxKind.CatchKeyword); } if (tryStatement.finallyBlock) { - pushIfKeyword(keywords, tryStatement.finallyBlock.getFirstToken()); + pushIfKeyword(keywords, tryStatement.finallyBlock.getFirstToken(), SyntaxKind.FinallyKeyword); } return keywordsToReferenceEntries(keywords); } function getSwitchCaseDefaultOccurrences(switchStatement: SwitchStatement) { - if (!switchStatement || switchStatement.kind !== SyntaxKind.SwitchStatement) { - return undefined; - } - var keywords: Node[] = []; - pushIfKeyword(keywords, switchStatement.getFirstToken()); + pushIfKeyword(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword); // Go through each clause in the switch statement, collecting the clause keywords. switchStatement.clauses.forEach(clause => { - pushIfKeyword(keywords, clause.getFirstToken()); + pushIfKeyword(keywords, clause.getFirstToken(), [SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword]); // For each clause, also recursively traverse the statements where we can find analogous breaks. forEachChild(clause, function aggregateBreakKeywords(node: Node): void { switch (node.kind) { case SyntaxKind.BreakStatement: - // If the break statement has a label, cannot be part of + // If the break statement has a label, it cannot be part of a switch block. if (!(node).label) { - pushIfKeyword(keywords, node.getFirstToken()); + pushIfKeyword(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword); } // Fall through case SyntaxKind.ForStatement: @@ -2251,10 +2251,6 @@ module ts { } function getBreakStatementOccurences(breakStatement: BreakOrContinueStatement): ReferenceEntry[]{ - if (!breakStatement || breakStatement.kind !== SyntaxKind.BreakStatement) { - return undefined; - } - // TODO (drosen): Deal with labeled statements. if (breakStatement.label) { return undefined; @@ -2280,10 +2276,30 @@ module ts { return undefined; } - function pushIfKeyword(keywordList: Node[], token: Node) { - if (token && isKeyword(token.kind)) { + // returns true if 'node' is defined and has a matching 'kind'. + function hasKind(node: Node, kind: SyntaxKind) { + return !!(node && node.kind === kind); + } + + // Null-propagating 'parent' function. + function parent(node: Node): Node { + return node && node.parent; + } + + function pushIfKeyword(keywordList: Node[], token: Node, expected: SyntaxKind): void; + function pushIfKeyword(keywordList: Node[], token: Node, expected: SyntaxKind[]): void; + function pushIfKeyword(keywordList: Node[], token: Node, expected: any): void { + if (!token) { + return; + } + + if (token.kind === expected || + (expected.length && (expected).some(expectedKind => expectedKind === token.kind))) { keywordList.push(token); } + else { + Debug.assert("Expected keyword, got " + token.getFullText().substring(token.getLeadingTriviaWidth())); + } } function keywordsToReferenceEntries(keywords: Node[]): ReferenceEntry[]{ From 50d0cdc42824495c24d8921430bf39ab294ea6c8 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 16:33:35 -0700 Subject: [PATCH 08/13] Better coverage against function boundaries. --- src/services/services.ts | 39 ++++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 5ec359724c..8acf069d21 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2168,8 +2168,7 @@ module ts { var result: ReferenceEntry[]; - // Each of these helper functions bails out if the node is undefined, - // which is why you'll see much of this 'node.parent && node.parent.parent' pattern. + // 'parent' and 'hasKind' are falsey-propagating convenience functions. switch (node.kind) { case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: @@ -2201,14 +2200,14 @@ module ts { function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { var keywords: Node[] = []; - pushIfKeyword(keywords, tryStatement.getFirstToken(), SyntaxKind.TryKeyword); + pushKeyword(keywords, tryStatement.getFirstToken(), SyntaxKind.TryKeyword); if (tryStatement.catchBlock) { - pushIfKeyword(keywords, tryStatement.catchBlock.getFirstToken(), SyntaxKind.CatchKeyword); + pushKeyword(keywords, tryStatement.catchBlock.getFirstToken(), SyntaxKind.CatchKeyword); } if (tryStatement.finallyBlock) { - pushIfKeyword(keywords, tryStatement.finallyBlock.getFirstToken(), SyntaxKind.FinallyKeyword); + pushKeyword(keywords, tryStatement.finallyBlock.getFirstToken(), SyntaxKind.FinallyKeyword); } return keywordsToReferenceEntries(keywords); @@ -2217,11 +2216,11 @@ module ts { function getSwitchCaseDefaultOccurrences(switchStatement: SwitchStatement) { var keywords: Node[] = []; - pushIfKeyword(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword); + pushKeyword(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword); // Go through each clause in the switch statement, collecting the clause keywords. switchStatement.clauses.forEach(clause => { - pushIfKeyword(keywords, clause.getFirstToken(), [SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword]); + pushKeyword(keywords, clause.getFirstToken(), [SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword]); // For each clause, also recursively traverse the statements where we can find analogous breaks. forEachChild(clause, function aggregateBreakKeywords(node: Node): void { @@ -2229,7 +2228,7 @@ module ts { case SyntaxKind.BreakStatement: // If the break statement has a label, it cannot be part of a switch block. if (!(node).label) { - pushIfKeyword(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword); + pushKeyword(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword); } // Fall through case SyntaxKind.ForStatement: @@ -2237,13 +2236,13 @@ module ts { case SyntaxKind.DoStatement: case SyntaxKind.WhileStatement: case SyntaxKind.SwitchStatement: - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.ArrowFunction: return; } - forEachChild(node, aggregateBreakKeywords); + // Do not cross function boundaries. + if (!isAnyFunction(node)) { + forEachChild(node, aggregateBreakKeywords); + } }); }); @@ -2264,12 +2263,14 @@ module ts { case SyntaxKind.WhileStatement: // TODO (drosen): Handle loops! return undefined; - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.ArrowFunction: - return undefined; + case SyntaxKind.SwitchStatement: return getSwitchCaseDefaultOccurrences(owner); + + default: + if (isAnyFunction(owner)) { + return undefined; + } } } @@ -2286,9 +2287,9 @@ module ts { return node && node.parent; } - function pushIfKeyword(keywordList: Node[], token: Node, expected: SyntaxKind): void; - function pushIfKeyword(keywordList: Node[], token: Node, expected: SyntaxKind[]): void; - function pushIfKeyword(keywordList: Node[], token: Node, expected: any): void { + function pushKeyword(keywordList: Node[], token: Node, expected: SyntaxKind): void; + function pushKeyword(keywordList: Node[], token: Node, expected: SyntaxKind[]): void; + function pushKeyword(keywordList: Node[], token: Node, expected: any): void { if (!token) { return; } From ea613fd0d8a92dd70b054752da6a6a420f045a8d Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 16:39:05 -0700 Subject: [PATCH 09/13] Replaced ES5 functions with analogous core.ts ones. --- src/services/services.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 8acf069d21..344acccded 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2219,7 +2219,7 @@ module ts { pushKeyword(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword); // Go through each clause in the switch statement, collecting the clause keywords. - switchStatement.clauses.forEach(clause => { + forEach(switchStatement.clauses, clause => { pushKeyword(keywords, clause.getFirstToken(), [SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword]); // For each clause, also recursively traverse the statements where we can find analogous breaks. @@ -2294,8 +2294,7 @@ module ts { return; } - if (token.kind === expected || - (expected.length && (expected).some(expectedKind => expectedKind === token.kind))) { + if (token.kind === expected || (expected.length && contains(expected, token.kind))) { keywordList.push(token); } else { @@ -2304,7 +2303,7 @@ module ts { } function keywordsToReferenceEntries(keywords: Node[]): ReferenceEntry[]{ - return keywords.map(keyword => + return map(keywords, keyword => new ReferenceEntry(filename, TypeScript.TextSpan.fromBounds(keyword.getStart(), keyword.end), /* isWriteAccess */ false) ); } From 04456a261aaf54aa7dceb3b75b7a4c6e486abf95 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 16:58:59 -0700 Subject: [PATCH 10/13] Made 'isAnyFunction' more exhaustive as it should be. --- src/services/services.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index 344acccded..b1fc4d4884 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1314,12 +1314,13 @@ module ts { function isAnyFunction(node: Node): boolean { switch (node.kind) { - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.Method: case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ArrowFunction: + case SyntaxKind.Method: case SyntaxKind.GetAccessor: case SyntaxKind.SetAccessor: - case SyntaxKind.ArrowFunction: + case SyntaxKind.Constructor: return true; } return false; From 0ce39a3c2ae441a537ace1952856de77fb4286d2 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 17:19:08 -0700 Subject: [PATCH 11/13] Addressed CR feedback. --- src/services/services.ts | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index b1fc4d4884..d1ddf70370 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2167,36 +2167,33 @@ module ts { return getReferencesForNode(node, [sourceFile]); } - var result: ReferenceEntry[]; - - // 'parent' and 'hasKind' are falsey-propagating convenience functions. switch (node.kind) { case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: if (hasKind(parent(parent(node)), SyntaxKind.TryStatement)) { - result = getTryCatchFinallyOccurrences((node.parent.parent)); + return getTryCatchFinallyOccurrences(node.parent.parent); } break; case SyntaxKind.SwitchKeyword: if (hasKind(node.parent, SyntaxKind.SwitchStatement)) { - result = getSwitchCaseDefaultOccurrences(node.parent); + return getSwitchCaseDefaultOccurrences(node.parent); } break; case SyntaxKind.CaseKeyword: case SyntaxKind.DefaultKeyword: if (hasKind(parent(parent(node)), SyntaxKind.SwitchStatement)) { - result = getSwitchCaseDefaultOccurrences((node.parent.parent)); + return getSwitchCaseDefaultOccurrences(node.parent.parent); } break; case SyntaxKind.BreakKeyword: if (hasKind(node.parent, SyntaxKind.BreakStatement)) { - result = getBreakStatementOccurences(node.parent); + return getBreakStatementOccurences(node.parent); } break; } - return result; + return undefined; function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { var keywords: Node[] = []; @@ -2221,7 +2218,7 @@ module ts { // Go through each clause in the switch statement, collecting the clause keywords. forEach(switchStatement.clauses, clause => { - pushKeyword(keywords, clause.getFirstToken(), [SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword]); + pushKeyword(keywords, clause.getFirstToken(), SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword); // For each clause, also recursively traverse the statements where we can find analogous breaks. forEachChild(clause, function aggregateBreakKeywords(node: Node): void { @@ -2288,18 +2285,16 @@ module ts { return node && node.parent; } - function pushKeyword(keywordList: Node[], token: Node, expected: SyntaxKind): void; - function pushKeyword(keywordList: Node[], token: Node, expected: SyntaxKind[]): void; - function pushKeyword(keywordList: Node[], token: Node, expected: any): void { + function pushKeyword(keywordList: Node[], token: Node, ...expected: SyntaxKind[]): void { if (!token) { return; } - if (token.kind === expected || (expected.length && contains(expected, token.kind))) { + if (contains(expected, token.kind)) { keywordList.push(token); } else { - Debug.assert("Expected keyword, got " + token.getFullText().substring(token.getLeadingTriviaWidth())); + Debug.assert("Got '" + token.getFullText().substring(token.getLeadingTriviaWidth()) + "' instead of expected keyword."); } } From fd93a3b4f7868fafdaaa8c50c31364fc00405cd1 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 17:26:58 -0700 Subject: [PATCH 12/13] What's in a name anyhow? --- src/services/services.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index d1ddf70370..ca15d8a46b 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2198,14 +2198,14 @@ module ts { function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { var keywords: Node[] = []; - pushKeyword(keywords, tryStatement.getFirstToken(), SyntaxKind.TryKeyword); + pushKeywordIf(keywords, tryStatement.getFirstToken(), SyntaxKind.TryKeyword); if (tryStatement.catchBlock) { - pushKeyword(keywords, tryStatement.catchBlock.getFirstToken(), SyntaxKind.CatchKeyword); + pushKeywordIf(keywords, tryStatement.catchBlock.getFirstToken(), SyntaxKind.CatchKeyword); } if (tryStatement.finallyBlock) { - pushKeyword(keywords, tryStatement.finallyBlock.getFirstToken(), SyntaxKind.FinallyKeyword); + pushKeywordIf(keywords, tryStatement.finallyBlock.getFirstToken(), SyntaxKind.FinallyKeyword); } return keywordsToReferenceEntries(keywords); @@ -2214,11 +2214,11 @@ module ts { function getSwitchCaseDefaultOccurrences(switchStatement: SwitchStatement) { var keywords: Node[] = []; - pushKeyword(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword); + pushKeywordIf(keywords, switchStatement.getFirstToken(), SyntaxKind.SwitchKeyword); // Go through each clause in the switch statement, collecting the clause keywords. forEach(switchStatement.clauses, clause => { - pushKeyword(keywords, clause.getFirstToken(), SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword); + pushKeywordIf(keywords, clause.getFirstToken(), SyntaxKind.CaseKeyword, SyntaxKind.DefaultKeyword); // For each clause, also recursively traverse the statements where we can find analogous breaks. forEachChild(clause, function aggregateBreakKeywords(node: Node): void { @@ -2226,7 +2226,7 @@ module ts { case SyntaxKind.BreakStatement: // If the break statement has a label, it cannot be part of a switch block. if (!(node).label) { - pushKeyword(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword); + pushKeywordIf(keywords, node.getFirstToken(), SyntaxKind.BreakKeyword); } // Fall through case SyntaxKind.ForStatement: @@ -2285,7 +2285,7 @@ module ts { return node && node.parent; } - function pushKeyword(keywordList: Node[], token: Node, ...expected: SyntaxKind[]): void { + function pushKeywordIf(keywordList: Node[], token: Node, ...expected: SyntaxKind[]): void { if (!token) { return; } From 813f28d865e0d816d8700c63aedf3198a31878e7 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Wed, 27 Aug 2014 17:28:45 -0700 Subject: [PATCH 13/13] Removed assertion. --- src/services/services.ts | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index ca15d8a46b..862ca387e8 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2293,9 +2293,6 @@ module ts { if (contains(expected, token.kind)) { keywordList.push(token); } - else { - Debug.assert("Got '" + token.getFullText().substring(token.getLeadingTriviaWidth()) + "' instead of expected keyword."); - } } function keywordsToReferenceEntries(keywords: Node[]): ReferenceEntry[]{ @@ -3093,7 +3090,7 @@ module ts { // ["// hack 1", "// ", "hack 1", undefined, "hack"] // // Here are the relevant capture groups: - // 0) The full match for hte entire regex. + // 0) The full match for the entire regex. // 1) The preamble to the message portion. // 2) The message portion. // 3...N) The descriptor that was matched - by index. 'undefined' for each