From 16d969c9cac62486dae9abdfcf84b3013eae5d10 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Sun, 28 Sep 2014 20:36:08 -0400 Subject: [PATCH 1/3] Support getOccurrencesAtPosition for 'throw' keywords. Also revised behavior for 'return' keywords in that when the position resides on a 'return' statement, 'throw' keywords in the same function scope that are not within a try-block are also highlighted. --- src/services/services.ts | 92 ++++++++++++++++++- tests/cases/fourslash/getOccurrencesThrow.ts | 58 ++++++++++++ tests/cases/fourslash/getOccurrencesThrow2.ts | 58 ++++++++++++ tests/cases/fourslash/getOccurrencesThrow3.ts | 58 ++++++++++++ tests/cases/fourslash/getOccurrencesThrow4.ts | 58 ++++++++++++ tests/cases/fourslash/getOccurrencesThrow5.ts | 58 ++++++++++++ tests/cases/fourslash/getOccurrencesThrow6.ts | 28 ++++++ 7 files changed, 409 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/getOccurrencesThrow.ts create mode 100644 tests/cases/fourslash/getOccurrencesThrow2.ts create mode 100644 tests/cases/fourslash/getOccurrencesThrow3.ts create mode 100644 tests/cases/fourslash/getOccurrencesThrow4.ts create mode 100644 tests/cases/fourslash/getOccurrencesThrow5.ts create mode 100644 tests/cases/fourslash/getOccurrencesThrow6.ts diff --git a/src/services/services.ts b/src/services/services.ts index 04e63f6dbe1..d35d64ab5f7 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2378,6 +2378,11 @@ module ts { return getReturnOccurrences(node.parent); } break; + case SyntaxKind.ThrowKeyword: + if (hasKind(node.parent, SyntaxKind.ThrowStatement)) { + return getThrowOccurrences(node.parent); + } + break; case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: @@ -2491,12 +2496,97 @@ module ts { } var keywords: Node[] = [] - forEachReturnStatement((func).body, returnStatement => { + forEachReturnStatement(func.body, returnStatement => { pushKeywordIf(keywords, returnStatement.getFirstToken(), SyntaxKind.ReturnKeyword); }); + // Include 'throw' statements that do not occur within a try block. + forEach(aggregateOwnedThrowStatements(func.body), throwStatement => { + pushKeywordIf(keywords, throwStatement.getFirstToken(), SyntaxKind.ThrowKeyword); + }); + return map(keywords, getReferenceEntryFromNode); } + + function getThrowOccurrences(throwStatement: ThrowStatement) { + var owner = getContextualThrowStatementOwner(throwStatement); + + if (!owner) { + return undefined; + } + + var keywords: Node[] = []; + + forEach(aggregateOwnedThrowStatements(owner), throwStatement => { + pushKeywordIf(keywords, throwStatement.getFirstToken(), SyntaxKind.ThrowKeyword); + }); + + // If the "owner" is a function, then we equate 'return' and 'throw' statements in their + // ability to "jump out" of the function, and include occurrences for both. + if (owner.kind === SyntaxKind.FunctionBlock) { + forEachReturnStatement(owner, returnStatement => { + pushKeywordIf(keywords, returnStatement.getFirstToken(), SyntaxKind.ReturnKeyword); + }); + } + + return map(keywords, getReferenceEntryFromNode); + } + + /** + * Aggregates all throw-statements within this node *without* crossing + * into function boundaries and try-blocks. + */ + function aggregateOwnedThrowStatements(node: Node): ThrowStatement[] { + var statementAccumulator: ThrowStatement[] = [] + aggregate(node); + return statementAccumulator; + + function aggregate(node: Node): void { + if (node.kind === SyntaxKind.ThrowStatement) { + statementAccumulator.push(node); + } + else if (node.kind === SyntaxKind.TryStatement) { + var tryStatement = node; + + if (tryStatement.catchBlock) { + aggregate(tryStatement.catchBlock); + } + if (tryStatement.finallyBlock) { + aggregate(tryStatement.finallyBlock); + } + } + // Do not cross function boundaries. + else if (!isAnyFunction(node)) { + forEachChild(node, aggregate); + } + }; + } + + /** + * For lack of a better name, this function takes a throw statement and returns the first + * encountered ancestor that is a try-block, function-block, or source file. + */ + function getContextualThrowStatementOwner(throwStatement: ThrowStatement): Node { + var child: Node = throwStatement; + + while (child.parent) { + var parent = child.parent; + + if (parent.kind === SyntaxKind.FunctionBlock || parent.kind === SyntaxKind.SourceFile) { + return parent; + } + + // A throw-statement is only owned by a try-statement if it occurs in the try block. + // Otherwise, it is owned by the next closest function-block or try-block. + if (parent.kind === SyntaxKind.TryStatement && child === (parent).tryBlock) { + return child; + } + + child = parent; + } + + return undefined; + } function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { var keywords: Node[] = []; diff --git a/tests/cases/fourslash/getOccurrencesThrow.ts b/tests/cases/fourslash/getOccurrencesThrow.ts new file mode 100644 index 00000000000..c25551db92e --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow.ts @@ -0,0 +1,58 @@ +/// + +////function f(a: number) { +//// try { +//// throw "Hello"; +//// +//// try { +//// throw 10; +//// } +//// catch (x) { +//// [|return|] 100; +//// } +//// finally { +//// throw 10; +//// } +//// } +//// catch (x) { +//// [|throw|] "Something"; +//// } +//// finally { +//// [|throw|] "Also something"; +//// } +//// if (a > 0) { +//// [|return|] (function () { +//// return; +//// return; +//// return; +//// +//// if (false) { +//// return true; +//// } +//// throw "Hello!"; +//// })() || true; +//// } +//// +//// [|th/**/row|] 10; +//// +//// var unusued = [1, 2, 3, 4].map(x => { throw 4 }) +//// +//// [|return|]; +//// [|return|] true; +//// [|throw|] false; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); diff --git a/tests/cases/fourslash/getOccurrencesThrow2.ts b/tests/cases/fourslash/getOccurrencesThrow2.ts new file mode 100644 index 00000000000..99e18020396 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow2.ts @@ -0,0 +1,58 @@ +/// + +////function f(a: number) { +//// try { +//// throw "Hello"; +//// +//// try { +//// [|t/**/hrow|] 10; +//// } +//// catch (x) { +//// return 100; +//// } +//// finally { +//// throw 10; +//// } +//// } +//// catch (x) { +//// throw "Something"; +//// } +//// finally { +//// throw "Also something"; +//// } +//// if (a > 0) { +//// return (function () { +//// return; +//// return; +//// return; +//// +//// if (false) { +//// return true; +//// } +//// throw "Hello!"; +//// })() || true; +//// } +//// +//// throw 10; +//// +//// var unusued = [1, 2, 3, 4].map(x => { throw 4 }) +//// +//// return; +//// return true; +//// throw false; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); diff --git a/tests/cases/fourslash/getOccurrencesThrow3.ts b/tests/cases/fourslash/getOccurrencesThrow3.ts new file mode 100644 index 00000000000..313d04b8e38 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow3.ts @@ -0,0 +1,58 @@ +/// + +////function f(a: number) { +//// try { +//// [|throw|] "Hello"; +//// +//// try { +//// throw 10; +//// } +//// catch (x) { +//// return 100; +//// } +//// finally { +//// [|thr/**/ow|] 10; +//// } +//// } +//// catch (x) { +//// throw "Something"; +//// } +//// finally { +//// throw "Also something"; +//// } +//// if (a > 0) { +//// return (function () { +//// return; +//// return; +//// return; +//// +//// if (false) { +//// return true; +//// } +//// throw "Hello!"; +//// })() || true; +//// } +//// +//// throw 10; +//// +//// var unusued = [1, 2, 3, 4].map(x => { throw 4 }) +//// +//// return; +//// return true; +//// throw false; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); diff --git a/tests/cases/fourslash/getOccurrencesThrow4.ts b/tests/cases/fourslash/getOccurrencesThrow4.ts new file mode 100644 index 00000000000..adf321526af --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow4.ts @@ -0,0 +1,58 @@ +/// + +////function f(a: number) { +//// try { +//// throw "Hello"; +//// +//// try { +//// throw 10; +//// } +//// catch (x) { +//// return 100; +//// } +//// finally { +//// throw 10; +//// } +//// } +//// catch (x) { +//// throw "Something"; +//// } +//// finally { +//// throw "Also something"; +//// } +//// if (a > 0) { +//// return (function () { +//// [|return|]; +//// [|return|]; +//// [|return|]; +//// +//// if (false) { +//// [|return|] true; +//// } +//// [|th/**/row|] "Hello!"; +//// })() || true; +//// } +//// +//// throw 10; +//// +//// var unusued = [1, 2, 3, 4].map(x => { throw 4 }) +//// +//// return; +//// return true; +//// throw false; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); diff --git a/tests/cases/fourslash/getOccurrencesThrow5.ts b/tests/cases/fourslash/getOccurrencesThrow5.ts new file mode 100644 index 00000000000..3e5ba4bca13 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow5.ts @@ -0,0 +1,58 @@ +/// + +////function f(a: number) { +//// try { +//// throw "Hello"; +//// +//// try { +//// throw 10; +//// } +//// catch (x) { +//// return 100; +//// } +//// finally { +//// throw 10; +//// } +//// } +//// catch (x) { +//// throw "Something"; +//// } +//// finally { +//// throw "Also something"; +//// } +//// if (a > 0) { +//// return (function () { +//// return; +//// return; +//// return; +//// +//// if (false) { +//// return true; +//// } +//// throw "Hello!"; +//// })() || true; +//// } +//// +//// throw 10; +//// +//// var unusued = [1, 2, 3, 4].map(x => { [|thr/**/ow|] 4 }) +//// +//// return; +//// return true; +//// throw false; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); diff --git a/tests/cases/fourslash/getOccurrencesThrow6.ts b/tests/cases/fourslash/getOccurrencesThrow6.ts new file mode 100644 index 00000000000..2769c9933db --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow6.ts @@ -0,0 +1,28 @@ +/// + +////[|throw|] 100; +//// +////try { +//// throw 0; +//// var x = () => { throw 0; }; +////} +////catch (y) { +//// var x = () => { throw 0; }; +//// [|throw|] 200; +////} +////finally { +//// [|throw|] 300; +////} + + + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + From 2503e50a5d76edb29df9e49b9811556335cb1311 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 29 Sep 2014 12:41:47 -0700 Subject: [PATCH 2/3] Changed "ownership" relation of try blocks on throw statements. A try-block now only owns a throw statement if its try statement has a catch-clause. --- src/services/services.ts | 25 ++++++++++----- tests/cases/fourslash/getOccurrencesThrow7.ts | 31 +++++++++++++++++++ tests/cases/fourslash/getOccurrencesThrow8.ts | 31 +++++++++++++++++++ 3 files changed, 80 insertions(+), 7 deletions(-) create mode 100644 tests/cases/fourslash/getOccurrencesThrow7.ts create mode 100644 tests/cases/fourslash/getOccurrencesThrow8.ts diff --git a/src/services/services.ts b/src/services/services.ts index 6a6e379869c..dc6b6353ef3 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2803,7 +2803,7 @@ module ts { /** * Aggregates all throw-statements within this node *without* crossing - * into function boundaries and try-blocks. + * into function boundaries and try-blocks with catch-clauses. */ function aggregateOwnedThrowStatements(node: Node): ThrowStatement[] { var statementAccumulator: ThrowStatement[] = [] @@ -2816,10 +2816,16 @@ module ts { } else if (node.kind === SyntaxKind.TryStatement) { var tryStatement = node; - + + // If a try statement has a catch clause, then any thrown exceptions in the try block + // will be propagated to the next owner. if (tryStatement.catchBlock) { aggregate(tryStatement.catchBlock); } + else { + aggregate(tryStatement.tryBlock); + } + if (tryStatement.finallyBlock) { aggregate(tryStatement.finallyBlock); } @@ -2833,7 +2839,8 @@ module ts { /** * For lack of a better name, this function takes a throw statement and returns the first - * encountered ancestor that is a try-block, function-block, or source file. + * encountered ancestor that is a try-block (whose try statement has a catch clause), + * function-block, or source file. */ function getContextualThrowStatementOwner(throwStatement: ThrowStatement): Node { var child: Node = throwStatement; @@ -2845,10 +2852,14 @@ module ts { return parent; } - // A throw-statement is only owned by a try-statement if it occurs in the try block. - // Otherwise, it is owned by the next closest function-block or try-block. - if (parent.kind === SyntaxKind.TryStatement && child === (parent).tryBlock) { - return child; + // A throw-statement is only owned by a try-statement if the try-statement has + // a catch clause, and if the throw-statement occurs within the try block. + if (parent.kind === SyntaxKind.TryStatement) { + var tryStatement = parent; + + if (tryStatement.tryBlock === child && tryStatement.catchBlock) { + return child; + } } child = parent; diff --git a/tests/cases/fourslash/getOccurrencesThrow7.ts b/tests/cases/fourslash/getOccurrencesThrow7.ts new file mode 100644 index 00000000000..46e899a891b --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow7.ts @@ -0,0 +1,31 @@ +/// + +////try { +//// [|throw|] 10; +//// +//// try { +//// throw 10; +//// } +//// catch (x) { +//// [|throw|] 10; +//// } +//// finally { +//// [|throw|] 10; +//// } +////} +////finally { +//// [|throw|] 10; +////} +//// +////[|throw|] 10; + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + diff --git a/tests/cases/fourslash/getOccurrencesThrow8.ts b/tests/cases/fourslash/getOccurrencesThrow8.ts new file mode 100644 index 00000000000..679e13c8d5a --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesThrow8.ts @@ -0,0 +1,31 @@ +/// + +////try { +//// throw 10; +//// +//// try { +//// [|throw|] 10; +//// } +//// catch (x) { +//// throw 10; +//// } +//// finally { +//// throw 10; +//// } +////} +////finally { +//// throw 10; +////} +//// +////throw 10; + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); + + verify.occurrencesAtPositionCount(test.ranges().length); +}); + From fc469538da2c58063e4aaa0e98edfcc48865b14a Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 30 Sep 2014 13:49:45 -0700 Subject: [PATCH 3/3] Minor naming/comment changes. --- src/services/services.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index dc6b6353ef3..264d0bc816e 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2778,7 +2778,7 @@ module ts { } function getThrowOccurrences(throwStatement: ThrowStatement) { - var owner = getContextualThrowStatementOwner(throwStatement); + var owner = getThrowStatementOwner(throwStatement); if (!owner) { return undefined; @@ -2817,12 +2817,12 @@ module ts { else if (node.kind === SyntaxKind.TryStatement) { var tryStatement = node; - // If a try statement has a catch clause, then any thrown exceptions in the try block - // will be propagated to the next owner. if (tryStatement.catchBlock) { aggregate(tryStatement.catchBlock); } else { + // Exceptions thrown within a try block lacking a catch clause + // are "owned" in the current context. aggregate(tryStatement.tryBlock); } @@ -2838,11 +2838,11 @@ module ts { } /** - * For lack of a better name, this function takes a throw statement and returns the first - * encountered ancestor that is a try-block (whose try statement has a catch clause), + * For lack of a better name, this function takes a throw statement and returns the + * nearest ancestor that is a try-block (whose try statement has a catch clause), * function-block, or source file. */ - function getContextualThrowStatementOwner(throwStatement: ThrowStatement): Node { + function getThrowStatementOwner(throwStatement: ThrowStatement): Node { var child: Node = throwStatement; while (child.parent) {