Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

confusion on #[serde(default)] between struct and field #2424

Closed
Geal opened this issue Jan 18, 2023 · 0 comments · Fixed by #2452
Closed

confusion on #[serde(default)] between struct and field #2424

Geal opened this issue Jan 18, 2023 · 0 comments · Fixed by #2452
Assignees

Comments

@Geal
Copy link
Contributor

Geal commented Jan 18, 2023

The #[serde(default)] attribute can be set on the struct and on the filed:

  • on the field (what we're using in config right now): fills the field if not present with its Default implementation
  • on the struct: fills any fields that are not present with the value from the struct's Default implementation

This was confusing to me because in some cases, I did not get the default values I was expecting. Example:

#[derive(Deserialize)]
struct Parent {
  #[serde(default)]
  thing: Thing,
  other: u8,
}

#[derive(Default, Deserialize)]
struct Thing {
  #[serde(default = "enable_true")]
  enabled: bool,
}

fn enable_true() -> bool {
  true
}

With this code, if I deserialize { "thing": { }, "other": 0, }, I end up with enabled = true because the enabled field is not present, so we use the enabled_true function to fill it, but if I deserialize { "other": 0 }, then thing is not present, so we use its Default implementation, which will set enabled = false.

Proposal: let's move to #[serde(default)] on the struct everywhere, with a custom Default implementation for the struct if we need some specific default values.

bnjjj added a commit that referenced this issue Jan 26, 2023
- Fix #2424 

Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants