Skip to content

Avoid clippy::unit_arg in generated builder code for unit-typed properties#1037

Open
danieleades wants to merge 1 commit into
oxidecomputer:mainfrom
danieleades:fix/builder-unit-arg
Open

Avoid clippy::unit_arg in generated builder code for unit-typed properties#1037
danieleades wants to merge 1 commit into
oxidecomputer:mainfrom
danieleades:fix/builder-unit-arg

Conversation

@danieleades

Copy link
Copy Markdown
Contributor

Problem

For struct properties whose type is the unit type () (produced by null schemas), builder-mode codegen emits expressions that pass a unit value to Ok(..):

  • in impl Default for builder::T: field: Ok(Default::default())
  • in impl From<super::T> for builder::T: field: Ok(value.field)

Both trip clippy's warn-by-default unit_arg lint. Generating builder-mode output for the typify-impl/tests/github.json fixture produces 105 such warnings. cargo-typify's generated #![allow(..)] prelude doesn't cover unit_arg, and consumers using the macro or build.rs get the warnings in their own builds.

Fix

Detect unit-typed properties in output_struct and emit Ok(()) directly in the builder's Default and From impls. This is semantically identical: reading or defaulting a unit field always yields () and has no effects.

Verified by compiling the github.json builder output (with the #![allow(..)] prelude stripped) under cargo clippy: unit_arg warnings drop from 105 to 0. No existing golden files change, since none of them exercise a unit-typed property in builder mode.

Trade-offs

None behavioral; only the spelling of the generated initializers for unit-typed fields changes.

…rties

For struct properties whose type is the unit type () (produced by null
schemas), the generated builder code passed a unit value to Ok(..) in
the builder's Default impl and in the From<T> impl, tripping clippy's
warn-by-default unit_arg lint. Emit Ok(()) directly for such properties
instead; this is semantically identical.
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