Preserve definition-level defaults when types are reached via $ref#1042
Open
danieleades wants to merge 1 commit into
Open
Preserve definition-level defaults when types are reached via $ref#1042danieleades wants to merge 1 commit into
danieleades wants to merge 1 commit into
Conversation
When a schema definition carries its own `default` value, that default was dropped when the type was converted via a reference: convert_ref_type recomputed the default from the metadata returned by convert_schema — which is `None` for struct conversions, since TypeEntryStruct::from_metadata consumes the metadata and records the default itself — and unconditionally assigned it, clobbering the already-recorded default. As a result the generated type lacked its `Default` impl (and default-dependent codegen) even though the same schema, inlined, produced one. Only override the recorded default when the metadata returned by the conversion actually specifies one. This preserves the existing precedence: a default declared at the reference site (merged sibling metadata alongside $ref) still comes back as `Some` and takes precedence over the definition-level default. Adds a golden test covering a definition-level default reached via $ref (struct and enum) and a ref-site override.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
When a schema definition carries its own
defaultvalue, that default is dropped when the type is reached via$ref. For example:{ "definitions": { "DefaultedStruct": { "type": "object", "properties": { "a": { "type": "string" } }, "default": { "a": "hello" } } } }Referenced via
"$ref": "#/definitions/DefaultedStruct", the generated type loses the definition-level default (itsDefaultimpl falls back to per-field defaults, yieldinga: None). Inlining the identical schema preserves it (a: Some("hello")).Cause
convert_ref_typerecomputes the default from the metadata returned byconvert_schemaand assigns it unconditionally. For struct conversions that returned metadata isNone—TypeEntryStruct::from_metadataconsumes the metadata and records the default itself — so the already-recorded default was clobbered withNone. (Enums were unaffected becauseconvert_enum_stringreturnsSome(metadata), which is why the asymmetry went unnoticed.)Fix
Only override the recorded default when the metadata returned by the conversion actually specifies one.
Precedence is preserved: a default declared at the reference site (sibling metadata alongside
$ref, handled by the merge path) comes back asSomeand still takes precedence over the definition-level default.Testing
New golden test
typify/tests/schemas/ref-with-default.jsoncovering:$ref— now generatesDefault for DefaultedStructwitha: Some("hello");$ref— unchanged behavior;allOf+ siblingdefault) — still wins over the definition-level default.No other golden outputs change.
cargo test --workspace --lockedpasses;cargo fmt --all -- --checkis clean.