Cleanup ImmediateLocation determination

Author: Veykril

crates/ide_completion/src/completions/keyword.rs

@@ -104,7 +104,7 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
     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 expects_item || expects_assoc_item || has_block_expr_parent {

crates/ide_completion/src/completions/qualified_path.rs

@@ -20,7 +20,6 @@ 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);
@@ -606,14 +605,33 @@ fn main() { T::$0; }
 macro_rules! foo { () => {} }
 
 fn main() { let _ = crate::$0 }
-        "#,
+"#,
             expect![[r##"
                 fn main()  fn()
                 ma foo!(…) #[macro_export] macro_rules! foo
             "##]],
         );
     }
 
+    #[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/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,
@@ -301,15 +288,15 @@ impl<'a> CompletionContext<'a> {
         matches!(self.completion_location, Some(ImmediateLocation::BlockExpr))
     }
 
-    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 expect_record_field(&self) -> bool {
-        matches!(self.completion_location, Some(ImmediateLocation::RecordFieldList))
+        matches!(self.completion_location, Some(ImmediateLocation::RecordField))
     }
 
     pub(crate) fn has_impl_or_trait_prev_sibling(&self) -> bool {
@@ -324,9 +311,8 @@ impl<'a> CompletionContext<'a> {
     }
 
     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);
+        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());
@@ -336,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());
@@ -364,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,102 +11,133 @@ 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 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)
 }
 
-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()
-}
-#[test]
-fn test_has_impl_parent() {
-    check_pattern_is_applicable(r"impl A { f$0 }", has_impl_parent);
+#[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
+    });
 }
 
-pub(crate) fn inside_impl_trait_block(element: SyntaxElement) -> bool {
-    // Here we search `impl` keyword up through the all ancestors, unlike in `has_impl_parent`,
-    // where we only check the first parent with different text range.
-    element
-        .ancestors()
-        .find(|it| it.kind() == IMPL)
-        .map(|it| ast::Impl::cast(it).unwrap())
-        .map(|it| it.trait_().is_some())
-        .unwrap_or(false)
-}
 #[test]
-fn test_inside_impl_trait_block() {
-    check_pattern_is_applicable(r"impl Foo for Bar { f$0 }", inside_impl_trait_block);
-    check_pattern_is_applicable(r"impl Foo for Bar { fn f$0 }", inside_impl_trait_block);
-    check_pattern_is_not_applicable(r"impl A { f$0 }", inside_impl_trait_block);
-    check_pattern_is_not_applicable(r"impl A { fn f$0 }", inside_impl_trait_block);
+fn test_has_trait_parent() {
+    check_location(r"trait A { f$0 }", ImmediateLocation::Trait);
 }
 
-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_impl_parent() {
+    check_location(r"impl A { f$0 }", ImmediateLocation::Impl);
 }
 #[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);
+    check_location(r"struct Foo { f$0 }", ImmediateLocation::RecordField);
+    check_location(r"struct Foo { f$0 pub f: i32}", ImmediateLocation::RecordField);
 }
 
-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);
+    check_location(r"fn my_fn() { let a = 2; f$0 }", ImmediateLocation::BlockExpr);
 }
 
-pub(crate) fn has_bind_pat_parent(element: SyntaxElement) -> bool {
-    element.ancestors().any(|it| it.kind() == IDENT_PAT)
+#[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_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);
+fn test_has_ref_expr_parent() {
+    check_location(r"fn my_fn() { let x = &m$0 foo; }", ImmediateLocation::RefExpr);
 }
 
-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);
+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 has_item_list_or_source_file_parent(element: SyntaxElement) -> bool {
-    let it = element
+pub(crate) fn inside_impl_trait_block(element: SyntaxElement) -> bool {
+    // Here we search `impl` keyword up through the all ancestors, unlike in `has_impl_parent`,
+    // where we only check the first parent with different text range.
+    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,
-    }
+        .find(|it| it.kind() == IMPL)
+        .map(|it| ast::Impl::cast(it).unwrap())
+        .map(|it| it.trait_().is_some())
+        .unwrap_or(false)
 }
 #[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);
+fn test_inside_impl_trait_block() {
+    check_pattern_is_applicable(r"impl Foo for Bar { f$0 }", inside_impl_trait_block);
+    check_pattern_is_applicable(r"impl Foo for Bar { fn f$0 }", inside_impl_trait_block);
+    check_pattern_is_not_applicable(r"impl A { f$0 }", inside_impl_trait_block);
+    check_pattern_is_not_applicable(r"impl A { fn f$0 }", inside_impl_trait_block);
 }
 
 pub(crate) fn is_match_arm(element: SyntaxElement) -> bool {
@@ -166,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);