Merge #9301

9301: internal: Start refactoring ide_completion tests r=Veykril a=Veykril

Our current completion test infra resovles around usually just checking a specific `CompletionKind` which is suboptimal. We only see what we want to see in tests with this causing us to miss a lot of incorrect completions we are doing. Instead we should test for different cursor locations for all kinds(sans the magic kind maybe? not sure yet). This way we will also see potential duplicate completions that merely different in their kind.

Also since most completion submodules complete things in tests of other modules due to the tests overlapping it makes more sense to group these tests differently which implies moving them to a new module. Exceptions for this might be stuff like attribute completion as these cannot currently interfere.

I only wrote a few tests to check for completions in `ItemList` position so far and I already found a few incorrect/irrelevant completions as these haven't been tested properly due to them being hidden by the `CompletionKind` filtering.

I think `CompletionKind` doesn't really seem to be beneficial to me as to I can't think of a occasion where we would want to only check a specific completion kind.


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

16 files changed, 60 insertions, 211 deletions

diff --git a/crates/ide_completion/src/completions/attribute.rs b/crates/ide_completion/src/completions/attribute.rs
index 6df569c2a..3866c5917 100644
--- a/crates/ide_completion/src/completions/attribute.rs
+++ b/crates/ide_completion/src/completions/attribute.rs
@@ -322,7 +322,7 @@ mod tests {
 
     use expect_test::{expect, Expect};
 
-    use crate::{test_utils::completion_list, CompletionKind};
+    use crate::{tests::filtered_completion_list, CompletionKind};
 
     #[test]
     fn attributes_are_sorted() {
@@ -341,7 +341,7 @@ mod tests {
     }
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Attribute);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Attribute);
         expect.assert_eq(&actual);
     }
 
diff --git a/crates/ide_completion/src/completions/attribute/derive.rs b/crates/ide_completion/src/completions/attribute/derive.rs
index 20bbbba46..5201095e8 100644
--- a/crates/ide_completion/src/completions/attribute/derive.rs
+++ b/crates/ide_completion/src/completions/attribute/derive.rs
@@ -82,7 +82,7 @@ const DEFAULT_DERIVE_COMPLETIONS: &[DeriveDependencies] = &[
 mod tests {
     use expect_test::{expect, Expect};
 
-    use crate::{test_utils::completion_list, CompletionKind};
+    use crate::{tests::filtered_completion_list, CompletionKind};
 
     fn check(ra_fixture: &str, expect: Expect) {
         let builtin_derives = r#"
@@ -106,7 +106,7 @@ pub macro PartialOrd {}
 pub macro Ord {}
 
 "#;
-        let actual = completion_list(
+        let actual = filtered_completion_list(
             &format!("{} {}", builtin_derives, ra_fixture),
             CompletionKind::Attribute,
         );
diff --git a/crates/ide_completion/src/completions/attribute/lint.rs b/crates/ide_completion/src/completions/attribute/lint.rs
index ca99e9759..4812b075c 100644
--- a/crates/ide_completion/src/completions/attribute/lint.rs
+++ b/crates/ide_completion/src/completions/attribute/lint.rs
@@ -34,7 +34,7 @@ pub(super) fn complete_lint(
 #[cfg(test)]
 mod tests {
 
-    use crate::test_utils::check_edit;
+    use crate::tests::check_edit;
 
     #[test]
     fn check_empty() {
diff --git a/crates/ide_completion/src/completions/dot.rs b/crates/ide_completion/src/completions/dot.rs
index 9552875c1..7f75d4298 100644
--- a/crates/ide_completion/src/completions/dot.rs
+++ b/crates/ide_completion/src/completions/dot.rs
@@ -101,10 +101,10 @@ fn complete_methods(
 mod tests {
     use expect_test::{expect, Expect};
 
-    use crate::{test_utils::completion_list, CompletionKind};
+    use crate::{tests::filtered_completion_list, CompletionKind};
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Reference);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Reference);
         expect.assert_eq(&actual);
     }
 
diff --git a/crates/ide_completion/src/completions/flyimport.rs b/crates/ide_completion/src/completions/flyimport.rs
index 30b8d44bd..4604feb5d 100644
--- a/crates/ide_completion/src/completions/flyimport.rs
+++ b/crates/ide_completion/src/completions/flyimport.rs
@@ -227,11 +227,11 @@ mod tests {
 
     use crate::{
         item::CompletionKind,
-        test_utils::{check_edit, check_edit_with_config, completion_list, TEST_CONFIG},
+        tests::{check_edit, check_edit_with_config, filtered_completion_list, TEST_CONFIG},
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Magic);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Magic);
         expect.assert_eq(&actual);
     }
 
diff --git a/crates/ide_completion/src/completions/fn_param.rs b/crates/ide_completion/src/completions/fn_param.rs
index cb90e8a3e..c9f0e2473 100644
--- a/crates/ide_completion/src/completions/fn_param.rs
+++ b/crates/ide_completion/src/completions/fn_param.rs
@@ -64,10 +64,10 @@ pub(crate) fn complete_fn_param(acc: &mut Completions, ctx: &CompletionContext)
 mod tests {
     use expect_test::{expect, Expect};
 
-    use crate::{test_utils::completion_list, CompletionKind};
+    use crate::{tests::filtered_completion_list, CompletionKind};
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Magic);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Magic);
         expect.assert_eq(&actual);
     }
 
diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs
index 0fccbeccf..73bbc4345 100644
--- a/crates/ide_completion/src/completions/keyword.rs
+++ b/crates/ide_completion/src/completions/keyword.rs
@@ -75,7 +75,9 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
         return;
     }
 
-    if expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_record_field() {
+    if !ctx.has_visibility_prev_sibling()
+        && (expects_item || ctx.expects_non_trait_assoc_item() || ctx.expect_record_field())
+    {
         add_keyword("pub(crate)", "pub(crate) ");
         add_keyword("pub", "pub ");
     }
@@ -88,11 +90,13 @@ pub(crate) fn complete_expr_keyword(acc: &mut Completions, ctx: &CompletionConte
     }
 
     if expects_item || has_block_expr_parent {
+        if !ctx.has_visibility_prev_sibling() {
+            add_keyword("impl", "impl $1 {\n    $0\n}");
+            add_keyword("extern", "extern $0");
+        }
         add_keyword("use", "use $0");
-        add_keyword("impl", "impl $1 {\n    $0\n}");
         add_keyword("trait", "trait $1 {\n    $0\n}");
         add_keyword("static", "static $0");
-        add_keyword("extern", "extern $0");
         add_keyword("mod", "mod $0");
     }
 
@@ -186,12 +190,12 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list},
+        tests::{check_edit, filtered_completion_list},
         CompletionKind,
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Keyword);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Keyword);
         expect.assert_eq(&actual)
     }
 
@@ -231,30 +235,6 @@ mod tests {
     }
 
     #[test]
-    fn test_keywords_at_source_file_level() {
-        check(
-            r"m$0",
-            expect![[r#"
-                kw pub(crate)
-                kw pub
-                kw unsafe
-                kw fn
-                kw const
-                kw type
-                kw use
-                kw impl
-                kw trait
-                kw static
-                kw extern
-                kw mod
-                kw enum
-                kw struct
-                kw union
-            "#]],
-        );
-    }
-
-    #[test]
     fn test_keywords_in_function() {
         check(
             r"fn quux() { $0 }",
@@ -263,11 +243,11 @@ mod tests {
                 kw fn
                 kw const
                 kw type
-                kw use
                 kw impl
+                kw extern
+                kw use
                 kw trait
                 kw static
-                kw extern
                 kw mod
                 kw match
                 kw while
@@ -291,11 +271,11 @@ mod tests {
                 kw fn
                 kw const
                 kw type
-                kw use
                 kw impl
+                kw extern
+                kw use
                 kw trait
                 kw static
-                kw extern
                 kw mod
                 kw match
                 kw while
@@ -319,11 +299,11 @@ mod tests {
                 kw fn
                 kw const
                 kw type
-                kw use
                 kw impl
+                kw extern
+                kw use
                 kw trait
                 kw static
-                kw extern
                 kw mod
                 kw match
                 kw while
@@ -370,49 +350,6 @@ fn quux() -> i32 {
     }
 
     #[test]
-    fn test_keywords_in_trait_def() {
-        check(
-            r"trait My { $0 }",
-            expect![[r#"
-                kw unsafe
-                kw fn
-                kw const
-                kw type
-            "#]],
-        );
-    }
-
-    #[test]
-    fn test_keywords_in_impl_def() {
-        check(
-            r"impl My { $0 }",
-            expect![[r#"
-                kw pub(crate)
-                kw pub
-                kw unsafe
-                kw fn
-                kw const
-                kw type
-            "#]],
-        );
-    }
-
-    #[test]
-    fn test_keywords_in_impl_def_with_attr() {
-        check(
-            r"impl My { #[foo] $0 }",
-            expect![[r#"
-                kw pub(crate)
-                kw pub
-                kw unsafe
-                kw fn
-                kw const
-                kw type
-            "#]],
-        );
-    }
-
-    #[test]
     fn test_keywords_in_loop() {
         check(
             r"fn my() { loop { $0 } }",
@@ -421,11 +358,11 @@ fn quux() -> i32 {
                 kw fn
                 kw const
                 kw type
-                kw use
                 kw impl
+                kw extern
+                kw use
                 kw trait
                 kw static
-                kw extern
                 kw mod
                 kw match
                 kw while
@@ -443,18 +380,6 @@ fn quux() -> i32 {
     }
 
     #[test]
-    fn test_keywords_after_unsafe_in_item_list() {
-        check(
-            r"unsafe $0",
-            expect![[r#"
-                kw fn
-                kw trait
-                kw impl
-            "#]],
-        );
-    }
-
-    #[test]
     fn test_keywords_after_unsafe_in_block_expr() {
         check(
             r"fn my_fn() { unsafe $0 }",
diff --git a/crates/ide_completion/src/completions/lifetime.rs b/crates/ide_completion/src/completions/lifetime.rs
index 8ccccb646..36f595164 100644
--- a/crates/ide_completion/src/completions/lifetime.rs
+++ b/crates/ide_completion/src/completions/lifetime.rs
@@ -50,7 +50,7 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list_with_config, TEST_CONFIG},
+        tests::{check_edit, filtered_completion_list_with_config, TEST_CONFIG},
         CompletionConfig, CompletionKind,
     };
 
@@ -59,7 +59,8 @@ mod tests {
     }
 
     fn check_with_config(config: CompletionConfig, ra_fixture: &str, expect: Expect) {
-        let actual = completion_list_with_config(config, ra_fixture, CompletionKind::Reference);
+        let actual =
+            filtered_completion_list_with_config(config, ra_fixture, CompletionKind::Reference);
         expect.assert_eq(&actual)
     }
 
diff --git a/crates/ide_completion/src/completions/mod_.rs b/crates/ide_completion/src/completions/mod_.rs
index 6a5746fb9..5def0d06a 100644
--- a/crates/ide_completion/src/completions/mod_.rs
+++ b/crates/ide_completion/src/completions/mod_.rs
@@ -141,11 +141,11 @@ fn module_chain_to_containing_module_file(
 
 #[cfg(test)]
 mod tests {
-    use crate::{test_utils::completion_list, CompletionKind};
+    use crate::{tests::filtered_completion_list, CompletionKind};
     use expect_test::{expect, Expect};
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Magic);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Magic);
         expect.assert_eq(&actual);
     }
 
diff --git a/crates/ide_completion/src/completions/pattern.rs b/crates/ide_completion/src/completions/pattern.rs
index 1daa8595a..efe3c957a 100644
--- a/crates/ide_completion/src/completions/pattern.rs
+++ b/crates/ide_completion/src/completions/pattern.rs
@@ -13,7 +13,7 @@ pub(crate) fn complete_pattern(acc: &mut Completions, ctx: &CompletionContext) {
         if let Some(hir::Adt::Enum(e)) =
             ctx.expected_type.as_ref().and_then(|ty| ty.strip_references().as_adt())
         {
-            super::complete_enum_variants(acc, ctx, e, |acc, ctx, variant, path| {
+            super::enum_variants_with_paths(acc, ctx, e, |acc, ctx, variant, path| {
                 acc.add_qualified_variant_pat(ctx, variant, path.clone());
                 acc.add_qualified_enum_variant(ctx, variant, path);
             });
@@ -61,17 +61,17 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list},
+        tests::{check_edit, filtered_completion_list},
         CompletionKind,
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Reference);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Reference);
         expect.assert_eq(&actual)
     }
 
     fn check_snippet(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Snippet);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Snippet);
         expect.assert_eq(&actual)
     }
 
diff --git a/crates/ide_completion/src/completions/postfix.rs b/crates/ide_completion/src/completions/postfix.rs
index 9f98b21be..c3c7e4589 100644
--- a/crates/ide_completion/src/completions/postfix.rs
+++ b/crates/ide_completion/src/completions/postfix.rs
@@ -307,12 +307,12 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list},
+        tests::{check_edit, filtered_completion_list},
         CompletionKind,
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Postfix);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Postfix);
         expect.assert_eq(&actual)
     }
 
diff --git a/crates/ide_completion/src/completions/qualified_path.rs b/crates/ide_completion/src/completions/qualified_path.rs
index 1643eeed4..9432caa22 100644
--- a/crates/ide_completion/src/completions/qualified_path.rs
+++ b/crates/ide_completion/src/completions/qualified_path.rs
@@ -198,17 +198,17 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list},
+        tests::{check_edit, filtered_completion_list},
         CompletionKind,
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Reference);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Reference);
         expect.assert_eq(&actual);
     }
 
     fn check_builtin(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::BuiltinType);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::BuiltinType);
         expect.assert_eq(&actual);
     }
 
@@ -714,24 +714,6 @@ impl MyStruct {
     }
 
     #[test]
-    fn completes_in_item_list() {
-        check(
-            r#"
-struct MyStruct {}
-#[macro_export]
-macro_rules! foo {}
-mod bar {}
-
-crate::$0
-"#,
-            expect![[r#"
-                md bar
-                ma foo!(…) #[macro_export] macro_rules! foo
-            "#]],
-        )
-    }
-
-    #[test]
     fn test_super_super_completion() {
         check(
             r#"
diff --git a/crates/ide_completion/src/completions/record.rs b/crates/ide_completion/src/completions/record.rs
index 0ac47cdbe..47523f72f 100644
--- a/crates/ide_completion/src/completions/record.rs
+++ b/crates/ide_completion/src/completions/record.rs
@@ -51,17 +51,17 @@ mod tests {
     use ide_db::helpers::FamousDefs;
 
     use crate::{
-        test_utils::{self, completion_list},
+        tests::{self, filtered_completion_list},
         CompletionKind,
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Reference);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Reference);
         expect.assert_eq(&actual);
     }
 
     fn check_snippet(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(
+        let actual = filtered_completion_list(
             &format!("//- /main.rs crate:main deps:core\n{}\n{}", ra_fixture, FamousDefs::FIXTURE),
             CompletionKind::Snippet,
         );
@@ -69,7 +69,7 @@ mod tests {
     }
 
     fn check_edit(what: &str, ra_fixture_before: &str, ra_fixture_after: &str) {
-        test_utils::check_edit(
+        tests::check_edit(
             what,
             &format!(
                 "//- /main.rs crate:main deps:core{}\n{}",
diff --git a/crates/ide_completion/src/completions/snippet.rs b/crates/ide_completion/src/completions/snippet.rs
index b9862de67..81ddfa34f 100644
--- a/crates/ide_completion/src/completions/snippet.rs
+++ b/crates/ide_completion/src/completions/snippet.rs
@@ -1,6 +1,7 @@
 //! This file provides snippet completions, like `pd` => `eprintln!(...)`.
 
 use ide_db::helpers::SnippetCap;
+use syntax::T;
 
 use crate::{
     context::PathCompletionContext, item::Builder, CompletionContext, CompletionItem,
@@ -35,9 +36,13 @@ pub(crate) fn complete_expr_snippet(acc: &mut Completions, ctx: &CompletionConte
 }
 
 pub(crate) fn complete_item_snippet(acc: &mut Completions, ctx: &CompletionContext) {
-    if !ctx.expects_item() {
+    if !ctx.expects_item() || ctx.previous_token_is(T![unsafe]) || ctx.path_qual().is_some() {
         return;
     }
+    if ctx.has_visibility_prev_sibling() {
+        return; // technically we could do some of these snippet completions if we were to put the
+                // attributes before the vis node.
+    }
     let cap = match ctx.config.snippet_cap {
         Some(it) => it,
         None => return,
@@ -82,10 +87,10 @@ fn ${1:feature}() {
 mod tests {
     use expect_test::{expect, Expect};
 
-    use crate::{test_utils::completion_list, CompletionKind};
+    use crate::{tests::filtered_completion_list, CompletionKind};
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Snippet);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Snippet);
         expect.assert_eq(&actual)
     }
 
@@ -105,21 +110,4 @@ mod tests {
         check(r#"fn foo(x: i32) { ::foo$0 }"#, expect![[""]]);
         check(r#"fn foo(x: i32) { ::$0 }"#, expect![[""]]);
     }
-
-    #[test]
-    fn completes_snippets_in_items() {
-        check(
-            r#"
-#[cfg(test)]
-mod tests {
-    $0
-}
-"#,
-            expect![[r#"
-                sn tmod (Test module)
-                sn tfn (Test function)
-                sn macro_rules
-            "#]],
-        )
-    }
 }
diff --git a/crates/ide_completion/src/completions/trait_impl.rs b/crates/ide_completion/src/completions/trait_impl.rs
index a60e5f43c..dc1d198cc 100644
--- a/crates/ide_completion/src/completions/trait_impl.rs
+++ b/crates/ide_completion/src/completions/trait_impl.rs
@@ -246,12 +246,12 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list},
+        tests::{check_edit, filtered_completion_list},
         CompletionKind,
     };
 
     fn check(ra_fixture: &str, expect: Expect) {
-        let actual = completion_list(ra_fixture, CompletionKind::Magic);
+        let actual = filtered_completion_list(ra_fixture, CompletionKind::Magic);
         expect.assert_eq(&actual)
     }
 
diff --git a/crates/ide_completion/src/completions/unqualified_path.rs b/crates/ide_completion/src/completions/unqualified_path.rs
index b5af1c810..2868d9b18 100644
--- a/crates/ide_completion/src/completions/unqualified_path.rs
+++ b/crates/ide_completion/src/completions/unqualified_path.rs
@@ -40,7 +40,7 @@ pub(crate) fn complete_unqualified_path(acc: &mut Completions, ctx: &CompletionC
         if let Some(hir::Adt::Enum(e)) =
             ctx.expected_type.as_ref().and_then(|ty| ty.strip_references().as_adt())
         {
-            super::complete_enum_variants(acc, ctx, e, |acc, ctx, variant, path| {
+            super::enum_variants_with_paths(acc, ctx, e, |acc, ctx, variant, path| {
                 acc.add_qualified_enum_variant(ctx, variant, path)
             });
         }
@@ -93,7 +93,7 @@ mod tests {
     use expect_test::{expect, Expect};
 
     use crate::{
-        test_utils::{check_edit, completion_list_with_config, TEST_CONFIG},
+        tests::{check_edit, filtered_completion_list_with_config, TEST_CONFIG},
         CompletionConfig, CompletionKind,
     };
 
@@ -102,7 +102,8 @@ mod tests {
     }
 
     fn check_with_config(config: CompletionConfig, ra_fixture: &str, expect: Expect) {
-        let actual = completion_list_with_config(config, ra_fixture, CompletionKind::Reference);
+        let actual =
+            filtered_completion_list_with_config(config, ra_fixture, CompletionKind::Reference);
         expect.assert_eq(&actual)
     }
 
@@ -500,18 +501,6 @@ fn f() {$0}
         check(
             r#"
 #[rustc_builtin_macro]
-pub macro Clone {}
-
-struct S;
-impl S {
-    $0
-}
-"#,
-            expect![[r#""#]],
-        );
-        check(
-            r#"
-#[rustc_builtin_macro]
 pub macro bench {}
 
 fn f() {$0}
@@ -773,42 +762,6 @@ impl My$0
     }
 
     #[test]
-    fn completes_in_assoc_item_list() {
-        check(
-            r#"
-macro_rules! foo {}
-mod bar {}
-
-struct MyStruct {}
-impl MyStruct {
-    $0
-}
-"#,
-            expect![[r#"
-                md bar
-                ma foo!(…) macro_rules! foo
-            "#]],
-        )
-    }
-
-    #[test]
-    fn completes_in_item_list() {
-        check(
-            r#"
-struct MyStruct {}
-macro_rules! foo {}
-mod bar {}
-
-$0
-"#,
-            expect![[r#"
-                md bar
-                ma foo!(…) macro_rules! foo
-            "#]],
-        )
-    }
-
-    #[test]
     fn completes_types_and_const_in_arg_list() {
         check(
             r#"