Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 121 additions & 1 deletion pyrefly/lib/lsp/wasm/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,27 @@ use pyrefly_python::docstring::Docstring;
use pyrefly_python::ignore::Ignore;
use pyrefly_python::ignore::find_comment_start_in_line;
use pyrefly_python::symbol_kind::SymbolKind;
use pyrefly_types::callable::Callable;
use pyrefly_types::callable::Param;
use pyrefly_types::callable::ParamList;
use pyrefly_types::callable::Params;
use pyrefly_types::callable::Required;
use pyrefly_types::types::BoundMethodType;
use pyrefly_types::types::Forallable;
use pyrefly_types::types::OverloadType;
use pyrefly_types::types::Type;
use pyrefly_util::lined_buffer::LineNumber;
use pyrefly_util::visit::VisitMut;
use ruff_python_ast::name::Name;
use ruff_text_size::TextSize;
use starlark_map::small_set::SmallSet;

use crate::alt::answers_solver::AnswersSolver;
use crate::error::error::Error;
use crate::state::lsp::FindDefinitionItemWithDocstring;
use crate::state::lsp::FindPreference;
use crate::state::state::Transaction;
use crate::state::state::TransactionHandle;

/// Gets all suppressed errors that overlap with the given line.
///
Expand Down Expand Up @@ -95,6 +107,7 @@ pub struct HoverValue {
pub name: Option<String>,
pub type_: Type,
pub docstring: Option<Docstring>,
pub display: Option<String>,
}

impl HoverValue {
Expand Down Expand Up @@ -159,6 +172,10 @@ impl HoverValue {
.map_or("".to_owned(), |s| format!("{s}: "));
let symbol_def_formatted =
HoverValue::format_symbol_def_locations(&self.type_).unwrap_or("".to_owned());
let type_display = self
.display
.clone()
.unwrap_or_else(|| self.type_.as_hover_string());

Hover {
contents: HoverContents::Markup(MarkupContent {
Expand All @@ -167,7 +184,7 @@ impl HoverValue {
"```python\n{}{}{}\n```{}{}",
kind_formatted,
name_formatted,
self.type_.as_hover_string(),
type_display,
docstring_formatted,
symbol_def_formatted
),
Expand All @@ -177,6 +194,101 @@ impl HoverValue {
}
}

fn collect_typed_dict_fields_for_hover<'a>(
solver: &AnswersSolver<TransactionHandle<'a>>,
ty: &Type,
) -> Option<Vec<(Name, Type, Required)>> {
match ty {
Type::Unpack(inner) => match inner.as_ref() {
Type::TypedDict(typed_dict) => {
let fields = solver.type_order().typed_dict_kw_param_info(typed_dict);
if fields.is_empty() {
None
} else {
Some(fields)
}
}
_ => None,
},
_ => None,
}
}

fn expand_callable_kwargs_for_hover<'a>(
solver: &AnswersSolver<TransactionHandle<'a>>,
callable: &mut Callable,
) {
if let Params::List(param_list) = &mut callable.params {
let mut expanded = Vec::with_capacity(param_list.len());
let mut changed = false;
for param in param_list.items() {
if let Param::Kwargs(_, ty) = param {
if let Some(fields) = collect_typed_dict_fields_for_hover(solver, ty) {
changed = true;
for (field_name, field_type, required) in fields {
expanded.push(Param::KwOnly(field_name, field_type, required));
Copy link
Contributor

@yangdanny97 yangdanny97 Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since required parameters can't come after non-required ones (there isn't a type checking issue here, just looks weird when we display it), we'd probably want to put the required TypedDict fields ahead of any non-required parameters. After the list is expanded, we can partition expanded by required-ness to make the new list.

}
}
}
expanded.push(param.clone());
}
if changed {
*param_list = ParamList::new(expanded);
}
}
}

/// Walks the type and rewrites any callable signatures so they display expanded TypedDict-based
/// `**kwargs` entries, ensuring hover text shows the actual keyword names users can pass.
fn expand_type_for_hover<'a>(solver: &AnswersSolver<TransactionHandle<'a>>, ty: &mut Type) {
ty.visit_mut(&mut |t| match t {
Type::Callable(callable) => expand_callable_kwargs_for_hover(solver, callable.as_mut()),
Type::Function(function) => {
expand_callable_kwargs_for_hover(solver, &mut function.signature)
}
Type::BoundMethod(bound_method) => match &mut bound_method.func {
BoundMethodType::Function(function) => {
expand_callable_kwargs_for_hover(solver, &mut function.signature)
}
BoundMethodType::Forall(forall) => {
expand_callable_kwargs_for_hover(solver, &mut forall.body.signature)
}
BoundMethodType::Overload(overload) => {
for sig in overload.signatures.iter_mut() {
match sig {
OverloadType::Function(function) => {
expand_callable_kwargs_for_hover(solver, &mut function.signature)
}
OverloadType::Forall(forall) => {
expand_callable_kwargs_for_hover(solver, &mut forall.body.signature)
}
}
}
}
},
Type::Forall(forall) => match &mut forall.body {
Forallable::Callable(callable) => expand_callable_kwargs_for_hover(solver, callable),
Forallable::Function(function) => {
expand_callable_kwargs_for_hover(solver, &mut function.signature)
}
Forallable::TypeAlias(_) => {}
},
Type::Overload(overload) => {
for sig in overload.signatures.iter_mut() {
match sig {
OverloadType::Function(function) => {
expand_callable_kwargs_for_hover(solver, &mut function.signature)
}
OverloadType::Forall(forall) => {
expand_callable_kwargs_for_hover(solver, &mut forall.body.signature)
}
}
}
}
_ => {}
});
}

pub fn get_hover(
transaction: &Transaction<'_>,
handle: &Handle,
Expand Down Expand Up @@ -214,6 +326,13 @@ pub fn get_hover(

// Otherwise, fall through to the existing type hover logic
let type_ = transaction.get_type_at(handle, position)?;
let type_display = transaction.ad_hoc_solve(handle, {
let mut cloned = type_.clone();
move |solver| {
expand_type_for_hover(&solver, &mut cloned);
cloned.as_hover_string()
}
});
let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
metadata,
definition_range: definition_location,
Expand Down Expand Up @@ -254,6 +373,7 @@ pub fn get_hover(
name,
type_,
docstring,
display: type_display,
}
.format(),
)
Expand Down
37 changes: 37 additions & 0 deletions pyrefly/lib/test/lsp/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,43 @@ from lib import foo_renamed
);
}

#[test]
fn hover_shows_unpacked_kwargs_fields() {
let code = r#"
from typing import TypedDict, Unpack

class Payload(TypedDict):
foo: int
bar: str
baz: bool | None

def takes(**kwargs: Unpack[Payload]) -> None:
...

takes(foo=1, bar="x", baz=None)
#^
"#;
let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report);
assert_eq!(
r#"
# main.py
12 | takes(foo=1, bar="x", baz=None)
^
```python
(function) takes: def takes(
*,
foo: int,
bar: str,
baz: bool | None,
**kwargs: Unpack[TypedDict[Payload]]
) -> None: ...
```
"#
.trim(),
report.trim(),
);
}

#[test]
fn hover_over_inline_ignore_comment() {
let code = r#"
Expand Down