allow calling goToDef on modifiers of named declarations
#60384
Conversation
typescript-bot
added
Author: Team
For Milestone Bug
iisaduan
requested review from
DanielRosenwasser,
gabritto,
navya9singh,
sandersn and
mjbvz
and removed request for
navya9singh
| switch (node.kind) { | ||
| case SyntaxKind.ReturnKeyword: | ||
| findFunctionDecl = n => { | ||
| return isClassStaticBlockDeclaration(n) |
There was a problem hiding this comment.
This feels like it's very much a special-case. Are you trying to just not break some sort of test case?
There was a problem hiding this comment.
It was existing code, so I'm not sure if there are more cases that would be an issue, but it breaks https://github.com/microsoft/TypeScript/blob/main/tests/cases/fourslash/goToDefinitionReturn5.ts
There was a problem hiding this comment.
It's probably just fine to update the baseline there.
| if (findFunctionDecl) { | ||
| const functionDeclaration = findAncestor(node, findFunctionDecl) as FunctionLikeDeclaration | undefined; | ||
| const isCorrectDeclaration = functionDeclaration && (!checkFunctionDeclaration || checkFunctionDeclaration(functionDeclaration)); | ||
| return isCorrectDeclaration ? [createDefinitionFromSignatureDeclaration(typeChecker, functionDeclaration)] : undefined; |
There was a problem hiding this comment.
I kind of find the indirection here hard to follow. I can see wanting to find some common logic, but I'd suggest just writing it out explicitly in each branch for now.
There was a problem hiding this comment.
Also, the idea that you wouldn't want to still jump to the nearest function even if it's not async or a generator is something I'm not so sure of. So the idea that you need a checkFunctionDeclaration to validate that you got a function of the right kind feels like it would break down unnecessarily during refactoring.
Fixes #60308.
There are some symbols where goToDef/goToType previously did not return a definition. For those cases, I have not changed the behavior, this PR just makes it so that calling
goTo(Type)Definitionon a modifier will do the same behavior as if the call was triggered on the name.The first commit refactors some existing
goToDefinitionkeyword cases. It may be easier to review by looking at the first commit, and then the rest of the commits without the first one.