fix(compilers/openapi): stop rounding raw-preserved numbers - #244
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix(compilers/openapi): stop rounding raw-preserved numbers#244OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
RawFromNode decoded a YAML node into `any` and re-marshalled it. yaml.v3 resolves an out-of-int64 integer and every decimal into float64, so the one channel whose documented promise is verbatim preservation quietly rewrote its numbers: a 23-digit extension value came back as 1.2345678901234568e+22, and 1.000000000000000000001 came back as 1. That reached every raw-preservation site — x-* extensions, oneOf/anyOf sibling preservation, the validation-only carve-out, and tuple items-after-prefix — because all four share this one conversion. Render the node tree directly instead, taking numeric scalars from their source text through value.NumericLiteral, which already backs the Value/BigVal channel and resolves YAML's integer bases (0o17 is still 15, not 17). Walking the tree means reimplementing what the decode did structurally, so merge-key expansion, duplicate-key rejection, non-string-key refusal, alias following and sorted object keys are all kept, and pinned by tests. Two behaviour changes beyond the rounding. An explicitly tagged huge integer (`!!int 12345678901234567890123`) used to be dropped with a diagnostic and is now preserved. A literal whose rendered form is not JSON is refused rather than spliced into the document, which keeps the open NewBigVal binary-exponent gap (#45) out of the IR without settling it here. Timestamps and !!binary scalars are still rewritten rather than kept verbatim. That is a different mechanism from the rounding and is filed as #242 rather than swept in. Fixes #32
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.
Summary
annotation.RawFromNodedecoded a YAML node intoanyand re-marshalled it as JSON. yaml.v3 resolves an out-of-int64 integer and every decimal intofloat64, so the one channel whose documented promise is verbatim preservation quietly rewrote its numbers:123456789012345678901231.2345678901234568e+221.0000000000000000000011-9223372036854775809-92233720368547760001.101.1One conversion backs every raw-preservation site, so all four were affected:
x-*extensions, oneOf/anyOf sibling preservation, the §4.7 validation-only carve-out, and tuple items-after-prefix. #23 fixed the same class of bug in theValue/BigValchannel; this is the raw channel's version of it.The fix renders the node tree directly, taking numeric scalars from their source text via
value.NumericLiteral— the helper that already backs theValuechannel, and that resolves YAML's integer bases, so0o17still reads 15 rather than 17.Walking the tree means the structural work the decode used to do has to be done here, so merge-key (
<<) expansion, duplicate-key rejection, non-string-key refusal, alias following and sorted object keys are all reproduced deliberately and pinned by tests.ir.UnmodeledEntry.Value's doc comment described the rounding as intended behaviour ("normalizes … number spelling"), so it is corrected to say what now holds.Behaviour changes beyond the rounding
!!int 12345678901234567890123) used to fail to convert and be dropped with a diagnostic. It is now preserved exactly.NewBigValaccepts binary-exponent literals and stores them verbatim (ir: NewBigVal accepts binary-exponent and leading-zero literals, storing non-JSON canonical forms #45, open), so!!float 1p4would otherwise have put1p4into aRawValue. Refusing matches what the old decode did with that input and leaves ir: NewBigVal accepts binary-exponent and leading-zero literals, storing non-JSON canonical forms #45 to be settled on its own terms.Deliberately out of scope
Timestamps still normalize to RFC 3339 and
!!binarystill carries decoded bytes rather than its base64 text, which costs ill-formed UTF-8 its identity to U+FFFD. Those are a different mechanism from the float64 rounding and are filed as #242, noted in the code and in theValuedoc comment rather than swept in here.Test plan
TestRawFromNode_KeepsNumericLiteralsExact— the regression, over the spellings the old conversion rewrote, including a significant trailing zero and both integer boundaries.TestRawFromNode_ResolvesYAMLIntegerBases— the half that must not become verbatim, so the fix cannot be "read the source text" applied too far.TestRawFromNode_PreservesMergeAndOrderingSemanticsandTestRawFromNode_RefusesWhatJSONCannotName— the structural behaviour the walk had to reimplement, and the refusals compilers/openapi: a failed raw conversion preserves nothing while its diagnostic claims otherwise #144 depends on.TestRawFromNode_DiffersFromTheOldDecodeOnlyInNumbers— the equivalence oracle. The old conversion is kept in the test file, and the claim is that rounding the new output through float64 reproduces it. Reading two implementations cannot show they agree; this can.<<chain that recurses without passing back through the node walk.testdata/conformance/openapi/numeric-precision.yamlgains apreservedproperty carrying high-precision numbers in an extension and undernot. The corpus had no case with a number float64 cannot hold, which is why nothing caught this.Verified by mutation rather than by inspection — each of these was planted and confirmed to redden: reverting to the float64 decode, inverting merge precedence, dropping the duplicate-key check, dropping key sorting, accepting non-string keys, and removing the bounds (which turns a clean refusal into a stack overflow). Restoring the original bug reddens the conformance case on all four preserved values, so the fixture is load-bearing rather than decorative.
Full gate green:
gofmt,go vet,golangci-lint,go build, and./scripts/check-coverage.shat 100% of 4469 statements.Fixes #32