From c456ef496d714f720bb485954db2650f54c86e2f Mon Sep 17 00:00:00 2001 From: Vladimir Matveev Date: Thu, 25 Sep 2014 14:29:32 -0700 Subject: [PATCH] renamed getNodeAtPosition to getExactTokenAtPosition, added predicate parameter that will determine if token with end === position should be returned --- src/services/services.ts | 65 +++++++++++++---- src/services/utilities.ts | 37 ++++++---- .../cases/fourslash/getOccurrencesIfElse5.ts | 42 +++++++++++ .../getOccurrencesIfElseNegatives.ts | 29 -------- .../getOccurrencesLoopBreakContinue6.ts | 70 +++++++++++++++++++ ...nNegatives.ts => getOccurrencesReturn4.ts} | 16 +++-- .../getOccurrencesSwitchCaseDefault5.ts | 40 +++++++++++ ...etOccurrencesSwitchCaseDefaultNegatives.ts | 25 ------- ...hisNegatives.ts => getOccurrencesThis6.ts} | 14 ++-- .../getOccurrencesTryCatchFinally4.ts | 30 ++++++++ .../getOccurrencesTryCatchFinallyNegatives.ts | 23 ------ tests/cases/fourslash/goToDefinitionSimple.ts | 5 ++ 12 files changed, 286 insertions(+), 110 deletions(-) create mode 100644 tests/cases/fourslash/getOccurrencesIfElse5.ts delete mode 100644 tests/cases/fourslash/getOccurrencesIfElseNegatives.ts create mode 100644 tests/cases/fourslash/getOccurrencesLoopBreakContinue6.ts rename tests/cases/fourslash/{getOccurrencesReturnNegatives.ts => getOccurrencesReturn4.ts} (52%) create mode 100644 tests/cases/fourslash/getOccurrencesSwitchCaseDefault5.ts delete mode 100644 tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts rename tests/cases/fourslash/{getOccurrencesThisNegatives.ts => getOccurrencesThis6.ts} (85%) create mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinally4.ts delete mode 100644 tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts diff --git a/src/services/services.ts b/src/services/services.ts index f83202df864..6c5b3363e33 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2088,7 +2088,7 @@ module ts { } // TODO: this is a hack for now, we need a proper walking mechanism to verify that we have the correct node - var mappedNode = getNodeAtPosition(sourceFile, TypeScript.end(node) - 1); + var mappedNode = getExactTokenAtPosition(sourceFile, TypeScript.end(node) - 1, /*includeItemAtEndPosition*/ undefined); if (isPunctuation(mappedNode.kind)) { mappedNode = mappedNode.parent; } @@ -2325,7 +2325,7 @@ module ts { fileName = TypeScript.switchToForwardSlashes(fileName); var sourceFile = getSourceFile(fileName); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, /*includeItemAtEndPosition*/ undefined); if (!node) { return undefined; } @@ -2434,7 +2434,7 @@ module ts { fileName = TypeScript.switchToForwardSlashes(fileName); var sourceFile = getSourceFile(fileName); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, /*includeItemAtEndPosition*/ undefined); if (!node) { return undefined; } @@ -2512,12 +2512,25 @@ module ts { return false; } + function isValidGotoDefinitionTarget(n: Node): boolean { + switch (n.kind) { + case SyntaxKind.Identifier: + case SyntaxKind.SuperKeyword: + case SyntaxKind.ThisKeyword: + case SyntaxKind.NumericLiteral: + case SyntaxKind.StringLiteral: + return true; + default: + return false; + } + } + synchronizeHostData(); filename = TypeScript.switchToForwardSlashes(filename); var sourceFile = getSourceFile(filename); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, isValidGotoDefinitionTarget); if (!node) { return undefined; } @@ -2581,7 +2594,7 @@ module ts { filename = TypeScript.switchToForwardSlashes(filename); var sourceFile = getSourceFile(filename); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, isValidFindOccurencesTarget); if (!node) { return undefined; } @@ -2647,6 +2660,32 @@ module ts { return undefined; + function isValidFindOccurencesTarget(n: Node): boolean { + switch (n.kind) { + case SyntaxKind.Identifier: + case SyntaxKind.SuperKeyword: + case SyntaxKind.ThisKeyword: + case SyntaxKind.IfKeyword: + case SyntaxKind.ElseKeyword: + case SyntaxKind.ReturnKeyword: + case SyntaxKind.TryKeyword: + case SyntaxKind.CatchKeyword: + case SyntaxKind.FinallyKeyword: + case SyntaxKind.SwitchKeyword: + case SyntaxKind.CaseKeyword: + case SyntaxKind.DefaultKeyword: + case SyntaxKind.BreakKeyword: + case SyntaxKind.ContinueKeyword: + case SyntaxKind.ForKeyword: + case SyntaxKind.WhileKeyword: + case SyntaxKind.DoKeyword: + case SyntaxKind.Constructor: + return true; + default: + return false; + } + } + function getIfElseOccurrences(ifStatement: IfStatement): ReferenceEntry[] { var keywords: Node[] = []; @@ -2903,7 +2942,7 @@ module ts { filename = TypeScript.switchToForwardSlashes(filename); var sourceFile = getSourceFile(filename); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, /*includeItemAtEndPosition*/ undefined); if (!node) { return undefined; } @@ -3086,7 +3125,7 @@ module ts { forEach(possiblePositions, position => { cancellationToken.throwIfCancellationRequested(); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, /*includeItemAtEndPosition*/ undefined); if (!node || node.getWidth() !== labelName.length) { return; } @@ -3142,7 +3181,7 @@ module ts { forEach(possiblePositions, position => { cancellationToken.throwIfCancellationRequested(); - var referenceLocation = getNodeAtPosition(sourceFile, position); + var referenceLocation = getExactTokenAtPosition(sourceFile, position, /*includeItemAtEndPosition*/ undefined); if (!isValidReferencePosition(referenceLocation, searchText)) { return; } @@ -3194,7 +3233,7 @@ module ts { forEach(possiblePositions, position => { cancellationToken.throwIfCancellationRequested(); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, n => n.kind === SyntaxKind.SuperKeyword); if (!node || node.kind !== SyntaxKind.SuperKeyword) { return; @@ -3260,7 +3299,7 @@ module ts { forEach(possiblePositions, position => { cancellationToken.throwIfCancellationRequested(); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, n => n.kind === SyntaxKind.ThisKeyword); if (!node || node.kind !== SyntaxKind.ThisKeyword) { return; } @@ -4022,7 +4061,7 @@ module ts { var sourceFile = getCurrentSourceFile(filename); var result: TypeScript.TextSpan[] = []; - var token = getTokenAtPosition(sourceFile, position); + var token = getTokenContainingPosition(sourceFile, position); if (token.getStart(sourceFile) === position) { var matchKind = getMatchingTokenKind(token); @@ -4178,7 +4217,7 @@ module ts { // OK, we have found a match in the file. This is only an acceptable match if // it is contained within a comment. - var token = getTokenAtPosition(sourceFile, matchPosition); + var token = getTokenContainingPosition(sourceFile, matchPosition); if (token.getStart() <= matchPosition && matchPosition < token.getEnd()) { // match was within the token itself. Not in the comment. Keep searching @@ -4306,7 +4345,7 @@ module ts { fileName = TypeScript.switchToForwardSlashes(fileName); var sourceFile = getSourceFile(fileName); - var node = getNodeAtPosition(sourceFile, position); + var node = getExactTokenAtPosition(sourceFile, position, n => n.kind === SyntaxKind.Identifier); // Can only rename an identifier. if (node && node.kind === SyntaxKind.Identifier) { diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 2824ab5d2da..0e17699a1b1 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -49,7 +49,7 @@ module ts { /** Get a token that contains the position. This is guaranteed to return a token, the position can be in the * leading trivia or within the token text. */ - export function getTokenAtPosition(sourceFile: SourceFile, position: number) { + export function getTokenContainingPosition(sourceFile: SourceFile, position: number) { var current: Node = sourceFile; outer: while (true) { // find the child that has this @@ -64,16 +64,29 @@ module ts { } } - /** Get the token whose text contains the position, or the containing node. */ - export function getNodeAtPosition(sourceFile: SourceFile, position: number) { + /** Get the token whose text contains the position */ + export function getExactTokenAtPosition(sourceFile: SourceFile, position: number, includeItemAtEndPosition: (n: Node) => boolean) { var current: Node = sourceFile; outer: while (true) { - // find the child that has this - for (var i = 0, n = current.getChildCount(); i < n; i++) { + if (isToken(current)) { + // exit early + return current; + } + + // find the child that contains 'position' + for (var i = 0, n = current.getChildCount(sourceFile); i < n; i++) { var child = current.getChildAt(i); - if (child.getStart() <= position && position < child.getEnd()) { - current = child; - continue outer; + if (child.getStart(sourceFile) <= position) { + if (position < child.getEnd()) { + current = child; + continue outer; + } + else if (includeItemAtEndPosition && child.getEnd() === position) { + var previousToken = findPrecedingToken(position, sourceFile, child); + if (previousToken && includeItemAtEndPosition(previousToken)) { + return previousToken; + } + } } } return current; @@ -91,7 +104,7 @@ module ts { export function findTokenOnLeftOfPosition(file: SourceFile, position: number): Node { // Ideally, getTokenAtPosition should return a token. However, it is currently // broken, so we do a check to make sure the result was indeed a token. - var tokenAtPosition = getTokenAtPosition(file, position); + var tokenAtPosition = getTokenContainingPosition(file, position); if (isToken(tokenAtPosition) && position > tokenAtPosition.getStart(file) && position < tokenAtPosition.getEnd()) { return tokenAtPosition; } @@ -126,8 +139,8 @@ module ts { } } - export function findPrecedingToken(position: number, sourceFile: SourceFile): Node { - return find(sourceFile); + export function findPrecedingToken(position: number, sourceFile: SourceFile, startNode?: Node): Node { + return find(startNode || sourceFile); function findRightmostToken(n: Node): Node { if (isToken(n)) { @@ -163,7 +176,7 @@ module ts { } } - Debug.assert(n.kind === SyntaxKind.SourceFile); + Debug.assert(startNode || n.kind === SyntaxKind.SourceFile); // Here we know that none of child token nodes embrace the position, // the only known case is when position is at the end of the file. diff --git a/tests/cases/fourslash/getOccurrencesIfElse5.ts b/tests/cases/fourslash/getOccurrencesIfElse5.ts new file mode 100644 index 00000000000..b0519630b7b --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesIfElse5.ts @@ -0,0 +1,42 @@ +/// + +////if/*1*/ (true) { +//// if/*2*/ (false) { +//// } +//// else/*3*/ { +//// } +//// if/*4*/ (true) { +//// } +//// else/*5*/ { +//// if/*6*/ (false) +//// if/*7*/ (true) +//// var x = undefined; +//// } +////} +////else/*8*/ if (null) { +////} +////else/*9*/ /* whar garbl */ if/*10*/ (undefined) { +////} +////else/*11*/ +////if/*12*/ (false) { +////} +////else/*13*/ { } + +function verifyOccurencesAtMarker(marker: string, count: number) { + goTo.marker(marker); + verify.occurrencesAtPositionCount(count); +} + +verifyOccurencesAtMarker("1", 7); +verifyOccurencesAtMarker("2", 2); +verifyOccurencesAtMarker("3", 2); +verifyOccurencesAtMarker("4", 2); +verifyOccurencesAtMarker("5", 2); +verifyOccurencesAtMarker("6", 1); +verifyOccurencesAtMarker("7", 1); +verifyOccurencesAtMarker("8", 7); +verifyOccurencesAtMarker("9", 7); +verifyOccurencesAtMarker("10", 7); +verifyOccurencesAtMarker("11", 7); +verifyOccurencesAtMarker("12", 7); +verifyOccurencesAtMarker("13", 7); diff --git a/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts b/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts deleted file mode 100644 index 601eba63c64..00000000000 --- a/tests/cases/fourslash/getOccurrencesIfElseNegatives.ts +++ /dev/null @@ -1,29 +0,0 @@ -/// - -////if/*1*/ (true) { -//// if/*2*/ (false) { -//// } -//// else/*3*/ { -//// } -//// if/*4*/ (true) { -//// } -//// else/*5*/ { -//// if/*6*/ (false) -//// if/*7*/ (true) -//// var x = undefined; -//// } -////} -////else/*8*/ if (null) { -////} -////else/*9*/ /* whar garbl */ if/*10*/ (undefined) { -////} -////else/*11*/ -////if/*12*/ (false) { -////} -////else/*13*/ { } - - -test.markers().forEach(m => { - goTo.position(m.position, m.fileName) - verify.occurrencesAtPositionCount(0); -}); diff --git a/tests/cases/fourslash/getOccurrencesLoopBreakContinue6.ts b/tests/cases/fourslash/getOccurrencesLoopBreakContinue6.ts new file mode 100644 index 00000000000..0127245dd50 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesLoopBreakContinue6.ts @@ -0,0 +1,70 @@ +/// + +////var arr = [1, 2, 3, 4]; +////label1: for (var n in arr) { +//// break; +//// continue; +//// break label1; +//// continue label1; +//// +//// label2: for (var i = 0; i < arr[n]; i++) { +//// break label1; +//// continue label1; +//// +//// break; +//// continue; +//// break label2; +//// continue label2; +//// +//// function foo() { +//// label3: while (true) { +//// break; +//// continue; +//// break label3; +//// continue label3; +//// +//// // these cross function boundaries +//// br/*1*/eak label1; +//// cont/*2*/inue label1; +//// bre/*3*/ak label2; +//// c/*4*/ontinue label2; +//// +//// label4: do { +//// break; +//// continue; +//// break label4; +//// continue label4; +//// +//// break label3; +//// continue label3; +//// +//// switch (10) { +//// case 1: +//// case 2: +//// break; +//// break label4; +//// default: +//// continue; +//// } +//// +//// // these cross function boundaries +//// br/*5*/eak label1; +//// co/*6*/ntinue label1; +//// br/*7*/eak label2; +//// con/*8*/tinue label2; +//// () => { b/*9*/reak; } +//// } while (true) +//// } +//// } +//// } +////} +//// +////label5: while (true) break label5; +//// +////label7: while (true) co/*10*/ntinue label5; + +test.markers().forEach(m => { + goTo.position(m.position); + + verify.occurrencesAtPositionCount(0); +}); diff --git a/tests/cases/fourslash/getOccurrencesReturnNegatives.ts b/tests/cases/fourslash/getOccurrencesReturn4.ts similarity index 52% rename from tests/cases/fourslash/getOccurrencesReturnNegatives.ts rename to tests/cases/fourslash/getOccurrencesReturn4.ts index 79cb3c659c6..4e25162f80f 100644 --- a/tests/cases/fourslash/getOccurrencesReturnNegatives.ts +++ b/tests/cases/fourslash/getOccurrencesReturn4.ts @@ -19,7 +19,15 @@ //// return/*7*/ true; ////} -test.markers().forEach(m => { - goTo.position(m.position, m.fileName) - verify.occurrencesAtPositionCount(0); -}); \ No newline at end of file +function verifyOccurencesAtMarker(marker: string, count: number) { + goTo.marker(marker); + verify.occurrencesAtPositionCount(count); +} + +verifyOccurencesAtMarker("1", 4); +verifyOccurencesAtMarker("2", 4); +verifyOccurencesAtMarker("3", 4); +verifyOccurencesAtMarker("4", 4); +verifyOccurencesAtMarker("5", 1); +verifyOccurencesAtMarker("6", 3); +verifyOccurencesAtMarker("7", 3); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefault5.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefault5.ts new file mode 100644 index 00000000000..162c2e1a94f --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSwitchCaseDefault5.ts @@ -0,0 +1,40 @@ +/// + +////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*/: +////} + +function verifyOccurencesAtMarker(marker: string, count: number) { + goTo.marker(marker); + verify.occurrencesAtPositionCount(count); +} + +verifyOccurencesAtMarker("1", 9); +verifyOccurencesAtMarker("2", 9); +verifyOccurencesAtMarker("3", 9); +verifyOccurencesAtMarker("4", 9); +verifyOccurencesAtMarker("5", 9); +verifyOccurencesAtMarker("6", 6); +verifyOccurencesAtMarker("7", 6); +verifyOccurencesAtMarker("8", 6); +verifyOccurencesAtMarker("9", 6); +verifyOccurencesAtMarker("10", 6); +verifyOccurencesAtMarker("11", 9); +verifyOccurencesAtMarker("12", 9); +verifyOccurencesAtMarker("13", 9); +verifyOccurencesAtMarker("14", 0); + diff --git a/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts b/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts deleted file mode 100644 index cfb70b0c11a..00000000000 --- a/tests/cases/fourslash/getOccurrencesSwitchCaseDefaultNegatives.ts +++ /dev/null @@ -1,25 +0,0 @@ -/// - -////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); -} diff --git a/tests/cases/fourslash/getOccurrencesThisNegatives.ts b/tests/cases/fourslash/getOccurrencesThis6.ts similarity index 85% rename from tests/cases/fourslash/getOccurrencesThisNegatives.ts rename to tests/cases/fourslash/getOccurrencesThis6.ts index 95c676e0add..6ff779c0b46 100644 --- a/tests/cases/fourslash/getOccurrencesThisNegatives.ts +++ b/tests/cases/fourslash/getOccurrencesThis6.ts @@ -142,8 +142,14 @@ ////} -test.markers().forEach(m => { - goTo.position(m.position, m.fileName) +function verifyOccurencesAtMarker(marker: string, count: number) { + goTo.marker(marker); + verify.occurrencesAtPositionCount(count); +} - verify.occurrencesAtPositionCount(0); -}); \ No newline at end of file +verifyOccurencesAtMarker("1", 2); +verifyOccurencesAtMarker("2", 6); +verifyOccurencesAtMarker("3", 1); +verifyOccurencesAtMarker("4", 1); +verifyOccurencesAtMarker("5", 1); +verifyOccurencesAtMarker("6", 0); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinally4.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinally4.ts new file mode 100644 index 00000000000..dc6b0540312 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesTryCatchFinally4.ts @@ -0,0 +1,30 @@ +/// + +////try/*1*/ { +//// try/*2*/ { +//// } +//// catch/*3*/ (x) { +//// } +//// +//// try/*4*/ { +//// } +//// finally/*5*/ {/*8*/ +//// } +////} +////catch/*6*/ (e) { +////} +////finally/*7*/ { +////} +function verifyOccurencesAtMarker(marker: string, count: number) { + goTo.marker(marker); + verify.occurrencesAtPositionCount(count); +} + +verifyOccurencesAtMarker("1", 3); +verifyOccurencesAtMarker("2", 2); +verifyOccurencesAtMarker("3", 2); +verifyOccurencesAtMarker("4", 2); +verifyOccurencesAtMarker("5", 2); +verifyOccurencesAtMarker("6", 3); +verifyOccurencesAtMarker("7", 3); +verifyOccurencesAtMarker("8", 0); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts b/tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts deleted file mode 100644 index d5889ff84e9..00000000000 --- a/tests/cases/fourslash/getOccurrencesTryCatchFinallyNegatives.ts +++ /dev/null @@ -1,23 +0,0 @@ -/// - -////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 diff --git a/tests/cases/fourslash/goToDefinitionSimple.ts b/tests/cases/fourslash/goToDefinitionSimple.ts index f931610aa2a..47c1909cf12 100644 --- a/tests/cases/fourslash/goToDefinitionSimple.ts +++ b/tests/cases/fourslash/goToDefinitionSimple.ts @@ -5,7 +5,12 @@ // @Filename: Consumption.ts //// var n = new /*1*/c(); +//// var n = new c/*3*/(); goTo.marker('1'); goTo.definition(); verify.caretAtMarker('2'); + +goTo.marker('3'); +goTo.definition(); +verify.caretAtMarker('2');