mirror of
https://github.com/astral-sh/ruff
synced 2026-01-21 13:30:49 -05:00
[ty] Get rid of high-level completion deduplication
It turns out this wasn't actually doing much and was just papering over the possibility of duplicates being returned from `SemanticModel`. So this was doing a fair bit of work for no good reason. Instead, we push deduplication down into semantic completions directly. This will run over a (typically) much smaller number of completions. With that said, this doesn't seem to lead to improvement in ad hoc benchmarking. However, perf wasn't really the main motivation here: this change is primarily to prep for switching to a max-heap. This deduplication was problematic because it was being done *after* sorting, which meant it dependend on having the entire set of completions in memory. But with a max-heap, we'll only keep around the top-K completions, so deduplication has to be done "earlier" (if at all).
This commit is contained in:
committed by
Andrew Gallant
parent
61abbb94e3
commit
2784461741
@@ -122,8 +122,6 @@ impl<'db> Completions<'db> {
|
||||
/// sequence of completions.
|
||||
fn into_completions(mut self) -> Vec<Completion<'db>> {
|
||||
self.items.sort_unstable();
|
||||
self.items
|
||||
.dedup_by(|c1, c2| (&c1.0.name, c1.0.module_name) == (&c2.0.name, c2.0.module_name));
|
||||
self.items.truncate(Completions::LIMIT);
|
||||
self.items
|
||||
.into_iter()
|
||||
@@ -134,8 +132,6 @@ impl<'db> Completions<'db> {
|
||||
// Convert this collection into a list of "import..." fixes
|
||||
fn into_imports(mut self) -> Vec<ImportEdit> {
|
||||
self.items.sort_unstable();
|
||||
self.items
|
||||
.dedup_by(|c1, c2| (&c1.0.name, c1.0.module_name) == (&c2.0.name, c2.0.module_name));
|
||||
self.items
|
||||
.into_iter()
|
||||
.map(|CompletionRanker(c)| c)
|
||||
@@ -151,8 +147,6 @@ impl<'db> Completions<'db> {
|
||||
// Convert this collection into a list of "qualify..." fixes
|
||||
fn into_qualifications(mut self, range: TextRange) -> Vec<ImportEdit> {
|
||||
self.items.sort_unstable();
|
||||
self.items
|
||||
.dedup_by(|c1, c2| (&c1.0.name, c1.0.module_name) == (&c2.0.name, c2.0.module_name));
|
||||
self.items
|
||||
.into_iter()
|
||||
.map(|CompletionRanker(c)| c)
|
||||
@@ -208,11 +202,8 @@ impl<'db> Completions<'db> {
|
||||
if self.context.exclude(self.db, &builder) {
|
||||
return false;
|
||||
}
|
||||
self.items.push(CompletionRanker(builder.build(
|
||||
self.db,
|
||||
&self.context,
|
||||
&self.query,
|
||||
)));
|
||||
let completion = builder.build(self.db, &self.context, &self.query);
|
||||
self.items.push(CompletionRanker(completion));
|
||||
true
|
||||
}
|
||||
}
|
||||
@@ -1409,11 +1400,11 @@ fn add_function_arg_completions<'db>(
|
||||
let Some(sig_help) = signature_help(db, file, cursor.offset) else {
|
||||
return;
|
||||
};
|
||||
let set_function_args = detect_set_function_args(cursor);
|
||||
let mut set_function_args = detect_set_function_args(cursor);
|
||||
|
||||
for sig in &sig_help.signatures {
|
||||
for p in &sig.parameters {
|
||||
if p.is_positional_only || set_function_args.contains(&p.name.as_str()) {
|
||||
if p.is_positional_only || !set_function_args.insert(p.name.as_str()) {
|
||||
continue;
|
||||
}
|
||||
let mut builder = CompletionBuilder::argument(&p.name).ty(p.ty);
|
||||
@@ -8170,6 +8161,20 @@ def f(x: Intersection[int, Any] | str):
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn no_duplicate_keyword_arg() {
|
||||
let builder = completion_test_builder(
|
||||
r#"
|
||||
import re
|
||||
re.match('', '', fla<CURSOR>
|
||||
"#,
|
||||
);
|
||||
assert_snapshot!(
|
||||
builder.skip_auto_import().skip_builtins().build().snapshot(),
|
||||
@"flags=",
|
||||
);
|
||||
}
|
||||
|
||||
/// A way to create a simple single-file (named `main.py`) completion test
|
||||
/// builder.
|
||||
///
|
||||
|
||||
@@ -251,6 +251,11 @@ impl<'db> SemanticModel<'db> {
|
||||
// Builtins are available in all scopes.
|
||||
let builtins = ModuleName::new_static("builtins").expect("valid module name");
|
||||
completions.extend(self.module_completions(&builtins));
|
||||
|
||||
// The above can sometimes result in duplicates. Get rid of them.
|
||||
completions.sort_by(|c1, c2| c1.name.cmp(&c2.name));
|
||||
completions.dedup_by(|c1, c2| c1.name == c2.name);
|
||||
|
||||
completions
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user