From a5bf4f6d3e413f1a74f754c1836e4f65f4c7909c Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 06:54:36 +0300 Subject: [PATCH] fix(compilers/openapi): keep a false schema beside its model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../openapi/conformance_unmodeled_test.go | 22 +++++++++++++ compilers/openapi/internal/schema/schema.go | 17 +++++++++- docs/ir-design.md | 21 +++++++++++- .../openapi/allof-boolean-branch.golden.json | 33 +++++++++++++++++-- .../openapi/allof-boolean-branch.yaml | 7 ++++ 5 files changed, 96 insertions(+), 4 deletions(-) diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index 7244a8c5..bb996bdd 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -125,6 +125,28 @@ func assertAllOfBooleanBranch(t *testing.T, doc *ir.Document, diags []ir.Diagnos require.True(t, ok) assert.Equal(t, ir.AdditionalClosed, bare.Additional, "the rule the composed case is held to") assert.Empty(t, bare.Properties) + + // The whole-schema half keeps itself verbatim for the same reason the branch + // half does (GitHub #350). A closed empty Model admits {} and the source + // admits nothing, so the lowered type is one value wider than what was + // written; the entry is what makes that recoverable. + bareEntry := unmodeledEntry(t, bare.Unmodeled, "openapi:schema") + assert.Equal(t, ir.ReasonDegradedLowering, bareEntry.Reason) + assert.JSONEq(t, `false`, string(bareEntry.Value)) + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/false-schema", "/components/schemas/BareFalse")) + + // And the claim that motivates it: the two shapes are now distinguishable. + // Both are closed empty models, so nothing but the preserved schema separates + // them, and asserting the pair is what stops the entry being dropped again. + closed, ok := doc.Types[namedID("ClosedEmpty")].(*ir.Model) + require.True(t, ok) + assert.Equal(t, bare.Additional, closed.Additional, "the two agree on everything else") + assert.Empty(t, closed.Properties) + assert.Empty(t, closed.Unmodeled, + "additionalProperties: false is exactly what the IR says, so it keeps nothing") + assert.NotEqual(t, bare.Unmodeled, closed.Unmodeled, + "a false schema and a closed empty model are no longer the same node") } // assertAllOfInlineResidue pins the residue of an inline allOf branch: the merge diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb9..ed70c4fa 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -428,8 +428,23 @@ func falseSchema(c lowering.Ctx, ts *compile.Types, pointer, hint string) (ir.Ty // it is the honest place for it, not a load-bearing one. var diags []ir.Diagnostic id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { + // Kept verbatim beside the approximation, which is what §4.8 asks of + // every degraded lowering that loses something, and what the composition + // half of this same rule already did. Without it a `false` schema and a + // schema that merely wrote `additionalProperties: false` are the same + // node, so nothing downstream can tell "no instance" from "the empty + // object" — the diagnostic says which one it was, but a diagnostic is not + // part of the document. + // + // The key names the position rather than a keyword, because a boolean + // schema writes none. Nothing can collide with it: a schema that is a + // boolean has no other keywords to preserve. + Preserve(c, &common.Unmodeled, "openapi:schema", + ir.RawValue("false"), ir.ReasonDegradedLowering, pointer) + diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.FalseSchema, pointer, - "boolean false schema matches nothing; lowered as a closed empty model")) + "boolean false schema matches nothing; lowered as a closed empty model "+ + "with the schema kept verbatim under Unmodeled")) return &ir.Model{TypeCommon: common, Additional: ir.AdditionalClosed} }) return id, diags diff --git a/docs/ir-design.md b/docs/ir-design.md index a72ceea6..da88578d 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -608,7 +608,26 @@ IR declines to model as written. It is not a list of every lowering that can wea whole-schema fallbacks sit outside it, because they answer to a compiler's own limits rather than to anything about the construct: a schema nested past the compiler's lowering depth cap becomes `any` with an `error` diagnostic, and a boolean `false` schema becomes a closed empty `Model` with -an `info` one. +an `info` one and the schema itself kept verbatim under `ReasonDegradedLowering` at +`Unmodeled["openapi:schema"]` — the key names the 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. + +That preservation is load-bearing, not decorative: a closed empty `Model` admits `{}`, and the +source admits nothing, so the lowered type is one value wider than what was written. Without the +verbatim entry, `false` and `{type: object, additionalProperties: false}` are the same node, and +nothing downstream can tell "no instance" from "the empty object". The `info` diagnostic says which +one it was, but a diagnostic is not part of the document. + +The approximation is preferred to an exactly empty shape, and the reason is worth stating because +the IR does have one. A closed `Enum` with no members is an exactly empty value space (§4.5), 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 — an exact-looking node resting on a fabricated +field, which is not more faithful than an approximate one with the source beside it. It would also +split the rule: the composition case below must stay a `Model`, because it keeps what the other +branches contributed, so one source construct would lower to two different kinds depending on where +it appeared. The overstatement is bounded, recorded, and recoverable from the `Unmodeled` entry; +an emitter that wants the exact reading has everything it needs to produce it. The `false` rule holds wherever the schema appears, composition included. A `false` conjunct (`allOf: [false, …]`) admits nothing, so the composition admits nothing: the composed `Model` is diff --git a/testdata/conformance/openapi/allof-boolean-branch.golden.json b/testdata/conformance/openapi/allof-boolean-branch.golden.json index 3cfe0746..a89a7550 100644 --- a/testdata/conformance/openapi/allof-boolean-branch.golden.json +++ b/testdata/conformance/openapi/allof-boolean-branch.golden.json @@ -75,6 +75,16 @@ "anonymous": false, "docs": {}, "sensitive": false, + "unmodeled": { + "openapi:schema": { + "reason": "degraded_lowering", + "value": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/BareFalse" + } + } + }, "provenance": { "source": 0, "pointer": "/components/schemas/BareFalse" @@ -84,6 +94,25 @@ "positional": false, "inputOnly": false }, + "t/openapi/components/schemas/ClosedEmpty": { + "kind": "model", + "id": "t/openapi/components/schemas/ClosedEmpty", + "name": { + "source": "ClosedEmpty", + "canonical": "closed_empty" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/ClosedEmpty" + }, + "additional": "closed", + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/Never": { "kind": "model", "id": "t/openapi/components/schemas/Never", @@ -178,7 +207,7 @@ { "severity": "info", "code": "openapi/false-schema", - "message": "boolean false schema matches nothing; lowered as a closed empty model", + "message": "boolean false schema matches nothing; lowered as a closed empty model with the schema kept verbatim under Unmodeled", "provenance": { "source": 0, "pointer": "/components/schemas/BareFalse" @@ -189,7 +218,7 @@ { "format": "openapi@3.1", "path": "allof-boolean-branch.yaml", - "hash": "8dfa63f600320e6f52e6a34aeaf1723eac8e8595ed3031b284d1116d631c1d1e" + "hash": "36740dfe66d0792d6a325af84d776552173f3d1f3e2cba97a40b361f01d372f5" } ] } diff --git a/testdata/conformance/openapi/allof-boolean-branch.yaml b/testdata/conformance/openapi/allof-boolean-branch.yaml index ba0baeac..895354b2 100644 --- a/testdata/conformance/openapi/allof-boolean-branch.yaml +++ b/testdata/conformance/openapi/allof-boolean-branch.yaml @@ -25,3 +25,10 @@ components: # The same rule reached the other way: a bare `false` schema in its own # right, which is the lowering the composed case is being held to. BareFalse: false + # The shape a closed empty model is otherwise written as. A closed empty + # Model admits `{}` and `false` admits nothing, so the two are one value + # apart -- and until the bare `false` kept itself verbatim, they lowered to + # byte-identical nodes and nothing downstream could tell them apart. + ClosedEmpty: + type: object + additionalProperties: false