Merge #9170

9170: internal: Remove unnecessary `completion::macro_in_item_position` module r=Veykril a=Veykril

bors r+

Co-authored-by: Lukas Wirth <[email protected]>

7 files changed, 32 insertions, 90 deletions

diff --git a/crates/ide_completion/src/completions.rs b/crates/ide_completion/src/completions.rs
index ffdcdc930..7a4d71e91 100644
--- a/crates/ide_completion/src/completions.rs
+++ b/crates/ide_completion/src/completions.rs
@@ -6,7 +6,6 @@ pub(crate) mod flyimport;
 pub(crate) mod fn_param;
 pub(crate) mod keyword;
 pub(crate) mod lifetime;
-pub(crate) mod macro_in_item_position;
 pub(crate) mod mod_;
 pub(crate) mod pattern;
 pub(crate) mod postfix;
diff --git a/crates/ide_completion/src/completions/macro_in_item_position.rs b/crates/ide_completion/src/completions/macro_in_item_position.rs
deleted file mode 100644
index 781b96ff1..000000000
--- a/crates/ide_completion/src/completions/macro_in_item_position.rs
+++ /dev/null
@@ -1,48 +0,0 @@
-//! Completes macro invocations used in item position.
-
-use crate::{CompletionContext, Completions};
-
-// Ideally this should be removed and moved into `(un)qualified_path` respectively
-pub(crate) fn complete_macro_in_item_position(acc: &mut Completions, ctx: &CompletionContext) {
-    // Show only macros in top level.
-    if !ctx.expects_item() {
-        return;
-    }
-
-    ctx.scope.process_all_names(&mut |name, res| {
-        if let hir::ScopeDef::MacroDef(mac) = res {
-            acc.add_macro(ctx, Some(name.clone()), mac);
-        }
-        // FIXME: This should be done in qualified_path/unqualified_path instead?
-        if let hir::ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = res {
-            acc.add_resolution(ctx, name, &res);
-        }
-    })
-}
-
-#[cfg(test)]
-mod tests {
-    use expect_test::{expect, Expect};
-
-    use crate::{test_utils::completion_list, CompletionKind};
-
-    fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Reference);
-        expect.assert_eq(&actual)
-    }
-
-    #[test]
-    fn completes_macros_as_item() {
-        check(
-            r#"
-macro_rules! foo { () => {} }
-fn foo() {}
-
-$0
-"#,
-            expect![[r#"
-                ma foo!(…) macro_rules! foo
-            "#]],
-        )
-    }
-}
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index c072de7b5..d58745fb4 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -7,7 +7,7 @@ use syntax::AstNode;
 use crate::{CompletionContext, Completions};
 
 pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionContext) {
-    if ctx.is_path_disallowed() || ctx.expects_item() {
+    if ctx.is_path_disallowed() {
         return;
     }
     let path = match ctx.path_qual() {
@@ -20,7 +20,8 @@ pub(crate) fn complete_qualified_path(acc: &mut Completions, ctx: &CompletionCon
         None => return,
     };
     let context_module = ctx.scope.module();
-    if ctx.expects_assoc_item() {
+
+    if ctx.expects_item() || ctx.expects_assoc_item() {
         if let hir::PathResolution::Def(hir::ModuleDef::Module(module)) = resolution {
             let module_scope = module.scope(ctx.db, context_module);
             for (name, def) in module_scope {
@@ -631,17 +632,17 @@ impl MyStruct {
 "#,
             expect![[r##"
                 md bar
-                ma foo! #[macro_export] macro_rules! foo
+                ma foo!(…) #[macro_export] macro_rules! foo
             "##]],
         );
     }
 
     #[test]
-    #[ignore] // FIXME doesn't complete anything atm
     fn completes_in_item_list() {
         check(
             r#"
 struct MyStruct {}
+#[macro_export]
 macro_rules! foo {}
 mod bar {}
 
@@ -649,7 +650,7 @@ crate::$0
 "#,
             expect![[r#"
                 md bar
-                ma foo! macro_rules! foo
+                ma foo!(…) #[macro_export] macro_rules! foo
             "#]],
         )
     }
diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs
index f321ed52b..8b22933e0 100644
--- a/crates/ide_completion/src/completions/unqualified_path.rs
+++ b/crates/ide_completion/src/completions/unqualified_path.rs
@@ -5,26 +5,25 @@ use hir::ScopeDef;
 use crate::{CompletionContext, Completions};
 
 pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionContext) {
-    if !ctx.is_trivial_path() {
-        return;
-    }
-    if ctx.is_path_disallowed() || ctx.expects_item() {
+    if ctx.is_path_disallowed() || !ctx.is_trivial_path() {
         return;
     }
 
-    if 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.clone()), macro_def);
+    if ctx.expects_item() || ctx.expects_assoc_item() {
+        // only show macros in {Assoc}ItemList
+        ctx.scope.process_all_names(&mut |name, res| {
+            if let hir::ScopeDef::MacroDef(mac) = res {
+                acc.add_macro(ctx, Some(name.clone()), mac);
             }
-            if let ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = def {
-                acc.add_resolution(ctx, name, &def);
+            if let hir::ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = res {
+                acc.add_resolution(ctx, name, &res);
             }
         });
         return;
     }
 
     if ctx.expects_use_tree() {
+        // only show modules in a fresh UseTree
         cov_mark::hit!(only_completes_modules_in_import);
         ctx.scope.process_all_names(&mut |name, res| {
             if let ScopeDef::ModuleDef(hir::ModuleDef::Module(_)) = res {
@@ -693,12 +692,11 @@ impl MyStruct {
 "#,
             expect![[r#"
                 md bar
-                ma foo! macro_rules! foo
+                ma foo!(…) macro_rules! foo
             "#]],
         )
     }
 
-    // FIXME: The completions here currently come from `macro_in_item_position`, but they shouldn't
     #[test]
     fn completes_in_item_list() {
         check(
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index 20e033d31..7e4b14926 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -67,14 +67,13 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) krate: Option<hir::Crate>,
     pub(super) expected_name: Option<NameOrNameRef>,
     pub(super) expected_type: Option<Type>,
-    pub(super) name_ref_syntax: Option<ast::NameRef>,
-
-    pub(super) use_item_syntax: Option<ast::Use>,
 
     /// The parent function of the cursor position if it exists.
     pub(super) function_def: Option<ast::Fn>,
     /// The parent impl of the cursor position if it exists.
     pub(super) impl_def: Option<ast::Impl>,
+    pub(super) name_ref_syntax: Option<ast::NameRef>,
+    pub(super) use_item_syntax: Option<ast::Use>,
 
     // potentially set if we are completing a lifetime
     pub(super) lifetime_syntax: Option<ast::Lifetime>,
@@ -89,13 +88,12 @@ pub(crate) struct CompletionContext<'a> {
     pub(super) completion_location: Option<ImmediateLocation>,
     pub(super) prev_sibling: Option<ImmediatePrevSibling>,
     pub(super) attribute_under_caret: Option<ast::Attr>,
+    pub(super) previous_token: Option<SyntaxToken>,
 
     pub(super) path_context: Option<PathCompletionContext>,
-    /// FIXME: `ActiveParameter` is string-based, which is very very wrong
     pub(super) active_parameter: Option<ActiveParameter>,
     pub(super) locals: Vec<(String, Local)>,
 
-    pub(super) previous_token: Option<SyntaxToken>,
     pub(super) in_loop_body: bool,
     pub(super) incomplete_let: bool,
 
@@ -143,28 +141,28 @@ impl<'a> CompletionContext<'a> {
             original_token,
             token,
             krate,
-            lifetime_allowed: false,
             expected_name: None,
             expected_type: None,
+            function_def: None,
+            impl_def: None,
             name_ref_syntax: None,
+            use_item_syntax: None,
             lifetime_syntax: None,
             lifetime_param_syntax: None,
-            function_def: None,
-            use_item_syntax: None,
-            impl_def: None,
-            active_parameter: ActiveParameter::at(db, position),
+            lifetime_allowed: false,
             is_label_ref: false,
-            is_param: false,
             is_pat_or_const: None,
-            path_context: None,
-            previous_token: None,
-            in_loop_body: false,
+            is_param: false,
             completion_location: None,
             prev_sibling: None,
-            no_completion_required: false,
-            incomplete_let: false,
             attribute_under_caret: None,
+            previous_token: None,
+            path_context: None,
+            active_parameter: ActiveParameter::at(db, position),
             locals,
+            in_loop_body: false,
+            incomplete_let: false,
+            no_completion_required: false,
         };
 
         let mut original_file = original_file.syntax().clone();
@@ -563,10 +561,6 @@ impl<'a> CompletionContext<'a> {
         self.name_ref_syntax =
             find_node_at_offset(original_file, name_ref.syntax().text_range().start());
 
-        if matches!(self.completion_location, Some(ImmediateLocation::ItemList)) {
-            return;
-        }
-
         self.use_item_syntax =
             self.sema.token_ancestors_with_macros(self.token.clone()).find_map(ast::Use::cast);
 
@@ -597,7 +591,7 @@ impl<'a> CompletionContext<'a> {
                 path_ctx.call_kind = match_ast! {
                     match p {
                         ast::PathExpr(it) => it.syntax().parent().and_then(ast::CallExpr::cast).map(|_| CallKind::Expr),
-                        ast::MacroCall(_it) => Some(CallKind::Mac),
+                        ast::MacroCall(it) => it.excl_token().and(Some(CallKind::Mac)),
                         ast::TupleStructPat(_it) => Some(CallKind::Pat),
                         _ => None
                     }
diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs
index 6fb38f50d..18983aa01 100644
--- a/crates/ide_completion/src/lib.rs
+++ b/crates/ide_completion/src/lib.rs
@@ -158,7 +158,6 @@ pub fn completions(
     completions::record::complete_record(&mut acc, &ctx);
     completions::pattern::complete_pattern(&mut acc, &ctx);
     completions::postfix::complete_postfix(&mut acc, &ctx);
-    completions::macro_in_item_position::complete_macro_in_item_position(&mut acc, &ctx);
     completions::trait_impl::complete_trait_impl(&mut acc, &ctx);
     completions::mod_::complete_mod(&mut acc, &ctx);
     completions::flyimport::import_on_the_fly(&mut acc, &ctx);
diff --git a/crates/ide_db/src/call_info.rs b/crates/ide_db/src/call_info.rs
index bad277a95..933bcad55 100644
--- a/crates/ide_db/src/call_info.rs
+++ b/crates/ide_db/src/call_info.rs
@@ -223,9 +223,8 @@ impl FnCallNode {
                 ast::Expr::PathExpr(path_expr) => path_expr.path()?.segment()?.name_ref()?,
                 _ => return None,
             }),
-
             FnCallNode::MethodCallExpr(call_expr) => {
-                call_expr.syntax().children().filter_map(ast::NameRef::cast).next()
+                call_expr.syntax().children().find_map(ast::NameRef::cast)
             }
         }
     }