Add support for attributes for struct fields #3880
Conversation
crates/ra_hir_def/src/adt.rs
Outdated
| }, | ||
| ); | ||
| } | ||
| StructKind::Tuple | ||
| } | ||
| 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 |
There was a problem hiding this comment.
Yeah, this is what we ideally should do here, but, in the current model, we just have one global set of active cfgs, so there won't be parent in the first place if it is disabled.
crates/ra_hir_ty/src/expr.rs
Outdated
| let lit_fields: FxHashSet<_> = fields | ||
| .iter() | ||
| .filter_map(|f| { | ||
| // TODO: check if cfg_is_enabled with .attrs ? |
There was a problem hiding this comment.
SO, if the cfg is disabled, there shouldn't be fields here in the first place, beucase they weren't alloced when we created corresponding VariantData
| @@ -319,3 +319,32 @@ fn no_such_field_diagnostics() { | |||
| "### | |||
| ); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn no_such_field_with_feature_flag_diagnostics() { | |||
|
Yeah, this generally moves in the right direction, except that, I think, you don't actually need to fetch attributes from the parent. There's just a global set of attributes enabled per-crate which you can use. Long-term, we should do something smarter here, like adding "deactivated" items, such that completion works even in cfged-out code, but that's not on the horizon yet. |
Signed-off-by: Benjamin Coenen <[email protected]>
|
Updated, the test now pass. Let me know what do you want to change :) |
bnjjj
changed the title
|
Yup, lgtm, although I think the orginal bit about attrs on structs was also good, and can be added back! Specifically, I think the following test would be good to add struct S {
#[cfg(feature = "foo")] foo: u32,
#[cfg(not(feature = "foo"))] bar: u32,
}
impl S {
#[cfg(feature = "foo")]
fn new(foo: u32) -> S { S { foo } }
#[cfg(not(feature = "foo"))]
fn new(bar u32) -> S { S { bar } }
}but bors r+ as this is indeed more correct than what we had before |
|
Ah, seems like Ci is failing though? |
|
Ok will check :) |
3880: Add support for attributes for struct fields r=matklad a=bnjjj
Hello I try to solve this example:
```rust
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 }
}
}
```
Here is a draft PR to try to solve this issue. In fact for now when i have this kind of example, rust-analyzer tells me that my second Self {} miss the bar field. Which is a bug.
I have some difficulties to add this features. Here in my draft I share my work about adding attributes support on struct field data. But I'm stuck when I have to fetch attributes from parent expressions. I don't really know how to do that. For the first iteration I just want to solve my issue without solving on all different expressions. And then after I will try to implement that on different kind of expression. I think I have to fetch my FunctionId and then I will be able to find attributes with myFunction.attrs() But I don't know if it's the right way.
@matklad (or anyone else) if you can help me it would be great :D
Co-authored-by: Benjamin Coenen <[email protected]>
Build failed |
Signed-off-by: Benjamin Coenen <[email protected]>
For sure I will add this support but if you don't care I would like to do it in another PR. In fact as I only access to an BTW I fix the failing test, it was a trailing whitespace in my test case. So next step, I will open a new PR to support also your example you can count on me :) |
|
bors r+ |
Build succeeded |
Hello I try to solve this example:
Here is a draft PR to try to solve this issue. In fact for now when i have this kind of example, rust-analyzer tells me that my second Self {} miss the bar field. Which is a bug.
I have some difficulties to add this features. Here in my draft I share my work about adding attributes support on struct field data. But I'm stuck when I have to fetch attributes from parent expressions. I don't really know how to do that. For the first iteration I just want to solve my issue without solving on all different expressions. And then after I will try to implement that on different kind of expression. I think I have to fetch my FunctionId and then I will be able to find attributes with myFunction.attrs() But I don't know if it's the right way.
@matklad (or anyone else) if you can help me it would be great :D