From 5dcbf03d0f114cab1ae1748dd3c3632a52f6f52d Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 9 Oct 2020 20:42:17 +0200 Subject: adt: correctly inherit field visibility from enum Previously, "find all references" on a variant field wouldn't find any references outside the defining module. This is because variant fields were incorrectly assumed to be private, like struct fields without explicit visibility, but they actually inherit the enum's visibility. --- crates/assists/src/handlers/fix_visibility.rs | 10 ++++------ crates/hir_def/src/adt.rs | 28 ++++++++++++++++++--------- crates/ide/src/references.rs | 24 +++++++++++++++++++++++ 3 files changed, 47 insertions(+), 15 deletions(-) (limited to 'crates') diff --git a/crates/assists/src/handlers/fix_visibility.rs b/crates/assists/src/handlers/fix_visibility.rs index 7cd76ea06..d505e9444 100644 --- a/crates/assists/src/handlers/fix_visibility.rs +++ b/crates/assists/src/handlers/fix_visibility.rs @@ -324,14 +324,14 @@ pub struct Foo { pub bar: () } #[test] fn fix_visibility_of_enum_variant_field() { - check_assist( + // Enum variants, as well as their fields, always get the enum's visibility. In fact, rustc + // rejects any visibility specifiers on them, so this assist should never fire on them. + check_assist_not_applicable( fix_visibility, r"mod foo { pub enum Foo { Bar { bar: () } } } fn main() { foo::Foo::Bar { <|>bar: () }; } ", - r"mod foo { pub enum Foo { Bar { $0pub(crate) bar: () } } } - fn main() { foo::Foo::Bar { bar: () }; } ", ); - check_assist( + check_assist_not_applicable( fix_visibility, r" //- /lib.rs @@ -339,8 +339,6 @@ mod foo; fn main() { foo::Foo::Bar { <|>bar: () }; } //- /foo.rs pub enum Foo { Bar { bar: () } } -", - r"pub enum Foo { Bar { $0pub(crate) bar: () } } ", ); check_assist_not_applicable( diff --git a/crates/hir_def/src/adt.rs b/crates/hir_def/src/adt.rs index d69ff2fc7..6539959c3 100644 --- a/crates/hir_def/src/adt.rs +++ b/crates/hir_def/src/adt.rs @@ -14,7 +14,7 @@ use tt::{Delimiter, DelimiterKind, Leaf, Subtree, TokenTree}; use crate::{ body::{CfgExpander, LowerCtx}, db::DefDatabase, - item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem}, + item_tree::{AttrOwner, Field, Fields, ItemTree, ModItem, RawVisibilityId}, src::HasChildSource, src::HasSource, trace::Trace, @@ -91,7 +91,7 @@ impl StructData { let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone(); let strukt = &item_tree[loc.id.value]; - let variant_data = lower_fields(&item_tree, &cfg_options, &strukt.fields); + let variant_data = lower_fields(&item_tree, &cfg_options, &strukt.fields, None); Arc::new(StructData { name: strukt.name.clone(), variant_data: Arc::new(variant_data), @@ -105,7 +105,7 @@ impl StructData { let cfg_options = db.crate_graph()[loc.container.module(db).krate].cfg_options.clone(); let union = &item_tree[loc.id.value]; - let variant_data = lower_fields(&item_tree, &cfg_options, &union.fields); + let variant_data = lower_fields(&item_tree, &cfg_options, &union.fields, None); Arc::new(StructData { name: union.name.clone(), @@ -126,7 +126,8 @@ impl EnumData { for var_id in enum_.variants.clone() { if item_tree.attrs(var_id.into()).is_cfg_enabled(&cfg_options) { let var = &item_tree[var_id]; - let var_data = lower_fields(&item_tree, &cfg_options, &var.fields); + let var_data = + lower_fields(&item_tree, &cfg_options, &var.fields, Some(enum_.visibility)); variants.alloc(EnumVariantData { name: var.name.clone(), @@ -296,13 +297,18 @@ fn lower_struct( } } -fn lower_fields(item_tree: &ItemTree, cfg_options: &CfgOptions, fields: &Fields) -> VariantData { +fn lower_fields( + item_tree: &ItemTree, + cfg_options: &CfgOptions, + fields: &Fields, + override_visibility: Option, +) -> VariantData { match fields { Fields::Record(flds) => { let mut arena = Arena::new(); for field_id in flds.clone() { if item_tree.attrs(field_id.into()).is_cfg_enabled(cfg_options) { - arena.alloc(lower_field(item_tree, &item_tree[field_id])); + arena.alloc(lower_field(item_tree, &item_tree[field_id], override_visibility)); } } VariantData::Record(arena) @@ -311,7 +317,7 @@ fn lower_fields(item_tree: &ItemTree, cfg_options: &CfgOptions, fields: &Fields) let mut arena = Arena::new(); for field_id in flds.clone() { if item_tree.attrs(field_id.into()).is_cfg_enabled(cfg_options) { - arena.alloc(lower_field(item_tree, &item_tree[field_id])); + arena.alloc(lower_field(item_tree, &item_tree[field_id], override_visibility)); } } VariantData::Tuple(arena) @@ -320,10 +326,14 @@ fn lower_fields(item_tree: &ItemTree, cfg_options: &CfgOptions, fields: &Fields) } } -fn lower_field(item_tree: &ItemTree, field: &Field) -> FieldData { +fn lower_field( + item_tree: &ItemTree, + field: &Field, + override_visibility: Option, +) -> FieldData { FieldData { name: field.name.clone(), type_ref: field.type_ref.clone(), - visibility: item_tree[field.visibility].clone(), + visibility: item_tree[override_visibility.unwrap_or(field.visibility)].clone(), } } diff --git a/crates/ide/src/references.rs b/crates/ide/src/references.rs index 571dd5452..9315f7354 100644 --- a/crates/ide/src/references.rs +++ b/crates/ide/src/references.rs @@ -732,6 +732,30 @@ fn f(e: En) { ); } + #[test] + fn test_find_all_refs_enum_var_privacy() { + check( + r#" +mod m { + pub enum En { + Variant { + field<|>: u8, + } + } +} + +fn f() -> m::En { + m::En::Variant { field: 0 } +} +"#, + expect![[r#" + field RECORD_FIELD FileId(0) 56..65 56..61 Other + + FileId(0) 125..130 Other Read + "#]], + ); + } + fn check(ra_fixture: &str, expect: Expect) { check_with_scope(ra_fixture, None, expect) } -- cgit v1.2.3