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
36 changes: 35 additions & 1 deletion pyrefly/lib/lsp/wasm/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

use std::collections::HashMap;

use dupe::Dupe;
use lsp_types::Hover;
use lsp_types::HoverContents;
use lsp_types::MarkupContent;
Expand Down Expand Up @@ -315,6 +316,39 @@ fn identifier_text_at(
.map(|id| id.identifier.id.to_string())
}

fn docstring_for_class_object_type(
transaction: &Transaction<'_>,
handle: &Handle,
type_: &Type,
) -> Option<Docstring> {
let qname = match type_ {
Type::ClassDef(cls) => cls.qname(),
Type::Type(inner) => match inner.as_ref() {
Type::ClassType(cls) => cls.qname(),
_ => return None,
},
_ => return None,
};
let definition_handle = Handle::new(
qname.module_name(),
qname.module_path().dupe(),
handle.sys_info().dupe(),
);
let definition = transaction
.find_definition(
&definition_handle,
qname.range().start(),
FindPreference {
resolve_call_dunders: false,
..Default::default()
},
)
.ok()?
.into_iter()
.find(|item| item.definition_range == qname.range())?;
Some(Docstring(definition.docstring_range?, definition.module))
}
Comment on lines +332 to +350
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

docstring_for_class_object_type relies on transaction.find_definition, which hard-depends on get_ast(handle) being present. ASTs can be evicted after computing answers (ModuleState::evict_ast stores None), so this fallback can silently stop working for many library modules even if their exports/types are available.

To make this robust, avoid the find_definition path here (or re-parse the module when get_ast is None). A practical alternative is to fetch the class symbol’s docstring_range directly from the target module’s exports (and follow ExportLocation::OtherModule if needed), which doesn’t require retaining the AST. Also note FindPreference::default() prefers .pyi; for docstrings you likely want to prefer executable .py when available.

Copilot uses AI. Check for mistakes.

fn collect_typed_dict_fields_for_hover<'a>(
solver: &AnswersSolver<TransactionHandle<'a>>,
ty: &Type,
Expand Down Expand Up @@ -636,7 +670,7 @@ pub fn get_hover(
let docstring = if let (Some(docstring), Some(module)) = (docstring_range, module) {
Some(Docstring(docstring, module))
} else {
None
docstring_for_class_object_type(transaction, handle, &type_)
};

let mut parameter_doc = keyword_argument_documentation(transaction, handle, position)
Expand Down
28 changes: 28 additions & 0 deletions pyrefly/lib/test/lsp/hover.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1029,6 +1029,34 @@ Widget docstring"#
);
}

#[test]
fn hover_on_property_returning_class_shows_class_docstring() {
let lib = r#"
from typing import Type

class current_timestamp:
"""The CURRENT_TIMESTAMP() SQL function."""
def __init__(self, *args: object, **kwargs: object) -> None: ...

class Func:
@property
def current_timestamp(self) -> Type[current_timestamp]: ...
"#;
let code = r#"
from lib import Func

f = Func()
f.current_timestamp
# ^
"#;
let report =
get_batched_lsp_operations_report(&[("main", code), ("lib", lib)], get_test_report);
assert!(
report.contains("The CURRENT_TIMESTAMP() SQL function."),
"Expected hover to show the class docstring, got: {report}"
);
}

#[test]
fn hover_on_first_component_of_multi_part_import() {
let mymod_init = r#"# mymod/__init__.py
Expand Down
Loading