From ab864ed259c10ff51f7c9c3421d098eeea7b0245 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 7 Apr 2020 17:58:05 +0200 Subject: feat: add attributes support on struct fields #3870 Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- crates/ra_hir_def/src/adt.rs | 31 ++++++++++++++++++++++++++++--- crates/ra_hir_def/src/data.rs | 6 +++++- crates/ra_hir_ty/src/expr.rs | 12 +++++++++--- crates/ra_hir_ty/src/tests.rs | 29 +++++++++++++++++++++++++++++ 4 files changed, 71 insertions(+), 7 deletions(-) diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index de07fc952..8527a6cad 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/crates/ra_hir_def/src/adt.rs @@ -4,17 +4,19 @@ 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::{ - db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, - visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId, - UnionId, VariantId, + attr::Attrs, db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, + type_ref::TypeRef, visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, + Lookup, StructId, UnionId, VariantId, }; /// Note that we use `StructData` for unions as well! @@ -49,11 +51,14 @@ pub struct StructFieldData { pub name: Name, pub type_ref: TypeRef, pub visibility: RawVisibility, + pub attrs: Attrs, + // TODO: add attributes } impl StructData { pub(crate) fn struct_data_query(db: &dyn DefDatabase, id: StructId) -> Arc { 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 = Arc::new(variant_data); @@ -181,6 +186,10 @@ 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>, @@ -189,12 +198,21 @@ 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)), }, ); } @@ -202,12 +220,19 @@ 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)), }, ); } diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 04bd4a305..934c3b94e 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -3,6 +3,7 @@ use std::sync::Arc; use hir_expand::{ + hygiene::Hygiene, name::{name, AsName, Name}, AstId, InFile, }; @@ -12,6 +13,7 @@ use ra_syntax::ast::{ }; use crate::{ + attr::Attrs, db::DefDatabase, path::{path, GenericArgs, Path}, src::HasSource, @@ -26,6 +28,7 @@ pub struct FunctionData { pub name: Name, pub params: Vec, pub ret_type: TypeRef, + pub attrs: Attrs, /// True if the first param is `self`. This is relevant to decide whether this /// can be called as a method. pub has_self_param: bool, @@ -63,6 +66,7 @@ impl FunctionData { params.push(type_ref); } } + 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 { @@ -81,7 +85,7 @@ impl FunctionData { let visibility = RawVisibility::from_ast_with_default(db, vis_default, src.map(|s| s.visibility())); - let sig = FunctionData { name, params, ret_type, has_self_param, visibility }; + let sig = FunctionData { name, params, ret_type, has_self_param, visibility, attrs }; Arc::new(sig) } } diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index b7b476b4c..eb1209d08 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/crates/ra_hir_ty/src/expr.rs @@ -8,8 +8,7 @@ use hir_def::{ AdtId, FunctionId, }; use hir_expand::{diagnostics::DiagnosticSink, name::Name}; -use ra_syntax::ast; -use ra_syntax::AstPtr; +use ra_syntax::{ast, AstPtr}; use rustc_hash::FxHashSet; use crate::{ @@ -82,7 +81,14 @@ impl<'a, 'b> ExprValidator<'a, 'b> { let variant_data = variant_data(db.upcast(), variant_def); - let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); + let lit_fields: FxHashSet<_> = fields + .iter() + .filter_map(|f| { + // TODO: check if cfg_is_enabled with .attrs ? + + Some(&f.name) + }) + .collect(); let missed_fields: Vec = variant_data .fields() .iter() diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index 027e5a8f8..c3d793cc2 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -318,3 +318,32 @@ fn no_such_field_diagnostics() { "### ); } + +#[test] +fn no_such_field_with_feature_flag_diagnostics() { + let diagnostics = TestDB::with_files( + r#" + //- /lib.rs + struct MyStruct { + my_val: usize, + #[cfg(feature = "foo")] + bar: bool, + } + + impl MyStruct { + #[cfg(feature = "foo")] + 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 } + } + } + "#, + ) + .diagnostics(); + + assert_snapshot!(diagnostics, ""); +} -- cgit v1.2.3 From 8f1dba6f9ae1d8d314dd9d007e4c582ed1403e8d Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 8 Apr 2020 18:12:15 +0200 Subject: 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 | 30 +++--------------------------- crates/ra_hir_def/src/data.rs | 25 +++++++++++++++++++++---- crates/ra_hir_ty/src/expr.rs | 9 +-------- crates/ra_hir_ty/src/tests.rs | 9 +++++---- 4 files changed, 30 insertions(+), 43 deletions(-) diff --git a/crates/ra_hir_def/src/adt.rs b/crates/ra_hir_def/src/adt.rs index 8527a6cad..7fc4cd76e 100644 --- a/crates/ra_hir_def/src/adt.rs +++ b/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, - type_ref::TypeRef, visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, - Lookup, StructId, UnionId, VariantId, + db::DefDatabase, src::HasChildSource, src::HasSource, trace::Trace, type_ref::TypeRef, + visibility::RawVisibility, EnumId, LocalEnumVariantId, LocalStructFieldId, Lookup, StructId, + 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>, @@ -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)), }, ); } diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 934c3b94e..606ec48b0 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/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, @@ -322,16 +329,26 @@ fn collect_impl_items( id: ImplId, ) -> Vec { 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() diff --git a/crates/ra_hir_ty/src/expr.rs b/crates/ra_hir_ty/src/expr.rs index 6547eedae..fb779cbef 100644 --- a/crates/ra_hir_ty/src/expr.rs +++ b/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 - .iter() - .filter_map(|f| { - // TODO: check if cfg_is_enabled with .attrs ? - - Some(&f.name) - }) - .collect(); + let lit_fields: FxHashSet<_> = fields.iter().map(|f| &f.name).collect(); let missed_fields: Vec = variant_data .fields() .iter() diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index e6ac0aec3..060814e53 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/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###""###); } -- cgit v1.2.3