Skip to content

Derive Default for structs when all field defaults are intrinsic#1035

Open
danieleades wants to merge 1 commit into
oxidecomputer:mainfrom
danieleades:fix/derive-default
Open

Derive Default for structs when all field defaults are intrinsic#1035
danieleades wants to merge 1 commit into
oxidecomputer:mainfrom
danieleades:fix/derive-default

Conversation

@danieleades

Copy link
Copy Markdown
Contributor

Problem

When every property of a struct has a default and none of them are custom, the generated code emits a hand-written impl Default whose every field initializer is Default::default(). That impl is exactly what #[derive(Default)] produces, so clippy's warn-by-default derivable_impls lint fires on it in consuming code — roughly 50 times on the output for a large schema like the github.json test fixture. It also bloats the generated output.

Fix

In output_struct, when there is no whole-type default value and every property's default is the intrinsic Default::default(), add Default to the struct's derive set instead of emitting the manual impl.

Hand-written impls are kept where they carry real information:

  • structs with a whole-type default value,
  • structs where any property has a custom default function,
  • builder-module Default impls (which initialize fields to Ok(..)/Err(..)).

Trade-offs

None behavioral — the derived impl produces identical values to the removed manual one. The golden-file diff is large but mechanical: every removed impl had all-Default::default() initializers, and its struct gains Default in the derive list.

When a struct has no whole-type default value and every property's
default is Default::default(), the generated manual
'impl Default' is exactly what '#[derive(Default)]' produces, and
trips clippy's warn-by-default 'derivable_impls' lint in consuming
code (~50 warnings on the github.json test fixture alone). Add
'Default' to the derive set in that case instead of emitting the
manual impl.

Structs with a whole-type default value or any custom field default
still get a hand-written impl, and builder-module Default impls are
unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant