Merge #3918

3918: Add support for feature attributes in struct literal r=matklad a=bnjjj

As promised here is the next PR to solve 2 different scenarios with feature flag on struct literal.
close #3870 

Co-authored-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>

crates/ra_hir_def/src/adt.rs

@@ -4,6 +4,7 @@ use std::sync::Arc;
 
 use either::Either;
 use hir_expand::{
+    hygiene::Hygiene,
     name::{AsName, Name},
     InFile,
 };
@@ -12,9 +13,9 @@ use ra_prof::profile;
 use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
 
 use crate::{
-    db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef,
+    attr::Attrs, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace,
-    visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId,
+    type_ref::TypeRef, visibility::RawVisibility, EnumId, HasModule, LocalEnumVariantId,
-    UnionId, VariantId,
+    LocalStructFieldId, Lookup, ModuleId, StructId, UnionId, VariantId,
 };
 
 /// Note that we use `StructData` for unions as well!
@@ -56,7 +57,8 @@ impl StructData {
         let src = id.lookup(db).source(db);
 
         let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
-        let variant_data = VariantData::new(db, src.map(|s| s.kind()));
+        let variant_data =
+            VariantData::new(db, src.map(|s| s.kind()), id.lookup(db).container.module(db));
         let variant_data = Arc::new(variant_data);
         Arc::new(StructData { name, variant_data })
     }
@@ -70,6 +72,7 @@ impl StructData {
                     .map(ast::StructKind::Record)
                     .unwrap_or(ast::StructKind::Unit)
             }),
+            id.lookup(db).container.module(db),
         );
         let variant_data = Arc::new(variant_data);
         Arc::new(StructData { name, variant_data })
@@ -82,7 +85,7 @@ impl EnumData {
         let src = e.lookup(db).source(db);
         let name = src.value.name().map_or_else(Name::missing, |n| n.as_name());
         let mut trace = Trace::new_for_arena();
-        lower_enum(db, &mut trace, &src);
+        lower_enum(db, &mut trace, &src, e.lookup(db).container.module(db));
         Arc::new(EnumData { name, variants: trace.into_arena() })
     }
 
@@ -98,7 +101,7 @@ impl HasChildSource for EnumId {
     fn child_source(&self, db: &dyn DefDatabase) -> InFile<ArenaMap<Self::ChildId, Self::Value>> {
         let src = self.lookup(db).source(db);
         let mut trace = Trace::new_for_map();
-        lower_enum(db, &mut trace, &src);
+        lower_enum(db, &mut trace, &src, self.lookup(db).container.module(db));
         src.with_value(trace.into_map())
     }
 }
@@ -107,22 +110,23 @@ fn lower_enum(
     db: &dyn DefDatabase,
     trace: &mut Trace<EnumVariantData, ast::EnumVariant>,
     ast: &InFile<ast::EnumDef>,
+    module_id: ModuleId,
 ) {
     for var in ast.value.variant_list().into_iter().flat_map(|it| it.variants()) {
         trace.alloc(
             || var.clone(),
             || EnumVariantData {
                 name: var.name().map_or_else(Name::missing, |it| it.as_name()),
-                variant_data: Arc::new(VariantData::new(db, ast.with_value(var.kind()))),
+                variant_data: Arc::new(VariantData::new(db, ast.with_value(var.kind()), module_id)),
             },
         );
     }
 }
 
 impl VariantData {
-    fn new(db: &dyn DefDatabase, flavor: InFile<ast::StructKind>) -> Self {
+    fn new(db: &dyn DefDatabase, flavor: InFile<ast::StructKind>, module_id: ModuleId) -> Self {
         let mut trace = Trace::new_for_arena();
-        match lower_struct(db, &mut trace, &flavor) {
+        match lower_struct(db, &mut trace, &flavor, module_id) {
             StructKind::Tuple => VariantData::Tuple(trace.into_arena()),
             StructKind::Record => VariantData::Record(trace.into_arena()),
             StructKind::Unit => VariantData::Unit,
@@ -155,22 +159,27 @@ impl HasChildSource for VariantId {
     type Value = Either<ast::TupleFieldDef, ast::RecordFieldDef>;
 
     fn child_source(&self, db: &dyn DefDatabase) -> InFile<ArenaMap<Self::ChildId, Self::Value>> {
-        let src = match self {
+        let (src, module_id) = match self {
             VariantId::EnumVariantId(it) => {
                 // I don't really like the fact that we call into parent source
                 // here, this might add to more queries then necessary.
                 let src = it.parent.child_source(db);
-                src.map(|map| map[it.local_id].kind())
+                (src.map(|map| map[it.local_id].kind()), it.parent.lookup(db).container.module(db))
             }
-            VariantId::StructId(it) => it.lookup(db).source(db).map(|it| it.kind()),
+            VariantId::StructId(it) => {
-            VariantId::UnionId(it) => it.lookup(db).source(db).map(|it| {
+                (it.lookup(db).source(db).map(|it| it.kind()), it.lookup(db).container.module(db))
-                it.record_field_def_list()
+            }
-                    .map(ast::StructKind::Record)
+            VariantId::UnionId(it) => (
-                    .unwrap_or(ast::StructKind::Unit)
+                it.lookup(db).source(db).map(|it| {
-            }),
+                    it.record_field_def_list()
+                        .map(ast::StructKind::Record)
+                        .unwrap_or(ast::StructKind::Unit)
+                }),
+                it.lookup(db).container.module(db),
+            ),
         };
         let mut trace = Trace::new_for_map();
-        lower_struct(db, &mut trace, &src);
+        lower_struct(db, &mut trace, &src, module_id);
         src.with_value(trace.into_map())
     }
 }
@@ -186,10 +195,17 @@ fn lower_struct(
     db: &dyn DefDatabase,
     trace: &mut Trace<StructFieldData, Either<ast::TupleFieldDef, ast::RecordFieldDef>>,
     ast: &InFile<ast::StructKind>,
+    module_id: ModuleId,
 ) -> StructKind {
+    let crate_graph = db.crate_graph();
     match &ast.value {
         ast::StructKind::Tuple(fl) => {
             for (i, fd) in fl.fields().enumerate() {
+                let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
+                if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
+                    continue;
+                }
+
                 trace.alloc(
                     || Either::Left(fd.clone()),
                     || StructFieldData {
@@ -203,6 +219,11 @@ fn lower_struct(
         }
         ast::StructKind::Record(fl) => {
             for fd in fl.fields() {
+                let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
+                if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
+                    continue;
+                }
+
                 trace.alloc(
                     || Either::Right(fd.clone()),
                     || StructFieldData {

crates/ra_hir_def/src/attr.rs

@@ -5,6 +5,7 @@ use std::{ops, sync::Arc};
 use either::Either;
 use hir_expand::{hygiene::Hygiene, AstId, InFile};
 use mbe::ast_to_token_tree;
+use ra_cfg::CfgOptions;
 use ra_syntax::{
     ast::{self, AstNode, AttrsOwner},
     SmolStr,
@@ -90,6 +91,10 @@ impl Attrs {
     pub fn by_key(&self, key: &'static str) -> AttrQuery<'_> {
         AttrQuery { attrs: self, key }
     }
+
+    pub(crate) fn is_cfg_enabled(&self, cfg_options: &CfgOptions) -> bool {
+        self.by_key("cfg").tt_values().all(|tt| cfg_options.is_cfg_enabled(tt) != Some(false))
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]

crates/ra_hir_def/src/body/lower.rs

@@ -4,6 +4,7 @@
 use either::Either;
 
 use hir_expand::{
+    hygiene::Hygiene,
     name::{name, AsName, Name},
     MacroDefId, MacroDefKind,
 };
@@ -20,6 +21,7 @@ use test_utils::tested_by;
 use super::{ExprSource, PatSource};
 use crate::{
     adt::StructKind,
+    attr::Attrs,
     body::{Body, BodySourceMap, Expander, PatPtr, SyntheticSyntax},
     builtin_type::{BuiltinFloat, BuiltinInt},
     db::DefDatabase,
@@ -31,8 +33,8 @@ use crate::{
     path::GenericArgs,
     path::Path,
     type_ref::{Mutability, TypeRef},
-    AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, Intern, ModuleDefId,
+    AdtId, ConstLoc, ContainerId, DefWithBodyId, EnumLoc, FunctionLoc, HasModule, Intern,
-    StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
+    ModuleDefId, StaticLoc, StructLoc, TraitLoc, TypeAliasLoc, UnionLoc,
 };
 
 pub(super) fn lower(
@@ -298,28 +300,41 @@ impl ExprCollector<'_> {
                 self.alloc_expr(Expr::Return { expr }, syntax_ptr)
             }
             ast::Expr::RecordLit(e) => {
+                let crate_graph = self.db.crate_graph();
                 let path = e.path().and_then(|path| self.expander.parse_path(path));
                 let mut field_ptrs = Vec::new();
                 let record_lit = if let Some(nfl) = e.record_field_list() {
                     let fields = nfl
                         .fields()
                         .inspect(|field| field_ptrs.push(AstPtr::new(field)))
-                        .map(|field| RecordLitField {
+                        .filter_map(|field| {
-                            name: field
+                            let module_id = ContainerId::DefWithBodyId(self.def).module(self.db);
-                                .name_ref()
+                            let attrs = Attrs::new(
-                                .map(|nr| nr.as_name())
+                                &field,
-                                .unwrap_or_else(Name::missing),
+                                &Hygiene::new(self.db.upcast(), self.expander.current_file_id),
-                            expr: if let Some(e) = field.expr() {
+                            );
-                                self.collect_expr(e)
+
-                            } else if let Some(nr) = field.name_ref() {
+                            if !attrs.is_cfg_enabled(&crate_graph[module_id.krate].cfg_options) {
-                                // field shorthand
+                                return None;
-                                self.alloc_expr_field_shorthand(
+                            }
-                                    Expr::Path(Path::from_name_ref(&nr)),
+
-                                    AstPtr::new(&field),
+                            Some(RecordLitField {
-                                )
+                                name: field
-                            } else {
+                                    .name_ref()
-                                self.missing_expr()
+                                    .map(|nr| nr.as_name())
-                            },
+                                    .unwrap_or_else(Name::missing),
+                                expr: if let Some(e) = field.expr() {
+                                    self.collect_expr(e)
+                                } else if let Some(nr) = field.name_ref() {
+                                    // field shorthand
+                                    self.alloc_expr_field_shorthand(
+                                        Expr::Path(Path::from_name_ref(&nr)),
+                                        AstPtr::new(&field),
+                                    )
+                                } else {
+                                    self.missing_expr()
+                                },
+                            })
                         })
                         .collect();
                     let spread = nfl.spread().map(|s| self.collect_expr(s));

crates/ra_hir_def/src/data.rs

@@ -7,7 +7,6 @@ use hir_expand::{
     name::{name, AsName, Name},
     AstId, InFile,
 };
-use ra_cfg::CfgOptions;
 use ra_prof::profile;
 use ra_syntax::ast::{
     self, AstNode, ImplItem, ModuleItemOwner, NameOwner, TypeAscriptionOwner, VisibilityOwner,
@@ -318,10 +317,6 @@ fn collect_impl_items_in_macro(
     }
 }
 
-fn is_cfg_enabled(cfg_options: &CfgOptions, attrs: &Attrs) -> bool {
-    attrs.by_key("cfg").tt_values().all(|tt| cfg_options.is_cfg_enabled(tt) != Some(false))
-}
-
 fn collect_impl_items(
     db: &dyn DefDatabase,
     impl_items: impl Iterator<Item = ImplItem>,
@@ -341,10 +336,11 @@ fn collect_impl_items(
                 }
                 .intern(db);
 
-                if !is_cfg_enabled(
+                if !db
-                    &crate_graph[module_id.krate].cfg_options,
+                    .function_data(def)
-                    &db.function_data(def).attrs,
+                    .attrs
-                ) {
+                    .is_cfg_enabled(&crate_graph[module_id.krate].cfg_options)
+                {
                     None
                 } else {
                     Some(def.into())

crates/ra_hir_ty/src/tests.rs

@@ -349,3 +349,63 @@ fn no_such_field_with_feature_flag_diagnostics() {
 
     assert_snapshot!(diagnostics, @r###""###);
 }
+
+#[test]
+fn no_such_field_with_feature_flag_diagnostics_on_struct_lit() {
+    let diagnostics = TestDB::with_files(
+        r#"
+        //- /lib.rs crate:foo cfg:feature=foo
+        struct S {
+            #[cfg(feature = "foo")]
+            foo: u32,
+            #[cfg(not(feature = "foo"))]
+            bar: u32,
+        }
+
+        impl S {
+            #[cfg(feature = "foo")]
+            fn new(foo: u32) -> Self {
+                Self { foo }
+            }
+            #[cfg(not(feature = "foo"))]
+            fn new(bar: u32) -> Self {
+                Self { bar }
+            }
+        }
+        "#,
+    )
+    .diagnostics()
+    .0;
+
+    assert_snapshot!(diagnostics, @r###""###);
+}
+
+#[test]
+fn no_such_field_with_feature_flag_diagnostics_on_struct_fields() {
+    let diagnostics = TestDB::with_files(
+        r#"
+        //- /lib.rs crate:foo cfg:feature=foo
+        struct S {
+            #[cfg(feature = "foo")]
+            foo: u32,
+            #[cfg(not(feature = "foo"))]
+            bar: u32,
+        }
+
+        impl S {
+            fn new(val: u32) -> Self {
+                Self {
+                    #[cfg(feature = "foo")]
+                    foo: val,
+                    #[cfg(not(feature = "foo"))]
+                    bar: val,
+                }
+            }
+        }
+        "#,
+    )
+    .diagnostics()
+    .0;
+
+    assert_snapshot!(diagnostics, @r###""###);
+}