Auto merge of #14156 - Veykril:completion-pod, r=Veykril

internal: Don't reconstruct ref match completion in to_proto manually

cc rust-lang/rust-analyzer#12571

Author: bors

crates/hir-def/src/adt.rs

@@ -2,9 +2,10 @@
 
 use std::sync::Arc;
 
-use crate::tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree};
 use base_db::CrateId;
+use cfg::CfgOptions;
 use either::Either;
+
 use hir_expand::{
     name::{AsName, Name},
     HirFileId, InFile,
@@ -24,12 +25,12 @@ use crate::{
     src::HasChildSource,
     src::HasSource,
     trace::Trace,
+    tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree},
     type_ref::TypeRef,
     visibility::RawVisibility,
     EnumId, LocalEnumVariantId, LocalFieldId, LocalModuleId, Lookup, ModuleId, StructId, UnionId,
     VariantId,
 };
-use cfg::CfgOptions;
 
 /// Note that we use `StructData` for unions as well!
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/ide-completion/src/context.rs

@@ -571,28 +571,25 @@ impl<'a> CompletionContext<'a> {
 
         // try to skip completions on path with invalid colons
         // this approach works in normal path and inside token tree
-        match original_token.kind() {
-            T![:] => {
-                // return if no prev token before colon
-                let prev_token = original_token.prev_token()?;
-
-                // only has a single colon
-                if prev_token.kind() != T![:] {
-                    return None;
-                }
+        if original_token.kind() == T![:] {
+            // return if no prev token before colon
+            let prev_token = original_token.prev_token()?;
 
-                // has 3 colon or 2 coloncolon in a row
-                // special casing this as per discussion in https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1031845205
-                // and https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1032812751
-                if prev_token
-                    .prev_token()
-                    .map(|t| t.kind() == T![:] || t.kind() == T![::])
-                    .unwrap_or(false)
-                {
-                    return None;
-                }
+            // only has a single colon
+            if prev_token.kind() != T![:] {
+                return None;
+            }
+
+            // has 3 colon or 2 coloncolon in a row
+            // special casing this as per discussion in https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1031845205
+            // and https://github.com/rust-lang/rust-analyzer/pull/13611#discussion_r1032812751
+            if prev_token
+                .prev_token()
+                .map(|t| t.kind() == T![:] || t.kind() == T![::])
+                .unwrap_or(false)
+            {
+                return None;
             }
-            _ => {}
         }
 
         let AnalysisResult {

crates/ide-completion/src/context/analysis.rs

@@ -29,6 +29,7 @@ pub(super) struct AnalysisResult {
     pub(super) analysis: CompletionAnalysis,
     pub(super) expected: (Option<Type>, Option<ast::NameOrNameRef>),
     pub(super) qualifier_ctx: QualifierCtx,
+    /// the original token of the expanded file
     pub(super) token: SyntaxToken,
     pub(super) offset: TextSize,
 }
@@ -213,15 +214,6 @@ fn analyze(
     let _p = profile::span("CompletionContext::analyze");
     let ExpansionResult { original_file, speculative_file, offset, fake_ident_token, derive_ctx } =
         expansion_result;
-    let syntax_element = NodeOrToken::Token(fake_ident_token);
-    if is_in_token_of_for_loop(syntax_element.clone()) {
-        // for pat $0
-        // there is nothing to complete here except `in` keyword
-        // don't bother populating the context
-        // FIXME: the completion calculations should end up good enough
-        // such that this special case becomes unnecessary
-        return None;
-    }
 
     // Overwrite the path kind for derives
     if let Some((original_file, file_with_fake_ident, offset, origin_attr)) = derive_ctx {
@@ -249,37 +241,35 @@ fn analyze(
         return None;
     }
 
-    let name_like = match find_node_at_offset(&speculative_file, offset) {
-        Some(it) => it,
-        None => {
-            let analysis = if let Some(original) = ast::String::cast(original_token.clone()) {
-                CompletionAnalysis::String {
-                    original,
-                    expanded: ast::String::cast(self_token.clone()),
+    let Some(name_like) = find_node_at_offset(&speculative_file, offset) else {
+        let analysis = if let Some(original) = ast::String::cast(original_token.clone()) {
+            CompletionAnalysis::String {
+                original,
+                expanded: ast::String::cast(self_token.clone()),
+            }
+        } else {
+            // Fix up trailing whitespace problem
+            // #[attr(foo = $0
+            let token = syntax::algo::skip_trivia_token(self_token.clone(), Direction::Prev)?;
+            let p = token.parent()?;
+            if p.kind() == SyntaxKind::TOKEN_TREE
+                && p.ancestors().any(|it| it.kind() == SyntaxKind::META)
+            {
+                let colon_prefix = previous_non_trivia_token(self_token.clone())
+                    .map_or(false, |it| T![:] == it.kind());
+                CompletionAnalysis::UnexpandedAttrTT {
+                    fake_attribute_under_caret: fake_ident_token
+                        .parent_ancestors()
+                        .find_map(ast::Attr::cast),
+                    colon_prefix,
                 }
             } else {
-                // Fix up trailing whitespace problem
-                // #[attr(foo = $0
-                let token = syntax::algo::skip_trivia_token(self_token.clone(), Direction::Prev)?;
-                let p = token.parent()?;
-                if p.kind() == SyntaxKind::TOKEN_TREE
-                    && p.ancestors().any(|it| it.kind() == SyntaxKind::META)
-                {
-                    let colon_prefix = previous_non_trivia_token(self_token.clone())
-                        .map_or(false, |it| T![:] == it.kind());
-                    CompletionAnalysis::UnexpandedAttrTT {
-                        fake_attribute_under_caret: syntax_element
-                            .ancestors()
-                            .find_map(ast::Attr::cast),
-                        colon_prefix,
-                    }
-                } else {
-                    return None;
-                }
-            };
-            return Some((analysis, (None, None), QualifierCtx::default()));
-        }
+                return None;
+            }
+        };
+        return Some((analysis, (None, None), QualifierCtx::default()));
     };
+
     let expected = expected_type_and_name(sema, self_token, &name_like);
     let mut qual_ctx = QualifierCtx::default();
     let analysis = match name_like {
@@ -290,6 +280,22 @@ fn analyze(
             let parent = name_ref.syntax().parent()?;
             let (nameref_ctx, qualifier_ctx) =
                 classify_name_ref(sema, &original_file, name_ref, parent)?;
+
+            if let NameRefContext {
+                kind:
+                    NameRefKind::Path(PathCompletionCtx { kind: PathKind::Expr { .. }, path, .. }, ..),
+                ..
+            } = &nameref_ctx
+            {
+                if is_in_token_of_for_loop(path) {
+                    // for pat $0
+                    // there is nothing to complete here except `in` keyword
+                    // don't bother populating the context
+                    // Ideally this special casing wouldn't be needed, but the parser recovers
+                    return None;
+                }
+            }
+
             qual_ctx = qualifier_ctx;
             CompletionAnalysis::NameRef(nameref_ctx)
         }
@@ -323,16 +329,14 @@ fn expected_type_and_name(
                     ast::FieldExpr(e) => e
                         .syntax()
                         .ancestors()
-                        .map_while(ast::FieldExpr::cast)
-                        .last()
-                        .map(|it| it.syntax().clone()),
+                        .take_while(|it| ast::FieldExpr::can_cast(it.kind()))
+                        .last(),
                     ast::PathSegment(e) => e
                         .syntax()
                         .ancestors()
                         .skip(1)
                         .take_while(|it| ast::Path::can_cast(it.kind()) || ast::PathExpr::can_cast(it.kind()))
-                        .find_map(ast::PathExpr::cast)
-                        .map(|it| it.syntax().clone()),
+                        .find(|it| ast::PathExpr::can_cast(it.kind())),
                     _ => None
                 }
             };
@@ -1270,40 +1274,29 @@ fn path_or_use_tree_qualifier(path: &ast::Path) -> Option<(ast::Path, bool)> {
     Some((use_tree.path()?, true))
 }
 
-pub(crate) fn is_in_token_of_for_loop(element: SyntaxElement) -> bool {
+fn is_in_token_of_for_loop(path: &ast::Path) -> bool {
     // oh my ...
     (|| {
-        let syntax_token = element.into_token()?;
-        let range = syntax_token.text_range();
-        let for_expr = syntax_token.parent_ancestors().find_map(ast::ForExpr::cast)?;
-
-        // check if the current token is the `in` token of a for loop
-        if let Some(token) = for_expr.in_token() {
-            return Some(syntax_token == token);
+        let expr = path.syntax().parent().and_then(ast::PathExpr::cast)?;
+        let for_expr = expr.syntax().parent().and_then(ast::ForExpr::cast)?;
+        if for_expr.in_token().is_some() {
+            return Some(false);
         }
         let pat = for_expr.pat()?;
-        if range.end() < pat.syntax().text_range().end() {
-            // if we are inside or before the pattern we can't be at the `in` token position
-            return None;
-        }
         let next_sibl = next_non_trivia_sibling(pat.syntax().clone().into())?;
         Some(match next_sibl {
-            // the loop body is some node, if our token is at the start we are at the `in` position,
-            // otherwise we could be in a recovered expression, we don't wanna ruin completions there
-            syntax::NodeOrToken::Node(n) => n.text_range().start() == range.start(),
-            // the loop body consists of a single token, if we are this we are certainly at the `in` token position
-            syntax::NodeOrToken::Token(t) => t == syntax_token,
+            syntax::NodeOrToken::Node(n) => {
+                n.text_range().start() == path.syntax().text_range().start()
+            }
+            syntax::NodeOrToken::Token(t) => {
+                t.text_range().start() == path.syntax().text_range().start()
+            }
         })
     })()
     .unwrap_or(false)
 }
 
-#[test]
-fn test_for_is_prev2() {
-    crate::tests::check_pattern_is_applicable(r"fn __() { for i i$0 }", is_in_token_of_for_loop);
-}
-
-pub(crate) fn is_in_loop_body(node: &SyntaxNode) -> bool {
+fn is_in_loop_body(node: &SyntaxNode) -> bool {
     node.ancestors()
         .take_while(|it| it.kind() != SyntaxKind::FN && it.kind() != SyntaxKind::CLOSURE_EXPR)
         .find_map(|it| {

crates/ide-completion/src/item.rs

@@ -14,9 +14,9 @@ use crate::{
     render::{render_path_resolution, RenderContext},
 };
 
-/// `CompletionItem` describes a single completion variant in the editor pop-up.
-/// It is basically a POD with various properties. To construct a
-/// `CompletionItem`, use `new` method and the `Builder` struct.
+/// `CompletionItem` describes a single completion entity which expands to 1 or more entries in the
+/// editor pop-up. It is basically a POD with various properties. To construct a
+/// [`CompletionItem`], use [`Builder::new`] method and the [`Builder`] struct.
 #[derive(Clone)]
 pub struct CompletionItem {
     /// Label in the completion pop up which identifies completion.
@@ -396,14 +396,20 @@ impl CompletionItem {
         self.trigger_call_info
     }
 
-    pub fn ref_match(&self) -> Option<(Mutability, TextSize, CompletionRelevance)> {
+    pub fn ref_match(&self) -> Option<(String, text_edit::Indel, CompletionRelevance)> {
         // Relevance of the ref match should be the same as the original
         // match, but with exact type match set because self.ref_match
         // is only set if there is an exact type match.
         let mut relevance = self.relevance;
         relevance.type_match = Some(CompletionRelevanceTypeMatch::Exact);
 
-        self.ref_match.map(|(mutability, offset)| (mutability, offset, relevance))
+        self.ref_match.map(|(mutability, offset)| {
+            (
+                format!("&{}{}", mutability.as_keyword_for_ref(), self.label()),
+                text_edit::Indel::insert(offset, format!("&{}", mutability.as_keyword_for_ref())),
+                relevance,
+            )
+        })
     }
 
     pub fn imports_to_add(&self) -> &[LocatedImport] {

crates/ide-completion/src/lib.rs

@@ -156,21 +156,23 @@ pub fn completions(
 
     // prevent `(` from triggering unwanted completion noise
     if trigger_character == Some('(') {
-        if let CompletionAnalysis::NameRef(NameRefContext { kind, .. }) = &analysis {
-            if let NameRefKind::Path(
-                path_ctx @ PathCompletionCtx { kind: PathKind::Vis { has_in_token }, .. },
-            ) = kind
-            {
-                completions::vis::complete_vis_path(&mut completions, ctx, path_ctx, has_in_token);
-            }
+        if let CompletionAnalysis::NameRef(NameRefContext {
+            kind:
+                NameRefKind::Path(
+                    path_ctx @ PathCompletionCtx { kind: PathKind::Vis { has_in_token }, .. },
+                ),
+            ..
+        }) = analysis
+        {
+            completions::vis::complete_vis_path(&mut completions, ctx, path_ctx, has_in_token);
         }
         return Some(completions.into());
     }
 
     {
         let acc = &mut completions;
 
-        match &analysis {
+        match analysis {
             CompletionAnalysis::Name(name_ctx) => completions::complete_name(acc, ctx, name_ctx),
             CompletionAnalysis::NameRef(name_ref_ctx) => {
                 completions::complete_name_ref(acc, ctx, name_ref_ctx)

crates/ide-completion/src/render.rs

@@ -529,8 +529,7 @@ mod tests {
                 let relevance = display_relevance(it.relevance());
                 items.push(format!("{tag} {} {relevance}\n", it.label()));
 
-                if let Some((mutability, _offset, relevance)) = it.ref_match() {
-                    let label = format!("&{}{}", mutability.as_keyword_for_ref(), it.label());
+                if let Some((label, _indel, relevance)) = it.ref_match() {
                     let relevance = display_relevance(relevance);
 
                     items.push(format!("{tag} {label} {relevance}\n"));

crates/ide-completion/src/tests.rs

@@ -23,15 +23,14 @@ mod type_pos;
 mod use_tree;
 mod visibility;
 
-use hir::{db::DefDatabase, PrefixKind, Semantics};
+use hir::{db::DefDatabase, PrefixKind};
 use ide_db::{
     base_db::{fixture::ChangeFixture, FileLoader, FilePosition},
     imports::insert_use::{ImportGranularity, InsertUseConfig},
     RootDatabase, SnippetCap,
 };
 use itertools::Itertools;
 use stdx::{format_to, trim_indent};
-use syntax::{AstNode, NodeOrToken, SyntaxElement};
 use test_utils::assert_eq_text;
 
 use crate::{
@@ -216,15 +215,6 @@ pub(crate) fn check_edit_with_config(
     assert_eq_text!(&ra_fixture_after, &actual)
 }
 
-pub(crate) fn check_pattern_is_applicable(code: &str, check: impl FnOnce(SyntaxElement) -> bool) {
-    let (db, pos) = position(code);
-
-    let sema = Semantics::new(&db);
-    let original_file = sema.parse(pos.file_id);
-    let token = original_file.syntax().token_at_offset(pos.offset).left_biased().unwrap();
-    assert!(check(NodeOrToken::Token(token)));
-}
-
 pub(crate) fn get_all_items(
     config: CompletionConfig,
     code: &str,
@@ -246,8 +236,9 @@ pub(crate) fn get_all_items(
 }
 
 #[test]
-fn test_no_completions_required() {
+fn test_no_completions_in_for_loop_in_kw_pos() {
     assert_eq!(completion_list(r#"fn foo() { for i i$0 }"#), String::new());
+    assert_eq!(completion_list(r#"fn foo() { for i in$0 }"#), String::new());
 }
 
 #[test]