From 856a4ec01d4cf31b305622d031a02090daafea99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 26 Nov 2025 22:41:06 +0100 Subject: [PATCH 1/4] Skip `deprecated` exports in autoimport --- pyrefly/lib/state/lsp.rs | 12 ++++-- pyrefly/lib/test/lsp/code_actions.rs | 63 ++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 28340e0cd7..7f11bd70e1 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -2936,9 +2936,15 @@ impl<'a> Transaction<'a> { pub fn search_exports_exact(&self, name: &str) -> Vec { self.search_exports(|handle, exports| { - if let Some(export) = exports.get(&Name::new(name)) { - match export { - ExportLocation::ThisModule(_) => vec![handle.dupe()], + if let Some(export_location) = exports.get(&Name::new(name)) { + match export_location { + ExportLocation::ThisModule(export) => { + // Ignore deprecated exports in autoimport suggestions + if export.deprecation.is_some() { + return Vec::new(); + } + vec![handle.dupe()] + } // Re-exported modules like `foo` in `from from_module import foo` // should likely be ignored in autoimport suggestions // because the original export in from_module will show it. diff --git a/pyrefly/lib/test/lsp/code_actions.rs b/pyrefly/lib/test/lsp/code_actions.rs index 203406f465..6bffd81e3d 100644 --- a/pyrefly/lib/test/lsp/code_actions.rs +++ b/pyrefly/lib/test/lsp/code_actions.rs @@ -236,3 +236,66 @@ my_export report.trim() ); } + +#[test] +fn test_import_from_stdlib() { + let report = get_batched_lsp_operations_report_allow_error( + &[("a", "TypeVar('T')\n# ^")], + get_test_report, + ); + // TODO: Ideally `typing` would be preferred over `ast`. + assert_eq!( + r#" +# a.py +1 | TypeVar('T') + ^ +Code Actions Results: +# Title: Insert import: `from ast import TypeVar` + +## Before: +TypeVar('T') +# ^ +## After: +from ast import TypeVar +TypeVar('T') +# ^ +# Title: Insert import: `from typing import TypeVar` + +## Before: +TypeVar('T') +# ^ +## After: +from typing import TypeVar +TypeVar('T') +# ^ +"# + .trim(), + report.trim() + ); +} + +#[test] +fn test_skip_deprecated_import() { + let report = get_batched_lsp_operations_report_allow_error( + &[ + ( + "a", + "from warnings import deprecated\n@deprecated('')\ndef my_func(): pass", + ), + ("b", "my_func()\n# ^"), + ], + get_test_report, + ); + assert_eq!( + r#" +# a.py + +# b.py +1 | my_func() + ^ +Code Actions Results: +"# + .trim(), + report.trim() + ); +} From d2d5ba911742832584f72fe216cb9a78d39947fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 6 Dec 2025 14:29:36 +0100 Subject: [PATCH 2/4] Don't remove, but sort deprecated imports lower --- pyrefly/lib/commands/infer.rs | 2 +- pyrefly/lib/state/lsp.rs | 33 ++++++++++++++++++---------- pyrefly/lib/test/lsp/code_actions.rs | 25 +++++++++++++++++++-- 3 files changed, 46 insertions(+), 14 deletions(-) diff --git a/pyrefly/lib/commands/infer.rs b/pyrefly/lib/commands/infer.rs index 6ae8dd84e2..9a8b65865f 100644 --- a/pyrefly/lib/commands/infer.rs +++ b/pyrefly/lib/commands/infer.rs @@ -295,7 +295,7 @@ impl InferArgs { let imports: Vec<(TextSize, String, String)> = transaction .search_exports_exact(unknown_name) .into_iter() - .map(|handle_to_import_from| { + .map(|(handle_to_import_from, _)| { insert_import_edit_with_forced_import_format( &ast, handle.dupe(), diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 7f11bd70e1..4127cf1139 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -5,6 +5,7 @@ * LICENSE file in the root directory of this source tree. */ +use std::cmp::Ordering; use std::cmp::Reverse; use std::collections::BTreeMap; @@ -1676,7 +1677,9 @@ impl<'a> Transaction<'a> { let error_range = error.range(); if error_range.contains_range(range) { let unknown_name = module_info.code_at(error_range); - for handle_to_import_from in self.search_exports_exact(unknown_name) { + for (handle_to_import_from, export) in + self.search_exports_exact(unknown_name) + { let (position, insert_text, _) = insert_import_edit( &ast, self.config_finder(), @@ -1686,7 +1689,12 @@ impl<'a> Transaction<'a> { import_format, ); let range = TextRange::at(position, TextSize::new(0)); - let title = format!("Insert import: `{}`", insert_text.trim()); + let mut title = format!("Insert import: `{}`", insert_text.trim()); + + if export.deprecation.is_some() { + title.push_str(" (deprecated)"); + } + code_actions.push((title, module_info.dupe(), range, insert_text)); } @@ -1709,7 +1717,16 @@ impl<'a> Transaction<'a> { _ => {} } } - code_actions.sort_by(|(title1, _, _, _), (title2, _, _, _)| title1.cmp(title2)); + + // Sort code actions: non-deprecated first, then alphabetically + code_actions.sort_by(|(title1, _, _, _), (title2, _, _, _)| { + match (title1.contains("deprecated"), title2.contains("deprecated")) { + (true, false) => Ordering::Greater, + (false, true) => Ordering::Less, + _ => title1.cmp(title2), + } + }); + Some(code_actions) } @@ -2934,17 +2951,11 @@ impl<'a> Transaction<'a> { (result, is_incomplete) } - pub fn search_exports_exact(&self, name: &str) -> Vec { + pub fn search_exports_exact(&self, name: &str) -> Vec<(Handle, Export)> { self.search_exports(|handle, exports| { if let Some(export_location) = exports.get(&Name::new(name)) { match export_location { - ExportLocation::ThisModule(export) => { - // Ignore deprecated exports in autoimport suggestions - if export.deprecation.is_some() { - return Vec::new(); - } - vec![handle.dupe()] - } + ExportLocation::ThisModule(export) => vec![(handle.dupe(), export.clone())], // Re-exported modules like `foo` in `from from_module import foo` // should likely be ignored in autoimport suggestions // because the original export in from_module will show it. diff --git a/pyrefly/lib/test/lsp/code_actions.rs b/pyrefly/lib/test/lsp/code_actions.rs index 6bffd81e3d..3258d7b853 100644 --- a/pyrefly/lib/test/lsp/code_actions.rs +++ b/pyrefly/lib/test/lsp/code_actions.rs @@ -275,14 +275,15 @@ TypeVar('T') } #[test] -fn test_skip_deprecated_import() { +fn test_take_deprecation_into_account_in_sorting_of_actions() { let report = get_batched_lsp_operations_report_allow_error( &[ ( "a", "from warnings import deprecated\n@deprecated('')\ndef my_func(): pass", ), - ("b", "my_func()\n# ^"), + ("b", "def my_func(): pass"), + ("c", "my_func()\n# ^"), ], get_test_report, ); @@ -291,9 +292,29 @@ fn test_skip_deprecated_import() { # a.py # b.py + +# c.py 1 | my_func() ^ Code Actions Results: +# Title: Insert import: `from b import my_func` + +## Before: +my_func() +# ^ +## After: +from b import my_func +my_func() +# ^ +# Title: Insert import: `from a import my_func` (deprecated) + +## Before: +my_func() +# ^ +## After: +from a import my_func +my_func() +# ^ "# .trim(), report.trim() From f799c78543732d4a1abb51e979744264dd20e409 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:44:37 +0100 Subject: [PATCH 3/4] Use `map_or` instead of making `title` `mut` --- pyrefly/lib/state/lsp.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index 4127cf1139..e608d83455 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -1689,11 +1689,11 @@ impl<'a> Transaction<'a> { import_format, ); let range = TextRange::at(position, TextSize::new(0)); - let mut title = format!("Insert import: `{}`", insert_text.trim()); - - if export.deprecation.is_some() { - title.push_str(" (deprecated)"); - } + let title = format!( + "Insert import: `{}`{}", + insert_text.trim(), + export.deprecation.map_or("", |_| " (deprecated)") + ); code_actions.push((title, module_info.dupe(), range, insert_text)); } From 187e0d77437a3bc0e350880ed2931e9242fd7bf4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Mon, 8 Dec 2025 20:47:02 +0100 Subject: [PATCH 4/4] Rename to `local_quickfix_code_actions_sorted` --- pyrefly/lib/lsp/non_wasm/server.rs | 2 +- pyrefly/lib/state/lsp.rs | 2 +- pyrefly/lib/test/lsp/code_actions.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pyrefly/lib/lsp/non_wasm/server.rs b/pyrefly/lib/lsp/non_wasm/server.rs index 6a95339601..706e01c3eb 100644 --- a/pyrefly/lib/lsp/non_wasm/server.rs +++ b/pyrefly/lib/lsp/non_wasm/server.rs @@ -2306,7 +2306,7 @@ impl Server { let module_info = transaction.get_module_info(&handle)?; let range = self.from_lsp_range(uri, &module_info, params.range); let code_actions = transaction - .local_quickfix_code_actions(&handle, range, import_format)? + .local_quickfix_code_actions_sorted(&handle, range, import_format)? .into_map(|(title, info, range, insert_text)| { CodeActionOrCommand::CodeAction(CodeAction { title, diff --git a/pyrefly/lib/state/lsp.rs b/pyrefly/lib/state/lsp.rs index e608d83455..c17ca04958 100644 --- a/pyrefly/lib/state/lsp.rs +++ b/pyrefly/lib/state/lsp.rs @@ -1661,7 +1661,7 @@ impl<'a> Transaction<'a> { } /// Produce code actions that makes edits local to the file. - pub fn local_quickfix_code_actions( + pub fn local_quickfix_code_actions_sorted( &self, handle: &Handle, range: TextRange, diff --git a/pyrefly/lib/test/lsp/code_actions.rs b/pyrefly/lib/test/lsp/code_actions.rs index 3258d7b853..2e83b38fa7 100644 --- a/pyrefly/lib/test/lsp/code_actions.rs +++ b/pyrefly/lib/test/lsp/code_actions.rs @@ -30,7 +30,7 @@ fn get_test_report(state: &State, handle: &Handle, position: TextSize) -> String let mut report = "Code Actions Results:\n".to_owned(); let transaction = state.transaction(); for (title, info, range, patch) in transaction - .local_quickfix_code_actions( + .local_quickfix_code_actions_sorted( handle, TextRange::new(position, position), ImportFormat::Absolute,