[ty] Added support for "go to definition" for attribute accesses and keyword arguments#19417
Conversation
…keyword arguments.
UnboundVariable
requested review from
AlexWaygood,
MichaReiser,
carljm,
dcreager and
sharkdp
as code owners
|
MichaReiser
added
server
| // Find the call expression that contains this keyword | ||
| let module = parsed_module(db, file).load(db); |
There was a problem hiding this comment.
Hmm, this seems a bit unfortunate that we need to go and search the covering node again. Would it make sense to instead store the CallExpr in KeywordArgument alongside the keyword?
Getting the function there should be cheaper because CoveringNode allows upward traversal whereas calling covering_node requires an AST traversal
There was a problem hiding this comment.
I'd advise against doing preemptive micro-optimizations here. This is not a hot code path, so using more memory in an attempt to speed it up is probably not the right tradeoff.
There was a problem hiding this comment.
I don't think this is a micro optimization. It just seems unnecessary if we can get this information when extracting the GoToDefinitionTarget. It's right there and I find it even less confusing
3 participants
This PR builds upon #19371. It addresses a few additional code review suggestions and adds support for attribute accesses (expressions of the form
x.y) and keyword arguments within call expressions.