[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
This commit is contained in:
Andrew Gallant 2025-10-27 14:17:07 -04:00 committed by Andrew Gallant
parent 2d4e0edee4
commit 765257bdce
1 changed files with 49 additions and 5 deletions

View File

@ -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.<CURSOR>
.contains("AbraKadabra");
}
#[test]
fn auto_import_should_not_include_symbols_in_current_module() {
let snapshot = CursorTest::builder()
.source("main.py", "Kadabra = 1\nKad<CURSOR>")
.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<Box<dyn Fn(&Completion) -> 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::<Vec<String>>()
.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,
}
}