dtolnay / request-for-implementation

Crates that don't exist, but should
610 stars 6 forks source link

Derive macro to derive Default from Serde default attributes #4

Closed dtolnay closed 5 years ago

dtolnay commented 5 years ago

The following data structures accept any of the following JSON inputs when deserializing:

{"http":{"address":"0.0.0.0","port":80}}
{"http":{"address":"0.0.0.0"}}
{"http":{"port":80}}
{"http":{}}
{}
#[derive(Deserialize)]
struct Config {
    #[serde(default)]
    http: HttpConfig,
}

#[derive(Deserialize)]
struct HttpConfig {
    #[serde(default = "default_address")]
    address: String,
    #[serde(default = "default_port")]
    port: u32,
}

fn default_address() -> String { "127.0.0.1".to_owned() }
fn default_port() -> u32 { 8080 }

impl Default for HttpConfig {
    fn default() -> Self {
        HttpConfig {
            address: default_address(),
            port: default_port(),
        }
    }
}

If there is an "http" entry but the "address" is missing, Serde will use the default address. If there is an "http" entry but the "port" is missing, we use the default port. If "http" is missing entirely, we use the Default impl of HttpConfig which comes with the default address and default port. This is working as intended.

But notice that the Default impl is repetitive and could be inferred from the serde attributes. I would like for there to be a Default_serde derive macro that derives the Default trait by using the Serde default value for each field.

#[derive(Deserialize, Default_serde)]
struct HttpConfig {
    #[serde(default = "default_address")]
    address: String,
    #[serde(default = "default_port")]
    port: u32,
}

expands to:

// generated code
impl core::default::Default for HttpConfig {
    fn default() -> Self {
        HttpConfig {
            address: default_address(),
            port: default_port(),
        }
    }
}
TedDriggs commented 5 years ago

Is the advantage of this over the reverse (hand-implementing Default and setting serde(default) at the struct level) that serde can then avoid creating the entire default struct if it only needs a couple fields?

Apart from that performance gain, my gut reaction is that it's more idiomatic to be explicit about the default and let serde pick that up.

TedDriggs commented 5 years ago

For reference, linking colin-kiegel/rust-derive-builder#117

TedDriggs commented 5 years ago

I have a version of this mostly working, filed dtolnay/quote#95 to try and sort out some error span situations and then I'll put it up for review.

TedDriggs commented 5 years ago

First draft is up: https://github.com/TedDriggs/serde_default

I really like the idea of declaring defaults at the field level so that proc-macro crates can fill in missing fields without creating an entire Default::default() instance. I maintain two libraries that would absolutely benefit from that: derive_builder and darling.

However, if multiple crates all require the same entry, then the worst case scenario becomes:

#[derive(Builder, Deserialize, CustomDefault)]
pub struct MyStruct {
    #[builder(default = "my_default")]
    #[serde(default = "my_default")]
    my_field: String,
}

IMO, that hurts readability and increases the likelihood of hard-to-debug errors coming from drift between the two attributes.

What if instead there was a crate that focused just on field-level defaults, using serde syntax, and any crate that wanted to read those attributes could do so? Then the code would read:

#[derive(Builder, Deserialize, CustomDefault)]
pub struct MyStruct {
    #[default(default = "my_default")]
    my_field: String,
}

serde, derive_builder, et al. could all look at that attribute and use it as a fallback for their own defaulting information.

dtolnay commented 5 years ago

Thanks, nicely done! I will go ahead and close out this issue and mark it off the list, and we can follow up on any remaining work on your issue tracker.

Is the advantage of this over the reverse (hand-implementing Default and setting serde(default) at the struct level) that serde can then avoid creating the entire default struct if it only needs a couple fields?

Another advantage is in this sort of situation:

#[derive(Deserialize, Default_serde)]
#[serde(default)]
struct S {
    // Almost all Default::default if omitted
    a: String,
    b: String,
    c: String,
    ...

    // Except for
    #[serde(default = "default_n")]
    n: String,
}

Here a handwritten Default would be many times more code.

What are your thoughts on the #[derive(Default_serde)] naming convention? #[derive(SerdeDefault)] looks wrong because there is no such trait as SerdeDefault. Instead Default_serde means we are deriving Default but it's the version of that derive that looks at serde attrs (out of possibly many other derives that also derive the same Default trait). Similar principle in the naming of serde_repr derives.