Skip to content

Allow to find all references of the 'this 'keyword #9270

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
4 commits merged into from
Jun 24, 2016
Merged

Allow to find all references of the 'this 'keyword #9270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e7acef1
Allow to find all references of the 'this 'keyword
Jun 17, 2016
07437a6
Make goto-definition work for `this` parameter
Jun 20, 2016
6260772
Remove Signature#thisType and use Signature#thisParameter everywhere
Jun 21, 2016
89c992a
Respond to PR comments
Jun 23, 2016
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
145 changes: 88 additions & 57 deletions src/compiler/checker.ts
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1866,7 +1866,7 @@ namespace ts {
buildTypeParameterDisplay(tp: TypeParameter, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
buildTypePredicateDisplay(predicate: TypePredicate, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
buildTypeParameterDisplayFromSymbol(symbol: Symbol, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
buildDisplayForParametersAndDelimiters(thisType: Type, parameters: Symbol[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
buildDisplayForParametersAndDelimiters(thisParameter: Symbol, parameters: Symbol[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
buildDisplayForTypeParametersAndDelimiters(typeParameters: TypeParameter[], writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
buildReturnTypeDisplay(signature: Signature, writer: SymbolWriter, enclosingDeclaration?: Node, flags?: TypeFormatFlags): void;
}
Expand Down Expand Up @@ -2386,7 +2386,8 @@ namespace ts {
declaration: SignatureDeclaration; // Originating declaration
typeParameters: TypeParameter[]; // Type parameters (undefined if non-generic)
parameters: Symbol[]; // Parameters
thisType?: Type; // type of this-type
/* @internal */
thisParameter?: Symbol; // symbol of this-type parameter
Copy link
Member

@sandersn sandersn Jun 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

It was never quite right to resolve thisType in getSignatureOfDeclaration so switching to a symbol is an improvement. But it means that you should remove thisType entirely and replace usages with getTypeOfSymbol(sig.thisParameter).

/* @internal */
resolvedReturnType: Type; // Resolved return type
/* @internal */
Expand Down
10 changes: 5 additions & 5 deletions src/harness/fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ namespace FourSlash {
// Find the unaccounted-for reference.
for (const actual of actualReferences) {
if (!ts.forEach(expectedReferences, r => r.start === actual.textSpan.start)) {
this.raiseError(`A reference ${actual} is unaccounted for.`);
this.raiseError(`A reference ${stringify(actual)} is unaccounted for.`);
}
}
// Probably will never reach here.
Expand Down Expand Up @@ -907,13 +907,13 @@ namespace FourSlash {
assert.equal(getDisplayPartsJson(actualQuickInfo.documentation), getDisplayPartsJson(documentation), this.messageAtLastKnownMarker("QuickInfo documentation"));
}

public verifyRenameLocations(findInStrings: boolean, findInComments: boolean) {
public verifyRenameLocations(findInStrings: boolean, findInComments: boolean, ranges?: Range[]) {
const renameInfo = this.languageService.getRenameInfo(this.activeFile.fileName, this.currentCaretPosition);
if (renameInfo.canRename) {
let references = this.languageService.findRenameLocations(
this.activeFile.fileName, this.currentCaretPosition, findInStrings, findInComments);

let ranges = this.getRanges();
ranges = ranges || this.getRanges();

if (!references) {
if (ranges.length !== 0) {
Expand Down Expand Up @@ -3128,8 +3128,8 @@ namespace FourSlashInterface {
this.state.verifyRenameInfoFailed(message);
}

public renameLocations(findInStrings: boolean, findInComments: boolean) {
this.state.verifyRenameLocations(findInStrings, findInComments);
public renameLocations(findInStrings: boolean, findInComments: boolean, ranges?: FourSlash.Range[]) {
this.state.verifyRenameLocations(findInStrings, findInComments, ranges);
}

public verifyQuickInfoDisplayParts(kind: string, kindModifiers: string, textSpan: { start: number; length: number; },
Expand Down
43 changes: 29 additions & 14 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,7 +779,7 @@ namespace ts {
declaration: SignatureDeclaration;
typeParameters: TypeParameter[];
parameters: Symbol[];
thisType: Type;
thisParameter: Symbol;
resolvedReturnType: Type;
minArgumentCount: number;
hasRestParameter: boolean;
Expand Down Expand Up @@ -5811,17 +5811,32 @@ namespace ts {
return undefined;
}

if (node.kind !== SyntaxKind.Identifier &&
// TODO (drosen): This should be enabled in a later release - currently breaks rename.
// node.kind !== SyntaxKind.ThisKeyword &&
// node.kind !== SyntaxKind.SuperKeyword &&
node.kind !== SyntaxKind.StringLiteral &&
!isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) {
return undefined;
switch (node.kind) {
case SyntaxKind.NumericLiteral:
if (!isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) {
break;
}
// Fallthrough
case SyntaxKind.Identifier:
case SyntaxKind.ThisKeyword:
// case SyntaxKind.SuperKeyword: TODO:GH#9268
case SyntaxKind.StringLiteral:
return getReferencedSymbolsForNode(node, program.getSourceFiles(), findInStrings, findInComments);
}
return undefined;
}

Debug.assert(node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.NumericLiteral || node.kind === SyntaxKind.StringLiteral);
return getReferencedSymbolsForNode(node, program.getSourceFiles(), findInStrings, findInComments);
function isThis(node: Node): boolean {
switch (node.kind) {
case SyntaxKind.ThisKeyword:
// case SyntaxKind.ThisType: TODO: GH#9267
return true;
case SyntaxKind.Identifier:
// 'this' as a parameter
return (node as Identifier).originalKeywordKind === SyntaxKind.ThisKeyword && node.parent.kind === SyntaxKind.Parameter;
default:
return false;
}
}

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

if (node.kind === SyntaxKind.ThisKeyword || node.kind === SyntaxKind.ThisType) {
if (isThis(node)) {
return getReferencesForThisKeyword(node, sourceFiles);
}

Expand Down Expand Up @@ -6376,7 +6391,7 @@ namespace ts {
cancellationToken.throwIfCancellationRequested();

const node = getTouchingWord(sourceFile, position);
if (!node || (node.kind !== SyntaxKind.ThisKeyword && node.kind !== SyntaxKind.ThisType)) {
if (!node || !isThis(node)) {
return;
}

Expand Down Expand Up @@ -8003,11 +8018,11 @@ namespace ts {

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

// Can only rename an identifier.
if (node) {
if (node.kind === SyntaxKind.Identifier ||
node.kind === SyntaxKind.StringLiteral ||
isLiteralNameOfPropertyDeclarationOrIndexAccess(node)) {
isLiteralNameOfPropertyDeclarationOrIndexAccess(node) ||
isThis(node)) {
const symbol = typeChecker.getSymbolAtLocation(node);

// Only allow a symbol to be renamed if it actually has at least one declaration.
Expand Down
10 changes: 5 additions & 5 deletions src/services/signatureHelp.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,8 +357,8 @@ namespace ts.SignatureHelp {
}

function getArgumentIndex(argumentsList: Node, node: Node) {
// The list we got back can include commas. In the presence of errors it may
// also just have nodes without commas. For example "Foo(a b c)" will have 3
// The list we got back can include commas. In the presence of errors it may
// also just have nodes without commas. For example "Foo(a b c)" will have 3
// args without commas. We want to find what index we're at. So we count
// forward until we hit ourselves, only incrementing the index if it isn't a
// comma.
Expand Down Expand Up @@ -390,8 +390,8 @@ namespace ts.SignatureHelp {
// 'a' '<comma>'. So, in the case where the last child is a comma, we increase the
// arg count by one to compensate.
//
// Note: this subtlety only applies to the last comma. If you had "Foo(a,," then
// we'll have: 'a' '<comma>' '<missing>'
// Note: this subtlety only applies to the last comma. If you had "Foo(a,," then
// we'll have: 'a' '<comma>' '<missing>'
// That will give us 2 non-commas. We then add one for the last comma, givin us an
// arg count of 3.
const listChildren = argumentsList.getChildren();
Expand Down Expand Up @@ -563,7 +563,7 @@ namespace ts.SignatureHelp {
signatureHelpParameters = typeParameters && typeParameters.length > 0 ? map(typeParameters, createSignatureHelpParameterForTypeParameter) : emptyArray;
suffixDisplayParts.push(punctuationPart(SyntaxKind.GreaterThanToken));
const parameterParts = mapToDisplayParts(writer =>
typeChecker.getSymbolDisplayBuilder().buildDisplayForParametersAndDelimiters(candidateSignature.thisType, candidateSignature.parameters, writer, invocation));
typeChecker.getSymbolDisplayBuilder().buildDisplayForParametersAndDelimiters(candidateSignature.thisParameter, candidateSignature.parameters, writer, invocation));
addRange(suffixDisplayParts, parameterParts);
}
else {
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/inferParameterWithMethodCallInitializer.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ function weird(this: Example, a = this.getNumber()) {
>Example : Symbol(Example, Decl(inferParameterWithMethodCallInitializer.ts, 2, 1))
>a : Symbol(a, Decl(inferParameterWithMethodCallInitializer.ts, 11, 29))
>this.getNumber : Symbol(Example.getNumber, Decl(inferParameterWithMethodCallInitializer.ts, 3, 15))
>this : Symbol(Example, Decl(inferParameterWithMethodCallInitializer.ts, 2, 1))
>this : Symbol(this, Decl(inferParameterWithMethodCallInitializer.ts, 11, 15))
>getNumber : Symbol(Example.getNumber, Decl(inferParameterWithMethodCallInitializer.ts, 3, 15))

return a;
Expand All @@ -45,7 +45,7 @@ class Weird {
>Example : Symbol(Example, Decl(inferParameterWithMethodCallInitializer.ts, 2, 1))
>a : Symbol(a, Decl(inferParameterWithMethodCallInitializer.ts, 15, 30))
>this.getNumber : Symbol(Example.getNumber, Decl(inferParameterWithMethodCallInitializer.ts, 3, 15))
>this : Symbol(Example, Decl(inferParameterWithMethodCallInitializer.ts, 2, 1))
>this : Symbol(this, Decl(inferParameterWithMethodCallInitializer.ts, 15, 16))
>getNumber : Symbol(Example.getNumber, Decl(inferParameterWithMethodCallInitializer.ts, 3, 15))

return a;
Expand Down
20 changes: 10 additions & 10 deletions tests/baselines/reference/thisTypeInAccessors.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ const explicit = {
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 7, 10))
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 7, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))

set x(this: Foo, n: number) { this.n = n; }
Expand All @@ -29,7 +29,7 @@ const explicit = {
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 8, 20))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 8, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 8, 20))
}
Expand All @@ -44,14 +44,14 @@ const copiedFromGetter = {
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 12, 10))
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 12, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))

set x(n) { this.n = n; }
>x : Symbol(x, Decl(thisTypeInAccessors.ts, 11, 10), Decl(thisTypeInAccessors.ts, 12, 48))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 13, 10))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 12, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 13, 10))
}
Expand All @@ -64,7 +64,7 @@ const copiedFromSetter = {
get x() { return this.n },
>x : Symbol(x, Decl(thisTypeInAccessors.ts, 16, 10), Decl(thisTypeInAccessors.ts, 17, 30))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 18, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))

set x(this: Foo, n: number) { this.n = n; }
Expand All @@ -73,7 +73,7 @@ const copiedFromSetter = {
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 18, 20))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 18, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 18, 20))
}
Expand All @@ -88,15 +88,15 @@ const copiedFromGetterUnannotated = {
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 22, 10))
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 22, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))

set x(this, n) { this.n = n; }
>x : Symbol(x, Decl(thisTypeInAccessors.ts, 21, 10), Decl(thisTypeInAccessors.ts, 22, 39))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 23, 10))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 23, 15))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 23, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 23, 15))
}
Expand All @@ -112,7 +112,7 @@ class Explicit {
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 28, 10))
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 28, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))

set x(this: Foo, n: number) { this.n = n; }
Expand All @@ -121,7 +121,7 @@ class Explicit {
>Foo : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 29, 20))
>this.n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>this : Symbol(Foo, Decl(thisTypeInAccessors.ts, 0, 0))
>this : Symbol(this, Decl(thisTypeInAccessors.ts, 29, 10))
>n : Symbol(Foo.n, Decl(thisTypeInAccessors.ts, 0, 15))
>n : Symbol(n, Decl(thisTypeInAccessors.ts, 29, 20))
}
Expand Down
Loading