Skip to content

Commit 1a999ac

Browse files
authored
Merge pull request #4 from barbacane-dev/fix/required-fields-with-intrinsic-defaults
fix: honour required contract on serialize for fields with intrinsic Rust defaults
2 parents 25c35bb + 8203db1 commit 1a999ac

16 files changed

Lines changed: 1080 additions & 483 deletions

CHANGELOG.adoc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
=== Bug fixes
1717
* fix untagged enum variant ordering: Integer now comes before Number to prevent unreachable deserialization (#991)
1818
* generate `Default` impl for structs where all required fields have explicit defaults (#918)
19+
* honour the JSON Schema `required` contract on serialization for fields whose Rust type has an intrinsic default (`Vec`, `HashMap`, `Option<T>`). Previously an empty/`None` required field was silently omitted because `skip_serializing_if` was applied; now `#[serde(default)]` is emitted alone, so deserialize stays lenient (`{}` parses) but serialize always renders the field. **Wire-format change**: payloads that previously omitted empty required fields will now render them as `[]`, `{}`, or `null`
1920
* generate `TryFrom` instead of `From` for bounded integer newtypes, enforcing min/max constraints (#986)
2021
* render integer `minimum`/`maximum` as integers (not floats) in doc comments (#843)
2122
* handle special characters in enum variant names (`=`, `>`, `≥`, etc.) without panicking (#948)

typify-impl/src/defaults.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -596,7 +596,7 @@ fn all_props<'a>(
596596

597597
if let Some(name) = maybe_name {
598598
let required = match &property.state {
599-
StructPropertyState::Required => true,
599+
StructPropertyState::Required | StructPropertyState::RequiredWithDefault => true,
600600
StructPropertyState::Optional | StructPropertyState::Default(_) => false,
601601
};
602602

typify-impl/src/lib.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1327,7 +1327,10 @@ impl<'a> TypeStruct<'a> {
13271327
.map(move |prop| TypeStructPropInfo {
13281328
name: prop.name.as_str(),
13291329
description: prop.description.as_deref(),
1330-
required: matches!(&prop.state, StructPropertyState::Required),
1330+
required: matches!(
1331+
&prop.state,
1332+
StructPropertyState::Required | StructPropertyState::RequiredWithDefault
1333+
),
13311334
type_id: prop.type_id.clone(),
13321335
})
13331336
}

typify-impl/src/structs.rs

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,12 +134,23 @@ impl TypeSpace {
134134
let state = if required.contains(prop_name) {
135135
// Even required fields may have an explicit default value
136136
// specified in the schema. If so, we use it for the Default impl
137-
// and builder pre-population (and also make serde more lenient).
138-
has_default(
137+
// and builder pre-population (and also make deserialization
138+
// more lenient — but NOT serialization, since the schema
139+
// still requires the field on the wire).
140+
match has_default(
139141
self,
140142
&type_id,
141143
metadata.as_ref().and_then(|m| m.default.as_ref()),
142-
)
144+
) {
145+
// `Optional` here means an intrinsic-default type
146+
// (Vec, Option, Map, …) with no explicit schema
147+
// default. Promote to RequiredWithDefault so that
148+
// `generate_serde_attr` emits `#[serde(default)]`
149+
// (lenient deserialize) without `skip_serializing_if`
150+
// (serialization always emits the required field).
151+
StructPropertyState::Optional => StructPropertyState::RequiredWithDefault,
152+
other => other,
153+
}
143154
} else {
144155
// We can use serde's `default` and `skip_serializing_if`
145156
// construction for options, arrays, and maps--i.e. properties that
@@ -342,6 +353,16 @@ pub(crate) fn generate_serde_attr(
342353
}
343354

344355
(StructPropertyState::Required, _) => DefaultFunction::None,
356+
357+
// Required by the schema but with an intrinsic default for the
358+
// Rust type. Emit `#[serde(default)]` so that deserializing a
359+
// JSON object that lacks the field still succeeds (PR #918's
360+
// intent), but DO NOT emit `skip_serializing_if`: the schema
361+
// says the field is required on the wire.
362+
(StructPropertyState::RequiredWithDefault, _) => {
363+
serde_options.push(quote! { default });
364+
DefaultFunction::Default
365+
}
345366
};
346367

347368
let serde = if serde_options.is_empty() {

typify-impl/src/type_entry.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,12 @@ pub(crate) enum StructPropertyRename {
223223
#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
224224
pub(crate) enum StructPropertyState {
225225
Required,
226+
/// Required by the schema, but with an intrinsic default value
227+
/// (e.g. `Vec::new()`, `None`) — emit `#[serde(default)]` so
228+
/// deserializing `{}` succeeds (PR #918), but DON'T emit
229+
/// `skip_serializing_if`: the schema marks the field required, so
230+
/// it must always be present on the wire.
231+
RequiredWithDefault,
226232
Optional,
227233
Default(WrappedValue),
228234
}

0 commit comments

Comments
 (0)