From a0b6ed7b2594a39fcc29d92f801e88b5afc14774 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Mon, 9 Nov 2026 00:51:12 +0900 Subject: [PATCH 1/4] Test hover types for __all__ entries --- pyrefly/lib/test/lsp/hover.rs | 40 ++++++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index 78b863e9e4..1a3ad1e79d 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -42,8 +42,7 @@ fn get_test_report(state: &State, handle: &Handle, position: TextSize) -> String } } -// BUG: Hover shows string literal types for __all__ and __slots__ entries. -// It should show (variable) foo: int and (attribute) bar: str (issue #4344). +// BUG: Hover shows a string literal type for the __slots__ entry (issue #4344). #[test] fn hover_symbol_literals() { let code = r#" @@ -65,7 +64,7 @@ class C: 3 | __all__ = ["foo"] ^ ```python -(variable) foo: Literal['foo'] +(variable) foo: int ``` 7 | __slots__ = ("bar",) @@ -79,6 +78,41 @@ Literal['bar'] ); } +#[test] +fn hover_dunder_all_reexport_entries() { + let code = r#" +from lib import foo +__all__ = ["foo"] +# ^ +__all__.append("foo") +# ^ +__all__.extend(["foo"]) +# ^ +__all__.remove("foo") +# ^ +"#; + get_batched_lsp_operations_report( + &[("main", code), ("lib", "foo: int = 1\n")], + |state, handle, position| { + let report = get_test_report(state, handle, position); + assert!(report.contains("(variable) foo: int"), "got: {report}"); + report + }, + ); +} + +#[test] +fn hover_string_matching_dunder_all_entry() { + let code = r#" +foo: int = 1 +value = "foo" +# ^ +__all__ = ["foo"] +"#; + let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report); + assert!(report.contains("Literal['foo']"), "got: {report}"); +} + fn get_test_report_at_verbosity( state: &State, handle: &Handle, From a85faf4779354bfe7a1138ad93d7e492667d5d28 Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Mon, 9 Nov 2026 00:54:33 +0900 Subject: [PATCH 2/4] Show referenced symbol types when hovering over __all__ entries Use the existing definition lookup to read the exported symbol type. Keep string literal types for strings outside __all__. --- pyrefly/lib/lsp/wasm/hover.rs | 18 ++++++++++++++++-- pyrefly/lib/test/lsp/hover.rs | 22 +++++++++++++++++++++- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs index 67d8efd3d9..59f7126ad7 100644 --- a/pyrefly/lib/lsp/wasm/hover.rs +++ b/pyrefly/lib/lsp/wasm/hover.rs @@ -1606,7 +1606,8 @@ pub fn get_hover_with_verbosity( position, ); - let Some(type_) = resolve_hovered_type(transaction, handle, ast.as_deref(), position) else { + let Some(mut type_) = resolve_hovered_type(transaction, handle, ast.as_deref(), position) + else { return keyword_hover; }; @@ -1631,7 +1632,6 @@ pub fn get_hover_with_verbosity( }) }); - let fallback_name_from_type = fallback_hover_name_from_type(&type_); let keyword_argument_identifier = keyword_argument_identifier(transaction, handle, position); let definition = transaction .find_definition( @@ -1655,6 +1655,20 @@ pub fn get_hover_with_verbosity( item.module.code_at(item.definition_range) == identifier.id.as_str() }) }); + // An `__all__` entry refers to a symbol, so hover uses that symbol's type. + if let Some(definition) = &definition + && transaction + .get_exports_data(handle) + .dunder_all_name_at(position) + .is_some() + && let Some(definition_type) = transaction.get_type_at( + &handle.for_module(definition.module.name(), definition.module.path().dupe()), + definition.definition_range.start(), + ) + { + type_ = definition_type; + } + let fallback_name_from_type = fallback_hover_name_from_type(&type_); let interface_definition = definition.as_ref().and_then(|executable| { if keyword_argument_identifier.is_some() { return None; diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index 1a3ad1e79d..4353b35f04 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -57,7 +57,15 @@ class C: def __init__(self) -> None: self.bar: str = "" "#; - let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report); + let report = get_batched_lsp_operations_report(&[("main", code)], |state, handle, position| { + match get_hover(&state.transaction(), handle, position, false) { + Some(Hover { + contents: Contents::MarkupContent(markup), + .. + }) => markup.value, + _ => "None".to_owned(), + } + }); assert_eq!( r#" # main.py @@ -101,6 +109,17 @@ __all__.remove("foo") ); } +#[test] +fn hover_dunder_all_class_entry() { + let code = r#" +class C: ... +__all__ = ["C"] +# ^ +"#; + let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report); + assert!(report.contains("(class) C: type[C]"), "got: {report}"); +} + #[test] fn hover_string_matching_dunder_all_entry() { let code = r#" @@ -2643,6 +2662,7 @@ def exported() -> int: let position = extract_cursors_for_test(lib_code)[0]; let report = get_test_report(&state, &lib_handle, position); + assert!(report.contains("-> int"), "got: {report}"); assert!( report.contains("Public stub documentation."), "got: {report}" From 6e33a9c4ef47f1994594a4faa9076e75fe6ad31a Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Thu, 12 Nov 2026 04:29:54 +0900 Subject: [PATCH 3/4] Use solved export types for __all__ hover MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read the current checked module’s final export answer so buffers, stubs, and unopened dependencies supply the correct type. --- pyrefly/lib/lsp/wasm/hover.rs | 16 +---- pyrefly/lib/state/lsp.rs | 7 +++ pyrefly/lib/test/lsp/lsp_interaction/hover.rs | 63 +++++++++++++++++++ 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs index 59f7126ad7..f2d9f9c084 100644 --- a/pyrefly/lib/lsp/wasm/hover.rs +++ b/pyrefly/lib/lsp/wasm/hover.rs @@ -1606,8 +1606,7 @@ pub fn get_hover_with_verbosity( position, ); - let Some(mut type_) = resolve_hovered_type(transaction, handle, ast.as_deref(), position) - else { + let Some(type_) = resolve_hovered_type(transaction, handle, ast.as_deref(), position) else { return keyword_hover; }; @@ -1655,19 +1654,6 @@ pub fn get_hover_with_verbosity( item.module.code_at(item.definition_range) == identifier.id.as_str() }) }); - // An `__all__` entry refers to a symbol, so hover uses that symbol's type. - if let Some(definition) = &definition - && transaction - .get_exports_data(handle) - .dunder_all_name_at(position) - .is_some() - && let Some(definition_type) = transaction.get_type_at( - &handle.for_module(definition.module.name(), definition.module.path().dupe()), - definition.definition_range.start(), - ) - { - type_ = definition_type; - } let fallback_name_from_type = fallback_hover_name_from_type(&type_); let interface_definition = definition.as_ref().and_then(|executable| { if keyword_argument_identifier.is_some() { diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index ca5ff3723a..082b87324c 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -86,6 +86,7 @@ use crate::alt::attr::AttrInfo; use crate::binding::binding::Binding; use crate::binding::binding::Key; use crate::binding::binding::KeyAnnotation; +use crate::binding::binding::KeyExport; use crate::binding::metadata::BindingsMetadata; use crate::config::error_kind::ErrorKind; use crate::error::error::ErrorQuickFix; @@ -1489,6 +1490,12 @@ impl<'a> Transaction<'a> { position: TextSize, coerce_callees: bool, ) -> Option { + if let Some((_, name)) = self.get_exports_data(handle).dunder_all_name_at(position) { + return self + .get_solutions(handle)? + .get_hashed_opt(Hashed::new(&KeyExport(name))) + .cloned(); + } let Some(IdentifierWithContext { identifier, context, diff --git a/pyrefly/lib/test/lsp/lsp_interaction/hover.rs b/pyrefly/lib/test/lsp/lsp_interaction/hover.rs index 8db4ad9d6a..b8c6bdd448 100644 --- a/pyrefly/lib/test/lsp/lsp_interaction/hover.rs +++ b/pyrefly/lib/test/lsp/lsp_interaction/hover.rs @@ -5,13 +5,76 @@ * LICENSE file in the root directory of this source tree. */ +use std::fs; + use lsp_types::Uri; use pyrefly_lsp_test::object_model::InitializeSettings; use pyrefly_lsp_test::object_model::LspInteraction; use serde_json::json; +use tempfile::TempDir; use crate::test::lsp::lsp_interaction::util::get_test_files_root; +#[test] +fn hover_dunder_all_uses_open_buffer_export() { + let root = TempDir::new().unwrap(); + fs::write(root.path().join("main.pyi"), "value: bytes\n").unwrap(); + fs::write( + root.path().join("main.py"), + "value = 1\n__all__ = ['value']\n", + ) + .unwrap(); + let mut interaction = LspInteraction::new(); + interaction.set_root(root.path().to_path_buf()); + interaction.initialize(Default::default()).unwrap(); + interaction.client.did_open("main.py"); + interaction.client.did_change( + "main.py", + "\nvalue = 1\nvalue = 'final'\n__all__ = ['value']\n", + ); + interaction + .client + .hover("main.py", 3, 13) + .expect_hover_response_with_markup(|value| { + value.is_some_and(|text| text.contains("(variable) value: str")) + }) + .unwrap(); + interaction.shutdown().unwrap(); +} + +#[test] +fn hover_dunder_all_uses_unopened_dependency_export() { + for with_stub in [false, true] { + let root = TempDir::new().unwrap(); + fs::write(root.path().join("dep.py"), "value: int = 1\n").unwrap(); + if with_stub { + fs::write(root.path().join("dep.pyi"), "value: str\n").unwrap(); + } + fs::write( + root.path().join("main.py"), + "from dep import value as public\n__all__ = ['public']\n", + ) + .unwrap(); + let mut interaction = LspInteraction::new(); + interaction.set_root(root.path().to_path_buf()); + interaction.initialize(Default::default()).unwrap(); + interaction.client.did_open("main.py"); + let expected = if with_stub { + "value: str" + } else { + "value: int" + }; + interaction + .client + .hover("main.py", 1, 13) + .expect_hover_response_with_markup(|value| { + value.is_some_and(|text| text.contains(expected)) + }) + .unwrap(); + interaction.shutdown().unwrap(); + } +} + #[test] fn test_hover_basic() { let root = get_test_files_root(); From 3f68f536064df49314f21d8ce01f7d74c614b18f Mon Sep 17 00:00:00 2001 From: Asuka Minato Date: Sat, 14 Nov 2026 12:43:27 +0900 Subject: [PATCH 4/4] Keep export lookup off identifier type queries Resolve __all__ entries only after identifier lookup fails. Cover go-to-type-definition and remove unrelated hover changes. --- pyrefly/lib/lsp/wasm/hover.rs | 2 +- pyrefly/lib/state/lsp.rs | 12 ++++++------ pyrefly/lib/test/lsp/hover.rs | 1 - pyrefly/lib/test/lsp/type_definition.rs | 23 +++++++++++++++++++++++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/pyrefly/lib/lsp/wasm/hover.rs b/pyrefly/lib/lsp/wasm/hover.rs index f2d9f9c084..67d8efd3d9 100644 --- a/pyrefly/lib/lsp/wasm/hover.rs +++ b/pyrefly/lib/lsp/wasm/hover.rs @@ -1631,6 +1631,7 @@ pub fn get_hover_with_verbosity( }) }); + let fallback_name_from_type = fallback_hover_name_from_type(&type_); let keyword_argument_identifier = keyword_argument_identifier(transaction, handle, position); let definition = transaction .find_definition( @@ -1654,7 +1655,6 @@ pub fn get_hover_with_verbosity( item.module.code_at(item.definition_range) == identifier.id.as_str() }) }); - let fallback_name_from_type = fallback_hover_name_from_type(&type_); let interface_definition = definition.as_ref().and_then(|executable| { if keyword_argument_identifier.is_some() { return None; diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 082b87324c..eb8e8cb67c 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -1490,17 +1490,17 @@ impl<'a> Transaction<'a> { position: TextSize, coerce_callees: bool, ) -> Option { - if let Some((_, name)) = self.get_exports_data(handle).dunder_all_name_at(position) { - return self - .get_solutions(handle)? - .get_hashed_opt(Hashed::new(&KeyExport(name))) - .cloned(); - } let Some(IdentifierWithContext { identifier, context, }) = self.identifier_at(handle, position) else { + if let Some((_, name)) = self.get_exports_data(handle).dunder_all_name_at(position) { + return self + .get_solutions(handle)? + .get_hashed_opt(Hashed::new(&KeyExport(name))) + .cloned(); + } return self .type_from_match_wildcard_at_impl(handle, position) .or_else(|| self.type_from_expression_at_impl(handle, position, false)); diff --git a/pyrefly/lib/test/lsp/hover.rs b/pyrefly/lib/test/lsp/hover.rs index 4353b35f04..b2f9651d8f 100644 --- a/pyrefly/lib/test/lsp/hover.rs +++ b/pyrefly/lib/test/lsp/hover.rs @@ -2662,7 +2662,6 @@ def exported() -> int: let position = extract_cursors_for_test(lib_code)[0]; let report = get_test_report(&state, &lib_handle, position); - assert!(report.contains("-> int"), "got: {report}"); assert!( report.contains("Public stub documentation."), "got: {report}" diff --git a/pyrefly/lib/test/lsp/type_definition.rs b/pyrefly/lib/test/lsp/type_definition.rs index 03567397f1..7cdcb862e8 100644 --- a/pyrefly/lib/test/lsp/type_definition.rs +++ b/pyrefly/lib/test/lsp/type_definition.rs @@ -37,6 +37,29 @@ fn get_test_report(state: &State, handle: &Handle, position: TextSize) -> String } } +#[test] +fn dunder_all_entry_uses_exported_type_definition() { + let code = r#" +class Exported: ... +value: Exported = Exported() +__all__ = ["value"] +# ^ +"#; + let report = get_batched_lsp_operations_report(&[("main", code)], get_test_report); + assert_eq!( + r#" +# main.py +4 | __all__ = ["value"] + ^ +Type Definition Result: +2 | class Exported: ... + ^^^^^^^^ +"# + .trim(), + report.trim(), + ); +} + #[test] fn function_goes_to_its_own_def() { let code = r#"