From 9349769363624795db79a860b5a4b46fec9b0930 Mon Sep 17 00:00:00 2001 From: Max Heller Date: Thu, 27 Jul 2023 19:33:00 -0400 Subject: [PATCH] exclude non-identifier aliases from completion filtering text --- Cargo.lock | 1 + crates/ide-completion/Cargo.toml | 1 + crates/ide-completion/src/item.rs | 34 +++++++++++++++++++++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index f8806794979..f07c08a77bc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -715,6 +715,7 @@ dependencies = [ "syntax", "test-utils", "text-edit", + "unicode-ident", ] [[package]] diff --git a/crates/ide-completion/Cargo.toml b/crates/ide-completion/Cargo.toml index 092fb303668..c06ac55aae3 100644 --- a/crates/ide-completion/Cargo.toml +++ b/crates/ide-completion/Cargo.toml @@ -17,6 +17,7 @@ itertools = "0.10.5" once_cell = "1.17.0" smallvec.workspace = true +unicode-ident = "1.0.0" # local deps diff --git a/crates/ide-completion/src/item.rs b/crates/ide-completion/src/item.rs index e850f7bfdf3..d5713db0a36 100644 --- a/crates/ide-completion/src/item.rs +++ b/crates/ide-completion/src/item.rs @@ -427,9 +427,21 @@ impl Builder { let insert_text = self.insert_text.unwrap_or_else(|| label.to_string()); if !self.doc_aliases.is_empty() { - let doc_aliases = self.doc_aliases.into_iter().join(", "); + let doc_aliases = self.doc_aliases.iter().join(", "); label = SmolStr::from(format!("{label} (alias {doc_aliases})")); - lookup = SmolStr::from(format!("{lookup} {doc_aliases}")); + let lookup_doc_aliases = (self.doc_aliases.iter()) + // Don't include aliases in `lookup` that aren't valid identifiers as including + // them results in weird completion filtering behavior e.g. `Partial>` matching + // `PartialOrd` because it has an alias of ">". + .filter(|alias| { + let mut chars = alias.chars(); + chars.next().map(unicode_ident::is_xid_start).unwrap_or(false) + && chars.all(unicode_ident::is_xid_continue) + }) + .join(", "); + if !lookup_doc_aliases.is_empty() { + lookup = SmolStr::from(format!("{lookup} {lookup_doc_aliases}")); + } } if let [import_edit] = &*self.imports_to_add { // snippets can have multiple imports, but normal completions only have up to one @@ -553,9 +565,12 @@ impl Builder { #[cfg(test)] mod tests { + use ide_db::SymbolKind; use itertools::Itertools; use test_utils::assert_eq_text; + use crate::{CompletionItem, CompletionItemKind}; + use super::{ CompletionRelevance, CompletionRelevancePostfixMatch, CompletionRelevanceTypeMatch, }; @@ -630,4 +645,19 @@ mod tests { check_relevance_score_ordered(expected_relevance_order); } + + #[test] + fn exclude_non_identifier_aliases_from_lookup() { + let mut item = CompletionItem::new( + CompletionItemKind::SymbolKind(SymbolKind::Trait), + Default::default(), + "PartialOrd", + ); + let aliases = [">", "<", "<=", ">="]; + item.doc_aliases(aliases.iter().map(|&alias| alias.into()).collect()); + let item = item.build(&Default::default()); + for alias in aliases { + assert!(!item.lookup().contains(alias)); + } + } }