-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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.
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
Conversation
@@ -3198,6 +3198,13 @@ namespace ts { | |||
return undefined; | |||
} | |||
|
|||
//neater |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops, pushed WIP
aca2d6c
to
f1f82e1
Compare
@@ -2388,6 +2388,8 @@ namespace ts { | |||
parameters: Symbol[]; // Parameters | |||
thisType?: Type; // type of this-type | |||
/* @internal */ | |||
thisParameter?: Symbol; // symbol of this-type parameter |
There was a problem hiding this comment.
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)
.
Looks good except for getting rid of |
@yuit and I discussed the removal of if (declaration.kind === SyntaxKind.Identifier && declaration.originalKeyword.kind === SytaxKind.ThisKeyword) {
return links.type = declaration.type ? getTypeFromTypeNode(declaration.type) : unknownType;
} Of course you may need to add some type assertions in there. |
return parameter && parameter.symbol; | ||
} | ||
|
||
function getAnnotatedAccessorThisType(accessor: AccessorDeclaration): Type | undefined { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this used now?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
👍 |
Fixes #9198. Closes #9037 because we don't need to prevent renaming if it works.