Skip to content
This repository has been archived by the owner on Jul 9, 2023. It is now read-only.

Derive macro to derive Default from Serde default attributes #4

Closed
dtolnay opened this issue Jan 21, 2019 · 5 comments
Closed

Derive macro to derive Default from Serde default attributes #4

dtolnay opened this issue Jan 21, 2019 · 5 comments

Comments

@dtolnay
Copy link
Owner

dtolnay commented Jan 21, 2019

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
Copy link

TedDriggs commented Jan 23, 2019

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
Copy link

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

@TedDriggs
Copy link

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
Copy link

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
Copy link
Owner Author

dtolnay commented Jan 25, 2019

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants