Skip to content

Commit 765257b

Browse files
committed
[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
1 parent 2d4e0ed commit 765257b

File tree

1 file changed

+49
-5
lines changed

1 file changed

+49
-5
lines changed

crates/ty_ide/src/completion.rs

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,10 @@ fn add_unimported_completions<'db>(
330330
let members = importer.members_in_scope_at(scoped.node, scoped.node.start());
331331

332332
for symbol in all_symbols(db, typed) {
333+
if symbol.module.file(db) == Some(file) {
334+
continue;
335+
}
336+
333337
let request =
334338
ImportRequest::import_from(symbol.module.name(db).as_str(), &symbol.symbol.name);
335339
// FIXME: `all_symbols` doesn't account for wildcard imports.
@@ -866,6 +870,7 @@ fn compare_suggestions(c1: &Completion, c2: &Completion) -> Ordering {
866870
mod tests {
867871
use insta::assert_snapshot;
868872
use ruff_python_parser::{Mode, ParseOptions, TokenKind, Tokens};
873+
use ty_python_semantic::ModuleName;
869874

870875
use crate::completion::{Completion, completion};
871876
use crate::tests::{CursorTest, CursorTestBuilder};
@@ -3400,6 +3405,24 @@ from os.<CURSOR>
34003405
.contains("AbraKadabra");
34013406
}
34023407

3408+
#[test]
3409+
fn auto_import_should_not_include_symbols_in_current_module() {
3410+
let snapshot = CursorTest::builder()
3411+
.source("main.py", "Kadabra = 1\nKad<CURSOR>")
3412+
.source("package/__init__.py", "AbraKadabra = 1")
3413+
.completion_test_builder()
3414+
.auto_import()
3415+
.type_signatures()
3416+
.module_names()
3417+
.filter(|c| c.name.contains("Kadabra"))
3418+
.build()
3419+
.snapshot();
3420+
assert_snapshot!(snapshot, @r"
3421+
AbraKadabra :: Unavailable :: package
3422+
Kadabra :: Literal[1] :: Current module
3423+
");
3424+
}
3425+
34033426
#[test]
34043427
fn import_type_check_only_lowers_ranking() {
34053428
let builder = CursorTest::builder()
@@ -4058,6 +4081,9 @@ def f[T](x: T):
40584081
settings: CompletionSettings,
40594082
skip_builtins: bool,
40604083
type_signatures: bool,
4084+
module_names: bool,
4085+
// This doesn't seem like a "very complex" type to me... ---AG
4086+
#[allow(clippy::type_complexity)]
40614087
predicate: Option<Box<dyn Fn(&Completion) -> bool>>,
40624088
}
40634089

@@ -4086,6 +4112,7 @@ def f[T](x: T):
40864112
original,
40874113
filtered,
40884114
type_signatures: self.type_signatures,
4115+
module_names: self.module_names,
40894116
}
40904117
}
40914118

@@ -4123,6 +4150,13 @@ def f[T](x: T):
41234150
self
41244151
}
41254152

4153+
/// When set, the module name for each symbol is included
4154+
/// in the snapshot (if available).
4155+
fn module_names(mut self) -> CompletionTestBuilder {
4156+
self.module_names = true;
4157+
self
4158+
}
4159+
41264160
/// Apply arbitrary filtering to completions.
41274161
fn filter(
41284162
mut self,
@@ -4148,6 +4182,9 @@ def f[T](x: T):
41484182
/// Whether type signatures should be included in the snapshot
41494183
/// generated by `CompletionTest::snapshot`.
41504184
type_signatures: bool,
4185+
/// Whether module names should be included in the snapshot
4186+
/// generated by `CompletionTest::snapshot`.
4187+
module_names: bool,
41514188
}
41524189

41534190
impl<'db> CompletionTest<'db> {
@@ -4166,15 +4203,21 @@ def f[T](x: T):
41664203
self.filtered
41674204
.iter()
41684205
.map(|c| {
4169-
let name = c.name.as_str().to_string();
4170-
if !self.type_signatures {
4171-
name
4172-
} else {
4206+
let mut snapshot = c.name.as_str().to_string();
4207+
if self.type_signatures {
41734208
let ty =
41744209
c.ty.map(|ty| ty.display(self.db).to_string())
41754210
.unwrap_or_else(|| "Unavailable".to_string());
4176-
format!("{name} :: {ty}")
4211+
snapshot = format!("{snapshot} :: {ty}");
4212+
}
4213+
if self.module_names {
4214+
let module_name = c
4215+
.module_name
4216+
.map(ModuleName::as_str)
4217+
.unwrap_or("Current module");
4218+
snapshot = format!("{snapshot} :: {module_name}");
41774219
}
4220+
snapshot
41784221
})
41794222
.collect::<Vec<String>>()
41804223
.join("\n")
@@ -4216,6 +4259,7 @@ def f[T](x: T):
42164259
settings: CompletionSettings::default(),
42174260
skip_builtins: false,
42184261
type_signatures: false,
4262+
module_names: false,
42194263
predicate: None,
42204264
}
42214265
}

0 commit comments

Comments
 (0)