Go-to-def should include both declaration(s) and target call signature #1543
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request improves the "Go to Definition" functionality by ensuring that both general declarations and specific call signatures are included in the results. When a call signature can be resolved, it replaces function-like declarations while preserving non-function declarations.
- Refactored the definition resolution logic to extract it into a reusable function
- Modified the signature handling to include both general declarations and specific call signatures
- Simplified the location creation logic by removing body-based filtering
| calledDeclaration := tryGetSignatureDeclaration(c, node) | ||
| if calledDeclaration != nil { | ||
| // If we can resolve a call signature, remove all function-like declarations and add that signature. | ||
| nonFunctionDeclarations := core.Filter(slices.Clip(declarations), func(node *ast.Node) bool { return !ast.IsFunctionLike(node) }) |
There was a problem hiding this comment.
Using slices.Clip(declarations) creates an unnecessary copy of the slice. Since core.Filter likely creates a new slice anyway, you can pass declarations directly to avoid the extra allocation.
| nonFunctionDeclarations := core.Filter(slices.Clip(declarations), func(node *ast.Node) bool { return !ast.IsFunctionLike(node) }) | |
| nonFunctionDeclarations := core.Filter(declarations, func(node *ast.Node) bool { return !ast.IsFunctionLike(node) }) |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
core.Filter only creates a new slice if something was removed. Thus the need for slices.Clip.
There was a problem hiding this comment.
Maybe we should just make filter always clip its input for safety?
| calledDeclaration := tryGetSignatureDeclaration(c, node) | ||
| if calledDeclaration != nil { | ||
| // If we can resolve a call signature, remove all function-like declarations and add that signature. | ||
| nonFunctionDeclarations := core.Filter(slices.Clip(declarations), func(node *ast.Node) bool { return !ast.IsFunctionLike(node) }) |
There was a problem hiding this comment.
Maybe we should just make filter always clip its input for safety?
There are several options for slices returned from
The first option leads to excessive allocation and copying. The second option is subtle, but desirable when concatenating slices. The third option is simple to understand, but requires callers to do the right thing. We currently use option three. Not sure the other ones are better. |
Fixes #1528.