Skip to content
Open
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
35 changes: 27 additions & 8 deletions pyrefly/lib/lsp/wasm/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use pyrefly_types::types::Type;
use pyrefly_util::lined_buffer::LineNumber;
use pyrefly_util::visit::Visit;
use ruff_python_ast::AnyNodeRef;
use ruff_python_ast::Identifier;
use ruff_python_ast::Stmt;
use ruff_python_ast::name::Name;
use ruff_text_size::Ranged;
Expand Down Expand Up @@ -431,6 +432,16 @@ fn parameter_definition_documentation(
docs.get(key).cloned().map(|doc| (key.to_owned(), doc))
}

fn keyword_argument_identifier(
transaction: &Transaction<'_>,
handle: &Handle,
position: TextSize,
) -> Option<Identifier> {
let identifier = transaction.identifier_at(handle, position)?;
matches!(identifier.context, IdentifierContext::KeywordArgument(_))
.then_some(identifier.identifier)
}

/// Check if the cursor position is on the `in` keyword within a for loop or comprehension.
/// Returns Some(iterable_range) if found, None otherwise.
fn in_keyword_in_iteration_at(
Expand Down Expand Up @@ -549,13 +560,8 @@ pub fn get_hover(
}

let fallback_name_from_type = fallback_hover_name_from_type(&type_);
let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
metadata,
definition_range: definition_location,
module,
docstring_range,
display_name,
}) = transaction
let keyword_argument_identifier = keyword_argument_identifier(transaction, handle, position);
let definition = transaction
.find_definition(
handle,
position,
Expand All @@ -566,9 +572,22 @@ pub fn get_hover(
)
.map(Vec1::into_vec)
.unwrap_or_default()
// TODO: handle more than 1 definition
.into_iter()
.next()
.filter(|item| {
keyword_argument_identifier
.as_ref()
Comment on lines 575 to +579
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.
.is_none_or(|identifier| {
item.module.code_at(item.definition_range) == identifier.id.as_str()
})
});
let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
metadata,
definition_range: definition_location,
module,
docstring_range,
display_name,
}) = definition
{
let kind = metadata.symbol_kind();
let name = {
Expand Down
27 changes: 26 additions & 1 deletion pyrefly/lib/state/lsp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ use crate::export::exports::Export;
use crate::export::exports::ExportLocation;
use crate::lsp::module_helpers::collect_symbol_def_paths;
use crate::lsp::wasm::completion::CompletionOptions;
use crate::lsp::wasm::signature_help::is_constructor_call;
use crate::state::ide::IntermediateDefinition;
use crate::state::ide::common_alias_target_module;
use crate::state::ide::import_regular_import_edit;
Expand Down Expand Up @@ -872,6 +873,29 @@ impl<'a> Transaction<'a> {
}
}

fn get_constructor_keyword_argument_type(
&self,
handle: &Handle,
position: TextSize,
) -> Option<Type> {
let call_info = self.get_callables_from_call(handle, position)?;
let callee_type = self
.get_answers(handle)?
.get_type_trace(call_info.callee_range)?;
if !is_constructor_call(callee_type) {
return None;
}

let chosen_overload_index =
call_info
.chosen_overload_index
.or((call_info.callables.len() == 1).then_some(0))?;
let callable = call_info.callables.get(chosen_overload_index)?.clone();
let params = Self::normalize_singleton_function_type_into_params(callable)?;
let arg_index = Self::active_parameter_index(&params, &call_info.active_argument)?;
Some(params.get(arg_index)?.as_type().clone())
}

pub fn get_type_at(&self, handle: &Handle, position: TextSize) -> Option<Type> {
match self.identifier_at(handle, position) {
Some(IdentifierWithContext {
Expand Down Expand Up @@ -1024,7 +1048,8 @@ impl<'a> Transaction<'a> {
return None;
}
self.get_type(handle, &key)
}),
})
.or_else(|| self.get_constructor_keyword_argument_type(handle, position)),
Comment on lines 1048 to +1052
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Some(IdentifierWithContext {
identifier: _,
context: IdentifierContext::Attribute { range, .. },
Expand Down
23 changes: 23 additions & 0 deletions pyrefly/lib/test/lsp/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,29 @@ foo(x=1, y=2)
assert!(report.contains("documentation for y"));
}

#[test]
fn hover_on_dataclass_constructor_keyword_shows_field_type() {
let code = r#"
from dataclasses import dataclass

@dataclass
class Test:
foo: int

Test(foo=1)
# ^
"#;
let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report);
assert!(
report.contains("foo: int"),
"Expected dataclass constructor keyword hover to show field type, got: {report}"
);
assert!(
!report.contains("(class) Test") && !report.contains("Test: int"),
"Keyword hover should not be labeled as the class, got: {report}"
);
}

#[test]
fn hover_returns_none_for_docstring_literals() {
let code = r#"
Expand Down
25 changes: 25 additions & 0 deletions pyrefly/lib/test/lsp/hover_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,31 @@ Hover Result: `int`
);
}

#[test]
fn kwarg_dataclass_constructor() {
let code = r#"
from dataclasses import dataclass

@dataclass
class Test:
foo: int

Test(foo=1)
# ^
"#;
let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report);
assert_eq!(
r#"
# main.py
8 | Test(foo=1)
^
Hover Result: `int`
"#
.trim(),
report.trim(),
);
}

// todo(kylei): When the callee's implementation uses *args/**kwargs, we can't refine the
// keyword argument to a specific parameter. Ideally we'd resolve through the matched overload.
#[test]
Expand Down
Loading