Repository: rust-analyzer/rust-analyzer
Pull request #8128 (2021-03-21 00:46:01)

Expand legacy-scoped macro during collection
Fixes https://github.com/rust-analyzer/rust-analyzer/issues/8120




Patch:
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index e34be7e42f..30e5776712 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -2199,7 +2199,6 @@ pub enum ScopeDef {
     ImplSelfType(Impl),
     AdtSelfType(Adt),
     Local(Local),
-    Label(Label),
     Unknown,
 }
 
diff --git a/crates/hir/src/semantics.rs b/crates/hir/src/semantics.rs
index 1198e3f0be..15651bb222 100644
--- a/crates/hir/src/semantics.rs
+++ b/crates/hir/src/semantics.rs
@@ -839,10 +839,6 @@ impl<'a> SemanticsScope<'a> {
                     let parent = resolver.body_owner().unwrap();
                     ScopeDef::Local(Local { parent, pat_id })
                 }
-                resolver::ScopeDef::Label(label_id) => {
-                    let parent = resolver.body_owner().unwrap();
-                    ScopeDef::Label(Label { parent, label_id })
-                }
             };
             f(name, def)
         })
diff --git a/crates/hir_def/src/body/scope.rs b/crates/hir_def/src/body/scope.rs
index bd7005ca69..1bbb54fc6d 100644
--- a/crates/hir_def/src/body/scope.rs
+++ b/crates/hir_def/src/body/scope.rs
@@ -8,7 +8,7 @@ use rustc_hash::FxHashMap;
 use crate::{
     body::Body,
     db::DefDatabase,
-    expr::{Expr, ExprId, LabelId, Pat, PatId, Statement},
+    expr::{Expr, ExprId, Pat, PatId, Statement},
     BlockId, DefWithBodyId,
 };
 
@@ -40,7 +40,6 @@ impl ScopeEntry {
 pub struct ScopeData {
     parent: Option<ScopeId>,
     block: Option<BlockId>,
-    label: Option<(LabelId, Name)>,
     entries: Vec<ScopeEntry>,
 }
 
@@ -68,11 +67,6 @@ impl ExprScopes {
         self.scopes[scope].block
     }
 
-    /// If `scope` refers to a labeled expression scope, returns the corresponding `Label`.
-    pub fn label(&self, scope: ScopeId) -> Option<(LabelId, Name)> {
-        self.scopes[scope].label.clone()
-    }
-
     pub fn scope_chain(&self, scope: Option<ScopeId>) -> impl Iterator<Item = ScopeId> + '_ {
         std::iter::successors(scope, move |&scope| self.scopes[scope].parent)
     }
@@ -91,34 +85,15 @@ impl ExprScopes {
     }
 
     fn root_scope(&mut self) -> ScopeId {
-        self.scopes.alloc(ScopeData { parent: None, block: None, label: None, entries: vec![] })
+        self.scopes.alloc(ScopeData { parent: None, block: None, entries: vec![] })
     }
 
     fn new_scope(&mut self, parent: ScopeId) -> ScopeId {
-        self.scopes.alloc(ScopeData {
-            parent: Some(parent),
-            block: None,
-            label: None,
-            entries: vec![],
-        })
+        self.scopes.alloc(ScopeData { parent: Some(parent), block: None, entries: vec![] })
     }
 
-    fn new_labeled_scope(&mut self, parent: ScopeId, label: Option<(LabelId, Name)>) -> ScopeId {
-        self.scopes.alloc(ScopeData { parent: Some(parent), block: None, label, entries: vec![] })
-    }
-
-    fn new_block_scope(
-        &mut self,
-        parent: ScopeId,
-        block: BlockId,
-        label: Option<(LabelId, Name)>,
-    ) -> ScopeId {
-        self.scopes.alloc(ScopeData {
-            parent: Some(parent),
-            block: Some(block),
-            label,
-            entries: vec![],
-        })
+    fn new_block_scope(&mut self, parent: ScopeId, block: BlockId) -> ScopeId {
+        self.scopes.alloc(ScopeData { parent: Some(parent), block: Some(block), entries: vec![] })
     }
 
     fn add_bindings(&mut self, body: &Body, scope: ScopeId, pat: PatId) {
@@ -169,33 +144,21 @@ fn compute_block_scopes(
 }
 
 fn compute_expr_scopes(expr: ExprId, body: &Body, scopes: &mut ExprScopes, scope: ScopeId) {
-    let make_label =
-        |label: &Option<_>| label.map(|label| (label, body.labels[label].name.clone()));
-
     scopes.set_scope(expr, scope);
     match &body[expr] {
-        Expr::Block { statements, tail, id, label } => {
-            let scope = scopes.new_block_scope(scope, *id, make_label(label));
+        Expr::Block { statements, tail, id, .. } => {
+            let scope = scopes.new_block_scope(scope, *id);
             // Overwrite the old scope for the block expr, so that every block scope can be found
             // via the block itself (important for blocks that only contain items, no expressions).
             scopes.set_scope(expr, scope);
-            compute_block_scopes(statements, *tail, body, scopes, scope);
+            compute_block_scopes(&statements, *tail, body, scopes, scope);
         }
-        Expr::For { iterable, pat, body: body_expr, label } => {
+        Expr::For { iterable, pat, body: body_expr, .. } => {
             compute_expr_scopes(*iterable, body, scopes, scope);
-            let scope = scopes.new_labeled_scope(scope, make_label(label));
+            let scope = scopes.new_scope(scope);
             scopes.add_bindings(body, scope, *pat);
             compute_expr_scopes(*body_expr, body, scopes, scope);
         }
-        Expr::While { condition, body: body_expr, label } => {
-            let scope = scopes.new_labeled_scope(scope, make_label(label));
-            compute_expr_scopes(*condition, body, scopes, scope);
-            compute_expr_scopes(*body_expr, body, scopes, scope);
-        }
-        Expr::Loop { body: body_expr, label } => {
-            let scope = scopes.new_labeled_scope(scope, make_label(label));
-            compute_expr_scopes(*body_expr, body, scopes, scope);
-        }
         Expr::Lambda { args, body: body_expr, .. } => {
             let scope = scopes.new_scope(scope);
             scopes.add_params_bindings(body, scope, &args);
diff --git a/crates/hir_def/src/nameres/collector.rs b/crates/hir_def/src/nameres/collector.rs
index 46a3c60cdd..28b73c3a18 100644
--- a/crates/hir_def/src/nameres/collector.rs
+++ b/crates/hir_def/src/nameres/collector.rs
@@ -1467,12 +1467,13 @@ impl ModCollector<'_, '_> {
             },
         ) {
             Ok(Ok(macro_call_id)) => {
-                self.def_collector.unexpanded_macros.push(MacroDirective {
-                    module_id: self.module_id,
-                    ast_id,
-                    legacy: Some(macro_call_id),
-                    depth: self.macro_depth + 1,
-                });
+                // Legacy macros need to be expanded immediately, so that any macros they produce
+                // are in scope.
+                self.def_collector.collect_macro_expansion(
+                    self.module_id,
+                    macro_call_id,
+                    self.macro_depth + 1,
+                );
 
                 return;
             }
diff --git a/crates/hir_def/src/resolver.rs b/crates/hir_def/src/resolver.rs
index 4a2d1c0875..42736171eb 100644
--- a/crates/hir_def/src/resolver.rs
+++ b/crates/hir_def/src/resolver.rs
@@ -12,7 +12,7 @@ use crate::{
     body::scope::{ExprScopes, ScopeId},
     builtin_type::BuiltinType,
     db::DefDatabase,
-    expr::{ExprId, LabelId, PatId},
+    expr::{ExprId, PatId},
     generics::GenericParams,
     item_scope::{BuiltinShadowMode, BUILTIN_SCOPE},
     nameres::DefMap,
@@ -409,7 +409,6 @@ pub enum ScopeDef {
     AdtSelfType(AdtId),
     GenericParam(GenericParamId),
     Local(PatId),
-    Label(LabelId),
 }
 
 impl Scope {
@@ -471,9 +470,6 @@ impl Scope {
                 f(name![Self], ScopeDef::AdtSelfType(*i));
             }
             Scope::ExprScope(scope) => {
-                if let Some((label, name)) = scope.expr_scopes.label(scope.scope_id) {
-                    f(name.clone(), ScopeDef::Label(label))
-                }
                 scope.expr_scopes.entries(scope.scope_id).iter().for_each(|e| {
                     f(e.name().clone(), ScopeDef::Local(e.pat()));
                 });
diff --git a/crates/ide_completion/src/completions/lifetime.rs b/crates/ide_completion/src/completions/lifetime.rs
index 628c1fb9b6..74eb233602 100644
--- a/crates/ide_completion/src/completions/lifetime.rs
+++ b/crates/ide_completion/src/completions/lifetime.rs
@@ -1,4 +1,4 @@
-//! Completes lifetimes and labels.
+//! Completes lifetimes.
 use hir::ScopeDef;
 
 use crate::{completions::Completions, context::CompletionContext};
@@ -29,18 +29,6 @@ pub(crate) fn complete_lifetime(acc: &mut Completions, ctx: &CompletionContext)
     }
 }
 
-/// Completes labels.
-pub(crate) fn complete_label(acc: &mut Completions, ctx: &CompletionContext) {
-    if !ctx.is_label_ref {
-        return;
-    }
-    ctx.scope.process_all_names(&mut |name, res| {
-        if let ScopeDef::Label(_) = res {
-            acc.add_resolution(ctx, name.to_string(), &res);
-        }
-    });
-}
-
 #[cfg(test)]
 mod tests {
     use expect_test::{expect, Expect};
@@ -190,96 +178,4 @@ fn foo<'footime, 'lifetime: 'a$0>() {}
             "#]],
         );
     }
-
-    #[test]
-    fn complete_label_in_loop() {
-        check(
-            r#"
-fn foo() {
-    'foop: loop {
-        break '$0
-    }
-}
-"#,
-            expect![[r#"
-                lb 'foop
-            "#]],
-        );
-        check(
-            r#"
-fn foo() {
-    'foop: loop {
-        continue '$0
-    }
-}
-"#,
-            expect![[r#"
-                lb 'foop
-            "#]],
-        );
-    }
-
-    #[test]
-    fn complete_label_in_block_nested() {
-        check(
-            r#"
-fn foo() {
-    'foop: {
-        'baap: {
-            break '$0
-        }
-    }
-}
-"#,
-            expect![[r#"
-                lb 'baap
-                lb 'foop
-            "#]],
-        );
-    }
-
-    #[test]
-    fn complete_label_in_loop_with_value() {
-        check(
-            r#"
-fn foo() {
-    'foop: loop {
-        break '$0 i32;
-    }
-}
-"#,
-            expect![[r#"
-                lb 'foop
-            "#]],
-        );
-    }
-
-    #[test]
-    fn complete_label_in_while_cond() {
-        check(
-            r#"
-fn foo() {
-    'outer: while { 'inner: loop { break '$0 } } {}
-}
-"#,
-            expect![[r#"
-                lb 'inner
-                lb 'outer
-            "#]],
-        );
-    }
-
-    #[test]
-    fn complete_label_in_for_iterable() {
-        check(
-            r#"
-fn foo() {
-    'outer: for _ in [{ 'inner: loop { break '$0 } }] {}
-}
-"#,
-            expect![[r#"
-                lb 'inner
-            "#]],
-        );
-    }
 }
diff --git a/crates/ide_completion/src/context.rs b/crates/ide_completion/src/context.rs
index 67e2d6f6c2..4c2b31084d 100644
--- a/crates/ide_completion/src/context.rs
+++ b/crates/ide_completion/src/context.rs
@@ -53,7 +53,6 @@ pub(crate) struct CompletionContext<'a> {
     /// FIXME: `ActiveParameter` is string-based, which is very very wrong
     pub(super) active_parameter: Option<ActiveParameter>,
     pub(super) is_param: bool,
-    pub(super) is_label_ref: bool,
     /// If a name-binding or reference to a const in a pattern.
     /// Irrefutable patterns (like let) are excluded.
     pub(super) is_pat_binding_or_const: bool,
@@ -156,7 +155,6 @@ impl<'a> CompletionContext<'a> {
             record_field_syntax: None,
             impl_def: None,
             active_parameter: ActiveParameter::at(db, position),
-            is_label_ref: false,
             is_param: false,
             is_pat_binding_or_const: false,
             is_irrefutable_pat_binding: false,
@@ -470,24 +468,12 @@ impl<'a> CompletionContext<'a> {
     ) {
         self.lifetime_syntax =
             find_node_at_offset(original_file, lifetime.syntax().text_range().start());
-        if let Some(parent) = lifetime.syntax().parent() {
-            if parent.kind() == syntax::SyntaxKind::ERROR {
-                return;
-            }
-
-            match_ast! {
-                match parent {
-                    ast::LifetimeParam(_it) => {
-                        self.lifetime_allowed = true;
-                        self.lifetime_param_syntax =
-                            self.sema.find_node_at_offset_with_macros(original_file, offset);
-                    },
-                    ast::BreakExpr(_it) => self.is_label_ref = true,
-                    ast::ContinueExpr(_it) => self.is_label_ref = true,
-                    ast::Label(_it) => (),
-                    _ => self.lifetime_allowed = true,
-                }
-            }
+        if lifetime.syntax().parent().map_or(false, |p| p.kind() != syntax::SyntaxKind::ERROR) {
+            self.lifetime_allowed = true;
+        }
+        if let Some(_) = lifetime.syntax().parent().and_then(ast::LifetimeParam::cast) {
+            self.lifetime_param_syntax =
+                self.sema.find_node_at_offset_with_macros(original_file, offset);
         }
     }
 
diff --git a/crates/ide_completion/src/lib.rs b/crates/ide_completion/src/lib.rs
index 995970fcab..7a0eb6a966 100644
--- a/crates/ide_completion/src/lib.rs
+++ b/crates/ide_completion/src/lib.rs
@@ -131,7 +131,6 @@ pub fn completions(
     completions::mod_::complete_mod(&mut acc, &ctx);
     completions::flyimport::import_on_the_fly(&mut acc, &ctx);
     completions::lifetime::complete_lifetime(&mut acc, &ctx);
-    completions::lifetime::complete_label(&mut acc, &ctx);
 
     Some(acc)
 }
diff --git a/crates/ide_completion/src/render.rs b/crates/ide_completion/src/render.rs
index 23e00aa471..2b6e9ebd1f 100644
--- a/crates/ide_completion/src/render.rs
+++ b/crates/ide_completion/src/render.rs
@@ -219,7 +219,6 @@ impl<'a> Render<'a> {
                 hir::GenericParam::ConstParam(_) => SymbolKind::ConstParam,
             }),
             ScopeDef::Local(..) => CompletionItemKind::SymbolKind(SymbolKind::Local),
-            ScopeDef::Label(..) => CompletionItemKind::SymbolKind(SymbolKind::Label),
             ScopeDef::AdtSelfType(..) | ScopeDef::ImplSelfType(..) => {
                 CompletionItemKind::SymbolKind(SymbolKind::SelfParam)
             }


Test patch:
diff --git a/crates/hir_def/src/nameres/tests/macros.rs b/crates/hir_def/src/nameres/tests/macros.rs
index d59d3c0db1..6d3cb8d7af 100644
--- a/crates/hir_def/src/nameres/tests/macros.rs
+++ b/crates/hir_def/src/nameres/tests/macros.rs
@@ -712,6 +712,27 @@ b! { static = #[] ();}
     );
 }
 
+#[test]
+fn macros_defining_macros() {
+    check(
+        r#"
+macro_rules! item {
+    ($item:item) => { $item }
+}
+
+item! {
+    macro_rules! indirect_macro { () => { struct S {} } }
+}
+
+indirect_macro!();
+    "#,
+        expect![[r#"
+            crate
+            S: t
+        "#]],
+    );
+}
+
 #[test]
 fn resolves_proc_macros() {
     check(