EmbarkStudios / mirror-mirror

🪞 Powerful reflection library for Rust
Apache License 2.0
79 stars 2 forks source link

Remove unreachable `panic!` in generated code for enums #105

Closed davidpdrsn closed 1 year ago

davidpdrsn commented 1 year ago

If you have an enum like

#[derive(Debug, Clone, PartialEq, Reflect)]
pub enum Foo {
    A,
    B(i32, String),
    C { a: f32, b: Option<bool> },
}

Then #[derive(Reflect)] would previously generate a to_value method like so:

fn to_value(&self) -> Value {
    match self {
        Self::A => EnumValue::new_unit_variant("A").into(),
        Self::B(field_0, field_1) => {
            let mut value = EnumValue::new_tuple_variant("B");
            value.push_tuple_field(field_0.to_value());
            value.push_tuple_field(field_1.to_value());
            value.finish().into()
        }
        Self::C { a, b } => {
            let mut value = EnumValue::new_struct_variant("C");
            value.set_struct_field("a", a.to_value());
            value.set_struct_field("b", b.to_value());
            value.finish().into()
        }
        other => panic!(
            "`Reflection::to_value` called on `{0:?}` which doesn\'t suport reflection",
            other.as_reflect()
        ),
    }
}

The panic! is there in case you have #[reflect(skip)] on a variant but try to call .to_value() on that (maybe to_value should return an Option?)

However the panic! would be generated regardless if the enum actually had skipped variants or not. This changes that should its only generated if there are skipped variants.

davidpdrsn commented 1 year ago

Maybe should generate try_to_value that returns Result and then to_value auto-unwraps

Good idea!