Skip to content

Get occurrences for 'super' keywords #632

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DanielRosenwasser merged 5 commits into from
Sep 9, 2014
Merged

Get occurrences for 'super' keywords #632

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1121e11
Basic implementation without tests for findAllRefs/getOccs for 'super…
DanielRosenwasser Sep 6, 2014
1cd0b30
Added tests for getOccurrences on super.
DanielRosenwasser Sep 8, 2014
131ac2f
Disabled findAllRefs for 'this'/'super'.
DanielRosenwasser Sep 9, 2014
0e93d28
Separated 'super'/'this' keyword searching to simplify logic.
DanielRosenwasser Sep 9, 2014
83c35ad
Addressed CR feedback.
DanielRosenwasser Sep 9, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ module ts {
while (true) {
node = node.parent;
if (!node) {
return node;
return undefined;
}
switch (node.kind) {
case SyntaxKind.ArrowFunction:
Expand All @@ -434,6 +434,23 @@ module ts {
}
}

export function getSuperContainer(node: Node): Node {
while (true) {
node = node.parent;
if (!node) {

@CyrusNajmabadi CyrusNajmabadi Sep 9, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return undefined.

@CyrusNajmabadi CyrusNajmabadi Sep 9, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still return undefined :)

you can also make this simpler with just:

while (node) {
   switch (node.kind) { ... }
   node = node.parent;
}

@mhegazy mhegazy Sep 9, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for the undefined

@DanielRosenwasser DanielRosenwasser Sep 9, 2014

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The people have spoken, the undefined has prevailed 😉.

return undefined;
}
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;
}
Expand Down
99 changes: 70 additions & 29 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2134,7 +2134,6 @@ module ts {
return result;
}

/// Find references
function getOccurrencesAtPosition(filename: string, position: number): ReferenceEntry[] {
synchronizeHostData();

Expand All @@ -2146,7 +2145,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]);
}
Expand Down Expand Up @@ -2378,7 +2377,9 @@ module ts {
}

if (node.kind !== SyntaxKind.Identifier &&
node.kind !== SyntaxKind.ThisKeyword &&
// 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;
Expand Down Expand Up @@ -2406,6 +2407,10 @@ module ts {
return getReferencesForThisKeyword(node, sourceFiles);
}

if (node.kind === SyntaxKind.SuperKeyword) {
return getReferencesForSuperKeyword(node);
}

var symbol = typeInfoResolver.getSymbolInfo(node);

// Could not find a symbol e.g. unknown identifier
Expand Down Expand Up @@ -2627,32 +2632,75 @@ module ts {
}
}

function getReferencesForThisKeyword(thisKeyword: Node, sourceFiles: SourceFile[]) {
// Get the owner" of the 'this' keyword.
var thisContainer = getThisContainer(thisKeyword, /* includeArrowFunctions */ false);
function getReferencesForSuperKeyword(superKeyword: Node): ReferenceEntry[]{
var searchSpaceNode = getSuperContainer(superKeyword);
if (!searchSpaceNode) {
return undefined;
}
// Whether 'super' occurs in a static context within a class.
var staticFlag = NodeFlags.Static;

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;
}

var result: ReferenceEntry[] = [];

var searchSpaceNode: Node;
var sourceFile = searchSpaceNode.getSourceFile();
var possiblePositions = getPossibleSymbolReferencePositions(sourceFile, "super", searchSpaceNode.getStart(), searchSpaceNode.getEnd());
forEach(possiblePositions, position => {
cancellationToken.throwIfCancellationRequested();

// Whether 'this' occurs in a static context within a class;
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 (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(<SourceFile>thisContainer)) {
if (isExternalModule(<SourceFile>searchSpaceNode)) {
return undefined;
}
// Fall through
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.FunctionExpression:
searchSpaceNode = thisContainer;
break;
default:
return undefined;
Expand Down Expand Up @@ -2683,31 +2731,24 @@ module ts {
return;
}

// Get the owner of the 'this' keyword.
// This *should* be a node that occurs somewhere within searchSpaceNode.
var container = getThisContainer(node, /* includeArrowFunctions */ false);

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) {
switch (searchSpaceNode.kind) {
case SyntaxKind.FunctionExpression:
case SyntaxKind.FunctionDeclaration:
if (searchSpaceNode.symbol === container.symbol) {
result.push(getReferenceEntryFromNode(node));
}
break;
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.FunctionExpression:
if (searchSpaceNode.symbol === container.symbol) {
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:
// Add all 'this' keywords that belong to the top-level scope.
if (searchSpaceNode.kind === SyntaxKind.SourceFile && !isExternalModule(<SourceFile>searchSpaceNode)) {
if (container.kind === SyntaxKind.SourceFile && !isExternalModule(<SourceFile>container)) {
result.push(getReferenceEntryFromNode(node));
}
break;
Expand Down
5 changes: 4 additions & 1 deletion tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@

goTo.file("file1.ts");
goTo.marker();
verify.referencesCountIs(8);

// 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);
64 changes: 64 additions & 0 deletions tests/cases/fourslash/getOccurrencesSuper.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/// <reference path='fourslash.ts' />

////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);
});
64 changes: 64 additions & 0 deletions tests/cases/fourslash/getOccurrencesSuper2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/// <reference path='fourslash.ts' />

////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);
});
27 changes: 27 additions & 0 deletions tests/cases/fourslash/getOccurrencesSuperNegatives.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/// <reference path='fourslash.ts' />

////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);
});