Merge #9020

9020: fix: Don't complete non-macro item paths in impls and modules r=Veykril a=Veykril

Part of #8518
bors r+

Co-authored-by: Lukas Wirth <lukastw97@gmail.com>

crates/ide_completion/src/completions/flyimport.rs

@@ -110,7 +110,11 @@ pub(crate) fn import_on_the_fly(acc: &mut Completions, ctx: &CompletionContext)
     if !ctx.config.enable_imports_on_the_fly {
         return None;
     }
-    if ctx.use_item_syntax.is_some() || ctx.is_path_disallowed() {
+    if ctx.use_item_syntax.is_some()
+        || ctx.is_path_disallowed()
+        || ctx.expects_item()
+        || ctx.expects_assoc_item()
+    {
         return None;
     }
     let potential_import_name = {

crates/ide_completion/src/completions/keyword.rs

@@ -49,35 +49,35 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         return;
     }
 
-    let has_trait_or_impl_parent = ctx.has_impl_or_trait_parent();
+    let expects_assoc_item = ctx.expects_assoc_item();
     let has_block_expr_parent = ctx.has_block_expr_parent();
-    let has_item_list_parent = ctx.has_item_list_parent();
+    let expects_item = ctx.expects_item();
     if ctx.has_impl_or_trait_prev_sibling() {
         add_keyword(ctx, acc, "where", "where ");
         return;
     }
     if ctx.previous_token_is(T![unsafe]) {
-        if has_item_list_parent || has_block_expr_parent {
+        if expects_item || has_block_expr_parent {
             add_keyword(ctx, acc, "fn", "fn $1($2) {\n    $0\n}")
         }
 
-        if has_item_list_parent || has_block_expr_parent {
+        if expects_item || has_block_expr_parent {
             add_keyword(ctx, acc, "trait", "trait $1 {\n    $0\n}");
             add_keyword(ctx, acc, "impl", "impl $1 {\n    $0\n}");
         }
 
         return;
     }
-    if has_item_list_parent || has_trait_or_impl_parent || has_block_expr_parent {
+    if expects_item || expects_assoc_item || has_block_expr_parent {
         add_keyword(ctx, acc, "fn", "fn $1($2) {\n    $0\n}");
     }
-    if has_item_list_parent || has_block_expr_parent {
+    if expects_item || has_block_expr_parent {
         add_keyword(ctx, acc, "use", "use ");
         add_keyword(ctx, acc, "impl", "impl $1 {\n    $0\n}");
         add_keyword(ctx, acc, "trait", "trait $1 {\n    $0\n}");
     }
 
-    if has_item_list_parent {
+    if expects_item {
         add_keyword(ctx, acc, "enum", "enum $1 {\n    $0\n}");
         add_keyword(ctx, acc, "struct", "struct $0");
         add_keyword(ctx, acc, "union", "union $1 {\n    $0\n}");
@@ -101,24 +101,23 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         add_keyword(ctx, acc, "else", "else {\n    $0\n}");
         add_keyword(ctx, acc, "else if", "else if $1 {\n    $0\n}");
     }
-    if has_item_list_parent || has_block_expr_parent {
+    if expects_item || has_block_expr_parent {
         add_keyword(ctx, acc, "mod", "mod $0");
     }
-    if ctx.has_ident_or_ref_pat_parent() {
+    if ctx.expects_ident_pat_or_ref_expr() {
         add_keyword(ctx, acc, "mut", "mut ");
     }
-    if has_item_list_parent || has_trait_or_impl_parent || has_block_expr_parent {
+    if expects_item || expects_assoc_item || has_block_expr_parent {
         add_keyword(ctx, acc, "const", "const ");
         add_keyword(ctx, acc, "type", "type ");
     }
-    if has_item_list_parent || has_block_expr_parent {
+    if expects_item || has_block_expr_parent {
         add_keyword(ctx, acc, "static", "static ");
     };
-    if has_item_list_parent || has_block_expr_parent {
+    if expects_item || has_block_expr_parent {
         add_keyword(ctx, acc, "extern", "extern ");
     }
-    if has_item_list_parent || has_trait_or_impl_parent || has_block_expr_parent || ctx.is_match_arm
-    {
+    if expects_item || expects_assoc_item || has_block_expr_parent || ctx.is_match_arm {
         add_keyword(ctx, acc, "unsafe", "unsafe ");
     }
     if ctx.in_loop_body {
@@ -130,7 +129,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
             add_keyword(ctx, acc, "break", "break");
         }
     }
-    if has_item_list_parent || ctx.has_impl_parent() || ctx.has_field_list_parent() {
+    if expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_record_field() {
         add_keyword(ctx, acc, "pub(crate)", "pub(crate) ");
         add_keyword(ctx, acc, "pub", "pub ");
     }

crates/ide_completion/src/completions/qualified_path.rs

@@ -20,6 +20,17 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
         None => return,
     };
     let context_module = ctx.scope.module();
+    if ctx.expects_item() || ctx.expects_assoc_item() {
+        if let PathResolution::Def(hir::ModuleDef::Module(module)) = resolution {
+            let module_scope = module.scope(ctx.db, context_module);
+            for (name, def) in module_scope {
+                if let ScopeDef::MacroDef(macro_def) = def {
+                    acc.add_macro(ctx, Some(name.to_string()), macro_def);
+                }
+            }
+        }
+        return;
+    }
 
     // Add associated types on type parameters and `Self`.
     resolution.assoc_type_shorthand_candidates(ctx.db, |_, alias| {
@@ -594,7 +605,7 @@ fn main() { T::$0; }
 macro_rules! foo { () => {} }
 
 fn main() { let _ = crate::$0 }
-        "#,
+"#,
             expect![[r##"
                 fn main()  fn()
                 ma foo!(…) #[macro_export] macro_rules! foo
@@ -602,6 +613,25 @@ fn main() { let _ = crate::$0 }
         );
     }
 
+    #[test]
+    fn completes_qualified_macros_in_impl() {
+        check(
+            r#"
+#[macro_export]
+macro_rules! foo { () => {} }
+
+struct MyStruct {}
+
+impl MyStruct {
+    crate::$0
+}
+"#,
+            expect![[r##"
+                ma foo! #[macro_export] macro_rules! foo
+            "##]],
+        );
+    }
+
     #[test]
     fn test_super_super_completion() {
         check(

crates/ide_completion/src/completions/unqualified_path.rs

@@ -12,6 +12,14 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
     if ctx.is_path_disallowed() {
         return;
     }
+    if ctx.expects_item() || ctx.expects_assoc_item() {
+        ctx.scope.process_all_names(&mut |name, def| {
+            if let ScopeDef::MacroDef(macro_def) = def {
+                acc.add_macro(ctx, Some(name.to_string()), macro_def);
+            }
+        });
+        return;
+    }
 
     if let Some(hir::Adt::Enum(e)) =
         ctx.expected_type.as_ref().and_then(|ty| ty.strip_references().as_adt())
@@ -647,7 +655,7 @@ fn f() {}
     }
 
     #[test]
-    fn completes_type_or_trait_in_impl_block() {
+    fn completes_target_type_or_trait_in_impl_block() {
         check(
             r#"
 trait MyTrait {}
@@ -662,4 +670,21 @@ impl My$0
             "#]],
         )
     }
+
+    #[test]
+    fn only_completes_macros_in_assoc_item_list() {
+        check(
+            r#"
+struct MyStruct {}
+macro_rules! foo {}
+
+impl MyStruct {
+    $0
+}
+"#,
+            expect![[r#"
+                ma foo! macro_rules! foo
+            "#]],
+        )
+    }
 }

crates/ide_completion/src/context.rs

@@ -17,9 +17,8 @@ use text_edit::Indel;
 
 use crate::{
     patterns::{
-        for_is_prev2, has_bind_pat_parent, has_block_expr_parent, has_field_list_parent,
-        has_impl_parent, has_item_list_or_source_file_parent, has_prev_sibling, has_ref_parent,
-        has_trait_parent, inside_impl_trait_block, is_in_loop_body, is_match_arm, previous_token,
+        determine_location, for_is_prev2, has_prev_sibling, inside_impl_trait_block,
+        is_in_loop_body, is_match_arm, previous_token, ImmediateLocation,
     },
     CompletionConfig,
 };
@@ -30,18 +29,6 @@ pub(crate) enum PatternRefutability {
     Irrefutable,
 }
 
-/// Direct parent container of the cursor position
-#[derive(Copy, Clone, Debug, PartialEq, Eq)]
-pub(crate) enum ImmediateLocation {
-    Impl,
-    Trait,
-    RecordFieldList,
-    RefPatOrExpr,
-    IdentPat,
-    BlockExpr,
-    ItemList,
-}
-
 #[derive(Copy, Clone, Debug, PartialEq, Eq)]
 pub(crate) enum PrevSibling {
     Trait,
@@ -127,6 +114,7 @@ pub(crate) struct CompletionContext<'a> {
 
     no_completion_required: bool,
 }
+
 impl<'a> CompletionContext<'a> {
     pub(super) fn new(
         db: &'a RootDatabase,
@@ -281,34 +269,34 @@ impl<'a> CompletionContext<'a> {
         self.previous_token.as_ref().map_or(false, |tok| tok.kind() == kind)
     }
 
-    pub(crate) fn has_impl_or_trait_parent(&self) -> bool {
+    pub(crate) fn expects_assoc_item(&self) -> bool {
         matches!(
             self.completion_location,
             Some(ImmediateLocation::Trait) | Some(ImmediateLocation::Impl)
         )
     }
 
+    pub(crate) fn expects_non_trait_assoc_item(&self) -> bool {
+        matches!(self.completion_location, Some(ImmediateLocation::Impl))
+    }
+
+    pub(crate) fn expects_item(&self) -> bool {
+        matches!(self.completion_location, Some(ImmediateLocation::ItemList))
+    }
+
     pub(crate) fn has_block_expr_parent(&self) -> bool {
         matches!(self.completion_location, Some(ImmediateLocation::BlockExpr))
     }
 
-    pub(crate) fn has_item_list_parent(&self) -> bool {
-        matches!(self.completion_location, Some(ImmediateLocation::ItemList))
-    }
-
-    pub(crate) fn has_ident_or_ref_pat_parent(&self) -> bool {
+    pub(crate) fn expects_ident_pat_or_ref_expr(&self) -> bool {
         matches!(
             self.completion_location,
-            Some(ImmediateLocation::IdentPat) | Some(ImmediateLocation::RefPatOrExpr)
+            Some(ImmediateLocation::IdentPat) | Some(ImmediateLocation::RefExpr)
         )
     }
 
-    pub(crate) fn has_impl_parent(&self) -> bool {
-        matches!(self.completion_location, Some(ImmediateLocation::Impl))
-    }
-
-    pub(crate) fn has_field_list_parent(&self) -> bool {
-        matches!(self.completion_location, Some(ImmediateLocation::RecordFieldList))
+    pub(crate) fn expect_record_field(&self) -> bool {
+        matches!(self.completion_location, Some(ImmediateLocation::RecordField))
     }
 
     pub(crate) fn has_impl_or_trait_prev_sibling(&self) -> bool {
@@ -320,12 +308,11 @@ impl<'a> CompletionContext<'a> {
             || self.record_pat_syntax.is_some()
             || self.attribute_under_caret.is_some()
             || self.mod_declaration_under_caret.is_some()
-            || self.has_impl_or_trait_parent()
     }
 
     fn fill_keyword_patterns(&mut self, file_with_fake_ident: &SyntaxNode, offset: TextSize) {
         let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap();
-        let syntax_element = NodeOrToken::Token(fake_ident_token);
+        let syntax_element = NodeOrToken::Token(fake_ident_token.clone());
         self.previous_token = previous_token(syntax_element.clone());
         self.in_loop_body = is_in_loop_body(syntax_element.clone());
         self.is_match_arm = is_match_arm(syntax_element.clone());
@@ -335,22 +322,6 @@ impl<'a> CompletionContext<'a> {
             self.prev_sibling = Some(PrevSibling::Trait)
         }
 
-        if has_block_expr_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::BlockExpr);
-        } else if has_bind_pat_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::IdentPat);
-        } else if has_ref_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::RefPatOrExpr);
-        } else if has_impl_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::Impl);
-        } else if has_field_list_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::RecordFieldList);
-        } else if has_trait_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::Trait);
-        } else if has_item_list_or_source_file_parent(syntax_element.clone()) {
-            self.completion_location = Some(ImmediateLocation::ItemList);
-        }
-
         self.mod_declaration_under_caret =
             find_node_at_offset::<ast::Module>(&file_with_fake_ident, offset)
                 .filter(|module| module.item_list().is_none());
@@ -363,6 +334,8 @@ impl<'a> CompletionContext<'a> {
         let fn_is_prev = self.previous_token_is(T![fn]);
         let for_is_prev2 = for_is_prev2(syntax_element.clone());
         self.no_completion_required = (fn_is_prev && !inside_impl_trait_block) || for_is_prev2;
+
+        self.completion_location = determine_location(fake_ident_token);
     }
 
     fn fill_impl_def(&mut self) {

crates/ide_completion/src/patterns.rs

@@ -11,28 +11,115 @@ use syntax::{
 #[cfg(test)]
 use crate::test_utils::{check_pattern_is_applicable, check_pattern_is_not_applicable};
 
-pub(crate) fn has_trait_parent(element: SyntaxElement) -> bool {
-    not_same_range_ancestor(element)
-        .filter(|it| it.kind() == ASSOC_ITEM_LIST)
-        .and_then(|it| it.parent())
-        .filter(|it| it.kind() == TRAIT)
-        .is_some()
-}
-#[test]
-fn test_has_trait_parent() {
-    check_pattern_is_applicable(r"trait A { f$0 }", has_trait_parent);
+/// Direct parent container of the cursor position
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
+pub(crate) enum ImmediateLocation {
+    Impl,
+    Trait,
+    RecordField,
+    RefExpr,
+    IdentPat,
+    BlockExpr,
+    ItemList,
 }
 
-pub(crate) fn has_impl_parent(element: SyntaxElement) -> bool {
-    not_same_range_ancestor(element)
-        .filter(|it| it.kind() == ASSOC_ITEM_LIST)
-        .and_then(|it| it.parent())
-        .filter(|it| it.kind() == IMPL)
-        .is_some()
+pub(crate) fn determine_location(tok: SyntaxToken) -> Option<ImmediateLocation> {
+    // First "expand" the element we are completing to its maximum so that we can check in what
+    // context it immediately lies. This for example means if the token is a NameRef at the end of
+    // a path, we want to look at where the path is in the tree.
+    let node = match tok.parent().and_then(ast::NameLike::cast)? {
+        ast::NameLike::NameRef(name_ref) => {
+            if let Some(segment) = name_ref.syntax().parent().and_then(ast::PathSegment::cast) {
+                let p = segment.parent_path();
+                if p.parent_path().is_none() {
+                    p.syntax()
+                        .ancestors()
+                        .take_while(|it| it.text_range() == p.syntax().text_range())
+                        .last()?
+                } else {
+                    return None;
+                }
+            } else {
+                return None;
+            }
+        }
+        it @ ast::NameLike::Name(_) | it @ ast::NameLike::Lifetime(_) => it.syntax().clone(),
+    };
+    let parent = match node.parent() {
+        Some(parent) => parent,
+        // SourceFile
+        None => {
+            return match node.kind() {
+                MACRO_ITEMS | SOURCE_FILE => Some(ImmediateLocation::ItemList),
+                _ => None,
+            }
+        }
+    };
+    let res = match_ast! {
+        match parent {
+            ast::IdentPat(_it) => ImmediateLocation::IdentPat,
+            ast::BlockExpr(_it) => ImmediateLocation::BlockExpr,
+            ast::SourceFile(_it) => ImmediateLocation::ItemList,
+            ast::ItemList(_it) => ImmediateLocation::ItemList,
+            ast::RefExpr(_it) => ImmediateLocation::RefExpr,
+            ast::RefPat(_it) => ImmediateLocation::RefExpr,
+            ast::RecordField(_it) => ImmediateLocation::RecordField,
+            ast::AssocItemList(it) => match it.syntax().parent().map(|it| it.kind()) {
+                Some(IMPL) => ImmediateLocation::Impl,
+                Some(TRAIT) => ImmediateLocation::Trait,
+                _ => return None,
+            },
+            _ => return None,
+        }
+    };
+    Some(res)
 }
+
+#[cfg(test)]
+fn check_location(code: &str, loc: ImmediateLocation) {
+    check_pattern_is_applicable(code, |e| {
+        assert_eq!(determine_location(e.into_token().expect("Expected a token")), Some(loc));
+        true
+    });
+}
+
+#[test]
+fn test_has_trait_parent() {
+    check_location(r"trait A { f$0 }", ImmediateLocation::Trait);
+}
+
 #[test]
 fn test_has_impl_parent() {
-    check_pattern_is_applicable(r"impl A { f$0 }", has_impl_parent);
+    check_location(r"impl A { f$0 }", ImmediateLocation::Impl);
+}
+#[test]
+fn test_has_field_list_parent() {
+    check_location(r"struct Foo { f$0 }", ImmediateLocation::RecordField);
+    check_location(r"struct Foo { f$0 pub f: i32}", ImmediateLocation::RecordField);
+}
+
+#[test]
+fn test_has_block_expr_parent() {
+    check_location(r"fn my_fn() { let a = 2; f$0 }", ImmediateLocation::BlockExpr);
+}
+
+#[test]
+fn test_has_ident_pat_parent() {
+    check_location(r"fn my_fn(m$0) {}", ImmediateLocation::IdentPat);
+    check_location(r"fn my_fn() { let m$0 }", ImmediateLocation::IdentPat);
+    check_location(r"fn my_fn(&m$0) {}", ImmediateLocation::IdentPat);
+    check_location(r"fn my_fn() { let &m$0 }", ImmediateLocation::IdentPat);
+}
+
+#[test]
+fn test_has_ref_expr_parent() {
+    check_location(r"fn my_fn() { let x = &m$0 foo; }", ImmediateLocation::RefExpr);
+}
+
+#[test]
+fn test_has_item_list_or_source_file_parent() {
+    check_location(r"i$0", ImmediateLocation::ItemList);
+    check_location(r"mod foo { f$0 }", ImmediateLocation::ItemList);
 }
 
 pub(crate) fn inside_impl_trait_block(element: SyntaxElement) -> bool {
@@ -53,56 +140,6 @@ fn test_inside_impl_trait_block() {
     check_pattern_is_not_applicable(r"impl A { fn f$0 }", inside_impl_trait_block);
 }
 
-pub(crate) fn has_field_list_parent(element: SyntaxElement) -> bool {
-    not_same_range_ancestor(element).filter(|it| it.kind() == RECORD_FIELD_LIST).is_some()
-}
-#[test]
-fn test_has_field_list_parent() {
-    check_pattern_is_applicable(r"struct Foo { f$0 }", has_field_list_parent);
-    check_pattern_is_applicable(r"struct Foo { f$0 pub f: i32}", has_field_list_parent);
-}
-
-pub(crate) fn has_block_expr_parent(element: SyntaxElement) -> bool {
-    not_same_range_ancestor(element).filter(|it| it.kind() == BLOCK_EXPR).is_some()
-}
-#[test]
-fn test_has_block_expr_parent() {
-    check_pattern_is_applicable(r"fn my_fn() { let a = 2; f$0 }", has_block_expr_parent);
-}
-
-pub(crate) fn has_bind_pat_parent(element: SyntaxElement) -> bool {
-    element.ancestors().any(|it| it.kind() == IDENT_PAT)
-}
-
-#[test]
-fn test_has_bind_pat_parent() {
-    check_pattern_is_applicable(r"fn my_fn(m$0) {}", has_bind_pat_parent);
-    check_pattern_is_applicable(r"fn my_fn() { let m$0 }", has_bind_pat_parent);
-}
-
-pub(crate) fn has_ref_parent(element: SyntaxElement) -> bool {
-    not_same_range_ancestor(element)
-        .filter(|it| it.kind() == REF_PAT || it.kind() == REF_EXPR)
-        .is_some()
-}
-#[test]
-fn test_has_ref_parent() {
-    check_pattern_is_applicable(r"fn my_fn(&m$0) {}", has_ref_parent);
-    check_pattern_is_applicable(r"fn my() { let &m$0 }", has_ref_parent);
-}
-
-pub(crate) fn has_item_list_or_source_file_parent(element: SyntaxElement) -> bool {
-    match not_same_range_ancestor(element) {
-        Some(it) => it.kind() == SOURCE_FILE || it.kind() == ITEM_LIST,
-        None => true,
-    }
-}
-#[test]
-fn test_has_item_list_or_source_file_parent() {
-    check_pattern_is_applicable(r"i$0", has_item_list_or_source_file_parent);
-    check_pattern_is_applicable(r"mod foo { f$0 }", has_item_list_or_source_file_parent);
-}
-
 pub(crate) fn is_match_arm(element: SyntaxElement) -> bool {
     not_same_range_ancestor(element.clone()).filter(|it| it.kind() == MATCH_ARM).is_some()
         && previous_sibling_or_ancestor_sibling(element)
@@ -160,12 +197,8 @@ pub(crate) fn is_in_loop_body(element: SyntaxElement) -> bool {
         .is_some()
 }
 
-fn not_same_range_ancestor(element: SyntaxElement) -> Option<SyntaxNode> {
-    element
-        .ancestors()
-        .take_while(|it| it.text_range() == element.text_range())
-        .last()
-        .and_then(|it| it.parent())
+pub(crate) fn not_same_range_ancestor(element: SyntaxElement) -> Option<SyntaxNode> {
+    element.ancestors().skip_while(|it| it.text_range() == element.text_range()).next()
 }
 
 fn previous_non_trivia_token(token: SyntaxToken) -> Option<SyntaxToken> {

crates/ide_completion/src/test_utils.rs

@@ -132,7 +132,7 @@ pub(crate) fn check_edit_with_config(
     assert_eq_text!(&ra_fixture_after, &actual)
 }
 
-pub(crate) fn check_pattern_is_applicable(code: &str, check: fn(SyntaxElement) -> bool) {
+pub(crate) fn check_pattern_is_applicable(code: &str, check: impl FnOnce(SyntaxElement) -> bool) {
     let (db, pos) = position(code);
 
     let sema = Semantics::new(&db);