fix LSP - typing.NamedTuple tooltip is not informative #2844#2846
fix LSP - typing.NamedTuple tooltip is not informative #2844#2846asukaminato0721 wants to merge 1 commit intofacebook:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes LSP hover/signature help for typing.NamedTuple constructor calls by preventing the constructor trace from being overwritten by __init__, so IDE tooltips show the meaningful field-based signature.
Changes:
- Adjust constructor trace recording priority to prefer metaclass
__call__, otherwise overridden__new__, otherwise__init__. - Add LSP signature help regression test for
NamedTupleconstructor showing named fields and return type. - Add LSP hover regression test for
NamedTupleconstructor showing named fields and return type.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pyrefly/lib/alt/call.rs |
Changes trace-recording order/conditions for constructor calls to avoid __init__ overwriting more-informative call traces. |
pyrefly/lib/test/lsp/signature_help.rs |
Adds signature help regression test asserting NamedTuple constructor shows field params (not *Unknown/**Unknown). |
pyrefly/lib/test/lsp/hover.rs |
Adds hover regression test asserting NamedTuple constructor shows field params (not *Unknown/**Unknown). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let hint = None; // discard hint | ||
| let class_metadata = self.get_metadata_for_class(cls.class_object()); | ||
| let metaclass_dunder_call = self.get_metaclass_dunder_call(&cls); | ||
| if let Some(ret) = | ||
| self.call_metaclass(&cls, arguments_range, args, keywords, errors, context, hint) |
There was a problem hiding this comment.
construct_class now calls get_metaclass_dunder_call unconditionally (and that internally calls get_metadata_for_class), even when the class has no custom metaclass __call__. Since constructor analysis is on a hot path, this adds an extra metadata/member lookup for every class construction. Consider deriving a has_metaclass_call bool from call_metaclass(...) (or returning the resolved __call__ type from it) so you only perform the metaclass lookup when it actually exists, or refactor get_metaclass_dunder_call to reuse the already-computed class_metadata.
| { | ||
| if let Some(metaclass_dunder_call) = self.get_metaclass_dunder_call(&cls) { | ||
| if let Some(metaclass_dunder_call) = metaclass_dunder_call.clone() { | ||
| if let Some(callee_range) = callee_range |
There was a problem hiding this comment.
The outer metaclass_dunder_call is an Option<...> but the inner if let Some(metaclass_dunder_call) = metaclass_dunder_call.clone() reuses the same identifier for the unwrapped value, which is easy to misread (especially since the outer value is still used later for gating trace recording). Renaming the inner binding (e.g. metaclass_dunder_call_ty) would make the trace-priority logic clearer.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Summary
Fixes #2844
The checker was still analyzing both
__new__and__init__, but the LSP trace for a constructor call was getting overwritten by init, which is why NamedTuple showed the useless*Unknown, **Unknowntooltip.I changed the trace priority so IDE features keep metaclass
__call__first, otherwise overridden__new__, otherwise__init__.Test Plan
add test