Merge #9489

9489: feat: always prefer postfix snippets if there's exact textual match r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

Author: bors[bot]

crates/ide_completion/src/completions/postfix.rs

@@ -15,7 +15,7 @@ use crate::{
     context::CompletionContext,
     item::{Builder, CompletionKind},
     patterns::ImmediateLocation,
-    CompletionItem, CompletionItemKind, Completions,
+    CompletionItem, CompletionItemKind, CompletionRelevance, Completions,
 };
 
 pub(crate) fn complete_postfix(acc: &mut Completions, ctx: &CompletionContext) {
@@ -299,6 +299,12 @@ fn postfix_snippet(
     };
     let mut item = CompletionItem::new(CompletionKind::Postfix, ctx.source_range(), label);
     item.detail(detail).kind(CompletionItemKind::Snippet).snippet_edit(cap, edit);
+    if ctx.original_token.text() == label {
+        let mut relevance = CompletionRelevance::default();
+        relevance.exact_postfix_snippet_match = true;
+        item.set_relevance(relevance);
+    }
+
     item
 }
 

crates/ide_completion/src/item.rs

@@ -144,6 +144,15 @@ pub struct CompletionRelevance {
     /// }
     /// ```
     pub is_local: bool,
+    /// This is set in cases like these:
+    ///
+    /// ```
+    /// (a > b).not$0
+    /// ```
+    ///
+    /// Basically, we want to guarantee that postfix snippets always takes
+    /// precedence over everything else.
+    pub exact_postfix_snippet_match: bool,
 }
 
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
@@ -194,7 +203,9 @@ impl CompletionRelevance {
         if self.is_local {
             score += 1;
         }
-
+        if self.exact_postfix_snippet_match {
+            score += 100;
+        }
         score
     }
 
@@ -598,6 +609,13 @@ mod tests {
                 exact_name_match: true,
                 type_match: Some(CompletionRelevanceTypeMatch::Exact),
                 is_local: true,
+                ..CompletionRelevance::default()
+            }],
+            vec![CompletionRelevance {
+                exact_name_match: false,
+                type_match: None,
+                is_local: false,
+                exact_postfix_snippet_match: true,
             }],
         ];
 

crates/ide_completion/src/render.rs

@@ -330,6 +330,8 @@ fn compute_ref_match(
 
 #[cfg(test)]
 mod tests {
+    use std::cmp;
+
     use expect_test::{expect, Expect};
     use itertools::Itertools;
 
@@ -339,32 +341,25 @@ mod tests {
         CompletionKind, CompletionRelevance,
     };
 
+    #[track_caller]
     fn check(ra_fixture: &str, expect: Expect) {
         let actual = do_completion(ra_fixture, CompletionKind::Reference);
         expect.assert_debug_eq(&actual);
     }
 
+    #[track_caller]
     fn check_relevance(ra_fixture: &str, expect: Expect) {
-        fn display_relevance(relevance: CompletionRelevance) -> String {
-            let relevance_factors = vec![
-                (relevance.type_match == Some(CompletionRelevanceTypeMatch::Exact), "type"),
-                (
-                    relevance.type_match == Some(CompletionRelevanceTypeMatch::CouldUnify),
-                    "type_could_unify",
-                ),
-                (relevance.exact_name_match, "name"),
-                (relevance.is_local, "local"),
-            ]
-            .into_iter()
-            .filter_map(|(cond, desc)| if cond { Some(desc) } else { None })
-            .join("+");
+        check_relevance_for_kinds(&[CompletionKind::Reference], ra_fixture, expect)
+    }
 
-            format!("[{}]", relevance_factors)
-        }
+    #[track_caller]
+    fn check_relevance_for_kinds(kinds: &[CompletionKind], ra_fixture: &str, expect: Expect) {
+        let mut actual = get_all_items(TEST_CONFIG, ra_fixture);
+        actual.retain(|it| kinds.contains(&it.completion_kind));
+        actual.sort_by_key(|it| cmp::Reverse(it.relevance().score()));
 
-        let actual = get_all_items(TEST_CONFIG, ra_fixture)
+        let actual = actual
             .into_iter()
-            .filter(|it| it.completion_kind == CompletionKind::Reference)
             .flat_map(|it| {
                 let mut items = vec![];
 
@@ -384,6 +379,24 @@ mod tests {
             .collect::<String>();
 
         expect.assert_eq(&actual);
+
+        fn display_relevance(relevance: CompletionRelevance) -> String {
+            let relevance_factors = vec![
+                (relevance.type_match == Some(CompletionRelevanceTypeMatch::Exact), "type"),
+                (
+                    relevance.type_match == Some(CompletionRelevanceTypeMatch::CouldUnify),
+                    "type_could_unify",
+                ),
+                (relevance.exact_name_match, "name"),
+                (relevance.is_local, "local"),
+                (relevance.exact_postfix_snippet_match, "snippet"),
+            ]
+            .into_iter()
+            .filter_map(|(cond, desc)| if cond { Some(desc) } else { None })
+            .join("+");
+
+            format!("[{}]", relevance_factors)
+        }
     }
 
     #[test]
@@ -528,6 +541,7 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
+                            exact_postfix_snippet_match: false,
                         },
                         trigger_call_info: true,
                     },
@@ -556,6 +570,7 @@ fn main() { let _: m::Spam = S$0 }
                                 Exact,
                             ),
                             is_local: false,
+                            exact_postfix_snippet_match: false,
                         },
                     },
                     CompletionItem {
@@ -649,6 +664,7 @@ fn foo() { A { the$0 } }
                                 CouldUnify,
                             ),
                             is_local: false,
+                            exact_postfix_snippet_match: false,
                         },
                     },
                 ]
@@ -910,9 +926,9 @@ fn test(bar: u32) { }
 fn foo(s: S) { test(s.$0) }
 "#,
             expect![[r#"
-                fd foo []
                 fd bar [type+name]
                 fd baz [type]
+                fd foo []
             "#]],
         );
     }
@@ -926,9 +942,9 @@ struct B { x: (), y: f32, bar: u32 }
 fn foo(a: A) { B { bar: a.$0 }; }
 "#,
             expect![[r#"
-                fd foo []
                 fd bar [type+name]
                 fd baz [type]
+                fd foo []
             "#]],
         )
     }
@@ -956,9 +972,9 @@ fn f(foo: i64) {  }
 fn foo(a: A) { f(B { bar: a.$0 }); }
 "#,
             expect![[r#"
-                fd foo []
                 fd bar [type+name]
                 fd baz [type]
+                fd foo []
             "#]],
         );
     }
@@ -1004,9 +1020,9 @@ fn bar() -> u8 { 0 }
 fn f() { A { bar: b$0 }; }
 "#,
             expect![[r#"
+                fn bar() [type+name]
                 fn baz() [type]
                 st A []
-                fn bar() [type+name]
                 fn f() []
             "#]],
         );
@@ -1329,14 +1345,54 @@ fn foo() {
 }
 "#,
             expect![[r#"
+                lc foo [type+local]
                 ev Foo::A(…) [type_could_unify]
                 ev Foo::B [type_could_unify]
-                lc foo [type+local]
                 en Foo []
                 fn baz() []
                 fn bar() []
                 fn foo() []
             "#]],
         );
     }
+
+    #[test]
+    fn postfix_completion_relevance() {
+        check_relevance_for_kinds(
+            &[CompletionKind::Postfix, CompletionKind::Magic],
+            r#"
+mod ops {
+    pub trait Not {
+        type Output;
+        fn not(self) -> Self::Output;
+    }
+
+    impl Not for bool {
+        type Output = bool;
+        fn not(self) -> bool { if self { false } else { true }}
+    }
+}
+
+fn main() {
+    let _: bool = (9 > 2).not$0;
+}
+"#,
+            expect![[r#"
+                sn not [snippet]
+                me not() (ops::Not) [type_could_unify]
+                sn if []
+                sn while []
+                sn ref []
+                sn refm []
+                sn match []
+                sn box []
+                sn ok []
+                sn err []
+                sn some []
+                sn dbg []
+                sn dbgr []
+                sn call []
+            "#]],
+        );
+    }
 }

crates/rust-analyzer/src/to_proto.rs

@@ -195,6 +195,7 @@ pub(crate) fn completion_items(
     tdpp: lsp_types::TextDocumentPositionParams,
     items: Vec<CompletionItem>,
 ) -> Vec<lsp_types::CompletionItem> {
+    let max_relevance = items.iter().map(|it| it.relevance().score()).max().unwrap_or_default();
     let mut res = Vec::with_capacity(items.len());
     for item in items {
         completion_item(
@@ -203,6 +204,7 @@ pub(crate) fn completion_items(
             enable_imports_on_the_fly,
             line_index,
             &tdpp,
+            max_relevance,
             item,
         )
     }
@@ -215,6 +217,7 @@ fn completion_item(
     enable_imports_on_the_fly: bool,
     line_index: &LineIndex,
     tdpp: &lsp_types::TextDocumentPositionParams,
+    max_relevance: u32,
     item: CompletionItem,
 ) {
     let mut additional_text_edits = Vec::new();
@@ -259,7 +262,7 @@ fn completion_item(
         ..Default::default()
     };
 
-    set_score(&mut lsp_item, item.relevance());
+    set_score(&mut lsp_item, max_relevance, item.relevance());
 
     if item.deprecated() {
         lsp_item.tags = Some(vec![lsp_types::CompletionItemTag::Deprecated])
@@ -288,7 +291,7 @@ fn completion_item(
 
     if let Some((mutability, relevance)) = item.ref_match() {
         let mut lsp_item_with_ref = lsp_item.clone();
-        set_score(&mut lsp_item_with_ref, relevance);
+        set_score(&mut lsp_item_with_ref, max_relevance, relevance);
         lsp_item_with_ref.label =
             format!("&{}{}", mutability.as_keyword_for_ref(), lsp_item_with_ref.label);
         if let Some(it) = &mut lsp_item_with_ref.text_edit {
@@ -304,8 +307,12 @@ fn completion_item(
 
     acc.push(lsp_item);
 
-    fn set_score(res: &mut lsp_types::CompletionItem, relevance: CompletionRelevance) {
-        if relevance.is_relevant() {
+    fn set_score(
+        res: &mut lsp_types::CompletionItem,
+        max_relevance: u32,
+        relevance: CompletionRelevance,
+    ) {
+        if relevance.is_relevant() && relevance.score() == max_relevance {
             res.preselect = Some(true);
         }
         // The relevance needs to be inverted to come up with a sort score