Skip to content

Commit 165ee55

Browse files
asukaminato0721meta-codesync[bot]
authored andcommitted
fix Feat: Expand **kwargs with field names on hover when **kwargs is typed with Unpack (i.e. **kwargs: Unpack[T]) #758 (#1514)
Summary: fix #758 Hover output now expands **kwargs typed with typing.Unpack[TypedDict] into their constituent keyword-only parameters so people hovering a call see the real argument names. Implemented the expansion by letting HoverValue accept a pre-computed display string, formatting callables via transaction.ad_hoc_solve, and inserting typed-dict fields as synthetic Param::KwOnly entries before the original **kwargs. Pull Request resolved: #1514 Reviewed By: kinto0 Differential Revision: D86638374 Pulled By: yangdanny97 fbshipit-source-id: a41c8debf417727fd3f297671ec789ff3ae2af0a
1 parent c0ecefa commit 165ee55

File tree

3 files changed

+142
-1
lines changed

3 files changed

+142
-1
lines changed

crates/pyrefly_types/src/types.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1162,6 +1162,44 @@ impl Type {
11621162
}
11631163
}
11641164

1165+
/// Apply `f` to this type if it is a callable. Note that we do *not* recurse into the type to
1166+
/// find nested callable types.
1167+
pub fn visit_toplevel_callable_mut<'a>(&'a mut self, mut f: impl FnMut(&'a mut Callable)) {
1168+
match self {
1169+
Type::Callable(callable) => f(callable),
1170+
Type::Forall(box Forall {
1171+
body: Forallable::Callable(callable),
1172+
..
1173+
}) => f(callable),
1174+
Type::Function(box func)
1175+
| Type::Forall(box Forall {
1176+
body: Forallable::Function(func),
1177+
..
1178+
})
1179+
| Type::BoundMethod(box BoundMethod {
1180+
func: BoundMethodType::Function(func),
1181+
..
1182+
})
1183+
| Type::BoundMethod(box BoundMethod {
1184+
func: BoundMethodType::Forall(Forall { body: func, .. }),
1185+
..
1186+
}) => f(&mut func.signature),
1187+
Type::Overload(overload)
1188+
| Type::BoundMethod(box BoundMethod {
1189+
func: BoundMethodType::Overload(overload),
1190+
..
1191+
}) => {
1192+
for x in overload.signatures.iter_mut() {
1193+
match x {
1194+
OverloadType::Function(function) => f(&mut function.signature),
1195+
OverloadType::Forall(forall) => f(&mut forall.body.signature),
1196+
}
1197+
}
1198+
}
1199+
_ => {}
1200+
}
1201+
}
1202+
11651203
fn is_toplevel_callable(&self) -> bool {
11661204
let mut is_callable = false;
11671205
self.visit_toplevel_callable(&mut |_| is_callable = true);

pyrefly/lib/lsp/wasm/hover.rs

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,23 @@ use pyrefly_python::docstring::Docstring;
1515
use pyrefly_python::ignore::Ignore;
1616
use pyrefly_python::ignore::find_comment_start_in_line;
1717
use pyrefly_python::symbol_kind::SymbolKind;
18+
use pyrefly_types::callable::Callable;
19+
use pyrefly_types::callable::Param;
20+
use pyrefly_types::callable::ParamList;
21+
use pyrefly_types::callable::Params;
22+
use pyrefly_types::callable::Required;
1823
use pyrefly_types::types::Type;
1924
use pyrefly_util::lined_buffer::LineNumber;
25+
use ruff_python_ast::name::Name;
2026
use ruff_text_size::TextSize;
2127

28+
use crate::alt::answers_solver::AnswersSolver;
2229
use crate::error::error::Error;
2330
use crate::lsp::module_helpers::collect_symbol_def_paths;
2431
use crate::state::lsp::FindDefinitionItemWithDocstring;
2532
use crate::state::lsp::FindPreference;
2633
use crate::state::state::Transaction;
34+
use crate::state::state::TransactionHandle;
2735

2836
/// Gets all suppressed errors that overlap with the given line.
2937
///
@@ -95,6 +103,7 @@ pub struct HoverValue {
95103
pub name: Option<String>,
96104
pub type_: Type,
97105
pub docstring: Option<Docstring>,
106+
pub display: Option<String>,
98107
}
99108

100109
impl HoverValue {
@@ -158,6 +167,10 @@ impl HoverValue {
158167
.map_or("".to_owned(), |s| format!("{s}: "));
159168
let symbol_def_formatted =
160169
HoverValue::format_symbol_def_locations(&self.type_).unwrap_or("".to_owned());
170+
let type_display = self
171+
.display
172+
.clone()
173+
.unwrap_or_else(|| self.type_.as_hover_string());
161174

162175
Hover {
163176
contents: HoverContents::Markup(MarkupContent {
@@ -166,7 +179,7 @@ impl HoverValue {
166179
"```python\n{}{}{}\n```{}{}",
167180
kind_formatted,
168181
name_formatted,
169-
self.type_.as_hover_string(),
182+
type_display,
170183
docstring_formatted,
171184
symbol_def_formatted
172185
),
@@ -176,6 +189,49 @@ impl HoverValue {
176189
}
177190
}
178191

192+
fn collect_typed_dict_fields_for_hover<'a>(
193+
solver: &AnswersSolver<TransactionHandle<'a>>,
194+
ty: &Type,
195+
) -> Option<Vec<(Name, Type, Required)>> {
196+
match ty {
197+
Type::Unpack(inner) => match inner.as_ref() {
198+
Type::TypedDict(typed_dict) => {
199+
let fields = solver.type_order().typed_dict_kw_param_info(typed_dict);
200+
if fields.is_empty() {
201+
None
202+
} else {
203+
Some(fields)
204+
}
205+
}
206+
_ => None,
207+
},
208+
_ => None,
209+
}
210+
}
211+
212+
fn expand_callable_kwargs_for_hover<'a>(
213+
solver: &AnswersSolver<TransactionHandle<'a>>,
214+
callable: &mut Callable,
215+
) {
216+
if let Params::List(param_list) = &mut callable.params {
217+
let mut expanded = Vec::with_capacity(param_list.len());
218+
let mut changed = false;
219+
for param in param_list.items() {
220+
if let Param::Kwargs(_, ty) = param
221+
&& let Some(fields) = collect_typed_dict_fields_for_hover(solver, ty)
222+
{
223+
changed = true;
224+
for (field_name, field_type, required) in fields {
225+
expanded.push(Param::KwOnly(field_name, field_type, required));
226+
}
227+
}
228+
expanded.push(param.clone());
229+
}
230+
if changed {
231+
*param_list = ParamList::new(expanded);
232+
}
233+
}
234+
}
179235
pub fn get_hover(
180236
transaction: &Transaction<'_>,
181237
handle: &Handle,
@@ -213,6 +269,15 @@ pub fn get_hover(
213269

214270
// Otherwise, fall through to the existing type hover logic
215271
let type_ = transaction.get_type_at(handle, position)?;
272+
let type_display = transaction.ad_hoc_solve(handle, {
273+
let mut cloned = type_.clone();
274+
move |solver| {
275+
// If the type is a callable, rewrite the signature to expand TypedDict-based
276+
// `**kwargs` entries, ensuring hover text shows the actual keyword names users can pass.
277+
cloned.visit_toplevel_callable_mut(|c| expand_callable_kwargs_for_hover(&solver, c));
278+
cloned.as_hover_string()
279+
}
280+
});
216281
let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
217282
metadata,
218283
definition_range: definition_location,
@@ -253,6 +318,7 @@ pub fn get_hover(
253318
name,
254319
type_,
255320
docstring,
321+
display: type_display,
256322
}
257323
.format(),
258324
)

pyrefly/lib/test/lsp/hover.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,43 @@ from lib import foo_renamed
8888
);
8989
}
9090

91+
#[test]
92+
fn hover_shows_unpacked_kwargs_fields() {
93+
let code = r#"
94+
from typing import TypedDict, Unpack
95+
96+
class Payload(TypedDict):
97+
foo: int
98+
bar: str
99+
baz: bool | None
100+
101+
def takes(**kwargs: Unpack[Payload]) -> None:
102+
...
103+
104+
takes(foo=1, bar="x", baz=None)
105+
#^
106+
"#;
107+
let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report);
108+
assert_eq!(
109+
r#"
110+
# main.py
111+
12 | takes(foo=1, bar="x", baz=None)
112+
^
113+
```python
114+
(function) takes: def takes(
115+
*,
116+
foo: int,
117+
bar: str,
118+
baz: bool | None,
119+
**kwargs: Unpack[TypedDict[Payload]]
120+
) -> None: ...
121+
```
122+
"#
123+
.trim(),
124+
report.trim(),
125+
);
126+
}
127+
91128
#[test]
92129
fn hover_over_inline_ignore_comment() {
93130
let code = r#"

0 commit comments

Comments
 (0)