Skip to content

Commit e7ad1cb

Browse files
fix
1 parent b430a08 commit e7ad1cb

4 files changed

Lines changed: 101 additions & 9 deletions

File tree

pyrefly/lib/lsp/wasm/hover.rs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ use pyrefly_types::types::Type;
3333
use pyrefly_util::lined_buffer::LineNumber;
3434
use pyrefly_util::visit::Visit;
3535
use ruff_python_ast::AnyNodeRef;
36+
use ruff_python_ast::Identifier;
3637
use ruff_python_ast::Stmt;
3738
use ruff_python_ast::name::Name;
3839
use ruff_text_size::Ranged;
@@ -431,6 +432,16 @@ fn parameter_definition_documentation(
431432
docs.get(key).cloned().map(|doc| (key.to_owned(), doc))
432433
}
433434

435+
fn keyword_argument_identifier(
436+
transaction: &Transaction<'_>,
437+
handle: &Handle,
438+
position: TextSize,
439+
) -> Option<Identifier> {
440+
let identifier = transaction.identifier_at(handle, position)?;
441+
matches!(identifier.context, IdentifierContext::KeywordArgument(_))
442+
.then_some(identifier.identifier)
443+
}
444+
434445
/// Check if the cursor position is on the `in` keyword within a for loop or comprehension.
435446
/// Returns Some(iterable_range) if found, None otherwise.
436447
fn in_keyword_in_iteration_at(
@@ -549,13 +560,8 @@ pub fn get_hover(
549560
}
550561

551562
let fallback_name_from_type = fallback_hover_name_from_type(&type_);
552-
let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
553-
metadata,
554-
definition_range: definition_location,
555-
module,
556-
docstring_range,
557-
display_name,
558-
}) = transaction
563+
let keyword_argument_identifier = keyword_argument_identifier(transaction, handle, position);
564+
let definition = transaction
559565
.find_definition(
560566
handle,
561567
position,
@@ -566,9 +572,22 @@ pub fn get_hover(
566572
)
567573
.map(Vec1::into_vec)
568574
.unwrap_or_default()
569-
// TODO: handle more than 1 definition
570575
.into_iter()
571576
.next()
577+
.filter(|item| {
578+
keyword_argument_identifier
579+
.as_ref()
580+
.is_none_or(|identifier| {
581+
item.module.code_at(item.definition_range) == identifier.id.as_str()
582+
})
583+
});
584+
let (kind, name, docstring_range, module) = if let Some(FindDefinitionItemWithDocstring {
585+
metadata,
586+
definition_range: definition_location,
587+
module,
588+
docstring_range,
589+
display_name,
590+
}) = definition
572591
{
573592
let kind = metadata.symbol_kind();
574593
let name = {

pyrefly/lib/state/lsp.rs

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ use crate::export::exports::Export;
7575
use crate::export::exports::ExportLocation;
7676
use crate::lsp::module_helpers::collect_symbol_def_paths;
7777
use crate::lsp::wasm::completion::CompletionOptions;
78+
use crate::lsp::wasm::signature_help::is_constructor_call;
7879
use crate::state::ide::IntermediateDefinition;
7980
use crate::state::ide::common_alias_target_module;
8081
use crate::state::ide::import_regular_import_edit;
@@ -872,6 +873,29 @@ impl<'a> Transaction<'a> {
872873
}
873874
}
874875

876+
fn get_constructor_keyword_argument_type(
877+
&self,
878+
handle: &Handle,
879+
position: TextSize,
880+
) -> Option<Type> {
881+
let call_info = self.get_callables_from_call(handle, position)?;
882+
let callee_type = self
883+
.get_answers(handle)?
884+
.get_type_trace(call_info.callee_range)?;
885+
if !is_constructor_call(callee_type) {
886+
return None;
887+
}
888+
889+
let chosen_overload_index =
890+
call_info
891+
.chosen_overload_index
892+
.or((call_info.callables.len() == 1).then_some(0))?;
893+
let callable = call_info.callables.get(chosen_overload_index)?.clone();
894+
let params = Self::normalize_singleton_function_type_into_params(callable)?;
895+
let arg_index = Self::active_parameter_index(&params, &call_info.active_argument)?;
896+
Some(params.get(arg_index)?.as_type().clone())
897+
}
898+
875899
pub fn get_type_at(&self, handle: &Handle, position: TextSize) -> Option<Type> {
876900
match self.identifier_at(handle, position) {
877901
Some(IdentifierWithContext {
@@ -1024,7 +1048,8 @@ impl<'a> Transaction<'a> {
10241048
return None;
10251049
}
10261050
self.get_type(handle, &key)
1027-
}),
1051+
})
1052+
.or_else(|| self.get_constructor_keyword_argument_type(handle, position)),
10281053
Some(IdentifierWithContext {
10291054
identifier: _,
10301055
context: IdentifierContext::Attribute { range, .. },

pyrefly/lib/test/lsp/hover.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,29 @@ foo(x=1, y=2)
396396
assert!(report.contains("documentation for y"));
397397
}
398398

399+
#[test]
400+
fn hover_on_dataclass_constructor_keyword_shows_field_type() {
401+
let code = r#"
402+
from dataclasses import dataclass
403+
404+
@dataclass
405+
class Test:
406+
foo: int
407+
408+
Test(foo=1)
409+
# ^
410+
"#;
411+
let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report);
412+
assert!(
413+
report.contains("foo: int"),
414+
"Expected dataclass constructor keyword hover to show field type, got: {report}"
415+
);
416+
assert!(
417+
!report.contains("(class) Test") && !report.contains("Test: int"),
418+
"Keyword hover should not be labeled as the class, got: {report}"
419+
);
420+
}
421+
399422
#[test]
400423
fn hover_returns_none_for_docstring_literals() {
401424
let code = r#"

pyrefly/lib/test/lsp/hover_type.rs

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,31 @@ Hover Result: `int`
567567
);
568568
}
569569

570+
#[test]
571+
fn kwarg_dataclass_constructor() {
572+
let code = r#"
573+
from dataclasses import dataclass
574+
575+
@dataclass
576+
class Test:
577+
foo: int
578+
579+
Test(foo=1)
580+
# ^
581+
"#;
582+
let report = get_batched_lsp_operations_report_allow_error(&[("main", code)], get_test_report);
583+
assert_eq!(
584+
r#"
585+
# main.py
586+
8 | Test(foo=1)
587+
^
588+
Hover Result: `int`
589+
"#
590+
.trim(),
591+
report.trim(),
592+
);
593+
}
594+
570595
// todo(kylei): When the callee's implementation uses *args/**kwargs, we can't refine the
571596
// keyword argument to a specific parameter. Ideally we'd resolve through the matched overload.
572597
#[test]

0 commit comments

Comments
 (0)