fix(compilers/openapi): keep a false schema beside its model - #370
Open
OmarAlJarrah wants to merge 1 commit into
Open
fix(compilers/openapi): keep a false schema beside its model#370OmarAlJarrah wants to merge 1 commit into
OmarAlJarrah wants to merge 1 commit into
Conversation
A boolean `false` schema matches no instance. It lowers to a closed empty
Model, which matches the empty JSON object -- so the lowered type admits
exactly one value the source forbids. That approximation is deliberate
and normative, but nothing recorded it: the node carried no trace of the
`false`, so a `false` schema and a schema that merely wrote
{type: object, additionalProperties: false}
produced byte-identical IR, and no consumer could tell "no instance"
from "the empty object".
ir-design §4.8 already asks this of every degraded lowering -- verbatim
preservation under ReasonDegradedLowering wherever the weaker shape
loses something -- and the composition half of this very rule already
did it, keeping a `false` allOf branch beside the closed model. Only the
whole-schema half did not. It now keeps the schema at
Unmodeled["openapi:schema"], keyed by position because a boolean schema
writes no keyword to name it by.
The approximation stays rather than moving to an exactly empty shape,
and §4.8 now says why. A closed Enum with no members is an exactly empty
value space, but Enum.ValueType is a PrimKind and a `false` schema
declares no type, so lowering to one means inventing a value type the
source never wrote. It would also split the rule, since the composition
case must stay a Model to keep what the other branches contributed.
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
A boolean
falseschema matches no instance. It lowers to a closed emptyModel,which matches the empty JSON object — so the lowered type admits exactly one value
the source forbids.
That approximation is deliberate and normative (
ir-design.md§4.8). What wasmissing is any trace of it in the document. Compiling
F: falsebesideH: {type: object, additionalProperties: false}onmain@dbf0054produced twoidentical nodes:
So no consumer could tell "no instance" from "the empty object". The
infodiagnostic says which one it was, but a diagnostic is not part of the document.
§4.8 already asks for verbatim preservation under
ReasonDegradedLowering"wherever that weaker shape actually loses something", and the composition half of
this very rule already did it — a
falseallOfbranch is kept beside the closedmodel, explicitly so it "distinguishes it from a model that merely wrote
additionalProperties: false". Only the whole-schema half did not. That asymmetry,inside one rule, is the defect.
falseSchemanow keeps the schema atUnmodeled["openapi:schema"], keyed byposition because a boolean schema writes no keyword to name it by, and nothing can
collide with it since such a schema has nothing else to preserve. After:
Why the approximation stays, rather than moving to an exact shape
The issue asked for one or the other, and §4.8 now states the reasoning instead of
leaving it to be re-derived. A closed
Enumwith no members is an exactly emptyvalue space, but
Enum.ValueTypeis aPrimKindand afalseschema declares notype — lowering to one means inventing a value type the source never wrote, and an
exact-looking node resting on a fabricated field is not more faithful than an
approximate one with the source beside it. It would also split the rule: the
composition case must stay a
Model, because it keeps what the other branchescontributed, so one source construct would lower to two kinds depending on where it
appeared — the asymmetry §4.8's "the
falserule holds wherever the schema appears"exists to prevent. With the source preserved, the overstatement is bounded, recorded
and recoverable, and an emitter that wants the exact reading has what it needs.
applyFalseBranchesis unchanged: its half was already correct.Test plan
testdata/conformance/openapi/allof-boolean-branch.yamlgainsClosedEmpty, theother way to write a closed empty model, beside the
BareFalseit already had.assertAllOfBooleanBranchpins the new entry, its reason, its value, thediagnostic at
/components/schemas/BareFalse, and — the claim that motivates thechange — that the two nodes are no longer equal.
Preservecall failsTestConformance/allof-boolean-branchwith
openapi:schema kept verbatim; got map[].-updateand inspected: the diff is exactly the newunmodeledblock onBareFalse, the newClosedEmptynode, and the rewordeddiagnostic. No other golden moved.
Full gate green:
gofmt,go vet ./...,golangci-lint run(0 issues),go build ./...,./scripts/check-coverage.sh(100% of statements).Note on that last figure: I deliberately do not quote a statement count, because the
gate's count turns out to depend on test-cache warmth. That is pre-existing on
mainand filed separately as #369; it does not affect whether the gate passes, since
duplicated blocks can only cause a false failure.
Closes #350