Merge #11444

11444: feat: Fix up syntax errors in attribute macro inputs to make completion work more often r=flodiebold a=flodiebold

This implements the "fix up syntax nodes" workaround mentioned in #11014. It isn't much more than a proof of concept; I have only implemented a few cases, but it already helps quite a bit.

Some notes:
 - I'm not super happy about how much the fixup procedure needs to interact with the syntax node -> token tree conversion code (e.g. needing to share the token map). This could maybe be simplified with some refactoring of that code.
 - It would maybe be nice to have the fixup procedure reuse or share information with the parser, though I'm not really sure how much that would actually help.

Co-authored-by: Florian Diebold <[email protected]>

crates/base_db/src/fixture.rs

@@ -43,6 +43,17 @@ pub trait WithFixture: Default + SourceDatabaseExt + 'static {
         db
     }
 
+    fn with_files_extra_proc_macros(
+        ra_fixture: &str,
+        proc_macros: Vec<(String, ProcMacro)>,
+    ) -> Self {
+        let fixture = ChangeFixture::parse_with_proc_macros(ra_fixture, proc_macros);
+        let mut db = Self::default();
+        fixture.change.apply(&mut db);
+        assert!(fixture.file_position.is_none());
+        db
+    }
+
     fn with_position(ra_fixture: &str) -> (Self, FilePosition) {
         let (db, file_id, range_or_offset) = Self::with_range_or_offset(ra_fixture);
         let offset = range_or_offset.expect_offset();
@@ -84,7 +95,14 @@ pub struct ChangeFixture {
 
 impl ChangeFixture {
     pub fn parse(ra_fixture: &str) -> ChangeFixture {
-        let (mini_core, proc_macros, fixture) = Fixture::parse(ra_fixture);
+        Self::parse_with_proc_macros(ra_fixture, Vec::new())
+    }
+
+    pub fn parse_with_proc_macros(
+        ra_fixture: &str,
+        mut proc_macros: Vec<(String, ProcMacro)>,
+    ) -> ChangeFixture {
+        let (mini_core, proc_macro_names, fixture) = Fixture::parse(ra_fixture);
         let mut change = Change::new();
 
         let mut files = Vec::new();
@@ -222,11 +240,12 @@ impl ChangeFixture {
             }
         }
 
-        if !proc_macros.is_empty() {
+        if !proc_macro_names.is_empty() {
             let proc_lib_file = file_id;
             file_id.0 += 1;
 
-            let (proc_macro, source) = test_proc_macros(&proc_macros);
+            proc_macros.extend(default_test_proc_macros());
+            let (proc_macro, source) = filter_test_proc_macros(&proc_macro_names, proc_macros);
             let mut fs = FileSet::default();
             fs.insert(
                 proc_lib_file,
@@ -272,52 +291,84 @@ impl ChangeFixture {
     }
 }
 
-fn test_proc_macros(proc_macros: &[String]) -> (Vec<ProcMacro>, String) {
-    // The source here is only required so that paths to the macros exist and are resolvable.
-    let source = r#"
+fn default_test_proc_macros() -> [(String, ProcMacro); 4] {
+    [
+        (
+            r#"
 #[proc_macro_attribute]
 pub fn identity(_attr: TokenStream, item: TokenStream) -> TokenStream {
     item
 }
+"#
+            .into(),
+            ProcMacro {
+                name: "identity".into(),
+                kind: crate::ProcMacroKind::Attr,
+                expander: Arc::new(IdentityProcMacroExpander),
+            },
+        ),
+        (
+            r#"
 #[proc_macro_derive(DeriveIdentity)]
 pub fn derive_identity(item: TokenStream) -> TokenStream {
     item
 }
+"#
+            .into(),
+            ProcMacro {
+                name: "DeriveIdentity".into(),
+                kind: crate::ProcMacroKind::CustomDerive,
+                expander: Arc::new(IdentityProcMacroExpander),
+            },
+        ),
+        (
+            r#"
 #[proc_macro_attribute]
 pub fn input_replace(attr: TokenStream, _item: TokenStream) -> TokenStream {
     attr
 }
+"#
+            .into(),
+            ProcMacro {
+                name: "input_replace".into(),
+                kind: crate::ProcMacroKind::Attr,
+                expander: Arc::new(AttributeInputReplaceProcMacroExpander),
+            },
+        ),
+        (
+            r#"
 #[proc_macro]
 pub fn mirror(input: TokenStream) -> TokenStream {
     input
 }
-"#;
-    let proc_macros = [
-        ProcMacro {
-            name: "identity".into(),
-            kind: crate::ProcMacroKind::Attr,
-            expander: Arc::new(IdentityProcMacroExpander),
-        },
-        ProcMacro {
-            name: "DeriveIdentity".into(),
-            kind: crate::ProcMacroKind::CustomDerive,
-            expander: Arc::new(IdentityProcMacroExpander),
-        },
-        ProcMacro {
-            name: "input_replace".into(),
-            kind: crate::ProcMacroKind::Attr,
-            expander: Arc::new(AttributeInputReplaceProcMacroExpander),
-        },
-        ProcMacro {
-            name: "mirror".into(),
-            kind: crate::ProcMacroKind::FuncLike,
-            expander: Arc::new(MirrorProcMacroExpander),
-        },
+"#
+            .into(),
+            ProcMacro {
+                name: "mirror".into(),
+                kind: crate::ProcMacroKind::FuncLike,
+                expander: Arc::new(MirrorProcMacroExpander),
+            },
+        ),
     ]
-    .into_iter()
-    .filter(|pm| proc_macros.iter().any(|name| name == &stdx::to_lower_snake_case(&pm.name)))
-    .collect();
-    (proc_macros, source.into())
+}
+
+fn filter_test_proc_macros(
+    proc_macro_names: &[String],
+    proc_macro_defs: Vec<(String, ProcMacro)>,
+) -> (Vec<ProcMacro>, String) {
+    // The source here is only required so that paths to the macros exist and are resolvable.
+    let mut source = String::new();
+    let mut proc_macros = Vec::new();
+
+    for (c, p) in proc_macro_defs {
+        if !proc_macro_names.iter().any(|name| name == &stdx::to_lower_snake_case(&p.name)) {
+            continue;
+        }
+        proc_macros.push(p);
+        source += &c;
+    }
+
+    (proc_macros, source)
 }
 
 #[derive(Debug, Clone, Copy)]

crates/hir_def/src/macro_expansion_tests.rs

@@ -14,10 +14,10 @@ mod builtin_fn_macro;
 mod builtin_derive_macro;
 mod proc_macros;
 
-use std::{iter, ops::Range};
+use std::{iter, ops::Range, sync::Arc};
 
 use ::mbe::TokenMap;
-use base_db::{fixture::WithFixture, SourceDatabase};
+use base_db::{fixture::WithFixture, ProcMacro, SourceDatabase};
 use expect_test::Expect;
 use hir_expand::{
     db::{AstDatabase, TokenExpander},
@@ -39,7 +39,21 @@ use crate::{
 
 #[track_caller]
 fn check(ra_fixture: &str, mut expect: Expect) {
-    let db = TestDB::with_files(ra_fixture);
+    let extra_proc_macros = vec![(
+        r#"
+#[proc_macro_attribute]
+pub fn identity_when_valid(_attr: TokenStream, item: TokenStream) -> TokenStream {
+    item
+}
+"#
+        .into(),
+        ProcMacro {
+            name: "identity_when_valid".into(),
+            kind: base_db::ProcMacroKind::Attr,
+            expander: Arc::new(IdentityWhenValidProcMacroExpander),
+        },
+    )];
+    let db = TestDB::with_files_extra_proc_macros(ra_fixture, extra_proc_macros);
     let krate = db.crate_graph().iter().next().unwrap();
     let def_map = db.crate_def_map(krate);
     let local_id = def_map.root();
@@ -172,7 +186,7 @@ fn check(ra_fixture: &str, mut expect: Expect) {
         let range: Range<usize> = range.into();
 
         if show_token_ids {
-            if let Some((tree, map)) = arg.as_deref() {
+            if let Some((tree, map, _)) = arg.as_deref() {
                 let tt_range = call.token_tree().unwrap().syntax().text_range();
                 let mut ranges = Vec::new();
                 extract_id_ranges(&mut ranges, &map, &tree);
@@ -201,10 +215,19 @@ fn check(ra_fixture: &str, mut expect: Expect) {
     }
 
     for decl_id in def_map[local_id].scope.declarations() {
-        if let ModuleDefId::AdtId(AdtId::StructId(struct_id)) = decl_id {
-            let src = struct_id.lookup(&db).source(&db);
+        // FIXME: I'm sure there's already better way to do this
+        let src = match decl_id {
+            ModuleDefId::AdtId(AdtId::StructId(struct_id)) => {
+                Some(struct_id.lookup(&db).source(&db).syntax().cloned())
+            }
+            ModuleDefId::FunctionId(function_id) => {
+                Some(function_id.lookup(&db).source(&db).syntax().cloned())
+            }
+            _ => None,
+        };
+        if let Some(src) = src {
             if src.file_id.is_attr_macro(&db) || src.file_id.is_custom_derive(&db) {
-                let pp = pretty_print_macro_expansion(src.value.syntax().clone(), None);
+                let pp = pretty_print_macro_expansion(src.value, None);
                 format_to!(expanded_text, "\n{}", pp)
             }
         }
@@ -304,3 +327,25 @@ fn pretty_print_macro_expansion(expn: SyntaxNode, map: Option<&TokenMap>) -> Str
     }
     res
 }
+
+// Identity mapping, but only works when the input is syntactically valid. This
+// simulates common proc macros that unnecessarily parse their input and return
+// compile errors.
+#[derive(Debug)]
+struct IdentityWhenValidProcMacroExpander;
+impl base_db::ProcMacroExpander for IdentityWhenValidProcMacroExpander {
+    fn expand(
+        &self,
+        subtree: &Subtree,
+        _: Option<&Subtree>,
+        _: &base_db::Env,
+    ) -> Result<Subtree, base_db::ProcMacroExpansionError> {
+        let (parse, _) =
+            ::mbe::token_tree_to_syntax_node(subtree, ::mbe::TopEntryPoint::MacroItems);
+        if parse.errors().is_empty() {
+            Ok(subtree.clone())
+        } else {
+            panic!("got invalid macro input: {:?}", parse.errors());
+        }
+    }
+}

crates/hir_def/src/macro_expansion_tests/proc_macros.rs

@@ -52,3 +52,43 @@ struct S;
 #[attr2] struct S;"##]],
     );
 }
+
+#[test]
+fn attribute_macro_syntax_completion_1() {
+    // this is just the case where the input is actually valid
+    check(
+        r#"
+//- proc_macros: identity_when_valid
+#[proc_macros::identity_when_valid]
+fn foo() { bar.baz(); blub }
+"#,
+        expect![[r##"
+#[proc_macros::identity_when_valid]
+fn foo() { bar.baz(); blub }
+
+fn foo() {
+    bar.baz();
+    blub
+}"##]],
+    );
+}
+
+#[test]
+fn attribute_macro_syntax_completion_2() {
+    // common case of dot completion while typing
+    check(
+        r#"
+//- proc_macros: identity_when_valid
+#[proc_macros::identity_when_valid]
+fn foo() { bar.; blub }
+"#,
+        expect![[r##"
+#[proc_macros::identity_when_valid]
+fn foo() { bar.; blub }
+
+fn foo() {
+    bar. ;
+    blub
+}"##]],
+    );
+}

crates/hir_expand/Cargo.toml

@@ -27,3 +27,6 @@ profile = { path = "../profile", version = "0.0.0" }
 tt = { path = "../tt", version = "0.0.0" }
 mbe = { path = "../mbe", version = "0.0.0" }
 limit = { path = "../limit", version = "0.0.0" }
+
+[dev-dependencies]
+expect-test = "1.2.0-pre.1"