Skip to content

Commit e7acef1

Browse files
author
Andy Hanson
committed
Allow to find all references of the 'this 'keyword
1 parent ba78d1e commit e7acef1

6 files changed

Lines changed: 116 additions & 29 deletions

File tree

‎src/harness/fourslash.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ namespace FourSlash {
760760
// Find the unaccounted-for reference.
761761
for (const actual of actualReferences) {
762762
if (!ts.forEach(expectedReferences, r => r.start === actual.textSpan.start)) {
763-
this.raiseError(`A reference ${actual} is unaccounted for.`);
763+
this.raiseError(`A reference ${stringify(actual)} is unaccounted for.`);
764764
}
765765
}
766766
// Probably will never reach here.
@@ -907,13 +907,13 @@ namespace FourSlash {
907907
assert.equal(getDisplayPartsJson(actualQuickInfo.documentation), getDisplayPartsJson(documentation), this.messageAtLastKnownMarker("QuickInfo documentation"));
908908
}
909909

910-
public verifyRenameLocations(findInStrings: boolean, findInComments: boolean) {
910+
public verifyRenameLocations(findInStrings: boolean, findInComments: boolean, ranges?: Range[]) {
911911
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition);
912912
if (renameInfo.canRename) {
913913
let references = this.languageService.findRenameLocations(
914914
this.activeFile.fileName, this.currentCaretPosition, findInStrings, findInComments);
915915

916-
let ranges = this.getRanges();
916+
ranges = ranges || this.getRanges();
917917

918918
if (!references) {
919919
if (ranges.length !== 0) {
@@ -3128,8 +3128,8 @@ namespace FourSlashInterface {
31283128
this.state.verifyRenameInfoFailed(message);
31293129
}
31303130

3131-
public renameLocations(findInStrings: boolean, findInComments: boolean) {
3132-
this.state.verifyRenameLocations(findInStrings, findInComments);
3131+
public renameLocations(findInStrings: boolean, findInComments: boolean, ranges?: FourSlash.Range[]) {
3132+
this.state.verifyRenameLocations(findInStrings, findInComments, ranges);
31333133
}
31343134

31353135
public verifyQuickInfoDisplayParts(kind: string, kindModifiers: string, textSpan: { start: number; length: number; },

‎src/services/services.ts‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5811,17 +5811,32 @@ namespace ts {
58115811
return undefined;
58125812
}
58135813

5814-
if (node.kind !== SyntaxKind.Identifier &&
5815-
// TODO (drosen): This should be enabled in a later release - currently breaks rename.
5816-
// node.kind !== SyntaxKind.ThisKeyword &&
5817-
// node.kind !== SyntaxKind.SuperKeyword &&
5818-
node.kind !== SyntaxKind.StringLiteral &&
5819-
!isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) {
5820-
return undefined;
5814+
switch (node.kind) {
5815+
case SyntaxKind.NumericLiteral:
5816+
if (!isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) {
5817+
break;
5818+
}
5819+
// Fallthrough
5820+
case SyntaxKind.Identifier:
5821+
case SyntaxKind.ThisKeyword:
5822+
// case SyntaxKind.SuperKeyword: TODO:GH#9268
5823+
case SyntaxKind.StringLiteral:
5824+
return getReferencedSymbolsForNode(node, program.getSourceFiles(), findInStrings, findInComments);
58215825
}
5826+
return undefined;
5827+
}
58225828

5823-
Debug.assert(node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.NumericLiteral || node.kind === SyntaxKind.StringLiteral);
5824-
return getReferencedSymbolsForNode(node, program.getSourceFiles(), findInStrings, findInComments);
5829+
function isThis(node: Node): boolean {
5830+
switch (node.kind) {
5831+
case SyntaxKind.ThisKeyword:
5832+
// case SyntaxKind.ThisType: TODO: GH#9267
5833+
return true;
5834+
case SyntaxKind.Identifier:
5835+
// 'this' as a parameter
5836+
return (node as Identifier).originalKeywordKind === SyntaxKind.ThisKeyword && node.parent.kind === SyntaxKind.Parameter;
5837+
default:
5838+
return false;
5839+
}
58255840
}
58265841

58275842
function getReferencedSymbolsForNode(node: Node, sourceFiles: SourceFile[], findInStrings: boolean, findInComments: boolean): ReferencedSymbol[] {
@@ -5841,7 +5856,7 @@ namespace ts {
58415856
}
58425857
}
58435858

5844-
if (node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.ThisType) {
5859+
if (isThis(node)) {
58455860
return getReferencesForThisKeyword(node, sourceFiles);
58465861
}
58475862

@@ -6376,7 +6391,7 @@ namespace ts {
63766391
cancellationToken.throwIfCancellationRequested();
63776392

63786393
const node = getTouchingWord(sourceFile, position);
6379-
if (!node || (node.kind !== SyntaxKind.ThisKeyword && node.kind !== SyntaxKind.ThisType)) {
6394+
if (!node || !isThis(node)) {
63806395
return;
63816396
}
63826397

@@ -8003,11 +8018,11 @@ namespace ts {
80038018

80048019
const node = getTouchingWord(sourceFile, position, /*includeJsDocComment*/ true);
80058020

8006-
// Can only rename an identifier.
80078021
if (node) {
80088022
if (node.kind === SyntaxKind.Identifier ||
80098023
node.kind === SyntaxKind.StringLiteral ||
8010-
isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) {
8024+
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
8025+
isThis(node)) {
80118026
const symbol = typeChecker.getSymbolAtLocation(node);
80128027

80138028
// Only allow a symbol to be renamed if it actually has at least one declaration.
@@ -8054,6 +8069,26 @@ namespace ts {
80548069
}
80558070
}
80568071
}
8072+
else if (node.kind === SyntaxKind.ThisKeyword) {
8073+
const container = ts.getThisContainer(node, /*includeArrowFunctions*/ false);
8074+
// Only allow rename to change a function with a 'this' type to one with a regular parameter,
8075+
// e.g. `function(this: number) { return this; }` to `function(x: number) { return x; }`
8076+
if (isFunctionLike(container)) {
8077+
const sig = typeChecker.getSignatureFromDeclaration(container);
8078+
if (sig.thisType) {
8079+
return {
8080+
canRename: true,
8081+
kind: ScriptElementKind.parameterElement,
8082+
displayName: "this",
8083+
localizedErrorMessage: undefined,
8084+
fullDisplayName: "this",
8085+
kindModifiers: "",
8086+
triggerSpan: createTriggerSpanForNode(node, sourceFile)
8087+
};
8088+
}
8089+
}
8090+
// fallthrough to error
8091+
}
80578092
}
80588093
}
80598094

‎tests/cases/fourslash/findAllRefsThisKeyword.ts‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
/// <reference path='fourslash.ts' />
2+
// @noLib: true
3+
4+
////[|this|];
5+
////function f([|this|]) {
6+
//// return [|this|];
7+
//// function g([|this|]) { return [|this|]; }
8+
////}
9+
////class C {
10+
//// static x() {
11+
//// [|this|];
12+
//// }
13+
//// static y() {
14+
//// () => [|this|];
15+
//// }
16+
//// constructor() {
17+
//// [|this|];
18+
//// }
19+
//// method() {
20+
//// () => [|this|];
21+
//// }
22+
////}
23+
////// These are *not* real uses of the 'this' keyword, they are identifiers.
24+
////const x = { [|this|]: 0 }
25+
////x.[|this|];
26+
27+
const [global, f0, f1, g0, g1, x, y, constructor, method, propDef, propUse] = test.ranges();
28+
verify.referencesOf(global, [global]);
29+
verify.rangesReferenceEachOther([f0, f1]);
30+
verify.rangesReferenceEachOther([g0, g1]);
31+
verify.rangesReferenceEachOther([x, y]);
32+
verify.rangesReferenceEachOther([constructor, method]);
33+
verify.rangesReferenceEachOther([propDef, propUse]);

‎tests/cases/fourslash/findAllRefsThisKeywordMultipleFiles.ts‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,15 @@
11
/// <reference path='fourslash.ts' />
22

33
// @Filename: file1.ts
4-
////this; this;
4+
////[|this|]; [|this|];
55

66
// @Filename: file2.ts
7-
////this;
8-
////this;
7+
////[|this|];
8+
////[|this|];
99

1010
// @Filename: file3.ts
11-
//// ((x = this, y) => t/**/his)(this, this);
11+
//// ((x = [|this|], y) => [|this|])([|this|], [|this|]);
12+
//// // different 'this'
13+
//// function f(this) { return this; }
1214

13-
goTo.file("file1.ts");
14-
goTo.marker();
15-
16-
// TODO (drosen): The CURRENT behavior is that findAllRefs doesn't work on 'this' or 'super' keywords.
17-
// This should change down the line.
18-
verify.referencesCountIs(0);
15+
verify.rangesReferenceEachOther();

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ declare namespace FourSlashInterface {
216216
}[]): void;
217217
renameInfoSucceeded(displayName?: string, fullDisplayName?: string, kind?: string, kindModifiers?: string): void;
218218
renameInfoFailed(message?: string): void;
219-
renameLocations(findInStrings: boolean, findInComments: boolean): void;
219+
renameLocations(findInStrings: boolean, findInComments: boolean, ranges?: Range[]): void;
220220
verifyQuickInfoDisplayParts(kind: string, kindModifiers: string, textSpan: {
221221
start: number;
222222
length: number;

‎tests/cases/fourslash/renameThis.ts‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/// <reference path='fourslash.ts'/>
2+
3+
////function f([|this|]) {
4+
//// return [|this|];
5+
////}
6+
////this/**/;
7+
////const _ = { [|this|]: 0 }.[|this|];
8+
9+
let [r0, r1, r2, r3] = test.ranges()
10+
for (let range of [r0, r1]) {
11+
goTo.position(range.start);
12+
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false, [r0, r1]);
13+
}
14+
15+
// Trying to rename a legitimate 'this' should fail
16+
goTo.marker();
17+
verify.renameInfoFailed("You cannot rename this element.");
18+
19+
for (let range of [r2, r3]) {
20+
goTo.position(range.start);
21+
verify.renameLocations(/*findInStrings*/ false, /*findInComments*/ false, [r2, r3]);
22+
}

0 commit comments

Comments
 (0)