Fix crash in goto-def on @override
#51016
Conversation
When the base type is not defined, getDefinitionFromOverriddenMember will have its type as errorType, which has no symbol. The error handling previously only handled the case of no baseType at all -- which I'm not sure ever actually happens.
1. getTypeAtLocation never returns undefined, only errorType, so check for that. 2. Return directly after missing baseTypeNode instead of continuing to return later.
typescript-bot
added
Author: Team
For Milestone Bug
|
baselines must use getSymbolAtLocation because lots of them change. I'll look at them after the weekend and see if the changes will affect anything but baselines. |
This reverts commit 4c1b031.
|
I reverted the change to getSymbolAtLocation and I'll make that a separate PR, since it's more of an improvement in code readability and possibly in language service functionality. |
|
@typescript-bot test tsserver top100 |
|
Heya @DanielRosenwasser, I've started to run the diff-based top-repos suite (tsserver) on this PR at c1babc8. You can monitor the build here. |
| if (!baseType) return; | ||
| if (!baseTypeNode) return; | ||
| const expression = skipParentheses(baseTypeNode.expression); | ||
| const base = isClassExpression(expression) ? expression.symbol : typeChecker.getSymbolAtLocation(expression); |
There was a problem hiding this comment.
Actually, why are you checking for isClassExpression? Can't you use getDeclaredTypeOfSymbol?
There was a problem hiding this comment.
I have to follow Node -> Symbol -> Type, and getSymbolAtLocation doesn't support (parenthesised) class expressions right now, which breaks the Node -> Symbol. Previously, the code used getTypeAtLocation, which does support class expressions. I happen to know that class expressions have a symbol, so I can special-case the Node -> Symbol step for now.
I'm going to follow this PR with one that moves the paren/class-expr check into getTypeAtLocation, but it changes lots of our baselines since, apparently, we use it for type baselines.
|
Half of the top100 failed to run, but the half that did has no new errors. I think that's good enough. |
getDefinitionFromOverriddenMember previously assumed that getTypeOfSymbolAtLocation returned
undefinedfor a missing type, noterrorType. This fixes that.Fixes #50892