From fbb10cd6b375da2120e3f538c814bdb58595db87 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 29 Aug 2014 14:18:13 -0700 Subject: [PATCH 1/5] Added getOccs support for return keywords. --- src/compiler/checker.ts | 62 ++++++++++++++++++++-------------------- src/compiler/types.ts | 2 +- src/services/services.ts | 24 ++++++++++++++++ 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e1b2adde64f..575a9978f2f 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -4394,37 +4394,6 @@ module ts { return voidType; } - // WARNING: This has the same semantics as the forEach family of functions, - // in that traversal terminates in the event that 'visitor' supplies a truthy value. - function forEachReturnStatement(body: Block, visitor: (stmt: ReturnStatement) => T): T { - - return traverse(body); - - function traverse(node: Node): T { - switch (node.kind) { - case SyntaxKind.ReturnStatement: - return visitor(node); - case SyntaxKind.Block: - case SyntaxKind.FunctionBlock: - case SyntaxKind.IfStatement: - case SyntaxKind.DoStatement: - case SyntaxKind.WhileStatement: - case SyntaxKind.ForStatement: - case SyntaxKind.ForInStatement: - case SyntaxKind.WithStatement: - case SyntaxKind.SwitchStatement: - case SyntaxKind.CaseClause: - case SyntaxKind.DefaultClause: - case SyntaxKind.LabelledStatement: - case SyntaxKind.TryStatement: - case SyntaxKind.TryBlock: - case SyntaxKind.CatchBlock: - case SyntaxKind.FinallyBlock: - return forEachChild(node, traverse); - } - } - } - /// Returns a set of types relating to every return expression relating to a function block. function checkAndAggregateReturnExpressionTypes(body: Block, contextualMapper?: TypeMapper): Type[] { var aggregatedTypes: Type[] = []; @@ -7155,4 +7124,35 @@ module ts { return checker; } + + // WARNING: This has the same semantics as the forEach family of functions, + // in that traversal terminates in the event that 'visitor' supplies a truthy value. + export function forEachReturnStatement(body: Block, visitor: (stmt: ReturnStatement) => T): T { + + return traverse(body); + + function traverse(node: Node): T { + switch (node.kind) { + case SyntaxKind.ReturnStatement: + return visitor(node); + case SyntaxKind.Block: + case SyntaxKind.FunctionBlock: + case SyntaxKind.IfStatement: + case SyntaxKind.DoStatement: + case SyntaxKind.WhileStatement: + case SyntaxKind.ForStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.WithStatement: + case SyntaxKind.SwitchStatement: + case SyntaxKind.CaseClause: + case SyntaxKind.DefaultClause: + case SyntaxKind.LabelledStatement: + case SyntaxKind.TryStatement: + case SyntaxKind.TryBlock: + case SyntaxKind.CatchBlock: + case SyntaxKind.FinallyBlock: + return forEachChild(node, traverse); + } + } + } } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index feccd3a298c..198182906f2 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -220,7 +220,7 @@ module ts { LastFutureReservedWord = YieldKeyword, FirstTypeNode = TypeReference, LastTypeNode = ArrayType, - FirstPunctuation= OpenBraceToken, + FirstPunctuation = OpenBraceToken, LastPunctuation = CaretEqualsToken } diff --git a/src/services/services.ts b/src/services/services.ts index e5bdd0bf751..0d75dbb2b36 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2174,6 +2174,11 @@ module ts { return getIfElseOccurrences(node.parent); } break; + case SyntaxKind.ReturnKeyword: + if (hasKind(node.parent, SyntaxKind.ReturnStatement)) { + return getReturnOccurrences(node.parent); + } + break; case SyntaxKind.TryKeyword: case SyntaxKind.CatchKeyword: case SyntaxKind.FinallyKeyword: @@ -2261,6 +2266,25 @@ module ts { return result; } + function getReturnOccurrences(returnStatement: ReturnStatement): ReferenceEntry[]{ + var node: Node = returnStatement; + while (!isAnyFunction(node) && node.parent) { + node = node.parent; + } + + // If we didn't find a containing function with a block body, bail out. + if (!(isAnyFunction(node) && hasKind((node).body, SyntaxKind.FunctionBlock))) { + return undefined; + } + + var keywords: Node[] = [] + forEachReturnStatement((node).body, returnStmt => { + pushKeywordIf(keywords, returnStmt.getFirstToken(), SyntaxKind.ReturnKeyword); + }); + + return map(keywords, keywordToReferenceEntry); + } + function getTryCatchFinallyOccurrences(tryStatement: TryStatement): ReferenceEntry[] { var keywords: Node[] = []; From 7e5802192ec172aed7a28e03bfe5efb66ade3158 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 29 Aug 2014 16:11:02 -0700 Subject: [PATCH 2/5] Added tests for getOccs on return keywords. --- tests/cases/fourslash/getOccurrencesReturn.ts | 33 ++++++++++++ .../cases/fourslash/getOccurrencesReturn2.ts | 33 ++++++++++++ .../cases/fourslash/getOccurrencesReturn3.ts | 28 ++++++++++ .../fourslash/getOccurrencesReturnBroken.ts | 54 +++++++++++++++++++ .../getOccurrencesReturnNegatives.ts | 25 +++++++++ 5 files changed, 173 insertions(+) create mode 100644 tests/cases/fourslash/getOccurrencesReturn.ts create mode 100644 tests/cases/fourslash/getOccurrencesReturn2.ts create mode 100644 tests/cases/fourslash/getOccurrencesReturn3.ts create mode 100644 tests/cases/fourslash/getOccurrencesReturnBroken.ts create mode 100644 tests/cases/fourslash/getOccurrencesReturnNegatives.ts diff --git a/tests/cases/fourslash/getOccurrencesReturn.ts b/tests/cases/fourslash/getOccurrencesReturn.ts new file mode 100644 index 00000000000..613e1645fb5 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesReturn.ts @@ -0,0 +1,33 @@ +/// + +////function f(a: number) { +//// if (a > 0) { +//// [|ret/**/urn|] (function () { +//// return; +//// return; +//// return; +//// +//// if (false) { +//// return true; +//// } +//// })() || true; +//// } +//// +//// var unusued = [1, 2, 3, 4].map(x => { return 4 }) +//// +//// [|return|]; +//// [|return|] true; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesReturn2.ts b/tests/cases/fourslash/getOccurrencesReturn2.ts new file mode 100644 index 00000000000..15a062433fe --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesReturn2.ts @@ -0,0 +1,33 @@ +/// + +////function f(a: number) { +//// if (a > 0) { +//// return (function () { +//// [|return|]; +//// [|ret/**/urn|]; +//// [|return|]; +//// +//// while (false) { +//// [|return|] true; +//// } +//// })() || true; +//// } +//// +//// var unusued = [1, 2, 3, 4].map(x => { return 4 }) +//// +//// return; +//// return true; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +}); + +goTo.marker(); +test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); +}); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesReturn3.ts b/tests/cases/fourslash/getOccurrencesReturn3.ts new file mode 100644 index 00000000000..030d700ef6e --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesReturn3.ts @@ -0,0 +1,28 @@ +/// + +////function f(a: number) { +//// if (a > 0) { +//// return (function () { +//// return; +//// return; +//// return; +//// +//// if (false) { +//// return true; +//// } +//// })() || true; +//// } +//// +//// var unusued = [1, 2, 3, 4].map(x => { [|return|] 4 }) +//// +//// return; +//// return true; +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +}); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesReturnBroken.ts b/tests/cases/fourslash/getOccurrencesReturnBroken.ts new file mode 100644 index 00000000000..e740e7f2bb3 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesReturnBroken.ts @@ -0,0 +1,54 @@ +/// + +////ret/*1*/urn; +////retu/*2*/rn; +////function f(a: number) { +//// if (a > 0) { +//// return (function () { +//// () => [|return|]; +//// [|return|]; +//// [|return|]; +//// +//// if (false) { +//// [|return|] true; +//// } +//// })() || true; +//// } +//// +//// var unusued = [1, 2, 3, 4].map(x => { return 4 }) +//// +//// return; +//// return true; +////} +//// +////class A { +//// ret/*3*/urn; +//// r/*4*/eturn 8675309; +////} + +// Note: For this test, these 'return's get highlighted as a result of a parse recovery +// where if an arrow function starts with a statement, we try to parse a body +// as if it was missing curly braces. If the behavior changes in the future, +// a change to this test is very much welcome. +test.ranges().forEach(r => { + goTo.position(r.start); + + test.ranges().forEach(range => { + verify.occurrencesAtPositionContains(range, false); + }); +}); + +for (var i = 1; i <= test.markers().length; i++) { + goTo.marker("" + i); + + switch (i) { + case 0: + case 1: + case 4: + verify.occurrencesAtPositionCount(0); + break; + case 3: + verify.occurrencesAtPositionCount(1); // 'return' is an instance member + break; + } +}); \ No newline at end of file diff --git a/tests/cases/fourslash/getOccurrencesReturnNegatives.ts b/tests/cases/fourslash/getOccurrencesReturnNegatives.ts new file mode 100644 index 00000000000..79cb3c659c6 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesReturnNegatives.ts @@ -0,0 +1,25 @@ +/// + +////function f(a: number) { +//// if (a > 0) { +//// return (function () { +//// return/*1*/; +//// return/*2*/; +//// return/*3*/; +//// +//// if (false) { +//// return/*4*/ true; +//// } +//// })() || true; +//// } +//// +//// var unusued = [1, 2, 3, 4].map(x => { return/*5*/ 4 }) +//// +//// return/*6*/; +//// return/*7*/ true; +////} + +test.markers().forEach(m => { + goTo.position(m.position, m.fileName) + verify.occurrencesAtPositionCount(0); +}); \ No newline at end of file From ba396ed28fc6060d3891b604fa228e9ed0518470 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 29 Aug 2014 17:13:14 -0700 Subject: [PATCH 3/5] Utilize getContainingFunction in services. --- src/compiler/checker.ts | 22 +++++++++++----------- src/services/services.ts | 13 +++++++------ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 575a9978f2f..0fc14f157d7 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -5774,17 +5774,6 @@ module ts { // TODO: Check that target label is valid } - function getContainingFunction(node: Node): SignatureDeclaration { - while (true) { - node = node.parent; - if (!node || node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression || - node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.Method || node.kind === SyntaxKind.Constructor || - node.kind === SyntaxKind.GetAccessor || node.kind === SyntaxKind.SetAccessor) { - return node; - } - } - } - function checkReturnStatement(node: ReturnStatement) { if (node.expression && !(getNodeLinks(node.expression).flags & NodeCheckFlags.TypeChecked)) { var func = getContainingFunction(node); @@ -7124,6 +7113,17 @@ module ts { return checker; } + + export function getContainingFunction(node: Node): SignatureDeclaration { + while (true) { + node = node.parent; + if (!node || node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression || + node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.Method || node.kind === SyntaxKind.Constructor || + node.kind === SyntaxKind.GetAccessor || node.kind === SyntaxKind.SetAccessor) { + return node; + } + } + } // WARNING: This has the same semantics as the forEach family of functions, // in that traversal terminates in the event that 'visitor' supplies a truthy value. diff --git a/src/services/services.ts b/src/services/services.ts index 0d75dbb2b36..2ad05fdb726 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1313,6 +1313,10 @@ module ts { } function isAnyFunction(node: Node): boolean { + if (!node) { + return false; + } + switch (node.kind) { case SyntaxKind.FunctionExpression: case SyntaxKind.FunctionDeclaration: @@ -2267,18 +2271,15 @@ module ts { } function getReturnOccurrences(returnStatement: ReturnStatement): ReferenceEntry[]{ - var node: Node = returnStatement; - while (!isAnyFunction(node) && node.parent) { - node = node.parent; - } + var func = getContainingFunction(returnStatement); // If we didn't find a containing function with a block body, bail out. - if (!(isAnyFunction(node) && hasKind((node).body, SyntaxKind.FunctionBlock))) { + if (!(isAnyFunction(func) && hasKind(func.body, SyntaxKind.FunctionBlock))) { return undefined; } var keywords: Node[] = [] - forEachReturnStatement((node).body, returnStmt => { + forEachReturnStatement((func).body, returnStmt => { pushKeywordIf(keywords, returnStmt.getFirstToken(), SyntaxKind.ReturnKeyword); }); From 837dddaec37630b70416d42d7e95e0e747b40f84 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 4 Sep 2014 11:54:16 -0700 Subject: [PATCH 4/5] Addressed CR feedback. --- src/compiler/checker.ts | 31 ------------------------------- src/compiler/parser.ts | 31 +++++++++++++++++++++++++++++++ src/services/services.ts | 4 ++-- 3 files changed, 33 insertions(+), 33 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 0fc14f157d7..d9b2b556b21 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7124,35 +7124,4 @@ module ts { } } } - - // WARNING: This has the same semantics as the forEach family of functions, - // in that traversal terminates in the event that 'visitor' supplies a truthy value. - export function forEachReturnStatement(body: Block, visitor: (stmt: ReturnStatement) => T): T { - - return traverse(body); - - function traverse(node: Node): T { - switch (node.kind) { - case SyntaxKind.ReturnStatement: - return visitor(node); - case SyntaxKind.Block: - case SyntaxKind.FunctionBlock: - case SyntaxKind.IfStatement: - case SyntaxKind.DoStatement: - case SyntaxKind.WhileStatement: - case SyntaxKind.ForStatement: - case SyntaxKind.ForInStatement: - case SyntaxKind.WithStatement: - case SyntaxKind.SwitchStatement: - case SyntaxKind.CaseClause: - case SyntaxKind.DefaultClause: - case SyntaxKind.LabelledStatement: - case SyntaxKind.TryStatement: - case SyntaxKind.TryBlock: - case SyntaxKind.CatchBlock: - case SyntaxKind.FinallyBlock: - return forEachChild(node, traverse); - } - } - } } diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 1e957cd2abe..7e03366bc1a 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -349,6 +349,37 @@ module ts { } } + // Warning: This has the same semantics as the forEach family of functions, + // in that traversal terminates in the event that 'visitor' supplies a truthy value. + export function forEachReturnStatement(body: Block, visitor: (stmt: ReturnStatement) => T): T { + + return traverse(body); + + function traverse(node: Node): T { + switch (node.kind) { + case SyntaxKind.ReturnStatement: + return visitor(node); + case SyntaxKind.Block: + case SyntaxKind.FunctionBlock: + case SyntaxKind.IfStatement: + case SyntaxKind.DoStatement: + case SyntaxKind.WhileStatement: + case SyntaxKind.ForStatement: + case SyntaxKind.ForInStatement: + case SyntaxKind.WithStatement: + case SyntaxKind.SwitchStatement: + case SyntaxKind.CaseClause: + case SyntaxKind.DefaultClause: + case SyntaxKind.LabelledStatement: + case SyntaxKind.TryStatement: + case SyntaxKind.TryBlock: + case SyntaxKind.CatchBlock: + case SyntaxKind.FinallyBlock: + return forEachChild(node, traverse); + } + } + } + export function hasRestParameters(s: SignatureDeclaration): boolean { return s.parameters.length > 0 && (s.parameters[s.parameters.length - 1].flags & NodeFlags.Rest) !== 0; } diff --git a/src/services/services.ts b/src/services/services.ts index 2ad05fdb726..5860dee8e8d 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2279,8 +2279,8 @@ module ts { } var keywords: Node[] = [] - forEachReturnStatement((func).body, returnStmt => { - pushKeywordIf(keywords, returnStmt.getFirstToken(), SyntaxKind.ReturnKeyword); + forEachReturnStatement((func).body, returnStatement => { + pushKeywordIf(keywords, returnStatement.getFirstToken(), SyntaxKind.ReturnKeyword); }); return map(keywords, keywordToReferenceEntry); From 7b5440bb8dda9135bb24c844448f3463f8ea7502 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Thu, 4 Sep 2014 12:17:35 -0700 Subject: [PATCH 5/5] Addressed more CR feedback. --- src/compiler/checker.ts | 11 ----------- src/compiler/parser.ts | 26 ++++++++++++++++++++++++++ src/services/services.ts | 20 +------------------- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index d9b2b556b21..e7c0b5bc32a 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -7113,15 +7113,4 @@ module ts { return checker; } - - export function getContainingFunction(node: Node): SignatureDeclaration { - while (true) { - node = node.parent; - if (!node || node.kind === SyntaxKind.FunctionDeclaration || node.kind === SyntaxKind.FunctionExpression || - node.kind === SyntaxKind.ArrowFunction || node.kind === SyntaxKind.Method || node.kind === SyntaxKind.Constructor || - node.kind === SyntaxKind.GetAccessor || node.kind === SyntaxKind.SetAccessor) { - return node; - } - } - } } diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 7e03366bc1a..eea647999ac 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -380,6 +380,32 @@ module ts { } } + export function isAnyFunction(node: Node): boolean { + if (node) { + switch (node.kind) { + case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + case SyntaxKind.ArrowFunction: + case SyntaxKind.Method: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + case SyntaxKind.Constructor: + return true; + } + } + + return false; + } + + export function getContainingFunction(node: Node): SignatureDeclaration { + while (true) { + node = node.parent; + if (!node || isAnyFunction(node)) { + return node; + } + } + } + export function hasRestParameters(s: SignatureDeclaration): boolean { return s.parameters.length > 0 && (s.parameters[s.parameters.length - 1].flags & NodeFlags.Rest) !== 0; } diff --git a/src/services/services.ts b/src/services/services.ts index 5860dee8e8d..2ed73a52cb7 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1312,24 +1312,6 @@ module ts { return node.parent.kind === SyntaxKind.NewExpression && (node.parent).func === node; } - function isAnyFunction(node: Node): boolean { - if (!node) { - return false; - } - - switch (node.kind) { - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.ArrowFunction: - case SyntaxKind.Method: - case SyntaxKind.GetAccessor: - case SyntaxKind.SetAccessor: - case SyntaxKind.Constructor: - return true; - } - return false; - } - function isNameOfFunctionDeclaration(node: Node): boolean { return node.kind === SyntaxKind.Identifier && isAnyFunction(node.parent) && (node.parent).name === node; @@ -2274,7 +2256,7 @@ module ts { var func = getContainingFunction(returnStatement); // If we didn't find a containing function with a block body, bail out. - if (!(isAnyFunction(func) && hasKind(func.body, SyntaxKind.FunctionBlock))) { + if (!(func && hasKind(func.body, SyntaxKind.FunctionBlock))) { return undefined; }