fix hover on __init__ parameter shows parent type #2503#3020
fix hover on __init__ parameter shows parent type #2503#3020asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes LSP hover behavior for constructor keyword arguments (notably dataclass-generated __init__ params) so that hovering the keyword shows the field/parameter type rather than the parent class type.
Changes:
- Add regression tests asserting dataclass constructor kwarg hover shows the field type.
- Extend keyword-argument type resolution to fall back to constructor signature parameter types when definition refinement fails.
- Adjust hover’s definition selection so keyword-argument hovers don’t get labeled as the callee class.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyrefly/lib/test/lsp/hover.rs |
Adds hover regression test for dataclass constructor keyword showing field type and not class label. |
pyrefly/lib/test/lsp/hover_type.rs |
Adds type-hover regression test ensuring dataclass constructor kwarg hover resolves to int. |
pyrefly/lib/state/lsp.rs |
Adds constructor-kwarg type fallback in get_type_at for keyword arguments when definition refinement fails. |
pyrefly/lib/lsp/wasm/hover.rs |
Filters hover definition metadata for keyword arguments to avoid mislabeling as the callee class. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .into_iter() | ||
| .next() | ||
| .filter(|item| { | ||
| keyword_argument_identifier | ||
| .as_ref() |
There was a problem hiding this comment.
The find_definition result is truncated with .into_iter().next() before applying the keyword-argument filter. If find_definition returns multiple candidates (e.g., multiple callee locations), the first item might be filtered out even though a later item matches the keyword identifier, causing hover to lose definition metadata/name. Consider iterating definitions and selecting the first item whose definition_range text matches the keyword identifier (or only falling back when none match).
| return None; | ||
| } | ||
| self.get_type(handle, &key) | ||
| }), | ||
| }) | ||
| .or_else(|| self.get_constructor_keyword_argument_type(handle, position)), |
There was a problem hiding this comment.
In the keyword-argument type path, only the first item from find_definition_for_keyword_argument(...).first() is considered; if that first location can’t be refined (definition_range points at the callee) but a later location can, the type will incorrectly fall back (now to get_constructor_keyword_argument_type) or return None. Since find_definition_for_keyword_argument can return multiple locations, consider scanning for a candidate whose definition_range text matches the keyword name before falling back.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #2503
Test Plan