From 1121e11c458ff3fdf39acf090fc90d8a5c676338 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Fri, 5 Sep 2014 18:06:36 -0700 Subject: [PATCH 1/5] Basic implementation without tests for findAllRefs/getOccs for 'super' keywords. --- src/compiler/parser.ts | 17 ++++++ src/services/services.ts | 114 +++++++++++++++++++++++---------------- 2 files changed, 84 insertions(+), 47 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index e2d2bb3886b..798c13125d5 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -434,6 +434,23 @@ module ts { } } + export function getSuperContainer(node: Node): Node { + while (true) { + node = node.parent; + if (!node) { + return node; + } + switch (node.kind) { + case SyntaxKind.Property: + case SyntaxKind.Method: + case SyntaxKind.Constructor: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + 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 12f7e0ce24e..a414e70cc54 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2146,7 +2146,7 @@ module ts { return undefined; } - if (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword || + if (node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.SuperKeyword || isLiteralNameOfPropertyDeclarationOrIndexAccess(node) || isNameOfExternalModuleImportOrDeclaration(node)) { return getReferencesForNode(node, [sourceFile]); } @@ -2379,6 +2379,7 @@ module ts { if (node.kind !== SyntaxKind.Identifier && node.kind !== SyntaxKind.ThisKeyword && + node.kind !== SyntaxKind.SuperKeyword && !isLiteralNameOfPropertyDeclarationOrIndexAccess(node) && !isNameOfExternalModuleImportOrDeclaration(node)) { return undefined; @@ -2402,8 +2403,8 @@ module ts { } } - if (node.kind === SyntaxKind.ThisKeyword) { - return getReferencesForThisKeyword(node, sourceFiles); + if (node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.SuperKeyword) { + return getReferencesForThisOrSuperKeyword(node, sourceFiles); } var symbol = typeInfoResolver.getSymbolInfo(node); @@ -2627,32 +2628,46 @@ module ts { } } - function getReferencesForThisKeyword(thisKeyword: Node, sourceFiles: SourceFile[]) { - // Get the owner" of the 'this' keyword. - var thisContainer = getThisContainer(thisKeyword, /* includeArrowFunctions */ false); - + function getReferencesForThisOrSuperKeyword(thisOrSuperKeyword: Node, sourceFiles: SourceFile[]): ReferenceEntry[] { + var keywordName: string; var searchSpaceNode: Node; - // Whether 'this' occurs in a static context within a class; + if (thisOrSuperKeyword.kind === SyntaxKind.ThisKeyword) { + keywordName = "this" + searchSpaceNode = getThisContainer(thisOrSuperKeyword, /* includeArrowFunctions */ false); + } + else { + keywordName = "super"; + searchSpaceNode = getSuperContainer(thisOrSuperKeyword); + + if (!searchSpaceNode) { + return undefined; + } + } + + // Whether 'this'/'super' occurs in a static context within a class. var staticFlag = NodeFlags.Static; - switch (thisContainer.kind) { + switch (searchSpaceNode.kind) { case SyntaxKind.Property: case SyntaxKind.Method: case SyntaxKind.Constructor: case SyntaxKind.GetAccessor: case SyntaxKind.SetAccessor: - searchSpaceNode = thisContainer.parent; // should be the owning class - staticFlag &= thisContainer.flags + staticFlag &= searchSpaceNode.flags + searchSpaceNode = searchSpaceNode.parent; // re-assign to be the owning class break; case SyntaxKind.SourceFile: - if (isExternalModule(thisContainer)) { + if (isExternalModule(searchSpaceNode)) { return undefined; } - // Fall through + break; case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: - searchSpaceNode = thisContainer; + // 'super' can only occur within a class. + if (thisOrSuperKeyword.kind === SyntaxKind.SuperKeyword) { + return undefined; + } break; default: return undefined; @@ -2662,55 +2677,60 @@ module ts { if (searchSpaceNode.kind === SyntaxKind.SourceFile) { forEach(sourceFiles, sourceFile => { - var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, "this", sourceFile.getStart(), sourceFile.getEnd()); - getThisReferencesInFile(sourceFile, sourceFile, possiblePositions, result); + var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, keywordName, sourceFile.getStart(), sourceFile.getEnd()); + getThisOrSuperReferencesInFile(sourceFile, sourceFile, possiblePositions, result); }); } else { var sourceFile = searchSpaceNode.getSourceFile(); - var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, "this", searchSpaceNode.getStart(), searchSpaceNode.getEnd()); - getThisReferencesInFile(sourceFile, searchSpaceNode, possiblePositions, result); + var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, keywordName, searchSpaceNode.getStart(), searchSpaceNode.getEnd()); + getThisOrSuperReferencesInFile(sourceFile, searchSpaceNode, possiblePositions, result); } return result; - function getThisReferencesInFile(sourceFile: SourceFile, searchSpaceNode: Node, possiblePositions: number[], result: ReferenceEntry[]): void { + function getThisOrSuperReferencesInFile(sourceFile: SourceFile, searchSpaceNode: Node, possiblePositions: number[], result: ReferenceEntry[]): void { forEach(possiblePositions, position => { cancellationToken.throwIfCancellationRequested(); var node = getNodeAtPosition(sourceFile, position); - if (!node || node.kind !== SyntaxKind.ThisKeyword) { + if (!node) { return; } - // Get the owner of the 'this' keyword. - // This *should* be a node that occurs somewhere within searchSpaceNode. - var container = getThisContainer(node, /* includeArrowFunctions */ false); + var container = getNodeAtPosition(sourceFile, position); + if (!container) { + return; + } - switch (container.kind) { - case SyntaxKind.Property: - case SyntaxKind.Method: - case SyntaxKind.Constructor: - case SyntaxKind.GetAccessor: - case SyntaxKind.SetAccessor: - // Make sure the container belongs to the same class - // and has the appropriate static modifier from the original container. - if (searchSpaceNode.symbol === container.parent.symbol && (container.flags & NodeFlags.Static) === staticFlag) { - result.push(getReferenceEntryFromNode(node)); - } - break; - case SyntaxKind.FunctionDeclaration: - case SyntaxKind.FunctionExpression: - if (searchSpaceNode.symbol === container.symbol) { - result.push(getReferenceEntryFromNode(node)); - } - break; - case SyntaxKind.SourceFile: - // Add all 'this' keywords that belong to the top-level scope. - if (searchSpaceNode.kind === SyntaxKind.SourceFile && !isExternalModule(searchSpaceNode)) { - result.push(getReferenceEntryFromNode(node)); - } - break; + if (node.kind === SyntaxKind.SuperKeyword) { + container = getSuperContainer(node); + } + else if (node.kind === SyntaxKind.ThisKeyword) { + container = getThisContainer(node, /* includeArrowFunctions */ false); + } + + if (container) { + switch (searchSpaceNode.kind) { + case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + if (searchSpaceNode.symbol === container.symbol) { + result.push(getReferenceEntryFromNode(node)); + } + break; + case SyntaxKind.ClassDeclaration: + // Make sure the container belongs to the same class + // and has the appropriate static modifier from the original container. + if (container.parent && searchSpaceNode.symbol === container.parent.symbol && (container.flags & NodeFlags.Static) === staticFlag) { + result.push(getReferenceEntryFromNode(node)); + } + break; + case SyntaxKind.SourceFile: + if (container.kind === SyntaxKind.SourceFile && !isExternalModule(container)) { + result.push(getReferenceEntryFromNode(node)); + } + break; + } } }); } From 1cd0b306ed309e294d5f89221a4b9205d5c26592 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 8 Sep 2014 14:40:44 -0700 Subject: [PATCH 2/5] Added tests for getOccurrences on super. --- tests/cases/fourslash/getOccurrencesSuper.ts | 64 +++++++++++++++++++ tests/cases/fourslash/getOccurrencesSuper2.ts | 64 +++++++++++++++++++ .../fourslash/getOccurrencesSuperNegatives.ts | 27 ++++++++ 3 files changed, 155 insertions(+) create mode 100644 tests/cases/fourslash/getOccurrencesSuper.ts create mode 100644 tests/cases/fourslash/getOccurrencesSuper2.ts create mode 100644 tests/cases/fourslash/getOccurrencesSuperNegatives.ts diff --git a/tests/cases/fourslash/getOccurrencesSuper.ts b/tests/cases/fourslash/getOccurrencesSuper.ts new file mode 100644 index 00000000000..f53d9aca62e --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSuper.ts @@ -0,0 +1,64 @@ +/// + +////class SuperType { +//// superMethod() { +//// } +//// +//// static superStaticMethod() { +//// return 10; +//// } +////} +//// +////class SubType extends SuperType { +//// public prop1 = [|s/**/uper|].superMethod; +//// private prop2 = [|super|].superMethod; +//// +//// constructor() { +//// [|super|](); +//// } +//// +//// public method1() { +//// return [|super|].superMethod(); +//// } +//// +//// private method2() { +//// return [|super|].superMethod(); +//// } +//// +//// public method3() { +//// var x = () => [|super|].superMethod(); +//// +//// // Bad but still gets highlighted +//// function f() { +//// [|super|].superMethod(); +//// } +//// } +//// +//// // Bad but still gets highlighted. +//// public static statProp1 = super.superStaticMethod; +//// +//// public static staticMethod1() { +//// return super.superStaticMethod(); +//// } +//// +//// private static staticMethod2() { +//// return super.superStaticMethod(); +//// } +//// +//// // Are not actually 'super' keywords. +//// super = 10; +//// static super = 20; +////} + +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/getOccurrencesSuper2.ts b/tests/cases/fourslash/getOccurrencesSuper2.ts new file mode 100644 index 00000000000..1392170ae18 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSuper2.ts @@ -0,0 +1,64 @@ +/// + +////class SuperType { +//// superMethod() { +//// } +//// +//// static superStaticMethod() { +//// return 10; +//// } +////} +//// +////class SubType extends SuperType { +//// public prop1 = super.superMethod; +//// private prop2 = super.superMethod; +//// +//// constructor() { +//// super(); +//// } +//// +//// public method1() { +//// return super.superMethod(); +//// } +//// +//// private method2() { +//// return super.superMethod(); +//// } +//// +//// public method3() { +//// var x = () => super.superMethod(); +//// +//// // Bad but still gets highlighted +//// function f() { +//// super.superMethod(); +//// } +//// } +//// +//// // Bad but still gets highlighted. +//// public static statProp1 = [|super|].superStaticMethod; +//// +//// public static staticMethod1() { +//// return [|super|].superStaticMethod(); +//// } +//// +//// private static staticMethod2() { +//// return [|supe/**/r|].superStaticMethod(); +//// } +//// +//// // Are not actually 'super' keywords. +//// super = 10; +//// static super = 20; +////} + +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/getOccurrencesSuperNegatives.ts b/tests/cases/fourslash/getOccurrencesSuperNegatives.ts new file mode 100644 index 00000000000..8c76da33c12 --- /dev/null +++ b/tests/cases/fourslash/getOccurrencesSuperNegatives.ts @@ -0,0 +1,27 @@ +/// + +////function f(x = [|super|]) { +//// [|super|]; +////} +//// +////module M { +//// [|super|]; +//// function f(x = [|super|]) { +//// [|super|]; +//// } +//// +//// class A { +//// } +//// +//// class B extends A { +//// constructor() { +//// super(); +//// } +//// } +////} + +test.ranges().forEach(r => { + goTo.position(r.start); + + verify.occurrencesAtPositionCount(0); +}); From 131ac2f188cb2ca5a5b9f16dba5a112cb5341eb5 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Mon, 8 Sep 2014 17:44:15 -0700 Subject: [PATCH 3/5] Disabled findAllRefs for 'this'/'super'. --- src/services/services.ts | 5 +++-- tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index a414e70cc54..fa491176269 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2378,8 +2378,9 @@ module ts { } if (node.kind !== SyntaxKind.Identifier && - node.kind !== SyntaxKind.ThisKeyword && - node.kind !== SyntaxKind.SuperKeyword && + // TODO (drosen): This should be enabled in a later release - currently breaks rename. + //node.kind !== SyntaxKind.ThisKeyword && + //node.kind !== SyntaxKind.SuperKeyword && !isLiteralNameOfPropertyDeclarationOrIndexAccess(node) && !isNameOfExternalModuleImportOrDeclaration(node)) { return undefined; diff --git a/tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts b/tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts index bc31ffd1579..4b9f7a450ec 100644 --- a/tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts +++ b/tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts @@ -12,4 +12,7 @@ goTo.file("file1.ts"); goTo.marker(); -verify.referencesCountIs(8); \ No newline at end of file + +// TODO (drosen): The CURRENT behavior is that findAllRefs doesn't work on 'this' or 'super' keywords. +// This should change down the line. +verify.referencesCountIs(0); \ No newline at end of file From 0e93d283e3135414b731b64094fdb4ece9cf0ef1 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 9 Sep 2014 12:41:57 -0700 Subject: [PATCH 4/5] Separated 'super'/'this' keyword searching to simplify logic. --- src/services/services.ts | 138 ++++++++++++++++++++++----------------- 1 file changed, 79 insertions(+), 59 deletions(-) diff --git a/src/services/services.ts b/src/services/services.ts index fa491176269..c8e73f657a4 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -2134,7 +2134,6 @@ module ts { return result; } - /// Find references function getOccurrencesAtPosition(filename: string, position: number): ReferenceEntry[] { synchronizeHostData(); @@ -2404,8 +2403,12 @@ module ts { } } - if (node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.SuperKeyword) { - return getReferencesForThisOrSuperKeyword(node, sourceFiles); + if (node.kind === SyntaxKind.ThisKeyword) { + return getReferencesForThisKeyword(node, sourceFiles); + } + + if (node.kind === SyntaxKind.SuperKeyword) { + return getReferencesForSuperKeyword(node); } var symbol = typeInfoResolver.getSymbolInfo(node); @@ -2629,24 +2632,57 @@ module ts { } } - function getReferencesForThisOrSuperKeyword(thisOrSuperKeyword: Node, sourceFiles: SourceFile[]): ReferenceEntry[] { - var keywordName: string; - var searchSpaceNode: Node; - - if (thisOrSuperKeyword.kind === SyntaxKind.ThisKeyword) { - keywordName = "this" - searchSpaceNode = getThisContainer(thisOrSuperKeyword, /* includeArrowFunctions */ false); + function getReferencesForSuperKeyword(superKeyword: Node): ReferenceEntry[]{ + var searchSpaceNode = getSuperContainer(superKeyword); + if (!searchSpaceNode) { + return undefined; } - else { - keywordName = "super"; - searchSpaceNode = getSuperContainer(thisOrSuperKeyword); + // Whether 'super' occurs in a static context within a class. + var staticFlag = NodeFlags.Static; - if (!searchSpaceNode) { + switch (searchSpaceNode.kind) { + case SyntaxKind.Property: + case SyntaxKind.Method: + case SyntaxKind.Constructor: + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + staticFlag &= searchSpaceNode.flags; + searchSpaceNode = searchSpaceNode.parent; // re-assign to be the owning class + break; + default: return undefined; - } } - // Whether 'this'/'super' occurs in a static context within a class. + var result: ReferenceEntry[] = []; + + var sourceFile = searchSpaceNode.getSourceFile(); + var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, "super", searchSpaceNode.getStart(), searchSpaceNode.getEnd()); + forEach(possiblePositions, position => { + cancellationToken.throwIfCancellationRequested(); + + var node = getNodeAtPosition(sourceFile, position); + + if (!node || node.kind !== SyntaxKind.SuperKeyword) { + return; + } + + var container = getSuperContainer(node); + + // If we have a 'super' container, we must have an enclosing class. + // Now make sure the owning class is the same as the search-space + // and has the same static qualifier as the original 'super's owner. + if (container && (NodeFlags.Static & container.flags) === staticFlag && container.parent.symbol === searchSpaceNode.symbol) { + result.push(getReferenceEntryFromNode(node)); + } + }); + + return result; + } + + function getReferencesForThisKeyword(thisOrSuperKeyword: Node, sourceFiles: SourceFile[]): ReferenceEntry[] { + var searchSpaceNode = getThisContainer(thisOrSuperKeyword, /* includeArrowFunctions */ false); + + // Whether 'this' occurs in a static context within a class. var staticFlag = NodeFlags.Static; switch (searchSpaceNode.kind) { @@ -2662,13 +2698,9 @@ module ts { if (isExternalModule(searchSpaceNode)) { return undefined; } - break; + // Fall through case SyntaxKind.FunctionDeclaration: case SyntaxKind.FunctionExpression: - // 'super' can only occur within a class. - if (thisOrSuperKeyword.kind === SyntaxKind.SuperKeyword) { - return undefined; - } break; default: return undefined; @@ -2678,60 +2710,48 @@ module ts { if (searchSpaceNode.kind === SyntaxKind.SourceFile) { forEach(sourceFiles, sourceFile => { - var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, keywordName, sourceFile.getStart(), sourceFile.getEnd()); - getThisOrSuperReferencesInFile(sourceFile, sourceFile, possiblePositions, result); + var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, "this", sourceFile.getStart(), sourceFile.getEnd()); + getThisReferencesInFile(sourceFile, sourceFile, possiblePositions, result); }); } else { var sourceFile = searchSpaceNode.getSourceFile(); - var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, keywordName, searchSpaceNode.getStart(), searchSpaceNode.getEnd()); - getThisOrSuperReferencesInFile(sourceFile, searchSpaceNode, possiblePositions, result); + var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, "this", searchSpaceNode.getStart(), searchSpaceNode.getEnd()); + getThisReferencesInFile(sourceFile, searchSpaceNode, possiblePositions, result); } return result; - function getThisOrSuperReferencesInFile(sourceFile: SourceFile, searchSpaceNode: Node, possiblePositions: number[], result: ReferenceEntry[]): void { + function getThisReferencesInFile(sourceFile: SourceFile, searchSpaceNode: Node, possiblePositions: number[], result: ReferenceEntry[]): void { forEach(possiblePositions, position => { cancellationToken.throwIfCancellationRequested(); var node = getNodeAtPosition(sourceFile, position); - if (!node) { + if (!node || node.kind !== SyntaxKind.ThisKeyword) { return; } - var container = getNodeAtPosition(sourceFile, position); - if (!container) { - return; - } + var container = getThisContainer(node, /* includeArrowFunctions */ false); - if (node.kind === SyntaxKind.SuperKeyword) { - container = getSuperContainer(node); - } - else if (node.kind === SyntaxKind.ThisKeyword) { - container = getThisContainer(node, /* includeArrowFunctions */ false); - } - - if (container) { - switch (searchSpaceNode.kind) { - case SyntaxKind.FunctionExpression: - case SyntaxKind.FunctionDeclaration: - if (searchSpaceNode.symbol === container.symbol) { - result.push(getReferenceEntryFromNode(node)); - } - break; - case SyntaxKind.ClassDeclaration: - // Make sure the container belongs to the same class - // and has the appropriate static modifier from the original container. - if (container.parent && searchSpaceNode.symbol === container.parent.symbol && (container.flags & NodeFlags.Static) === staticFlag) { - result.push(getReferenceEntryFromNode(node)); - } - break; - case SyntaxKind.SourceFile: - if (container.kind === SyntaxKind.SourceFile && !isExternalModule(container)) { - result.push(getReferenceEntryFromNode(node)); - } - break; - } + switch (searchSpaceNode.kind) { + case SyntaxKind.FunctionExpression: + case SyntaxKind.FunctionDeclaration: + if (searchSpaceNode.symbol === container.symbol) { + result.push(getReferenceEntryFromNode(node)); + } + break; + case SyntaxKind.ClassDeclaration: + // Make sure the container belongs to the same class + // and has the appropriate static modifier from the original container. + if (container.parent && searchSpaceNode.symbol === container.parent.symbol && (container.flags & NodeFlags.Static) === staticFlag) { + result.push(getReferenceEntryFromNode(node)); + } + break; + case SyntaxKind.SourceFile: + if (container.kind === SyntaxKind.SourceFile && !isExternalModule(container)) { + result.push(getReferenceEntryFromNode(node)); + } + break; } }); } From 83c35ad059a440c0a49e31365967bcab1c13b10e Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 9 Sep 2014 12:50:14 -0700 Subject: [PATCH 5/5] Addressed CR feedback. --- src/compiler/parser.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index 798c13125d5..a89152e043c 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -411,7 +411,7 @@ module ts { while (true) { node = node.parent; if (!node) { - return node; + return undefined; } switch (node.kind) { case SyntaxKind.ArrowFunction: @@ -438,7 +438,7 @@ module ts { while (true) { node = node.parent; if (!node) { - return node; + return undefined; } switch (node.kind) { case SyntaxKind.Property: