From 765257bdcee023c7ebb6ecbc02245b4c4990b06d Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Mon, 27 Oct 2025 14:17:07 -0400 Subject: [PATCH] [ty] Filter out "unimported" from the current module Note that this doesn't change the evaluation results unfortunately. In particular, prior to this fix, the correct result was ranked above the redundant result. Our MRR-based evaluation doesn't care about anything below the rank of the correct answer, and so this change isn't reflected in our evaluation. Fixes astral-sh/ty#1445 --- crates/ty_ide/src/completion.rs | 54 ++++++++++++++++++++++++++++++--- 1 file changed, 49 insertions(+), 5 deletions(-) diff --git a/crates/ty_ide/src/completion.rs b/crates/ty_ide/src/completion.rs index 1b384493fe..18af1da727 100644 --- a/crates/ty_ide/src/completion.rs +++ b/crates/ty_ide/src/completion.rs @@ -330,6 +330,10 @@ fn add_unimported_completions<'db>( let members = importer.members_in_scope_at(scoped.node, scoped.node.start()); for symbol in all_symbols(db, typed) { + if symbol.module.file(db) == Some(file) { + continue; + } + let request = ImportRequest::import_from(symbol.module.name(db).as_str(), &symbol.symbol.name); // FIXME: `all_symbols` doesn't account for wildcard imports. @@ -866,6 +870,7 @@ fn compare_suggestions(c1: &Completion, c2: &Completion) -> Ordering { mod tests { use insta::assert_snapshot; use ruff_python_parser::{Mode, ParseOptions, TokenKind, Tokens}; + use ty_python_semantic::ModuleName; use crate::completion::{Completion, completion}; use crate::tests::{CursorTest, CursorTestBuilder}; @@ -3400,6 +3405,24 @@ from os. .contains("AbraKadabra"); } + #[test] + fn auto_import_should_not_include_symbols_in_current_module() { + let snapshot = CursorTest::builder() + .source("main.py", "Kadabra = 1\nKad") + .source("package/__init__.py", "AbraKadabra = 1") + .completion_test_builder() + .auto_import() + .type_signatures() + .module_names() + .filter(|c| c.name.contains("Kadabra")) + .build() + .snapshot(); + assert_snapshot!(snapshot, @r" + AbraKadabra :: Unavailable :: package + Kadabra :: Literal[1] :: Current module + "); + } + #[test] fn import_type_check_only_lowers_ranking() { let builder = CursorTest::builder() @@ -4058,6 +4081,9 @@ def f[T](x: T): settings: CompletionSettings, skip_builtins: bool, type_signatures: bool, + module_names: bool, + // This doesn't seem like a "very complex" type to me... ---AG + #[allow(clippy::type_complexity)] predicate: Option bool>>, } @@ -4086,6 +4112,7 @@ def f[T](x: T): original, filtered, type_signatures: self.type_signatures, + module_names: self.module_names, } } @@ -4123,6 +4150,13 @@ def f[T](x: T): self } + /// When set, the module name for each symbol is included + /// in the snapshot (if available). + fn module_names(mut self) -> CompletionTestBuilder { + self.module_names = true; + self + } + /// Apply arbitrary filtering to completions. fn filter( mut self, @@ -4148,6 +4182,9 @@ def f[T](x: T): /// Whether type signatures should be included in the snapshot /// generated by `CompletionTest::snapshot`. type_signatures: bool, + /// Whether module names should be included in the snapshot + /// generated by `CompletionTest::snapshot`. + module_names: bool, } impl<'db> CompletionTest<'db> { @@ -4166,15 +4203,21 @@ def f[T](x: T): self.filtered .iter() .map(|c| { - let name = c.name.as_str().to_string(); - if !self.type_signatures { - name - } else { + let mut snapshot = c.name.as_str().to_string(); + if self.type_signatures { let ty = c.ty.map(|ty| ty.display(self.db).to_string()) .unwrap_or_else(|| "Unavailable".to_string()); - format!("{name} :: {ty}") + snapshot = format!("{snapshot} :: {ty}"); } + if self.module_names { + let module_name = c + .module_name + .map(ModuleName::as_str) + .unwrap_or("Current module"); + snapshot = format!("{snapshot} :: {module_name}"); + } + snapshot }) .collect::>() .join("\n") @@ -4216,6 +4259,7 @@ def f[T](x: T): settings: CompletionSettings::default(), skip_builtins: false, type_signatures: false, + module_names: false, predicate: None, } }