feat: emit typed wrapper for primitive oneOf schemas#549
Draft
plheide wants to merge 4 commits intoomissis:mainfrom
Draft
feat: emit typed wrapper for primitive oneOf schemas#549plheide wants to merge 4 commits intoomissis:mainfrom
plheide wants to merge 4 commits intoomissis:mainfrom
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
plheide
force-pushed
the
feat/p4-primitive-oneof
branch
from
599e3dd to
b747147
Compare
plheide
force-pushed
the
feat/p4-primitive-oneof
branch
4 times, most recently
from
dd6ee06 to
24ab94a
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
plheide
force-pushed
the
feat/p4-primitive-oneof
branch
2 times, most recently
from
bb53991 to
2b8da87
Compare
Both jsonFormatter.generate and yamlFormatter.generate were near-identical near-100-line blocks. Extracts the shared body into generateUnmarshalBody in a new pkg/generator/unmarshal_body.go and shrinks each formatter to a ~25-line wrapper that supplies an unmarshalContext describing only the format-specific parts (function signature, decode call). Adds a replacesUnmarshalBody bool to validatorDesc, honored by the helper so that future validators (e.g. oneOf primitives) can take full control of the function body. Currently unused but keeps the helper's contract self-contained. Behavior is preserved byte-for-byte: all golden-file generation tests pass with no diff.
Adds Config.FormatValidation (off by default, with optional AllowList) that turns on a stdlib-only formatValidator for the format keywords the type mapping previously left as bare string: uuid, email, uri, uri-reference, hostname, regex. Each format compiles to a small post-decode check on the typed field (net/mail.ParseAddress for email, net/url.Parse for uri, regexp for the others). Nillable pointer fields are gated by a nil check so absent optional values are not flagged. Includes golden-file fixtures under tests/data/formatValidation/ and 20 round-trip assertions in tests/unmarshal_json_test.go covering both acceptance and rejection.
Adds Config.StrictAdditionalProperties with three modes:
off (default) - silently drop unknown fields, preserving
historical behavior.
respect-schema - reject unknown fields only when the schema
declares additionalProperties: false.
strict - reject unknown fields for every generated
object type. Skipped when the schema declares
a typed additionalProperties (a catch-all map
field is generated instead).
Implementation is a strictFieldsValidator that reuses the existing
raw-map pre-validation seam in unmarshal_body.go. patternProperties
suppresses enforcement with a warning since it has no first-class
generator support.
Includes golden-file fixtures under tests/data/strictAdditionalProperties{,Always}/
and round-trip assertions covering accept/reject for both modes.
Detects schemas where every oneOf variant is a JSON primitive (string,
number, integer, boolean, null) and emits a wrapper Go type instead of
falling through to interface{}. The wrapper holds the decoded value in
a private field and exposes:
UnmarshalJSON - dispatch on json.Decoder.Token kind so a number
that also matches integer cannot ambiguously
satisfy two variants.
MarshalJSON - emit the underlying value or null.
UnmarshalYAML
MarshalYAML
Value() - typed getter returning any.
IsZero() - supports omitzero.
As<Kind>() - typed accessor per declared variant
(AsString / AsNumber / AsBool / IsNull).
Detection wires into both generateDeclaredType (root) and
generateTypeInline (object property) so the wrapper is emitted whether
the oneOf is the root schema or a nested field.
Includes golden-file fixtures under tests/data/oneOfPrimitive/ covering
number+string, number+string+bool, string+null, and an in-field case
that mirrors the original motivating use case (a "value" property
typed as number|string|bool).
plheide
force-pushed
the
feat/p4-primitive-oneof
branch
from
2b8da87 to
6570abd
Compare
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
1 participant
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.
Currently, a schema with
oneOfwhose variants are all JSON primitives falls through tointerface{}with no validation: the schema parser readsOneOfinto the model but the generator never references it. This produces broken or empty output (see e.g. #187 for the related top-level-oneOf-of-objects case, which this PR does not fix).Detects schemas where every
oneOfvariant declares a single primitive type (string,number,integer,boolean,null) with no nested constraints, and emits a small wrapper Go type instead of falling through tointerface{}. The wrapper holds the decoded value in a private field and exposes:UnmarshalJSON— dispatches onjson.Decoder.Tokenkind, so a number that also matches integer cannot ambiguously satisfy two variantsMarshalJSON— emits the underlying value ornullUnmarshalYAML/MarshalYAMLValue() any— typed getterIsZero() bool— supportsomitzeroAs<Kind>()— typed accessor per declared variant (AsString/AsNumber/AsBool/IsNull)Detection wires into both
generateDeclaredType(root) andgenerateTypeInline(object property) so the wrapper is emitted whether theoneOfis the root schema or a nested field. (Root-level oneOf-only schemas with no top-leveltypestill hit the existing early return ingenerateRootType; that's a separate fix.)Test coverage under
tests/data/oneOfPrimitive/coversnumber+string,number+string+bool,string+null, and the in-field case (object with avalueproperty typedoneOf:[number,string,bool]). Runtime tests intests/unmarshal_json_test.gocover acceptance per variant, rejection of disallowed kinds, and JSON round-trip.Known limitation
integerandnumbervariants both decode throughcase json.Numberintofloat64. For schemas that allowintegerand need to preserve int64 values ≥ 2^53, this loses precision and there is noAsIntaccessor. Adding a distinctoneOfKindIntegeris straightforward if needed; happy to roll it in here or as a follow-up.Final part of the small series; depends on #548.
Related: #187 (this PR partially addresses oneOf in field position; root-level oneOf-of-objects requires general discriminated-union support, which is the planned follow-up).