feat: add attributes support on struct fields and method #3870

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

crates/ra_hir_def/src/adt.rs

@@ -4,19 +4,17 @@ use std::sync::Arc;
 
 use either::Either;
 use hir_expand::{
-    hygiene::Hygiene,
     name::{AsName, Name},
     InFile,
 };
 use ra_arena::{map::ArenaMap, Arena};
-use ra_cfg::CfgOptions;
 use ra_prof::profile;
 use ra_syntax::ast::{self, NameOwner, TypeAscriptionOwner, VisibilityOwner};
 
 use crate::{
-    attr::Attrs, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace,
+    db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef,
-    type_ref::TypeRef, visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId,
+    visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId,
-    Lookup, StructId, UnionId, VariantId,
+    UnionId, VariantId,
 };
 
 /// Note that we use `StructData` for unions as well!
@@ -51,8 +49,6 @@ pub struct StructFieldData {
     pub name: Name,
     pub type_ref: TypeRef,
     pub visibility: RawVisibility,
-    pub attrs: Attrs,
-    // TODO: add attributes
 }
 
 impl StructData {
@@ -186,10 +182,6 @@ pub enum StructKind {
     Unit,
 }
 
-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 lower_struct(
     db: &dyn DefDatabase,
     trace: &mut Trace<StructFieldData, Either<ast::TupleFieldDef, ast::RecordFieldDef>>,
@@ -198,21 +190,12 @@ fn lower_struct(
     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));
-
-                // Need verification about parent cfg_options and current with current attributes
-                // If it is we are in a case where the cfg is not enabled then we don't have to add this field to check
-                // if !is_cfg_enabled(&crate_graph[module_id.krate].cfg_options, &attrs) {
-                //     continue;
-                // }
-
                 trace.alloc(
                     || Either::Left(fd.clone()),
                     || StructFieldData {
                         name: Name::new_tuple_field(i),
                         type_ref: TypeRef::from_ast_opt(fd.type_ref()),
                         visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())),
-                        attrs: Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id)),
                     },
                 );
             }
@@ -220,19 +203,12 @@ fn lower_struct(
         }
         ast::StructKind::Record(fl) => {
             for fd in fl.fields() {
-                let attrs = Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id));
-                // Need verification about parent cfg_options and current with current attributes
-                // If it is we are in a case where the cfg is not enabled then we don't have to add this field to check
-                // if !is_cfg_enabled(&crate_graph[module_id.krate].cfg_options, &attrs) {
-                //     continue;
-                // }
                 trace.alloc(
                     || Either::Right(fd.clone()),
                     || StructFieldData {
                         name: fd.name().map(|n| n.as_name()).unwrap_or_else(Name::missing),
                         type_ref: TypeRef::from_ast_opt(fd.ascribed_type()),
                         visibility: RawVisibility::from_ast(db, ast.with_value(fd.visibility())),
-                        attrs: Attrs::new(&fd, &Hygiene::new(db.upcast(), ast.file_id)),
                     },
                 );
             }

crates/ra_hir_def/src/data.rs

@@ -7,6 +7,7 @@ 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,
@@ -67,6 +68,7 @@ impl FunctionData {
             }
         }
         let attrs = Attrs::new(&src.value, &Hygiene::new(db.upcast(), src.file_id));
+
         let ret_type = if let Some(type_ref) = src.value.ret_type().and_then(|rt| rt.type_ref()) {
             TypeRef::from_ast(type_ref)
         } else {
@@ -215,6 +217,7 @@ impl ImplData {
         let module_id = impl_loc.container.module(db);
 
         let mut items = Vec::new();
+
         if let Some(item_list) = src.value.item_list() {
             items.extend(collect_impl_items(db, item_list.impl_items(), src.file_id, id));
             items.extend(collect_impl_items_in_macros(
@@ -315,6 +318,10 @@ 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>,
@@ -322,16 +329,26 @@ fn collect_impl_items(
     id: ImplId,
 ) -> Vec<AssocItemId> {
     let items = db.ast_id_map(file_id);
+    let crate_graph = db.crate_graph();
+    let module_id = id.lookup(db).container.module(db);
 
     impl_items
-        .map(|item_node| match item_node {
+        .filter_map(|item_node| match item_node {
             ast::ImplItem::FnDef(it) => {
                 let def = FunctionLoc {
                     container: AssocContainerId::ImplId(id),
                     ast_id: AstId::new(file_id, items.ast_id(&it)),
                 }
                 .intern(db);
-                def.into()
+
+                if !is_cfg_enabled(
+                    &crate_graph[module_id.krate].cfg_options,
+                    &db.function_data(def).attrs,
+                ) {
+                    None
+                } else {
+                    Some(def.into())
+                }
             }
             ast::ImplItem::ConstDef(it) => {
                 let def = ConstLoc {
@@ -339,7 +356,7 @@ fn collect_impl_items(
                     ast_id: AstId::new(file_id, items.ast_id(&it)),
                 }
                 .intern(db);
-                def.into()
+                Some(def.into())
             }
             ast::ImplItem::TypeAliasDef(it) => {
                 let def = TypeAliasLoc {
@@ -347,7 +364,7 @@ fn collect_impl_items(
                     ast_id: AstId::new(file_id, items.ast_id(&it)),
                 }
                 .intern(db);
-                def.into()
+                Some(def.into())
             }
         })
         .collect()

crates/ra_hir_ty/src/expr.rs

@@ -166,14 +166,7 @@ impl<'a, 'b> ExprValidator<'a, 'b> {
 
         let variant_data = variant_data(db.upcast(), variant_def);
 
-        let lit_fields: FxHashSet<_> = fields
+        let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect();
-            .iter()
-            .filter_map(|f| {
-                // TODO: check if cfg_is_enabled with .attrs ?
-
-                Some(&f.name)
-            })
-            .collect();
         let missed_fields: Vec<Name> = variant_data
             .fields()
             .iter()

crates/ra_hir_ty/src/tests.rs

@@ -324,7 +324,7 @@ fn no_such_field_diagnostics() {
 fn no_such_field_with_feature_flag_diagnostics() {
     let diagnostics = TestDB::with_files(
         r#"
-        //- /lib.rs
+        //- /lib.rs crate:foo cfg:feature=foo
         struct MyStruct {
             my_val: usize,
             #[cfg(feature = "foo")]
@@ -336,7 +336,7 @@ fn no_such_field_with_feature_flag_diagnostics() {
             pub(crate) fn new(my_val: usize, bar: bool) -> Self {
                 Self { my_val, bar }
             }
-
+        
             #[cfg(not(feature = "foo"))]
             pub(crate) fn new(my_val: usize, _bar: bool) -> Self {
                 Self { my_val }
@@ -344,7 +344,8 @@ fn no_such_field_with_feature_flag_diagnostics() {
         }
         "#,
     )
-    .diagnostics();
+    .diagnostics()
+    .0;
 
-    assert_snapshot!(diagnostics, "");
+    assert_snapshot!(diagnostics, @r###""###);
 }