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 2 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
37 changes: 29 additions & 8 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ namespace ts {

const noConstraintType = createAnonymousType(undefined, emptySymbols, emptyArray, emptyArray, undefined, undefined);

const anySignature = createSignature(undefined, undefined, undefined, emptyArray, anyType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasStringLiterals*/ false);
const unknownSignature = createSignature(undefined, undefined, undefined, emptyArray, unknownType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasStringLiterals*/ false);
const anySignature = createSignature(undefined, undefined, undefined, undefined, emptyArray, anyType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasStringLiterals*/ false);
const unknownSignature = createSignature(undefined, undefined, undefined, undefined, emptyArray, unknownType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasStringLiterals*/ false);

const enumNumberIndexInfo = createIndexInfo(stringType, /*isReadonly*/ true);

Expand Down Expand Up @@ -3198,6 +3198,11 @@ namespace ts {
return undefined;
}

function getAnnotatedAccessorThisParameter(accessor: AccessorDeclaration): Symbol {
Copy link
Member

@DanielRosenwasser DanielRosenwasser Jun 20, 2016

Choose a reason for hiding this comment

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

?

Copy link
Author

@ghost ghost Jun 20, 2016

Choose a reason for hiding this comment

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

Whoops, pushed WIP

const parameter = getAccessorThisParameter(accessor);
return parameter && parameter.symbol;
}

Copy link
Member

@sandersn sandersn Jun 23, 2016

Choose a reason for hiding this comment

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

is this used now?

Copy link
Author

@ghost ghost Jun 23, 2016

Choose a reason for hiding this comment

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

Linter would complain if it weren't.

Copy link
Member

@sandersn sandersn Jun 23, 2016

Choose a reason for hiding this comment

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

I looked it up: checkAccessorDeclaration still uses it.

function getAnnotatedAccessorThisType(accessor: AccessorDeclaration): Type {
if (accessor) {
const parameter = getAccessorThisParameter(accessor);
Expand Down Expand Up @@ -3888,12 +3893,13 @@ namespace ts {
resolveObjectTypeMembers(type, source, typeParameters, typeArguments);
}

function createSignature(declaration: SignatureDeclaration, typeParameters: TypeParameter[], thisType: Type, parameters: Symbol[],
function createSignature(declaration: SignatureDeclaration, typeParameters: TypeParameter[], thisParameter: Symbol | undefined, thisType: Type | undefined, parameters: Symbol[],
resolvedReturnType: Type, typePredicate: TypePredicate, minArgumentCount: number, hasRestParameter: boolean, hasStringLiterals: boolean): Signature {
const sig = new Signature(checker);
sig.declaration = declaration;
sig.typeParameters = typeParameters;
sig.parameters = parameters;
sig.thisParameter = thisParameter;
sig.thisType = thisType;
sig.resolvedReturnType = resolvedReturnType;
sig.typePredicate = typePredicate;
Expand All @@ -3904,15 +3910,15 @@ namespace ts {
}

function cloneSignature(sig: Signature): Signature {
return createSignature(sig.declaration, sig.typeParameters, sig.thisType, sig.parameters, sig.resolvedReturnType,
return createSignature(sig.declaration, sig.typeParameters, sig.thisParameter, sig.thisType, sig.parameters, sig.resolvedReturnType,
sig.typePredicate, sig.minArgumentCount, sig.hasRestParameter, sig.hasStringLiterals);
}

function getDefaultConstructSignatures(classType: InterfaceType): Signature[] {
const baseConstructorType = getBaseConstructorTypeOfClass(classType);
const baseSignatures = getSignaturesOfType(baseConstructorType, SignatureKind.Construct);
if (baseSignatures.length === 0) {
return [createSignature(undefined, classType.localTypeParameters, undefined, emptyArray, classType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasStringLiterals*/ false)];
return [createSignature(undefined, classType.localTypeParameters, undefined, undefined, emptyArray, classType, /*typePredicate*/ undefined, 0, /*hasRestParameter*/ false, /*hasStringLiterals*/ false)];
}
const baseTypeNode = getBaseTypeNodeOfClass(classType);
const typeArguments = map(baseTypeNode.typeArguments, getTypeFromTypeNode);
Expand Down Expand Up @@ -4454,6 +4460,7 @@ namespace ts {
const parameters: Symbol[] = [];
let hasStringLiterals = false;
let minArgumentCount = -1;
let thisParameter: Symbol = undefined;
let thisType: Type = undefined;
let hasThisParameter: boolean;
const isJSConstructSignature = isJSDocConstructSignature(declaration);
Expand All @@ -4472,6 +4479,7 @@ namespace ts {
}
if (i === 0 && paramSymbol.name === "this") {
hasThisParameter = true;
thisParameter = param.symbol;
thisType = param.type ? getTypeFromTypeNode(param.type) : unknownType;
}
else {
Expand All @@ -4498,8 +4506,11 @@ namespace ts {
!hasDynamicName(declaration) &&
(!hasThisParameter || thisType === unknownType)) {
const otherKind = declaration.kind === SyntaxKind.GetAccessor ? SyntaxKind.SetAccessor : SyntaxKind.GetAccessor;
const setter = <AccessorDeclaration>getDeclarationOfKind(declaration.symbol, otherKind);
thisType = getAnnotatedAccessorThisType(setter);
const other = <AccessorDeclaration>getDeclarationOfKind(declaration.symbol, otherKind);
if (other) {
thisParameter = getAnnotatedAccessorThisParameter(other);
thisType = getAnnotatedAccessorThisType(other);
}
}

if (minArgumentCount < 0) {
Expand All @@ -4520,7 +4531,7 @@ namespace ts {
createTypePredicateFromTypePredicateNode(declaration.type as TypePredicateNode) :
undefined;

links.resolvedSignature = createSignature(declaration, typeParameters, thisType, parameters, returnType, typePredicate, minArgumentCount, hasRestParameter(declaration), hasStringLiterals);
links.resolvedSignature = createSignature(declaration, typeParameters, thisParameter, thisType, parameters, returnType, typePredicate, minArgumentCount, hasRestParameter(declaration), hasStringLiterals);
}
return links.resolvedSignature;
}
Expand Down Expand Up @@ -5453,6 +5464,7 @@ namespace ts {
freshTypePredicate = cloneTypePredicate(signature.typePredicate, mapper);
}
const result = createSignature(signature.declaration, freshTypeParameters,
signature.thisParameter && instantiateSymbol(signature.thisParameter, mapper),
signature.thisType && instantiateType(signature.thisType, mapper),
instantiateList(signature.parameters, mapper, instantiateSymbol),
instantiateType(signature.resolvedReturnType, mapper),
Expand Down Expand Up @@ -17156,6 +17168,15 @@ namespace ts {
return getSymbolOfEntityNameOrPropertyAccessExpression(<EntityName | PropertyAccessExpression>node);

case SyntaxKind.ThisKeyword:
const container = getThisContainer(node, /*includeArrowFunctions*/ false);
if (isFunctionLike(container)) {
const sig = getSignatureFromDeclaration(container);
if (sig.thisParameter) {
return sig.thisParameter;
}
}
// fallthrough

case SyntaxKind.SuperKeyword:
const type = isExpression(node) ? checkExpression(<Expression>node) : getTypeFromTypeNode(<TypeNode>node);
return type.symbol;
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2388,6 +2388,8 @@ namespace ts {
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 */
minArgumentCount: number; // Number of non-optional parameters
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
42 changes: 29 additions & 13 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ namespace ts {
declaration: SignatureDeclaration;
typeParameters: TypeParameter[];
parameters: Symbol[];
thisParameter: Symbol;
thisType: Type;
resolvedReturnType: Type;
minArgumentCount: number;
Expand Down Expand Up @@ -5811,17 +5812,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 +5857,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 +6392,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 +8019,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
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, 22, 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