-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix completion and quick info crash in type parameter in function in type alias #5781
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
// For example | ||
// type list<T> = T[]; // Both T will go through same code path | ||
let declaration = <TypeAliasDeclaration>getDeclarationOfKind(symbol, SyntaxKind.TypeParameter).parent; | ||
displayParts.push(keywordPart(SyntaxKind.TypeKeyword)); | ||
displayParts.push(spacePart()); | ||
addFullSymbolName(declaration.symbol); |
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.
declaration
may not be defined here with the new if clause
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.
that is a good point. I will have to come up with different test case when that can happen. My first intuition is that that case should not happen since when we get type parameter, it must be in a declaration. Regardless I will handle the behavior.
if (signatureDeclaration.kind === SyntaxKind.ConstructSignature) { | ||
displayParts.push(keywordPart(SyntaxKind.NewKeyword)); | ||
displayParts.push(spacePart()); | ||
let declaration = <Node>getDeclarationOfKind(symbol, SyntaxKind.TypeParameter); |
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.
When can declaration be undefined? Would it suffice to assert instead ?
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 agree that it probably should be assert instead. I will update it
👍 |
@yuit the failing status on this PR was due to the issue in master - it's fixed now, however to update the status on your PR you'll need to refresh it. |
|
||
if (declaration) { | ||
if (isFunctionLikeKind(declaration.kind)) { | ||
let signature = typeChecker.getSignatureFromDeclaration(<SignatureDeclaration>declaration); |
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.
This should probably be const
@weswigham Thanks for the head up! |
Fix completion and quick info crash in type parameter in function in type alias
Fix #4715 and #4616