Skip to content

fix(compilers/openapi): keep a false schema beside its model - #370

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-false-schema-exact
Open

fix(compilers/openapi): keep a false schema beside its model#370
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-false-schema-exact

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

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 (ir-design.md §4.8). What was
missing is any trace of it in the document. Compiling F: false beside
H: {type: object, additionalProperties: false} on main @ dbf0054 produced two
identical nodes:

{"id":".../F","kind":"model","additional":"closed","unmodeled":null}
{"id":".../H","kind":"model","additional":"closed","unmodeled":null}

So no consumer could tell "no instance" from "the empty object". The info
diagnostic 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 false allOf branch is kept beside the closed
model, 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.

falseSchema now keeps the schema at Unmodeled["openapi:schema"], keyed by
position 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:

{"id":".../F","kind":"model","additional":"closed",
 "unmodeled":{"openapi:schema":{"reason":"degraded_lowering","value":false,
              "provenance":{"source":0,"pointer":"/components/schemas/F"}}}}
{"id":".../H","kind":"model","additional":"closed","unmodeled":null}

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 Enum with no members is an exactly empty
value space, but Enum.ValueType is a PrimKind and a false schema declares no
type — 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 branches
contributed, so one source construct would lower to two kinds depending on where it
appeared — the asymmetry §4.8's "the false rule 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.

applyFalseBranches is unchanged: its half was already correct.

Test plan

  • testdata/conformance/openapi/allof-boolean-branch.yaml gains ClosedEmpty, the
    other way to write a closed empty model, beside the BareFalse it already had.
  • assertAllOfBooleanBranch pins the new entry, its reason, its value, the
    diagnostic at /components/schemas/BareFalse, and — the claim that motivates the
    change — that the two nodes are no longer equal.
  • Watched red: dropping the Preserve call fails TestConformance/allof-boolean-branch
    with openapi:schema kept verbatim; got map[].
  • Golden regenerated with -update and inspected: the diff is exactly the new
    unmodeled block on BareFalse, the new ClosedEmpty node, and the reworded
    diagnostic. 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 main
and filed separately as #369; it does not affect whether the gate passes, since
duplicated blocks can only cause a false failure.

Closes #350

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openapi: a false schema lowers to a model that still admits {}

1 participant