Collect diagnostics in queries instead of nameres

5 files changed, 194 insertions, 116 deletions

diff --git a/crates/hir-def/src/adt.rs b/crates/hir-def/src/adt.rs
index 14f8629056f..af8ca8571ba 100644
--- a/crates/hir-def/src/adt.rs
+++ b/crates/hir-def/src/adt.rs
@@ -6,7 +6,7 @@ use base_db::CrateId;
 use either::Either;
 use hir_expand::{
     name::{AsName, Name},
-    InFile,
+    HirFileId, InFile,
 };
 use la_arena::{Arena, ArenaMap};
 use syntax::ast::{self, HasName, HasVisibility};
@@ -17,13 +17,15 @@ use crate::{
     builtin_type::{BuiltinInt, BuiltinUint},
     db::DefDatabase,
     intern::Interned,
-    item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem, RawVisibilityId},
+    item_tree::{AttrOwner, Field, FieldAstId, Fields, ItemTree, ModItem, RawVisibilityId},
+    nameres::diagnostics::DefDiagnostic,
     src::HasChildSource,
     src::HasSource,
     trace::Trace,
     type_ref::TypeRef,
     visibility::RawVisibility,
-    EnumId, LocalEnumVariantId, LocalFieldId, Lookup, ModuleId, StructId, UnionId, VariantId,
+    EnumId, LocalEnumVariantId, LocalFieldId, LocalModuleId, Lookup, ModuleId, StructId, UnionId,
+    VariantId,
 };
 use cfg::CfgOptions;
 
@@ -143,6 +145,13 @@ fn parse_repr_tt(tt: &Subtree) -> Option<ReprData> {
 
 impl StructData {
     pub(crate) fn struct_data_query(db: &dyn DefDatabase, id: StructId) -> Arc<StructData> {
+        db.struct_data_with_diagnostics(id).0
+    }
+
+    pub(crate) fn struct_data_with_diagnostics_query(
+        db: &dyn DefDatabase,
+        id: StructId,
+    ) -> (Arc<StructData>, Arc<Vec<DefDiagnostic>>) {
         let loc = id.lookup(db);
         let krate = loc.container.krate;
         let item_tree = loc.id.item_tree(db);
@@ -150,16 +159,35 @@ impl StructData {
         let cfg_options = db.crate_graph()[loc.container.krate].cfg_options.clone();
 
         let strukt = &item_tree[loc.id.value];
-        let variant_data = lower_fields(db, krate, &item_tree, &cfg_options, &strukt.fields, None);
-        Arc::new(StructData {
-            name: strukt.name.clone(),
-            variant_data: Arc::new(variant_data),
-            repr,
-            visibility: item_tree[strukt.visibility].clone(),
-        })
+        let (variant_data, diagnostics) = lower_fields(
+            db,
+            krate,
+            loc.id.file_id(),
+            loc.container.local_id,
+            &item_tree,
+            &cfg_options,
+            &strukt.fields,
+            None,
+        );
+        (
+            Arc::new(StructData {
+                name: strukt.name.clone(),
+                variant_data: Arc::new(variant_data),
+                repr,
+                visibility: item_tree[strukt.visibility].clone(),
+            }),
+            Arc::new(diagnostics),
+        )
     }
 
     pub(crate) fn union_data_query(db: &dyn DefDatabase, id: UnionId) -> Arc<StructData> {
+        db.union_data_with_diagnostics(id).0
+    }
+
+    pub(crate) fn union_data_with_diagnostics_query(
+        db: &dyn DefDatabase,
+        id: UnionId,
+    ) -> (Arc<StructData>, Arc<Vec<DefDiagnostic>>) {
         let loc = id.lookup(db);
         let krate = loc.container.krate;
         let item_tree = loc.id.item_tree(db);
@@ -167,19 +195,37 @@ impl StructData {
         let cfg_options = db.crate_graph()[loc.container.krate].cfg_options.clone();
 
         let union = &item_tree[loc.id.value];
-        let variant_data = lower_fields(db, krate, &item_tree, &cfg_options, &union.fields, None);
-
-        Arc::new(StructData {
-            name: union.name.clone(),
-            variant_data: Arc::new(variant_data),
-            repr,
-            visibility: item_tree[union.visibility].clone(),
-        })
+        let (variant_data, diagnostics) = lower_fields(
+            db,
+            krate,
+            loc.id.file_id(),
+            loc.container.local_id,
+            &item_tree,
+            &cfg_options,
+            &union.fields,
+            None,
+        );
+        (
+            Arc::new(StructData {
+                name: union.name.clone(),
+                variant_data: Arc::new(variant_data),
+                repr,
+                visibility: item_tree[union.visibility].clone(),
+            }),
+            Arc::new(diagnostics),
+        )
     }
 }
 
 impl EnumData {
     pub(crate) fn enum_data_query(db: &dyn DefDatabase, e: EnumId) -> Arc<EnumData> {
+        db.enum_data_with_diagnostics(e).0
+    }
+
+    pub(crate) fn enum_data_with_diagnostics_query(
+        db: &dyn DefDatabase,
+        e: EnumId,
+    ) -> (Arc<EnumData>, Arc<Vec<DefDiagnostic>>) {
         let loc = e.lookup(db);
         let krate = loc.container.krate;
         let item_tree = loc.id.item_tree(db);
@@ -188,31 +234,46 @@ impl EnumData {
 
         let enum_ = &item_tree[loc.id.value];
         let mut variants = Arena::new();
+        let mut diagnostics = Vec::new();
         for tree_id in enum_.variants.clone() {
-            if item_tree.attrs(db, krate, tree_id.into()).is_cfg_enabled(&cfg_options) {
-                let var = &item_tree[tree_id];
-                let var_data = lower_fields(
+            let attrs = item_tree.attrs(db, krate, tree_id.into());
+            let var = &item_tree[tree_id];
+            if attrs.is_cfg_enabled(&cfg_options) {
+                let (var_data, field_diagnostics) = lower_fields(
                     db,
                     krate,
+                    loc.id.file_id(),
+                    loc.container.local_id,
                     &item_tree,
                     &cfg_options,
                     &var.fields,
                     Some(enum_.visibility),
                 );
+                diagnostics.extend(field_diagnostics);
 
                 variants.alloc(EnumVariantData {
                     name: var.name.clone(),
                     variant_data: Arc::new(var_data),
                 });
+            } else {
+                diagnostics.push(DefDiagnostic::unconfigured_code(
+                    loc.container.local_id,
+                    InFile::new(loc.id.file_id(), var.ast_id.upcast()),
+                    attrs.cfg().unwrap(),
+                    cfg_options.clone(),
+                ))
             }
         }
 
-        Arc::new(EnumData {
-            name: enum_.name.clone(),
-            variants,
-            repr,
-            visibility: item_tree[enum_.visibility].clone(),
-        })
+        (
+            Arc::new(EnumData {
+                name: enum_.name.clone(),
+                variants,
+                repr,
+                visibility: item_tree[enum_.visibility].clone(),
+            }),
+            Arc::new(diagnostics),
+        )
     }
 
     pub fn variant(&self, name: &Name) -> Option<LocalEnumVariantId> {
@@ -384,31 +445,64 @@ fn lower_struct(
 fn lower_fields(
     db: &dyn DefDatabase,
     krate: CrateId,
+    current_file_id: HirFileId,
+    container: LocalModuleId,
     item_tree: &ItemTree,
     cfg_options: &CfgOptions,
     fields: &Fields,
     override_visibility: Option<RawVisibilityId>,
-) -> VariantData {
+) -> (VariantData, Vec<DefDiagnostic>) {
+    let mut diagnostics = Vec::new();
     match fields {
         Fields::Record(flds) => {
             let mut arena = Arena::new();
             for field_id in flds.clone() {
-                if item_tree.attrs(db, krate, field_id.into()).is_cfg_enabled(cfg_options) {
-                    arena.alloc(lower_field(item_tree, &item_tree[field_id], override_visibility));
+                let attrs = item_tree.attrs(db, krate, field_id.into());
+                let field = &item_tree[field_id];
+                if attrs.is_cfg_enabled(cfg_options) {
+                    arena.alloc(lower_field(item_tree, field, override_visibility));
+                } else {
+                    diagnostics.push(DefDiagnostic::unconfigured_code(
+                        container,
+                        InFile::new(
+                            current_file_id,
+                            match field.ast_id {
+                                FieldAstId::Record(it) => it.upcast(),
+                                FieldAstId::Tuple(it) => it.upcast(),
+                            },
+                        ),
+                        attrs.cfg().unwrap(),
+                        cfg_options.clone(),
+                    ))
                 }
             }
-            VariantData::Record(arena)
+            (VariantData::Record(arena), diagnostics)
         }
         Fields::Tuple(flds) => {
             let mut arena = Arena::new();
             for field_id in flds.clone() {
-                if item_tree.attrs(db, krate, field_id.into()).is_cfg_enabled(cfg_options) {
-                    arena.alloc(lower_field(item_tree, &item_tree[field_id], override_visibility));
+                let attrs = item_tree.attrs(db, krate, field_id.into());
+                let field = &item_tree[field_id];
+                if attrs.is_cfg_enabled(cfg_options) {
+                    arena.alloc(lower_field(item_tree, field, override_visibility));
+                } else {
+                    diagnostics.push(DefDiagnostic::unconfigured_code(
+                        container,
+                        InFile::new(
+                            current_file_id,
+                            match field.ast_id {
+                                FieldAstId::Record(it) => it.upcast(),
+                                FieldAstId::Tuple(it) => it.upcast(),
+                            },
+                        ),
+                        attrs.cfg().unwrap(),
+                        cfg_options.clone(),
+                    ))
                 }
             }
-            VariantData::Tuple(arena)
+            (VariantData::Tuple(arena), diagnostics)
         }
-        Fields::Unit => VariantData::Unit,
+        Fields::Unit => (VariantData::Unit, diagnostics),
     }
 }
 
diff --git a/crates/hir-def/src/db.rs b/crates/hir-def/src/db.rs
index 40b2f734b71..93ffe29a1fe 100644
--- a/crates/hir-def/src/db.rs
+++ b/crates/hir-def/src/db.rs
@@ -97,12 +97,27 @@ pub trait DefDatabase: InternDatabase + AstDatabase + Upcast<dyn AstDatabase> {
     #[salsa::invoke(StructData::struct_data_query)]
     fn struct_data(&self, id: StructId) -> Arc<StructData>;
 
+    #[salsa::invoke(StructData::struct_data_with_diagnostics_query)]
+    fn struct_data_with_diagnostics(
+        &self,
+        id: StructId,
+    ) -> (Arc<StructData>, Arc<Vec<DefDiagnostic>>);
+
     #[salsa::invoke(StructData::union_data_query)]
     fn union_data(&self, id: UnionId) -> Arc<StructData>;
 
+    #[salsa::invoke(StructData::union_data_with_diagnostics_query)]
+    fn union_data_with_diagnostics(
+        &self,
+        id: UnionId,
+    ) -> (Arc<StructData>, Arc<Vec<DefDiagnostic>>);
+
     #[salsa::invoke(EnumData::enum_data_query)]
     fn enum_data(&self, e: EnumId) -> Arc<EnumData>;
 
+    #[salsa::invoke(EnumData::enum_data_with_diagnostics_query)]
+    fn enum_data_with_diagnostics(&self, e: EnumId) -> (Arc<EnumData>, Arc<Vec<DefDiagnostic>>);
+
     #[salsa::invoke(ImplData::impl_data_query)]
     fn impl_data(&self, e: ImplId) -> Arc<ImplData>;
 
diff --git a/crates/hir-def/src/nameres/collector.rs b/crates/hir-def/src/nameres/collector.rs
index 5f0cf9c4ac8..9ffc218818c 100644
--- a/crates/hir-def/src/nameres/collector.rs
+++ b/crates/hir-def/src/nameres/collector.rs
@@ -32,8 +32,8 @@ use crate::{
     derive_macro_as_call_id,
     item_scope::{ImportType, PerNsGlobImports},
     item_tree::{
-        self, FieldAstId, Fields, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode,
-        MacroCall, MacroDef, MacroRules, Mod, ModItem, ModKind, TreeId,
+        self, Fields, FileItemTreeId, ImportKind, ItemTree, ItemTreeId, ItemTreeNode, MacroCall,
+        MacroDef, MacroRules, Mod, ModItem, ModKind, TreeId,
     },
     macro_call_as_call_id, macro_id_to_def_id,
     nameres::{
@@ -1511,10 +1511,7 @@ impl ModCollector<'_, '_> {
             let attrs = self.item_tree.attrs(self.def_collector.db, krate, item.into());
             if let Some(cfg) = attrs.cfg() {
                 if !self.is_cfg_enabled(&cfg) {
-                    self.emit_unconfigured_diagnostic(
-                        InFile::new(self.file_id(), item.ast_id(&self.item_tree).upcast()),
-                        &cfg,
-                    );
+                    self.emit_unconfigured_diagnostic(item, &cfg);
                     continue;
                 }
             }
@@ -1526,20 +1523,22 @@ impl ModCollector<'_, '_> {
             }
 
             let db = self.def_collector.db;
-            let module_id = self.module_id;
-            let module = self.def_collector.def_map.module_id(module_id);
+            let module = self.def_collector.def_map.module_id(self.module_id);
+            let def_map = &mut self.def_collector.def_map;
             let update_def =
                 |def_collector: &mut DefCollector<'_>, id, name: &Name, vis, has_constructor| {
-                    def_collector.def_map.modules[module_id].scope.declare(id);
+                    def_collector.def_map.modules[self.module_id].scope.declare(id);
                     def_collector.update(
-                        module_id,
+                        self.module_id,
                         &[(Some(name.clone()), PerNs::from_def(id, vis, has_constructor))],
                         vis,
                         ImportType::Named,
                     )
                 };
             let resolve_vis = |def_map: &DefMap, visibility| {
-                def_map.resolve_visibility(db, module_id, visibility).unwrap_or(Visibility::Public)
+                def_map
+                    .resolve_visibility(db, self.module_id, visibility)
+                    .unwrap_or(Visibility::Public)
             };
 
             match item {
@@ -1595,7 +1594,6 @@ impl ModCollector<'_, '_> {
                     let fn_id =
                         FunctionLoc { container, id: ItemTreeId::new(self.tree_id, id) }.intern(db);
 
-                    let def_map = &self.def_collector.def_map;
                     let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     if self.def_collector.is_proc_macro {
                         if self.module_id == def_map.root {
@@ -1616,10 +1614,7 @@ impl ModCollector<'_, '_> {
                 ModItem::Struct(id) => {
                     let it = &self.item_tree[id];
 
-                    self.process_fields(&it.fields);
-
-                    let vis =
-                        resolve_vis(&self.def_collector.def_map, &self.item_tree[it.visibility]);
+                    let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     update_def(
                         self.def_collector,
                         StructLoc { container: module, id: ItemTreeId::new(self.tree_id, id) }
@@ -1633,10 +1628,7 @@ impl ModCollector<'_, '_> {
                 ModItem::Union(id) => {
                     let it = &self.item_tree[id];
 
-                    self.process_fields(&it.fields);
-
-                    let vis =
-                        resolve_vis(&self.def_collector.def_map, &self.item_tree[it.visibility]);
+                    let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     update_def(
                         self.def_collector,
                         UnionLoc { container: module, id: ItemTreeId::new(self.tree_id, id) }
@@ -1650,21 +1642,7 @@ impl ModCollector<'_, '_> {
                 ModItem::Enum(id) => {
                     let it = &self.item_tree[id];
 
-                    for id in it.variants.clone() {
-                        let variant = &self.item_tree[id];
-                        let attrs = self.item_tree.attrs(self.def_collector.db, krate, id.into());
-                        if let Some(cfg) = attrs.cfg() {
-                            if !self.is_cfg_enabled(&cfg) {
-                                self.emit_unconfigured_diagnostic(
-                                    InFile::new(self.file_id(), variant.ast_id.upcast()),
-                                    &cfg,
-                                );
-                            }
-                        }
-                    }
-
-                    let vis =
-                        resolve_vis(&self.def_collector.def_map, &self.item_tree[it.visibility]);
+                    let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     update_def(
                         self.def_collector,
                         EnumLoc { container: module, id: ItemTreeId::new(self.tree_id, id) }
@@ -1682,10 +1660,7 @@ impl ModCollector<'_, '_> {
 
                     match &it.name {
                         Some(name) => {
-                            let vis = resolve_vis(
-                                &self.def_collector.def_map,
-                                &self.item_tree[it.visibility],
-                            );
+                            let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                             update_def(self.def_collector, const_id.into(), name, vis, false);
                         }
                         None => {
@@ -1699,8 +1674,7 @@ impl ModCollector<'_, '_> {
                 ModItem::Static(id) => {
                     let it = &self.item_tree[id];
 
-                    let vis =
-                        resolve_vis(&self.def_collector.def_map, &self.item_tree[it.visibility]);
+                    let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     update_def(
                         self.def_collector,
                         StaticLoc { container, id: ItemTreeId::new(self.tree_id, id) }
@@ -1714,8 +1688,7 @@ impl ModCollector<'_, '_> {
                 ModItem::Trait(id) => {
                     let it = &self.item_tree[id];
 
-                    let vis =
-                        resolve_vis(&self.def_collector.def_map, &self.item_tree[it.visibility]);
+                    let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     update_def(
                         self.def_collector,
                         TraitLoc { container: module, id: ItemTreeId::new(self.tree_id, id) }
@@ -1729,8 +1702,7 @@ impl ModCollector<'_, '_> {
                 ModItem::TypeAlias(id) => {
                     let it = &self.item_tree[id];
 
-                    let vis =
-                        resolve_vis(&self.def_collector.def_map, &self.item_tree[it.visibility]);
+                    let vis = resolve_vis(def_map, &self.item_tree[it.visibility]);
                     update_def(
                         self.def_collector,
                         TypeAliasLoc { container, id: ItemTreeId::new(self.tree_id, id) }
@@ -2143,44 +2115,17 @@ impl ModCollector<'_, '_> {
         }
     }
 
-    fn process_fields(&mut self, fields: &Fields) {
-        match fields {
-            Fields::Record(range) | Fields::Tuple(range) => {
-                for id in range.clone() {
-                    let field = &self.item_tree[id];
-                    let attrs = self.item_tree.attrs(
-                        self.def_collector.db,
-                        self.def_collector.def_map.krate,
-                        id.into(),
-                    );
-                    if let Some(cfg) = attrs.cfg() {
-                        if !self.is_cfg_enabled(&cfg) {
-                            self.emit_unconfigured_diagnostic(
-                                InFile::new(
-                                    self.file_id(),
-                                    match field.ast_id {
-                                        FieldAstId::Record(it) => it.upcast(),
-                                        FieldAstId::Tuple(it) => it.upcast(),
-                                    },
-                                ),
-                                &cfg,
-                            );
-                        }
-                    }
-                }
-            }
-            Fields::Unit => {}
-        }
-    }
-
     fn is_cfg_enabled(&self, cfg: &CfgExpr) -> bool {
         self.def_collector.cfg_options.check(cfg) != Some(false)
     }
 
-    fn emit_unconfigured_diagnostic(&mut self, ast: AstId<ast::AnyHasAttrs>, cfg: &CfgExpr) {
+    fn emit_unconfigured_diagnostic(&mut self, item: ModItem, cfg: &CfgExpr) {
+        let ast_id = item.ast_id(self.item_tree);
+
+        let ast_id = InFile::new(self.file_id(), ast_id.upcast());
         self.def_collector.def_map.diagnostics.push(DefDiagnostic::unconfigured_code(
             self.module_id,
-            ast,
+            ast_id,
             cfg.clone(),
             self.def_collector.cfg_options.clone(),
         ));
diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs
index e1f69001e80..d1c8fa59aef 100644
--- a/crates/hir/src/lib.rs
+++ b/crates/hir/src/lib.rs
@@ -540,9 +540,27 @@ impl Module {
                     }
                     acc.extend(decl.diagnostics(db))
                 }
-                ModuleDef::Adt(Adt::Enum(e)) => {
-                    for v in e.variants(db) {
-                        acc.extend(ModuleDef::Variant(v).diagnostics(db));
+                ModuleDef::Adt(adt) => {
+                    match adt {
+                        Adt::Struct(s) => {
+                            for diag in db.struct_data_with_diagnostics(s.id).1.iter() {
+                                emit_def_diagnostic(db, acc, diag);
+                            }
+                        }
+                        Adt::Union(u) => {
+                            for diag in db.union_data_with_diagnostics(u.id).1.iter() {
+                                emit_def_diagnostic(db, acc, diag);
+                            }
+                        }
+                        Adt::Enum(e) => {
+                            for v in e.variants(db) {
+                                acc.extend(ModuleDef::Variant(v).diagnostics(db));
+                            }
+
+                            for diag in db.enum_data_with_diagnostics(e.id).1.iter() {
+                                emit_def_diagnostic(db, acc, diag);
+                            }
+                        }
                     }
                     acc.extend(decl.diagnostics(db))
                 }
diff --git a/crates/ide-diagnostics/src/handlers/inactive_code.rs b/crates/ide-diagnostics/src/handlers/inactive_code.rs
index c22e25b0637..f558b7256a4 100644
--- a/crates/ide-diagnostics/src/handlers/inactive_code.rs
+++ b/crates/ide-diagnostics/src/handlers/inactive_code.rs
@@ -142,12 +142,18 @@ trait Bar {
     }
 
     #[test]
-    fn inactive_fields() {
+    fn inactive_fields_and_variants() {
         check(
             r#"
 enum Foo {
   #[cfg(a)] Bar,
 //^^^^^^^^^^^^^ weak: code is inactive due to #[cfg] directives: a is disabled
+  Baz {
+    #[cfg(a)] baz: String,
+  //^^^^^^^^^^^^^^^^^^^^^ weak: code is inactive due to #[cfg] directives: a is disabled
+  },
+  Qux(#[cfg(a)] String),
+    //^^^^^^^^^^^^^^^^ weak: code is inactive due to #[cfg] directives: a is disabled
 }
 
 struct Baz {