Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix name collisions #152

Merged
merged 2 commits into from
Sep 12, 2024
Merged

Fix name collisions #152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b4f609e
Fix variable name collisions in generated code
davidpdrsn Sep 12, 2024
9a54204
don't need this lint
davidpdrsn Sep 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 33 additions & 30 deletions crates/mirror-mirror-macros/src/derive_reflect/enum_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ fn expand_reflect(
let ident = field.ident;
let ident_string = stringify(ident);
quote! {
if let Some(new_value) = enum_.field(#ident_string) {
if let Some(new_value) = __mirror_mirror_enum.field(#ident_string) {
#ident.patch(new_value);
}
}
Expand All @@ -190,7 +190,7 @@ fn expand_reflect(
|(index, field)| {
let ident = &field.fake_ident;
quote! {
if let Some(new_value) = enum_.field_at(#index) {
if let Some(new_value) = __mirror_mirror_enum.field_at(#index) {
#ident.patch(new_value);
}
}
Expand All @@ -217,12 +217,12 @@ fn expand_reflect(

if attrs.clone_opt_out {
quote! {
fn patch(&mut self, value: &dyn Reflect) {
if let Some(enum_) = value.reflect_ref().as_enum() {
if let Some(new) = FromReflect::from_reflect(value) {
fn patch(&mut self, __mirror_mirror_value: &dyn Reflect) {
if let Some(__mirror_mirror_enum) = __mirror_mirror_value.reflect_ref().as_enum() {
if let Some(new) = FromReflect::from_reflect(__mirror_mirror_value) {
*self = new;
} else {
let variant_matches = self.variant_name() == enum_.variant_name();
let variant_matches = self.variant_name() == __mirror_mirror_enum.variant_name();
match self {
#(#match_arms)*
_ => {}
Expand All @@ -233,14 +233,14 @@ fn expand_reflect(
}
} else {
quote! {
fn patch(&mut self, value: &dyn Reflect) {
if let Some(new) = value.downcast_ref::<Self>() {
fn patch(&mut self, __mirror_mirror_value: &dyn Reflect) {
if let Some(new) = __mirror_mirror_value.downcast_ref::<Self>() {
*self = new.clone();
} else if let Some(enum_) = value.reflect_ref().as_enum() {
if let Some(new) = FromReflect::from_reflect(value) {
} else if let Some(__mirror_mirror_enum) = __mirror_mirror_value.reflect_ref().as_enum() {
if let Some(new) = FromReflect::from_reflect(__mirror_mirror_value) {
*self = new;
} else {
let variant_matches = self.variant_name() == enum_.variant_name();
let variant_matches = self.variant_name() == __mirror_mirror_enum.variant_name();
match self {
#(#match_arms)*
_ => {}
Expand All @@ -264,17 +264,20 @@ fn expand_reflect(
let ident = &field.ident;
let ident_string = stringify(ident);
quote! {
value.set_struct_field(#ident_string, #ident.to_value());
__mirror_mirror_value.set_struct_field(
#ident_string,
#ident.to_value(),
);
}
});

let fields_len = fields.len();

quote! {
Self::#variant_ident { #(#field_names,)* } => {
let mut value = EnumValue::new_struct_variant_with_capacity(#variant_ident_string, #fields_len);
let mut __mirror_mirror_value = EnumValue::new_struct_variant_with_capacity(#variant_ident_string, #fields_len);
#(#set_fields)*
value.finish().into()
__mirror_mirror_value.finish().into()
}
}
}
Expand All @@ -290,11 +293,11 @@ fn expand_reflect(

quote! {
Self::#variant_ident(#(#field_names,)*) => {
let mut value = EnumValue::new_tuple_variant_with_capacity(#variant_ident_string, #fields_len);
let mut __mirror_mirror_value = EnumValue::new_tuple_variant_with_capacity(#variant_ident_string, #fields_len);
#(
value.push_tuple_field(#included_fields.to_value());
__mirror_mirror_value.push_tuple_field(#included_fields.to_value());
)*
value.finish().into()
__mirror_mirror_value.finish().into()
}
}
}
Expand Down Expand Up @@ -387,21 +390,21 @@ fn expand_from_reflect(
if let Some(from_reflect_with) = field.from_reflect_with() {
quote! {
#ident: {
let value = enum_.field(#ident_string)?;
let value = __mirror_mirror_enum.field(#ident_string)?;
#from_reflect_with(value)?
},
}
} else if attrs.clone_opt_out {
quote! {
#ident: {
let value = enum_.field(#ident_string)?;
let value = __mirror_mirror_enum.field(#ident_string)?;
FromReflect::from_reflect(value)?
},
}
} else {
quote! {
#ident: {
let value = enum_.field(#ident_string)?;
let value = __mirror_mirror_enum.field(#ident_string)?;
if let Some(value) = value.downcast_ref::<#ty>() {
value.to_owned()
} else {
Expand Down Expand Up @@ -430,21 +433,21 @@ fn expand_from_reflect(
if let Some(from_reflect_with) = field.from_reflect_with() {
quote! {
{
let value = enum_.field_at(#idx)?;
let value = __mirror_mirror_enum.field_at(#idx)?;
#from_reflect_with(value)?
},
}
} else if attrs.clone_opt_out {
quote! {
{
let value = enum_.field_at(#idx)?;
let value = __mirror_mirror_enum.field_at(#idx)?;
FromReflect::from_reflect(value)?
},
}
} else {
quote! {
{
let value = enum_.field_at(#idx)?;
let value = __mirror_mirror_enum.field_at(#idx)?;
if let Some(value) = value.downcast_ref::<#ty>() {
value.to_owned()
} else {
Expand Down Expand Up @@ -480,9 +483,9 @@ fn expand_from_reflect(

quote! {
impl #impl_generics FromReflect for #ident #type_generics #where_clause {
fn from_reflect(reflect: &dyn Reflect) -> Option<Self> {
let enum_ = reflect.reflect_ref().as_enum()?;
match enum_.variant_name() {
fn from_reflect(__mirror_mirror_reflect: &dyn Reflect) -> Option<Self> {
let __mirror_mirror_enum = __mirror_mirror_reflect.reflect_ref().as_enum()?;
match __mirror_mirror_enum.variant_name() {
#(#match_arms)*
_ => None,
}
Expand Down Expand Up @@ -552,7 +555,7 @@ fn expand_enum(
let ident = &field.ident;
let ident_string = stringify(ident);
quote! {
if name == #ident_string {
if __mirror_mirror_name == #ident_string {
return Some(#ident);
}
}
Expand All @@ -575,7 +578,7 @@ fn expand_enum(

quote! {
#[allow(unused_variables, unreachable_code)]
fn field(&self, name: &str) -> Option<&dyn Reflect> {
fn field(&self, __mirror_mirror_name: &str) -> Option<&dyn Reflect> {
match self {
#(#match_arms)*
_ => {}
Expand All @@ -600,7 +603,7 @@ fn expand_enum(
let ident = &field.ident;
let ident_string = stringify(ident);
quote! {
if name == #ident_string {
if __mirror_mirror_name == #ident_string {
return Some(#ident);
}
}
Expand All @@ -623,7 +626,7 @@ fn expand_enum(

quote! {
#[allow(unused_variables, unreachable_code)]
fn field_mut(&mut self, name: &str) -> Option<&mut dyn Reflect> {
fn field_mut(&mut self, __mirror_mirror_name: &str) -> Option<&mut dyn Reflect> {
match self {
#(#match_arms)*
_ => {}
Expand Down
1 change: 0 additions & 1 deletion crates/mirror-mirror-macros/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
clippy::needless_borrow,
clippy::match_wildcard_for_single_variants,
clippy::if_let_mutex,
clippy::mismatched_target_os,
clippy::await_holding_lock,
clippy::match_on_vec_items,
clippy::imprecise_flops,
Expand Down
1 change: 0 additions & 1 deletion crates/mirror-mirror/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@
clippy::needless_borrow,
clippy::match_wildcard_for_single_variants,
clippy::if_let_mutex,
clippy::mismatched_target_os,
clippy::await_holding_lock,
clippy::match_on_vec_items,
clippy::imprecise_flops,
Expand Down
43 changes: 43 additions & 0 deletions crates/mirror-mirror/src/tests/enum_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,3 +505,46 @@ fn default_value_for_enum_variant_type() {
Foo::C { a: 0.0, b: None },
);
}

#[test]
fn field_named_named() {
#[derive(Reflect, Debug, Clone)]
#[reflect(crate_name(crate), opt_out(Default))]
enum A {
Struct {
name: String,
},
// some other field names that shouldn't collide with generated code
Struct2 {
value: (),
reflect: (),
enum_: (),
struct_: (),
},
}

let mut a = A::Struct {
name: "foo".to_owned(),
};

assert_eq!(
a.as_reflect()
.as_enum()
.unwrap()
.field("name")
.unwrap()
.downcast_ref::<String>()
.unwrap(),
"foo"
);
assert_eq!(
a.as_reflect_mut()
.as_enum_mut()
.unwrap()
.field_mut("name")
.unwrap()
.downcast_mut::<String>()
.unwrap(),
"foo"
);
}
42 changes: 42 additions & 0 deletions crates/mirror-mirror/src/tests/struct_.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,3 +410,45 @@ fn default_value() {
assert_eq!(foo_descriptor.default_value(), Some(foo_default));
assert_eq!(bar_descriptor.default_value(), None);
}

#[test]
fn field_named_named() {
#[derive(Reflect, Debug, Clone)]
#[reflect(crate_name(crate), opt_out(Default))]
struct A {
name: String,
value: (),
reflect: (),
enum_: (),
struct_: (),
}

let mut a = A {
name: "foo".to_owned(),
value: (),
reflect: (),
enum_: (),
struct_: (),
};

assert_eq!(
a.as_reflect()
.as_struct()
.unwrap()
.field("name")
.unwrap()
.downcast_ref::<String>()
.unwrap(),
"foo"
);
assert_eq!(
a.as_reflect_mut()
.as_struct_mut()
.unwrap()
.field_mut("name")
.unwrap()
.downcast_mut::<String>()
.unwrap(),
"foo"
);
}
Loading