Repository: rust-analyzer/rust-analyzer (rust)

Issue:
Reserved keywords such as `fn` and `pub` are not suggested just after `#[...]`
<!--
Troubleshooting guide: https://rust-analyzer.github.io/manual.html#troubleshooting
Forum for questions: https://users.rust-lang.org/c/ide/14

Before submitting, please make sure that you're not running into one of these known issues:

 1. local imports (`use` statements) don't work: #1165
 2. local items don't work: #1559
 3. on-the-fly diagnostics are mostly unimplemented (`cargo check` diagnostics will be shown when saving a file)
 4. some settings are required for procedural macro and build script support (`rust-analyzer.cargo.loadOutDirsFromCheck`, `rust-analyzer.procMacro.enable`): #6448
 5. some platform-specific imports are not resolved: #6038
 6. the official `rust-lang.rust` VS Code extension conflicts with `rust-analyzer`: #6463

Otherwise please try to provide information which will help us to fix the issue faster. Minimal reproducible examples with few dependencies are especially lovely <3.
-->

Initial state:

```
#[test]
f
```

Intended state:

```
#[test]
fn
```

Expected suggestion:

```
#[test]
f⌶
 +----------------------+
 |== fn                 |
 |品 f32                |
 |品 f64                |
 |🧊 file!              |
 |🧊 format!            |
 |🧊 format_args!       |
 |🧊 format_args_nl!    |
 +----------------------+
```

Current sugestion:

```
#[test]
f⌶
 +----------------------+
 |品 f32                |
 |品 f64                |
 |🧊 file!              |
 |🧊 format!            |
 |🧊 format_args!       |
 |🧊 format_args_nl!    |
 |品  File              |
 +----------------------+
```

Reserved keywords such as `fn` and `pub` are not suggested after `#[...]` (e.g. `#[test]` and `#[cfg(...)]`).
It is a little annoying that `fn` is not suggested immediately after `#[...]` when I am writing tests, for  example, so it would be nice if this could be improved.
I have installed this extension today.

VSCode: 1.53.1


Pull request description:
Complete keywords in (Assoc)ItemList with leading attribute
Fixes #7673
bors r+

Patch:
diff --git a/crates/ide_completion/src/completions/keyword.rs b/crates/ide_completion/src/completions/keyword.rs
index 96447a603c..c9673df85d 100644
--- a/crates/ide_completion/src/completions/keyword.rs
+++ b/crates/ide_completion/src/completions/keyword.rs
@@ -394,6 +394,21 @@ fn quux() -> i32 {
         );
     }
 
+    #[test]
+    fn test_keywords_in_impl_def_with_attr() {
+        check(
+            r"impl My { #[foo] $0 }",
+            expect![[r#"
+                kw fn
+                kw const
+                kw type
+                kw unsafe
+                kw pub(crate)
+                kw pub
+            "#]],
+        );
+    }
+
     #[test]
     fn test_keywords_in_loop() {
         check(
diff --git a/crates/ide_completion/src/patterns.rs b/crates/ide_completion/src/patterns.rs
index 7bae7d12c3..c8a88367d7 100644
--- a/crates/ide_completion/src/patterns.rs
+++ b/crates/ide_completion/src/patterns.rs
@@ -25,9 +25,10 @@ pub(crate) enum ImmediateLocation {
 }
 
 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.
+    // First walk the element we are completing up to its highest node that has the same text range
+    // as the element so that we can check in what context it immediately lies. We only do this for
+    // NameRef -> Path as that's the only thing that makes sense to being "expanded" semantically.
+    // We only wanna do this if the NameRef is the last segment of the path.
     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) {
@@ -47,7 +48,20 @@ pub(crate) fn determine_location(tok: SyntaxToken) -> Option<ImmediateLocation>
         it @ ast::NameLike::Name(_) | it @ ast::NameLike::Lifetime(_) => it.syntax().clone(),
     };
     let parent = match node.parent() {
-        Some(parent) => parent,
+        Some(parent) => match ast::MacroCall::cast(parent.clone()) {
+            // When a path is being typed in an (Assoc)ItemList the parser will always emit a macro_call.
+            // This is usually fine as the node expansion code above already accounts for that with
+            // the ancestors call, but there is one exception to this which is that when an attribute
+            // precedes it the code above will not walk the Path to the parent MacroCall as their ranges differ.
+            Some(call)
+                if call.excl_token().is_none()
+                    && call.token_tree().is_none()
+                    && call.semicolon_token().is_none() =>
+            {
+                call.syntax().parent()?
+            }
+            _ => parent,
+        },
         // SourceFile
         None => {
             return match node.kind() {


Test patch:
diff --git a/crates/hir_def/src/nameres/tests/incremental.rs b/crates/hir_def/src/nameres/tests/incremental.rs
index 7bf152e26c..d884a6eb49 100644
--- a/crates/hir_def/src/nameres/tests/incremental.rs
+++ b/crates/hir_def/src/nameres/tests/incremental.rs
@@ -1,8 +1,6 @@
 use std::sync::Arc;
 
-use base_db::{salsa::SweepStrategy, SourceDatabaseExt};
-
-use crate::{AdtId, ModuleDefId};
+use base_db::SourceDatabaseExt;
 
 use super::*;
 
@@ -165,73 +163,3 @@ m!(Z);
         assert_eq!(n_reparsed_macros, 0);
     }
 }
-
-#[test]
-fn item_tree_prevents_reparsing() {
-    // The `ItemTree` is used by both name resolution and the various queries in `adt.rs` and
-    // `data.rs`. After computing the `ItemTree` and deleting the parse tree, we should be able to
-    // run those other queries without triggering a reparse.
-
-    let (db, pos) = TestDB::with_position(
-        r#"
-pub struct S;
-pub union U {}
-pub enum E {
-    Variant,
-}
-pub fn f(_: S) { $0 }
-pub trait Tr {}
-impl Tr for () {}
-pub const C: u8 = 0;
-pub static ST: u8 = 0;
-pub type Ty = ();
-"#,
-    );
-    let krate = db.test_crate();
-    {
-        let events = db.log_executed(|| {
-            db.file_item_tree(pos.file_id.into());
-        });
-        let n_calculated_item_trees = events.iter().filter(|it| it.contains("item_tree")).count();
-        assert_eq!(n_calculated_item_trees, 1);
-        let n_parsed_files = events.iter().filter(|it| it.contains("parse(")).count();
-        assert_eq!(n_parsed_files, 1);
-    }
-
-    // Delete the parse tree.
-    let sweep = SweepStrategy::default().discard_values().sweep_all_revisions();
-    base_db::ParseQuery.in_db(&db).sweep(sweep);
-
-    {
-        let events = db.log_executed(|| {
-            let crate_def_map = db.crate_def_map(krate);
-            let (_, module_data) = crate_def_map.modules.iter().last().unwrap();
-            assert_eq!(module_data.scope.resolutions().count(), 8);
-            assert_eq!(module_data.scope.impls().count(), 1);
-
-            for imp in module_data.scope.impls() {
-                db.impl_data(imp);
-            }
-
-            for (_, res) in module_data.scope.resolutions() {
-                match res.values.or(res.types).unwrap().0 {
-                    ModuleDefId::FunctionId(f) => drop(db.function_data(f)),
-                    ModuleDefId::AdtId(adt) => match adt {
-                        AdtId::StructId(it) => drop(db.struct_data(it)),
-                        AdtId::UnionId(it) => drop(db.union_data(it)),
-                        AdtId::EnumId(it) => drop(db.enum_data(it)),
-                    },
-                    ModuleDefId::ConstId(it) => drop(db.const_data(it)),
-                    ModuleDefId::StaticId(it) => drop(db.static_data(it)),
-                    ModuleDefId::TraitId(it) => drop(db.trait_data(it)),
-                    ModuleDefId::TypeAliasId(it) => drop(db.type_alias_data(it)),
-                    ModuleDefId::EnumVariantId(_)
-                    | ModuleDefId::ModuleId(_)
-                    | ModuleDefId::BuiltinType(_) => unreachable!(),
-                }
-            }
-        });
-        let n_reparsed_files = events.iter().filter(|it| it.contains("parse(")).count();
-        assert_eq!(n_reparsed_files, 0);
-    }
-}
diff --git a/crates/hir_ty/src/tests.rs b/crates/hir_ty/src/tests.rs
index 9d726b0248..cc819373c6 100644
--- a/crates/hir_ty/src/tests.rs
+++ b/crates/hir_ty/src/tests.rs
@@ -7,7 +7,6 @@ mod traits;
 mod method_resolution;
 mod macros;
 mod display_source_code;
-mod incremental;
 
 use std::{env, sync::Arc};
 
@@ -318,6 +317,50 @@ fn ellipsize(mut text: String, max_len: usize) -> String {
     text
 }
 
+#[test]
+fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
+    let (mut db, pos) = TestDB::with_position(
+        "
+        //- /lib.rs
+        fn foo() -> i32 {
+            $01 + 1
+        }
+    ",
+    );
+    {
+        let events = db.log_executed(|| {
+            let module = db.module_for_file(pos.file_id);
+            let crate_def_map = module.def_map(&db);
+            visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
+                db.infer(def);
+            });
+        });
+        assert!(format!("{:?}", events).contains("infer"))
+    }
+
+    let new_text = "
+        fn foo() -> i32 {
+            1
+            +
+            1
+        }
+    "
+    .to_string();
+
+    db.set_file_text(pos.file_id, Arc::new(new_text));
+
+    {
+        let events = db.log_executed(|| {
+            let module = db.module_for_file(pos.file_id);
+            let crate_def_map = module.def_map(&db);
+            visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
+                db.infer(def);
+            });
+        });
+        assert!(!format!("{:?}", events).contains("infer"), "{:#?}", events)
+    }
+}
+
 fn check_infer(ra_fixture: &str, expect: Expect) {
     let mut actual = infer(ra_fixture);
     actual.push('\n');
diff --git a/crates/hir_ty/src/tests/incremental.rs b/crates/hir_ty/src/tests/incremental.rs
deleted file mode 100644
index 3e08e83e89..0000000000
--- a/crates/hir_ty/src/tests/incremental.rs
+++ /dev/null
@@ -1,51 +0,0 @@
-use std::sync::Arc;
-
-use base_db::{fixture::WithFixture, SourceDatabaseExt};
-
-use crate::{db::HirDatabase, test_db::TestDB};
-
-use super::visit_module;
-
-#[test]
-fn typing_whitespace_inside_a_function_should_not_invalidate_types() {
-    let (mut db, pos) = TestDB::with_position(
-        "
-        //- /lib.rs
-        fn foo() -> i32 {
-            $01 + 1
-        }
-    ",
-    );
-    {
-        let events = db.log_executed(|| {
-            let module = db.module_for_file(pos.file_id);
-            let crate_def_map = module.def_map(&db);
-            visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
-                db.infer(def);
-            });
-        });
-        assert!(format!("{:?}", events).contains("infer"))
-    }
-
-    let new_text = "
-        fn foo() -> i32 {
-            1
-            +
-            1
-        }
-    "
-    .to_string();
-
-    db.set_file_text(pos.file_id, Arc::new(new_text));
-
-    {
-        let events = db.log_executed(|| {
-            let module = db.module_for_file(pos.file_id);
-            let crate_def_map = module.def_map(&db);
-            visit_module(&db, &crate_def_map, module.local_id, &mut |def| {
-                db.infer(def);
-            });
-        });
-        assert!(!format!("{:?}", events).contains("infer"), "{:#?}", events)
-    }
-}