Repository navigation
[ty] Group overloaded methods like overloaded functions for docstrings - #23920
Conversation
Typing conformance resultsNo changes detected ✅Current numbersThe percentage of diagnostics emitted that were expected errors held steady at 87.94%. The percentage of expected errors that received a diagnostic held steady at 83.36%. The number of fully passing files held steady at 79/133. |
|
Memory usage reportMemory usage unchanged ✅ |
bd71b71 to
afb5ed1
Compare
|
After adding the suggested tests, I had to revise the implementation a bit since it surfaced some issues -- sorry for the churn. |
|
|
||
| let docs = if typed_dict_key.is_some() { | ||
| None | ||
| } else if let GotoTarget::Call { callable, call, .. } = goto_target { |
There was a problem hiding this comment.
Can you say more why you think it's preferred to handle overloads here vs in get_definition_targets as I did in #21920 ?
There was a problem hiding this comment.
Here, we want to return the precise overload that matches the currently-provided arguments. So we actually need to match on the arguments provided to the call. That seems a bit different than what you're doing in get_definition_targets, unless I'm misunderstanding.
There was a problem hiding this comment.
I never managed to finish this, but my thinking was that we want an option to parametrize go_to_definition_targets with what we're resolving: Is it definitions, declarations, exact definition etc. But given that this isn't in place today and i remember that I ran into some issues with it (which is why the PR is still in draft), I think it's okay to go with your current approach and I can revisit it in #21920.
afb5ed1 to
6cba58f
Compare
4e309c3 to
fb6ee71
Compare
1eb110b to
3e89515
Compare
3e89515 to
c85c37e
Compare
c85c37e to
e816157
Compare
| &model, | ||
| ty_python_semantic::ImportAliasResolution::ResolveAliases, | ||
| ) | ||
| .and_then(|definitions| definitions.docstring(db)) |
There was a problem hiding this comment.
This is different than signature help, where we want to make sure implementation and docstring are 1:1.
| /// When the resolved overload has no docstring and neither does the | ||
| /// implementation, we fall back to showing a sibling overload's docstring. |
There was a problem hiding this comment.
This is something where Pylance is inconsistent. Their signature help does show "a second overload" but hover doesn't. I don't feel strongly whether we should have this fallback, for as long as it is consistent (which we are :))
#23920) ## Summary Previously, attribute lookup for methods stopped after the first matching definition in the first matching class scope. That meant overloaded methods often resolved to just an overload declaration, which usually has no docstring... while overloaded free functions could still surface the implementation docstring. Closes astral-sh/ty#3024.
3 participants
Summary
Previously, attribute lookup for methods stopped after the first matching definition in the first matching class scope. That meant overloaded methods often resolved to just an overload declaration, which usually has no docstring... while overloaded free functions could still surface the implementation docstring.
Closes astral-sh/ty#3024.