EmbarkStudios / mirror-mirror

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

`TypeDescriptor::default_value` doesn't respect `Default` #118

Closed davidpdrsn closed 6 months ago

davidpdrsn commented 1 year ago

If you do

#[derive(Reflect)]
struct Foo {
    n: i32,
}

impl Default for Foo {
    fn default() -> Self {
        Self {
            n: 1337
        }
    }
}

and then

<Foo as DescribeType>::type_descriptor().default_value()

You'll get Foo { n: 0 }. That is because default_value doesn't respect the Default implementation and instead uses the default for each individual type. i32s default is 0.

This is a big footgun and something we should think about.

cc @fu5ha

fu5ha commented 1 year ago

I haven't fully thought through the options but we talked about putting a default value on TypeDescriptor but then I was thinking about how even if we do that it is constructed via <Foo as DescribeType>::type_descriptor() and realized we basically want to specialize this based on if Foo impls Default or not which is a bit of an issue.

davidpdrsn commented 1 year ago

We do a similar trick currently here #[derive(Reflect)] requires Clone and Debug but you can opt-out of that with an attribute:

pub trait Reflect {
    // new method on the reflect trait
    // needs to object safe so has to return either Value or Box<dyn Reflect> I suppose
    fn default(&self) -> Option<Value>;
}

#[derive(Reflect)]
// and to opt out of `Default` which would make `Reflect::default` return `None`
// #[reflect(opt_out(Default))]
struct Foo {
    n: i32,
}

impl Default for Foo {
    fn default() -> Self {
        Self {
            n: 1337
        }
    }
}
davidpdrsn commented 6 months ago

Fixed in https://github.com/EmbarkStudios/mirror-mirror/pull/135 and will be included in 0.2