fix Hover on datetime.datetime import does not show constructor signature & docstring #2814#2847
fix Hover on datetime.datetime import does not show constructor signature & docstring #2814#2847asukaminato0721 wants to merge 2 commits intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Pyrefly’s hover rendering so that hovering a non-definition class symbol (e.g., an imported class) shows the constructor call signature instead of the generic type[Class] form, while preserving docstring display. It also updates and adds tests to validate the new hover output.
Changes:
- Add hover formatting logic to display class constructor signatures for non-definition class symbols.
- Update existing LSP interaction hover tests to match the new hover rendering.
- Add a new hover regression test covering imported class constructor signature + docstring.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyrefly/lib/lsp/wasm/hover.rs |
Adds constructor-signature rendering for imported/non-definition class hovers and wires it into get_hover. |
pyrefly/lib/test/lsp/lsp_interaction/hover.rs |
Updates hover interaction assertions to match the new constructor-style output. |
pyrefly/lib/test/lsp/lsp_interaction/configuration.rs |
Updates hover interaction assertions in configuration-related tests to match new output. |
pyrefly/lib/test/lsp/hover.rs |
Adds a new regression test for imported class hover showing constructor signature + docstring. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn class_constructor_display( | ||
| transaction: &Transaction<'_>, | ||
| handle: &Handle, | ||
| position: TextSize, | ||
| kind: Option<SymbolKind>, | ||
| type_: &Type, | ||
| name: Option<&str>, | ||
| ) -> Option<String> { | ||
| if kind != Some(SymbolKind::Class) | ||
| || transaction | ||
| .identifier_at(handle, position) | ||
| .is_some_and(|id| matches!(id.context, IdentifierContext::ClassDef { .. })) | ||
| { | ||
| return None; | ||
| } |
There was a problem hiding this comment.
class_constructor_display will also rewrite hovers for class symbols whose displayed name differs from the underlying ClassType being hovered (e.g. from typing import List currently shows List: type[list], but this logic would likely switch it to the list constructor signature). That can be misleading because the hover header still says (class) List while the signature corresponds to a different class. Consider adding a guard that only applies this constructor formatting when the hovered symbol name matches the underlying class name (cls.name()), otherwise fall back to the existing type[...] display.
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D97669633. |
Summary
Fixes #2814
changing class hover rendering so non-definition class symbols show constructor information instead of the generic type[Class] form, while still keeping the class docstring.
Test Plan
add test