Report DefDiagnostics from inside item bodies

crates/hir/src/lib.rs

@@ -41,7 +41,7 @@ use hir_def::{
     body::{BodyDiagnostic, SyntheticSyntax},
     expr::{BindingAnnotation, LabelId, Pat, PatId},
     lang_item::LangItemTarget,
-    nameres,
+    nameres::{self, diagnostics::DefDiagnostic},
     per_ns::PerNs,
     resolver::{HasResolver, Resolver},
     AttrDefId, ConstId, ConstParamId, EnumId, FunctionId, GenericDefId, HasModule, LifetimeParamId,
@@ -523,6 +523,67 @@ impl Module {
                 // FIXME: This is accidentally quadratic.
                 continue;
             }
+            emit_def_diagnostic(db, acc, diag);
+        }
+        for decl in self.declarations(db) {
+            match decl {
+                ModuleDef::Module(m) => {
+                    // Only add diagnostics from inline modules
+                    if def_map[m.id.local_id].origin.is_inline() {
+                        m.diagnostics(db, acc)
+                    }
+                }
+                _ => acc.extend(decl.diagnostics(db)),
+            }
+        }
+
+        for impl_def in self.impl_defs(db) {
+            for item in impl_def.items(db) {
+                let def: DefWithBody = match item {
+                    AssocItem::Function(it) => it.into(),
+                    AssocItem::Const(it) => it.into(),
+                    AssocItem::TypeAlias(_) => continue,
+                };
+
+                def.diagnostics(db, acc);
+            }
+        }
+    }
+
+    pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> {
+        let def_map = self.id.def_map(db.upcast());
+        let scope = &def_map[self.id.local_id].scope;
+        scope
+            .declarations()
+            .map(ModuleDef::from)
+            .chain(scope.unnamed_consts().map(|id| ModuleDef::Const(Const::from(id))))
+            .collect()
+    }
+
+    pub fn impl_defs(self, db: &dyn HirDatabase) -> Vec<Impl> {
+        let def_map = self.id.def_map(db.upcast());
+        def_map[self.id.local_id].scope.impls().map(Impl::from).collect()
+    }
+
+    /// Finds a path that can be used to refer to the given item from within
+    /// this module, if possible.
+    pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
+        hir_def::find_path::find_path(db, item.into().into(), self.into())
+    }
+
+    /// Finds a path that can be used to refer to the given item from within
+    /// this module, if possible. This is used for returning import paths for use-statements.
+    pub fn find_use_path_prefixed(
+        self,
+        db: &dyn DefDatabase,
+        item: impl Into<ItemInNs>,
+        prefix_kind: PrefixKind,
+    ) -> Option<ModPath> {
+        hir_def::find_path::find_path_prefixed(db, item.into().into(), self.into(), prefix_kind)
+    }
+}
+
+fn emit_def_diagnostic(db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>, diag: &DefDiagnostic) {
     match &diag.kind {
         DefDiagnosticKind::UnresolvedModule { ast: declaration, candidate } => {
             let decl = declaration.to_node(db.upcast());
@@ -537,10 +598,7 @@ impl Module {
         DefDiagnosticKind::UnresolvedExternCrate { ast } => {
             let item = ast.to_node(db.upcast());
             acc.push(
-                        UnresolvedExternCrate {
+                UnresolvedExternCrate { decl: InFile::new(ast.file_id, AstPtr::new(&item)) }.into(),
-                            decl: InFile::new(ast.file_id, AstPtr::new(&item)),
-                        }
-                        .into(),
             );
         }
 
@@ -551,8 +609,7 @@ impl Module {
 
             let use_tree = import.use_tree_to_ast(db.upcast(), file_id, *index);
             acc.push(
-                        UnresolvedImport { decl: InFile::new(file_id, AstPtr::new(&use_tree)) }
+                UnresolvedImport { decl: InFile::new(file_id, AstPtr::new(&use_tree)) }.into(),
-                            .into(),
             );
         }
 
@@ -589,16 +646,12 @@ impl Module {
                         });
                     'outer: for attr in derive_attrs {
                         let tokens =
-                                    attr.syntax().children_with_tokens().filter_map(|elem| {
+                            attr.syntax().children_with_tokens().filter_map(|elem| match elem {
-                                        match elem {
                                 syntax::NodeOrToken::Node(_) => None,
                                 syntax::NodeOrToken::Token(tok) => Some(tok),
-                                        }
                             });
                         for token in tokens {
-                                    if token.kind() == SyntaxKind::IDENT
+                            if token.kind() == SyntaxKind::IDENT && token.text() == &**derive_name {
-                                        && token.text() == &**derive_name
-                                    {
                                 precise_location = Some(token.text_range());
                                 break 'outer;
                             }
@@ -616,9 +669,7 @@ impl Module {
                         .doc_comments_and_attrs()
                         .nth((*invoc_attr_index) as usize)
                         .and_then(Either::right)
-                                .unwrap_or_else(|| {
+                        .unwrap_or_else(|| panic!("cannot find attribute #{}", invoc_attr_index));
-                                    panic!("cannot find attribute #{}", invoc_attr_index)
-                                });
                     (
                         ast_id.with_value(SyntaxNodePtr::from(AstPtr::new(&attr))),
                         Some(attr_name.clone()),
@@ -626,11 +677,7 @@ impl Module {
                 }
             };
             acc.push(
-                        UnresolvedProcMacro {
+                UnresolvedProcMacro { node, precise_location, macro_name: name.map(Into::into) }
-                            node,
-                            precise_location,
-                            macro_name: name.map(Into::into),
-                        }
                     .into(),
             );
         }
@@ -709,63 +756,6 @@ impl Module {
         }
     }
 }
-        for decl in self.declarations(db) {
-            match decl {
-                ModuleDef::Module(m) => {
-                    // Only add diagnostics from inline modules
-                    if def_map[m.id.local_id].origin.is_inline() {
-                        m.diagnostics(db, acc)
-                    }
-                }
-                _ => acc.extend(decl.diagnostics(db)),
-            }
-        }
-
-        for impl_def in self.impl_defs(db) {
-            for item in impl_def.items(db) {
-                let def: DefWithBody = match item {
-                    AssocItem::Function(it) => it.into(),
-                    AssocItem::Const(it) => it.into(),
-                    AssocItem::TypeAlias(_) => continue,
-                };
-
-                def.diagnostics(db, acc);
-            }
-        }
-    }
-
-    pub fn declarations(self, db: &dyn HirDatabase) -> Vec<ModuleDef> {
-        let def_map = self.id.def_map(db.upcast());
-        let scope = &def_map[self.id.local_id].scope;
-        scope
-            .declarations()
-            .map(ModuleDef::from)
-            .chain(scope.unnamed_consts().map(|id| ModuleDef::Const(Const::from(id))))
-            .collect()
-    }
-
-    pub fn impl_defs(self, db: &dyn HirDatabase) -> Vec<Impl> {
-        let def_map = self.id.def_map(db.upcast());
-        def_map[self.id.local_id].scope.impls().map(Impl::from).collect()
-    }
-
-    /// Finds a path that can be used to refer to the given item from within
-    /// this module, if possible.
-    pub fn find_use_path(self, db: &dyn DefDatabase, item: impl Into<ItemInNs>) -> Option<ModPath> {
-        hir_def::find_path::find_path(db, item.into().into(), self.into())
-    }
-
-    /// Finds a path that can be used to refer to the given item from within
-    /// this module, if possible. This is used for returning import paths for use-statements.
-    pub fn find_use_path_prefixed(
-        self,
-        db: &dyn DefDatabase,
-        item: impl Into<ItemInNs>,
-        prefix_kind: PrefixKind,
-    ) -> Option<ModPath> {
-        hir_def::find_path::find_path_prefixed(db, item.into().into(), self.into(), prefix_kind)
-    }
-}
 
 impl HasVisibility for Module {
     fn visibility(&self, db: &dyn HirDatabase) -> Visibility {
@@ -1107,7 +1097,14 @@ impl DefWithBody {
     pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) {
         let krate = self.module(db).id.krate();
 
-        let source_map = db.body_with_source_map(self.into()).1;
+        let (body, source_map) = db.body_with_source_map(self.into());
+
+        for (_, def_map) in body.blocks(db.upcast()) {
+            for diag in def_map.diagnostics() {
+                emit_def_diagnostic(db, acc, diag);
+            }
+        }
+
         for diag in source_map.diagnostics() {
             match diag {
                 BodyDiagnostic::InactiveCode { node, cfg, opts } => acc.push(

crates/hir_def/src/body/lower.rs

@@ -552,7 +552,7 @@ impl ExprCollector<'_> {
             ast::Expr::MacroCall(e) => {
                 let macro_ptr = AstPtr::new(&e);
                 let mut ids = vec![];
-                self.collect_macro_call(e, macro_ptr, |this, expansion| {
+                self.collect_macro_call(e, macro_ptr, true, |this, expansion| {
                     ids.push(match expansion {
                         Some(it) => this.collect_expr(it),
                         None => this.alloc_expr(Expr::Missing, syntax_ptr.clone()),
@@ -576,6 +576,7 @@ impl ExprCollector<'_> {
         &mut self,
         e: ast::MacroCall,
         syntax_ptr: AstPtr<ast::MacroCall>,
+        record_diagnostics: bool,
         mut collector: F,
     ) {
         // File containing the macro call. Expansion errors will be attached here.
@@ -587,15 +588,18 @@ impl ExprCollector<'_> {
         let res = match res {
             Ok(res) => res,
             Err(UnresolvedMacro { path }) => {
+                if record_diagnostics {
                     self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedMacroCall {
                         node: InFile::new(outer_file, syntax_ptr),
                         path,
                     });
+                }
                 collector(self, None);
                 return;
             }
         };
 
+        if record_diagnostics {
             match &res.err {
                 Some(ExpandError::UnresolvedProcMacro) => {
                     self.source_map.diagnostics.push(BodyDiagnostic::UnresolvedProcMacro {
@@ -610,6 +614,7 @@ impl ExprCollector<'_> {
                 }
                 None => {}
             }
+        }
 
         match res.value {
             Some((mark, expansion)) => {
@@ -663,29 +668,33 @@ impl ExprCollector<'_> {
                     let macro_ptr = AstPtr::new(&m);
                     let syntax_ptr = AstPtr::new(&stmt.expr().unwrap());
 
-                    self.collect_macro_call(m, macro_ptr, |this, expansion| match expansion {
+                    self.collect_macro_call(
+                        m,
+                        macro_ptr,
+                        false,
+                        |this, expansion| match expansion {
                             Some(expansion) => {
                                 let statements: ast::MacroStmts = expansion;
 
                                 statements.statements().for_each(|stmt| this.collect_stmt(stmt));
                                 if let Some(expr) = statements.expr() {
                                     let expr = this.collect_expr(expr);
-                                this.statements_in_scope.push(Statement::Expr { expr, has_semi });
+                                    this.statements_in_scope
+                                        .push(Statement::Expr { expr, has_semi });
                                 }
                             }
                             None => {
                                 let expr = this.alloc_expr(Expr::Missing, syntax_ptr.clone());
                                 this.statements_in_scope.push(Statement::Expr { expr, has_semi });
                             }
-                    });
+                        },
+                    );
                 } else {
                     let expr = self.collect_expr_opt(stmt.expr());
                     self.statements_in_scope.push(Statement::Expr { expr, has_semi });
                 }
             }
-            ast::Stmt::Item(item) => {
+            ast::Stmt::Item(_item) => {}
-                self.check_cfg(&item);
-            }
         }
     }
 
@@ -878,7 +887,7 @@ impl ExprCollector<'_> {
                 Some(call) => {
                     let macro_ptr = AstPtr::new(&call);
                     let mut pat = None;
-                    self.collect_macro_call(call, macro_ptr, |this, expanded_pat| {
+                    self.collect_macro_call(call, macro_ptr, true, |this, expanded_pat| {
                         pat = Some(this.collect_pat_opt(expanded_pat));
                     });