Add default value for properties (takes a function instead of an expression)

[AlephAlpha]

Fix #775

Similar to #879 , but default values are specified by paths to functions, instead of expressions:

#[derive(Clone, Properties)]
struct MyProps {
    #[props(default = "default_value")]
    value: u32,
    #[props(default = "always_true")]
    enabled: bool
}

fn default_value() -> u32 {
  100
}

fn always_true() -> bool {
  true
}

But there is a minor problem: Fixed.

If the function has incompatible type:

#[derive(Clone, Properties)]
pub struct Props {
    // ERROR: the function returns incompatible types
    #[props(default = "foo")]
    value: String,
}

fn foo() -> i32 {
    100
}

It might produce an error message with a misleading "help":

error[E0308]: mismatched types
   --> $DIR/fail.rs:112:27
    |
112 |         #[props(default = "foo")]
    |                           ^^^^^
    |                           |
    |                           expected struct `std::string::String`, found i32
    |                           help: try using a conversion method: `"foo".to_string()`
    |
    = note: expected type `std::string::String`
               found type `i32`

I don't know how to fix this.

The expression version (#879 ) has a similar problem.

@mdtusz @jstarry @hgzimmerman

[hgzimmerman]

I think at the very least you should look at prior art in how Serde accomplishes this. If they can't get a correct error message in this situation, I don't think it would be reasonable to expect you to do the same - and I don't think it would be a deal breaker for the feature, although the documentation where this feature is mentioned should make light of the erroneous error message.

[AlephAlpha]

@hgzimmerman Thanks for pointing that out.

I checked serde's source code and did some experiment. The error message serde would provide in such case is match arms have incompatible types, without that misleading "help". They use a match expression because they really need to, not just to avoid a misleading error message.

Inspired by serde, I did something like:

let none: Option<#ty> = None;
match true {
    false => none,
    true => Some(#default())
}
.unwrap()

Now the error message becomes:

error[E0308]: match arms have incompatible types
   --> $DIR/fail.rs:112:27
    |
112 |         #[props(default = "foo")]
    |                           ^^^^^
    |                           |
    |                           expected struct `std::string::String`, found i32
    |                           `match` arms have incompatible types
    |                           this is found to be of type `std::option::Option<std::string::String>`
    |
    = note: expected type `std::option::Option<std::string::String>`
               found type `std::option::Option<i32>`

Still confusing, but at least it won't suggest a wrong fix.

There might be better ways to fix this.

[jstarry]

I think the Option is a bit confusing. How about the following:

                    #name: {
                        match true {
                            false => ::std::unimplemented!(),
                            true => #default()
                        }
                    },

[jstarry]

Updated suggestion to use ::std:: prefix

[jstarry]

Looks great, great test coverage 👍

[jstarry]

Updated suggestion to use ::std:: prefix

[jstarry]

Do you need an assign here? I think you can just do false => ::std::unreachable!() right?

[AlephAlpha]

The false arm must return the correct type, otherwise Rust would think that it has the same type as #default(), and might show that misleading message: "help: try using a conversion method: "foo".to_string()".

[jstarry]

Hmm your explanation makes sense but I had it working with unimplemented like that. This is fine though!

[AlephAlpha]

Sorry, I was wrong about that. I did not test it before leaving the comment. It seems that false => ::std::unreachable!() does work.