Don't complete non-macro item paths in impls and modules

6 files changed, 76 insertions, 29 deletions

diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs
index be9cfbded..df27e7a84 100644
--- a/crates/ide_completion/src/completions/flyimport.rs
+++ b/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 = {
diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs
index 58e35bad9..14d6ae54e 100644
--- a/crates/ide_completion/src/completions/keyword.rs
+++ b/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() {
         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 ");
     }
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index ed48f61af..a90325e06 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -21,6 +21,18 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
     };
     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| {
         acc.add_type_alias(ctx, alias);
diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs
index 046a393ae..cbac88240 100644
--- a/crates/ide_completion/src/completions/unqualified_path.rs
+++ b/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
+            "#]],
+        )
+    }
 }
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index 66577df94..b7e116dba 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -127,6 +127,7 @@ pub(crate) struct CompletionContext<'a> {
 
     no_completion_required: bool,
 }
+
 impl<'a> CompletionContext<'a> {
     pub(super) fn new(
         db: &'a RootDatabase,
@@ -281,21 +282,25 @@ 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 has_block_expr_parent(&self) -> bool {
-        matches!(self.completion_location, Some(ImmediateLocation::BlockExpr))
+    pub(crate) fn expects_non_trait_assoc_item(&self) -> bool {
+        matches!(self.completion_location, Some(ImmediateLocation::Impl))
     }
 
-    pub(crate) fn has_item_list_parent(&self) -> bool {
+    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_ident_or_ref_pat_parent(&self) -> bool {
         matches!(
             self.completion_location,
@@ -303,11 +308,7 @@ impl<'a> CompletionContext<'a> {
         )
     }
 
-    pub(crate) fn has_impl_parent(&self) -> bool {
-        matches!(self.completion_location, Some(ImmediateLocation::Impl))
-    }
-
-    pub(crate) fn has_field_list_parent(&self) -> bool {
+    pub(crate) fn expect_record_field(&self) -> bool {
         matches!(self.completion_location, Some(ImmediateLocation::RecordFieldList))
     }
 
@@ -320,10 +321,10 @@ 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) {
+        dbg!(file_with_fake_ident);
         let fake_ident_token = file_with_fake_ident.token_at_offset(offset).right_biased().unwrap();
         let syntax_element = NodeOrToken::Token(fake_ident_token);
         self.previous_token = previous_token(syntax_element.clone());
diff --git a/crates/ide_completion/src/patterns.rs b/crates/ide_completion/src/patterns.rs
index 04f2c532b..f7bf4d638 100644
--- a/crates/ide_completion/src/patterns.rs
+++ b/crates/ide_completion/src/patterns.rs
@@ -92,9 +92,15 @@ fn test_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,
+    let it = element
+        .ancestors()
+        .take_while(|it| it.text_range() == element.text_range())
+        .last()
+        .map(|it| (it.kind(), it.parent()));
+    match it {
+        Some((_, Some(it))) => it.kind() == SOURCE_FILE || it.kind() == ITEM_LIST,
+        Some((MACRO_ITEMS, None) | (SOURCE_FILE, None)) => true,
+        _ => false,
     }
 }
 #[test]