From 0e347144cf4bb6b2c9577820da6ad59f08b38c81 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Mon, 7 Sep 2026 23:59:06 +0300 Subject: [PATCH 1/6] feat(ir)!: give contentSchema a home on Encoding and lower it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 2020-12 content vocabulary was two-thirds modelled: contentEncoding reached Encoding.Name and contentMediaType Encoding.MediaType, while contentSchema had no field at any IR position and was always kept verbatim under Unmodeled. A consumer saw an opaque string where the source declared a full shape, and had to special-case one of three keywords (GitHub #426). ir.Encoding gains Schema *TypeRef. contentSchema's value is a schema, so it lowers like every other sub-schema position: hoisted at its own source pointer and referenced by ID, never carried beside the encoding as a raw payload a consumer would have to re-parse. The pointer it hoists at is the one the source wrote it at, which only that declaration can name, so the minted node needs no namespace of its own. The three keywords now share one home, so a position keeps them all or lowers them all: contentSchema joins contentKeywords, and the schema package decides its fate by asking the node that was built rather than the keyword that was written. That is why annotation.noIRHomeAt goes — whether a content keyword reached ir.Encoding is a question only the lowering can answer, and it was answering "never" from outside. Adding the scalar hoisters to the schema walk's recursion is what lets a contentSchema nest; the walk's depth counter already bounds it, and internal/archtest pins the widened cycle. BREAKING CHANGE: contentSchema no longer appears as an openapi:contentSchema Unmodeled entry at a position that lowers to a Scalar; it is Encoding.Schema there. A position with no Encoding field still keeps it verbatim, now alongside its two neighbours. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- .../openapi/conformance_unmodeled_test.go | 20 ++-- .../openapi/internal/annotation/annotation.go | 40 ++------ .../annotation/annotation_internal_test.go | 37 ------- compilers/openapi/internal/schema/schema.go | 99 ++++++++++++------- .../openapi/internal/schema/schema_test.go | 44 +++++++-- docs/ir-design.md | 21 ++-- internal/archtest/recursion_test.go | 10 +- ir/constraints.go | 10 +- .../openapi/content-vocabulary.golden.json | 93 ++++++++++++----- .../openapi/content-vocabulary.yaml | 8 +- 10 files changed, 222 insertions(+), 160 deletions(-) diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index b5af0493..801411c2 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -392,10 +392,11 @@ func assertDependentRequired(t *testing.T, doc *ir.Document, diags []ir.Diagnost diagsAt(diags, "openapi/validation-only-keyword", "/components/schemas/Card")) } -// assertContentVocabulary pins the 2020-12 content vocabulary: contentEncoding -// and contentMediaType are an encoding and lower into ir.Encoding, contentSchema -// is a schema and has no IR home anywhere, and a position with no Encoding field -// at all keeps both of the first two verbatim (GitHub #125). +// assertContentVocabulary pins the 2020-12 content vocabulary: all three +// keywords lower into ir.Encoding — contentEncoding and contentMediaType as +// names, contentSchema as a reference to the type it hoists — and a position +// with no Encoding field at all keeps all three verbatim (GitHub #125, +// GitHub #426). func assertContentVocabulary(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { thumb, ok := doc.Types[namedID("Thumbnail")].(*ir.Scalar) require.True(t, ok) @@ -408,13 +409,16 @@ func assertContentVocabulary(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) require.True(t, ok) require.NotNil(t, env.Encoding) assert.Equal(t, "application/json", env.Encoding.MediaType) - entry := unmodeledEntry(t, env.Unmodeled, "openapi:contentSchema") - assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) - assert.JSONEq(t, `{"type":"object","properties":{"id":{"type":"string"}}}`, string(entry.Value)) + require.NotNil(t, env.Encoding.Schema, "contentSchema has a home on ir.Encoding") + assert.Empty(t, env.Unmodeled, "so it is lowered, never also kept raw") + decoded, ok := doc.Types[env.Encoding.Schema.Target].(*ir.Model) + require.True(t, ok, "and it reaches the registry as a type rather than a blob") + require.Len(t, decoded.Properties, 1) + assert.Equal(t, "id", decoded.Properties[0].WireName) bag, ok := doc.Types[namedID("Bag")].(*ir.Model) require.True(t, ok) - for _, key := range []string{"openapi:contentEncoding", "openapi:contentMediaType"} { + for _, key := range []string{"openapi:contentEncoding", "openapi:contentMediaType", "openapi:contentSchema"} { assert.Equal(t, ir.ReasonNoIRHome, unmodeledEntry(t, bag.Unmodeled, key).Reason, "an object has no Encoding field, so %s is kept", key) } diff --git a/compilers/openapi/internal/annotation/annotation.go b/compilers/openapi/internal/annotation/annotation.go index fe28f587..635a1793 100644 --- a/compilers/openapi/internal/annotation/annotation.go +++ b/compilers/openapi/internal/annotation/annotation.go @@ -639,43 +639,23 @@ func subObjectKeys(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, // unmodeledAt collects every keyword a site declares that the IR keeps verbatim // instead of modelling, each under the reason that says which of those it is -// (§12): validation logic the IR draws a boundary against (§4.7), data with no IR -// field yet, and JSON Schema resource/dialect metadata the IR excludes on -// purpose. +// (§12): validation logic the IR draws a boundary against (§4.7), and JSON +// Schema resource/dialect metadata the IR excludes on purpose. +// +// The content vocabulary is not read here even though it is data with an IR +// home: whether contentEncoding, contentMediaType and contentSchema reached +// ir.Encoding depends on what the position lowered to, which only the schema +// package can answer — schema.recordUnplacedContent asks the node that was +// built rather than the keyword that was written. func unmodeledAt(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { vOnly, vDiags := validationOnlyAt(s, pointer, srcIndex) - noHome, nhDiags := noIRHomeAt(s, pointer, srcIndex) dialect, dDiags := dialectAt(s, pointer, srcIndex) - diags := make([]ir.Diagnostic, 0, len(vDiags)+len(nhDiags)+len(dDiags)) + diags := make([]ir.Diagnostic, 0, len(vDiags)+len(dDiags)) diags = append(diags, vDiags...) - diags = append(diags, nhDiags...) diags = append(diags, dDiags...) - return MergeUnmodeled(MergeUnmodeled(vOnly, noHome), dialect), diags -} - -// noIRHomeAt collects the keywords a schema declares that describe real data yet -// have no field at any IR position. Unlike the §4.7 family these are gaps -// expected to close rather than a boundary the IR draws, which is what -// ReasonNoIRHome says and ReasonValidationOnly would not (§12). -// -// Site-only: contentSchema describes the value at the position that wrote it. -func noIRHomeAt(s *oas3.Schema, pointer string, srcIndex int) (ir.Unmodeled, []ir.Diagnostic) { - if s.GetContentSchema() == nil { - return nil, nil - } - at := pointer + ids.Ptr("contentSchema") - var p ir.Unmodeled - kept, diags := PreserveNodeInto(&p, "openapi:contentSchema", RawPropertyNode(s, "contentSchema"), - ir.ReasonNoIRHome, at, srcIndex) - if !kept { - return nil, diags - } - return p, []ir.Diagnostic{diag.Newf(ir.SeverityInfo, diag.DegradedConstruct, - ir.Provenance{Source: srcIndex, Pointer: at}, - "contentSchema is the shape of the decoded content and no IR position has a field "+ - "for it; kept verbatim under Unmodeled")} + return MergeUnmodeled(vOnly, dialect), diags } // DialectKeywords are the JSON Schema resource and dialect keywords the IR diff --git a/compilers/openapi/internal/annotation/annotation_internal_test.go b/compilers/openapi/internal/annotation/annotation_internal_test.go index 5e65e015..400f8223 100644 --- a/compilers/openapi/internal/annotation/annotation_internal_test.go +++ b/compilers/openapi/internal/annotation/annotation_internal_test.go @@ -148,23 +148,6 @@ func TestDialectAt_KeepsEachKeywordOutOfScope(t *testing.T) { "and the hoist gate agrees a node is needed to hold them") } -// TestNoIRHomeAt_ContentSchemaIsKeptNotExcluded pins the reason split the other -// way: contentSchema is real data shape with no field yet, a gap expected to -// close, so it must not be filed as a deliberate exclusion. -func TestNoIRHomeAt_ContentSchemaIsKeptNotExcluded(t *testing.T) { - t.Parallel() - s := schemaFromYAML(t, "type: string\ncontentSchema: {type: object}\n") - - got, diags := noIRHomeAt(s, "/components/schemas/S", 0) - - entry, ok := got["openapi:contentSchema"] - require.True(t, ok) - assert.JSONEq(t, `{"type":"object"}`, string(entry.Value)) - assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) - require.Len(t, diags, 1) - assert.Equal(t, "/components/schemas/S/contentSchema", diags[0].Provenance.Pointer) -} - // schemaFromYAML unmarshals body as a bare schema through the same marshaller // the compiler's loader parses documents with, so the raw nodes the verbatim // readers read off are present. A schema built in Go carries none, which @@ -180,26 +163,6 @@ func schemaFromYAML(t *testing.T, body string) *oas3.Schema { return s } -// TestNoIRHomeAt_ModelSetWithoutRawSourceRecordsNothing pins the guard between -// the model and the raw tree. contentSchema is kept verbatim, so it is read off -// the source node rather than the parsed model — and a schema built in memory, -// or one whose value cannot be converted to JSON, has a model field set with no -// bytes behind it. Recording an entry there would announce a preservation with -// nothing preserved, so the collector reports nothing instead. -func TestNoIRHomeAt_ModelSetWithoutRawSourceRecordsNothing(t *testing.T) { - t.Parallel() - inner := oas3.NewJSONSchemaFromSchema[oas3.Referenceable]( - &oas3.Schema{Type: oas3.NewTypeFromString(oas3.SchemaTypeObject)}) - s := &oas3.Schema{ContentSchema: inner} - require.NotNil(t, s.GetContentSchema(), "the model reports the keyword as set") - require.Nil(t, RawPropertyNode(s, "contentSchema"), "and no raw node backs it") - - got, diags := noIRHomeAt(s, "/components/schemas/A", 0) - - assert.Nil(t, got, "no entry is recorded when there are no bytes to record") - assert.Empty(t, diags, "and nothing is announced, so the two channels agree") -} - // TestKind_String covers both named values and the default case, so an // assertion failure or test diff over a Kind prints a name instead of a bare // int. diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index e3f1c958..ee72f6f7 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -981,7 +981,7 @@ func lowerTyped(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i case oas3.SchemaTypeArray: return lowerArray(c, ts, anchors, depth, s, pointer, hint) default: - return scalarTypeID(c, ts, s, st, pointer, hint) + return scalarTypeID(c, ts, anchors, depth, s, st, pointer, hint) } } @@ -1337,10 +1337,10 @@ func buildTuple(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i // 2020-12 content vocabulary each hoist a named Scalar wrapping the base // primitive with an Encoding, so what the position wrote never leaks onto the // shared primitive every other declaration of that type also resolves to. -func scalarTypeID(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, st oas3.SchemaType, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { +func scalarTypeID(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, st oas3.SchemaType, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { format := s.GetFormat() if st == oas3.SchemaTypeString && format == "byte" { - return hoistByteScalar(c, ts, s, pointer, hint) + return hoistByteScalar(c, ts, anchors, depth, s, pointer, hint) } key := string(st) if format != "" { @@ -1348,12 +1348,12 @@ func scalarTypeID(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, st oas3.Sch } prim, known := formatTable[key] if !known { - return hoistFormatScalar(c, ts, s, baseForType(st), format, pointer, hint) + return hoistFormatScalar(c, ts, anchors, depth, s, baseForType(st), format, pointer, hint) } - if !declaresContent(s) { + if !declaresContentVocabulary(s) { return ts.PrimID(prim), nil } - return hoistContentScalar(c, ts, s, prim, pointer, hint) + return hoistContentScalar(c, ts, anchors, depth, s, prim, pointer, hint) } // hoistByteScalar hoists a base64-encoded byte scalar (string+byte). @@ -1363,12 +1363,12 @@ func scalarTypeID(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, st oas3.Sch // otherwise carry them: that fallback resolves to whatever node the pointer // already owns and returns early. A scalar that hoisted because it wrote a // format must not lose the bounds it wrote beside it (invariant 2). -func hoistByteScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { +func hoistByteScalar(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { var diags []ir.Diagnostic id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { base := ts.PrimRef(ir.PrimBytes) wire := ts.PrimRef(ir.PrimString) - enc, encDiags := scalarEncoding(c, s, "base64", &common, pointer) + enc, encDiags := scalarEncoding(c, ts, anchors, depth, s, "base64", &common, pointer, hint) diags = append(diags, encDiags...) enc.WireType = &wire cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) @@ -1385,11 +1385,11 @@ func hoistByteScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, // hoistFormatScalar hoists a scalar over base carrying an unknown format as its // encoding name, preserving the format losslessly. -func hoistFormatScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, base ir.PrimKind, format, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { +func hoistFormatScalar(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, base ir.PrimKind, format, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { var diags []ir.Diagnostic id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { baseRef := ts.PrimRef(base) - enc, encDiags := scalarEncoding(c, s, format, &common, pointer) + enc, encDiags := scalarEncoding(c, ts, anchors, depth, s, format, &common, pointer, hint) diags = append(diags, encDiags...) cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) @@ -1407,11 +1407,11 @@ func hoistFormatScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, base i // (type, format) pair maps to, giving the content vocabulary written here a node // of its own to sit on. It carries the position's value constraints for the // reason hoistByteScalar records. -func hoistContentScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, prim ir.PrimKind, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { +func hoistContentScalar(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, prim ir.PrimKind, pointer, hint string) (ir.TypeID, []ir.Diagnostic) { var diags []ir.Diagnostic id := internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { base := ts.PrimRef(prim) - enc, encDiags := scalarEncoding(c, s, "", &common, pointer) + enc, encDiags := scalarEncoding(c, ts, anchors, depth, s, "", &common, pointer, hint) diags = append(diags, encDiags...) cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) @@ -1425,25 +1425,34 @@ func hoistContentScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, prim return id, diags } -// scalarEncoding builds the Encoding a scalar position declares: the 2020-12 -// content vocabulary over the OpenAPI `format` spelling of the same thing. -// formatName is the encoding name the format contributes — "base64" for +// scalarEncoding builds the Encoding a scalar position declares: the whole +// 2020-12 content vocabulary over the OpenAPI `format` spelling of the encoding +// name. formatName is the encoding name the format contributes — "base64" for // format: byte, an unrecognized format verbatim, "" when the pairing is already // captured by the primitive kind. +func scalarEncoding(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, + formatName string, common *ir.TypeCommon, pointer, hint string, +) (*ir.Encoding, []ir.Diagnostic) { + name, diags := encodingName(c, s, formatName, common, pointer) + content, contentDiags := contentSchemaRef(c, ts, anchors, depth, s, pointer, hint) + enc := &ir.Encoding{Name: name, MediaType: s.GetContentMediaType(), Schema: content} + return enc, append(diags, contentDiags...) +} + +// encodingName elects the one name ir.Encoding holds from the two keywords that +// can name an encoding at a scalar position. // -// contentEncoding wins Encoding.Name: it is the standard keyword, where a format -// the IR could not place is only parked there. Encoding holds one name, so a -// format that named a *different* encoding is kept verbatim on c rather than +// contentEncoding wins: it is the standard keyword, where a format the IR could +// not place is only parked there. Encoding holds one name, so a format that +// named a *different* encoding is kept verbatim on the node rather than // overwritten away. -func scalarEncoding(c lowering.Ctx, s *oas3.Schema, formatName string, common *ir.TypeCommon, pointer string) (*ir.Encoding, []ir.Diagnostic) { - enc := &ir.Encoding{Name: formatName, MediaType: s.GetContentMediaType()} +func encodingName(c lowering.Ctx, s *oas3.Schema, formatName string, common *ir.TypeCommon, pointer string) (string, []ir.Diagnostic) { content := s.GetContentEncoding() if content == "" || content == formatName { - return enc, nil + return formatName, nil } - enc.Name = content if formatName == "" { - return enc, nil + return content, nil } at := pointer + ids.Ptr("format") kept, diags := PreserveSchemaKeyword(c, &common.Unmodeled, s, "format", ir.ReasonNoIRHome, at) @@ -1452,25 +1461,39 @@ func scalarEncoding(c lowering.Ctx, s *oas3.Schema, formatName string, common *i "format and contentEncoding both name an encoding and ir.Encoding holds one; "+ "contentEncoding %q is lowered and format is kept verbatim under Unmodeled", content)) } - return enc, diags + return content, diags } -// contentKeywords are the content-vocabulary keywords that lower into -// ir.Encoding: contentEncoding names Encoding.Name and contentMediaType names -// Encoding.MediaType (ir/constraints.go, ir-design §5.3). contentSchema is not -// one of them — it is a schema rather than an encoding, and noIRHomeAt keeps it -// verbatim at every position. -var contentKeywords = []string{"contentEncoding", "contentMediaType"} - -// declaresContent reports whether s writes a contentKeywords entry. -func declaresContent(s *oas3.Schema) bool { - return s.GetContentEncoding() != "" || s.GetContentMediaType() != "" +// contentSchemaRef lowers contentSchema — the shape the encoded value has once +// decoded — to Encoding.Schema. Its value is a schema, so it lowers like every +// other sub-schema position (fillAdditional, patternProps): hoisted at its own +// pointer and referenced by ID, never carried beside the encoding as a raw blob +// a consumer would have to re-parse. +// +// The pointer it hoists at is the one the source wrote it at, which only this +// declaration can name, so the node needs no namespace of its own (§4.3). +func contentSchemaRef(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer, hint string) (*ir.TypeRef, []ir.Diagnostic) { + cs := s.GetContentSchema() + if cs == nil { + return nil, nil + } + ref, diags := Ref(c, ts, anchors, depth, cs, pointer+ids.Ptr("contentSchema"), compile.SubHint(hint, "content")) + return &ref, diags } +// contentKeywords are the 2020-12 content-vocabulary keywords, all three of +// which lower into ir.Encoding: contentEncoding names Encoding.Name, +// contentMediaType names Encoding.MediaType, and contentSchema names +// Encoding.Schema (ir/constraints.go, ir-design §5.3). One list because they +// share one home — a position that reached no Encoding keeps all three, and one +// that reached an Encoding keeps none. +var contentKeywords = []string{"contentEncoding", "contentMediaType", "contentSchema"} + // declaresContentVocabulary reports whether s writes any content-vocabulary // keyword, so a position that wrote one owns a node to keep it on. func declaresContentVocabulary(s *oas3.Schema) bool { - return declaresContent(s) || s.GetContentSchema() != nil + return s.GetContentEncoding() != "" || s.GetContentMediaType() != "" || + s.GetContentSchema() != nil } // recordUnplacedContent keeps each content keyword verbatim on p, for a position @@ -1483,7 +1506,7 @@ func declaresContentVocabulary(s *oas3.Schema) bool { // node the position actually lowered to instead of re-deriving lower()'s // dispatch, so the two cannot drift apart. func recordUnplacedContent(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, td ir.TypeDef, pointer string) []ir.Diagnostic { - if !declaresContent(s) || scalarHasEncoding(td) { + if !declaresContentVocabulary(s) || scalarHasEncoding(td) { return nil } var diags []ir.Diagnostic @@ -1494,8 +1517,8 @@ func recordUnplacedContent(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, td i continue } diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, pointer+ids.Ptr(keyword), - "%s encodes a string value and this position lowered to a shape with no "+ - "Encoding field; kept verbatim under Unmodeled", keyword)) + "%s is content-vocabulary data the IR holds in ir.Encoding, and this position "+ + "lowered to a shape with no Encoding field; kept verbatim under Unmodeled", keyword)) } return diags } diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index 0c625104..2f4f6b14 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -2958,8 +2958,9 @@ func compileVocabIR(t *testing.T, schemas string) string { // TestContentVocabulary_LowersToEncoding pins where the 2020-12 content // vocabulary lands: contentEncoding on Encoding.Name, contentMediaType on -// Encoding.MediaType, on the Scalar node the position hoists rather than on the -// shared primitive every other declaration of that type also resolves to. +// Encoding.MediaType and contentSchema on Encoding.Schema, on the Scalar node +// the position hoists rather than on the shared primitive every other +// declaration of that type also resolves to. func TestContentVocabulary_LowersToEncoding(t *testing.T) { t.Parallel() cases := []struct { @@ -2997,6 +2998,30 @@ func TestContentVocabulary_LowersToEncoding(t *testing.T) { at: componentID("A"), want: ir.Encoding{Name: "base64", WireType: &ir.TypeRef{Target: "t/prim/string"}}, }, + { + name: "contentSchema beside contentMediaType", + schemas: " A: {type: string, contentMediaType: application/json, contentSchema: {type: object, properties: {id: {type: string}}}}\n", + at: componentID("A"), + want: ir.Encoding{ + MediaType: "application/json", + Schema: &ir.TypeRef{Target: "t/anon/components/schemas/A/contentSchema"}, + }, + }, + { + name: "contentSchema alone still hoists the scalar that holds it", + schemas: " A: {type: array, items: {type: string, contentSchema: {type: object}}}\n", + at: "t/anon/components/schemas/A/items", + want: ir.Encoding{ + Schema: &ir.TypeRef{Target: "t/anon/components/schemas/A/items/contentSchema"}, + }, + }, + { + name: "contentSchema naming a component resolves to it", + schemas: " A: {type: string, contentSchema: {$ref: '#/components/schemas/B'}}\n" + + " B: {type: object, properties: {id: {type: string}}}\n", + at: componentID("A"), + want: ir.Encoding{Schema: &ir.TypeRef{Target: componentID("B")}}, + }, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { @@ -3009,6 +3034,10 @@ func TestContentVocabulary_LowersToEncoding(t *testing.T) { require.NotNil(t, sc.Encoding) assert.Empty(t, cmp.Diff(tc.want, *sc.Encoding)) assert.Empty(t, sc.Unmodeled, "a keyword with a field is lowered, never also kept raw") + if tc.want.Schema != nil { + assert.Contains(t, doc.Types, tc.want.Schema.Target, + "contentSchema reaches the registry as a type, not a raw blob") + } }) } } @@ -3055,9 +3084,10 @@ func TestContentVocabulary_KeepsTheBoundsWrittenBesideIt(t *testing.T) { } } -// TestContentVocabulary_KeptWhereNoEncodingHolds covers the other half: a schema -// with no Encoding field to fill, and contentSchema, which has no IR field at any -// position. +// TestContentVocabulary_KeptWhereNoEncodingHolds covers the other half: a +// position that lowered to a shape with no Encoding field to fill keeps every +// content keyword verbatim, contentSchema included — the three share one home, +// so they are kept or lowered together. func TestContentVocabulary_KeptWhereNoEncodingHolds(t *testing.T) { t.Parallel() cases := []struct { @@ -3070,8 +3100,8 @@ func TestContentVocabulary_KeptWhereNoEncodingHolds(t *testing.T) { key: "openapi:contentMediaType", wantJSON: `"application/zip"`, at: componentID("A"), }, { - name: "contentSchema has no field anywhere", - schemas: " A: {type: string, contentSchema: {type: object}}\n", + name: "object position has no Encoding for contentSchema either", + schemas: " A: {type: object, properties: {p: {type: string}}, contentSchema: {type: object}}\n", key: "openapi:contentSchema", wantJSON: `{"type":"object"}`, at: componentID("A"), }, { diff --git a/docs/ir-design.md b/docs/ir-design.md index 40f7be3c..c68af204 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -665,11 +665,13 @@ is what turns "irreducible" into a confident expansion. Two neighbouring keyword groups are *not* part of this boundary, and are recorded here so their treatment is stated rather than assumed: -- The content vocabulary is modelled, not preserved. `contentEncoding` lowers to `Encoding.Name` - and `contentMediaType` to `Encoding.MediaType` (§5.3) on the scalar the position lowers to; - where the position lowers to a shape with no `Encoding` field, both stay verbatim under - `ReasonNoIRHome`. `contentSchema` is real data shape with no IR field at any position, so it is - always verbatim under `ReasonNoIRHome` — a gap expected to close, not a boundary (§12). +- The content vocabulary is modelled, not preserved. All three keywords share one home, so a + position keeps them all or lowers them all: `contentEncoding` to `Encoding.Name`, + `contentMediaType` to `Encoding.MediaType`, and `contentSchema` to `Encoding.Schema` (§5.3) on + the scalar the position lowers to. `contentSchema`'s value is a schema and lowers like one — + hoisted at its own source pointer and referenced by ID — rather than riding along as a payload + a consumer would have to re-parse. Where the position lowers to a shape with no `Encoding` + field, all three stay verbatim under `ReasonNoIRHome`. - `$id`, `$schema` and `$vocabulary` identify and configure a JSON Schema *resource*. The IR identifies every type by a synthetic ID derived from its source pointer rather than by `$id` (§3), and describes one API surface rather than a resource graph, so it has no dialect axis and @@ -1016,11 +1018,16 @@ type Encoding struct { // (utcDateTime encoded as int32; bytes as base64 string) MediaType string // content media type of the value itself (Smithy @mediaType on string/blob, // JSON Schema contentMediaType); "" = none + Schema *TypeRef // the shape the encoded value has once decoded (JSON Schema contentSchema): + // what a base64 blob or an application/json-typed string holds; nil = unstated } ``` The logical-type / encoding-name / wire-type triple is TCGC's reification of TypeSpec `@encode` -and also absorbs OpenAPI `format` and Protobuf's `sint*/fixed*` wire variants. Encoding attaches +and also absorbs OpenAPI `format` and Protobuf's `sint*/fixed*` wire variants. `MediaType` and +`Schema` sit beside it because they answer the same question one level in: what the encoded value +*is*. `Schema` is a `TypeRef` like any other schema position — the decoded shape is hoisted into +the registry at its own source pointer, never carried here as a raw payload. Encoding attaches at the scalar definition or overrides at the property — property wins. Protobuf editions features lower here per element after the compiler resolves the feature cascade (descriptors expose resolved values): `field_presence` → `Presence`, `enum_type` → `Closed`, @@ -1938,7 +1945,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; per-status responses/default → Conditions + ranges, with the responses-map key as declared and then neutralized → Response.Name.Hint and ErrorCase.Name.Hint alike (`404`, `5_xx`, `default`), which records the spelling a range cannot state though only `default` survives neutralization unchanged; two keys resolving to one range — `4XX` beside `4xx` — are both kept and reported `openapi/duplicate-status-key`, since they reach the IR with one name and one condition; an error response lowers exactly as a success one — its `headers` → ErrorCase.Headers and every media type of its `content` → ErrorCase.Payload.Contents, neither degraded and neither kept under Unmodeled, and the payload's naming hint derived from the declaration pointer on both sides so that one `components/responses` entry mounted at a success and an error status interns one type whichever side reaches it first; response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; per-status responses/default → Conditions + ranges, with the responses-map key as written → Response.Name.Hint and ErrorCase.Name.Hint alike, so `4XX` and `default` round-trip the spelling their range cannot state; an error response lowers exactly as a success one — its `headers` → ErrorCase.Headers and every media type of its `content` → ErrorCase.Payload.Contents, neither degraded and neither kept under Unmodeled; response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType`/`contentSchema` → `Encoding` on the scalar the position lowers to — the last as `Encoding.Schema`, a TypeRef to the decoded shape hoisted at its own pointer — and all three → Unmodeled (`no_ir_home`) at a position with no `Encoding` field, per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled | diff --git a/internal/archtest/recursion_test.go b/internal/archtest/recursion_test.go index 07d709cc..d5ccdf70 100644 --- a/internal/archtest/recursion_test.go +++ b/internal/archtest/recursion_test.go @@ -75,14 +75,18 @@ var loweringRecursions = [][]string{ // The two exported names are the walk's entry points, which is what the // operation lowering reaches it by; the rest of the set is unexported because // nothing outside the schema package has any business entering mid-walk. +// The scalar hoisters and the encoding reader joined it when contentSchema +// gained an IR home: its value is a schema, so lowering it re-enters the walk +// from a scalar position, which until then was the walk's one leaf. var schemaRecursion = []string{ "CarriedRef", "Ref", "buildComposedVariant", "buildTuple", - "composedVariant", "fillAdditional", "fillAllOf", "fillModelProperties", - "hoistSubSchema", "lower", "lowerAllOf", "lowerArray", + "composedVariant", "contentSchemaRef", "fillAdditional", "fillAllOf", + "fillModelProperties", "hoistByteScalar", "hoistContentScalar", + "hoistFormatScalar", "hoistSubSchema", "lower", "lowerAllOf", "lowerArray", "lowerBesideUnmodeledUnion", "lowerCoDeclaredUnion", "lowerDistributedUnion", "lowerModel", "lowerOneOfAnyOf", "lowerSchemaBody", "lowerTyped", "lowerUnion", "lowerUntyped", "patternProps", "refSiteRef", "refTypeRef", "resolveSchemaRef", - "schemaBody", "schemaRefHomed", + "scalarEncoding", "scalarTypeID", "schemaBody", "schemaRefHomed", } // loweringPackages are the directories whose sources the call graph reads, diff --git a/ir/constraints.go b/ir/constraints.go index a84f4880..338dabbd 100644 --- a/ir/constraints.go +++ b/ir/constraints.go @@ -42,8 +42,9 @@ type Constraints struct { } // Encoding is the logical-type / encoding-name / wire-type triple that reifies -// TypeSpec @encode and absorbs OpenAPI format and Protobuf wire variants -// (ir-design §5.3). Property encoding overrides scalar encoding. +// TypeSpec @encode and absorbs OpenAPI format and Protobuf wire variants, plus +// the media type and decoded shape of an encoded payload (ir-design §5.3). +// Property encoding overrides scalar encoding. type Encoding struct { // Name is the encoding scheme ("rfc3339", "base64", "zigzag", "packed", // "delimited", format strings, ...). @@ -54,6 +55,11 @@ type Encoding struct { // MediaType is the content media type of the value itself (Smithy @mediaType, // JSON Schema contentMediaType); "" = none. MediaType string `json:"mediaType,omitempty"` + // Schema is the shape the encoded value has once decoded — what a base64 blob + // or an application/json-typed string holds (JSON Schema contentSchema); nil = + // unstated. It is a reference into the type registry like any other schema, + // never the encoded value's own type. + Schema *TypeRef `json:"schema,omitempty"` } // XMLHints describes an XML wire shape that diverges from the JSON-implied one diff --git a/testdata/conformance/openapi/content-vocabulary.golden.json b/testdata/conformance/openapi/content-vocabulary.golden.json index 4da2e98a..e546670a 100644 --- a/testdata/conformance/openapi/content-vocabulary.golden.json +++ b/testdata/conformance/openapi/content-vocabulary.golden.json @@ -18,6 +18,52 @@ } ], "types": { + "t/anon/components/schemas/Envelope/contentSchema": { + "kind": "model", + "id": "t/anon/components/schemas/Envelope/contentSchema", + "name": { + "hint": "envelope_content" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Envelope/contentSchema" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Envelope/contentSchema/properties/id", + "name": { + "source": "id", + "canonical": "id" + }, + "wireName": "id", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Envelope/contentSchema/properties/id" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/Bag": { "kind": "model", "id": "t/openapi/components/schemas/Bag", @@ -44,6 +90,16 @@ "source": 0, "pointer": "/components/schemas/Bag/contentMediaType" } + }, + "openapi:contentSchema": { + "reason": "no_ir_home", + "value": { + "type": "object" + }, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Bag/contentSchema" + } } }, "provenance": { @@ -93,23 +149,6 @@ "anonymous": false, "docs": {}, "sensitive": false, - "unmodeled": { - "openapi:contentSchema": { - "reason": "no_ir_home", - "value": { - "properties": { - "id": { - "type": "string" - } - }, - "type": "object" - }, - "provenance": { - "source": 0, - "pointer": "/components/schemas/Envelope/contentSchema" - } - } - }, "provenance": { "source": 0, "pointer": "/components/schemas/Envelope" @@ -119,7 +158,11 @@ "nullable": false }, "encoding": { - "mediaType": "application/json" + "mediaType": "application/json", + "schema": { + "target": "t/anon/components/schemas/Envelope/contentSchema", + "nullable": false + } } }, "t/openapi/components/schemas/Thumbnail": { @@ -172,28 +215,28 @@ { "severity": "info", "code": "openapi/degraded-construct", - "message": "contentSchema is the shape of the decoded content and no IR position has a field for it; kept verbatim under Unmodeled", + "message": "contentEncoding is content-vocabulary data the IR holds in ir.Encoding, and this position lowered to a shape with no Encoding field; kept verbatim under Unmodeled", "provenance": { "source": 0, - "pointer": "/components/schemas/Envelope/contentSchema" + "pointer": "/components/schemas/Bag/contentEncoding" } }, { "severity": "info", "code": "openapi/degraded-construct", - "message": "contentEncoding encodes a string value and this position lowered to a shape with no Encoding field; kept verbatim under Unmodeled", + "message": "contentMediaType is content-vocabulary data the IR holds in ir.Encoding, and this position lowered to a shape with no Encoding field; kept verbatim under Unmodeled", "provenance": { "source": 0, - "pointer": "/components/schemas/Bag/contentEncoding" + "pointer": "/components/schemas/Bag/contentMediaType" } }, { "severity": "info", "code": "openapi/degraded-construct", - "message": "contentMediaType encodes a string value and this position lowered to a shape with no Encoding field; kept verbatim under Unmodeled", + "message": "contentSchema is content-vocabulary data the IR holds in ir.Encoding, and this position lowered to a shape with no Encoding field; kept verbatim under Unmodeled", "provenance": { "source": 0, - "pointer": "/components/schemas/Bag/contentMediaType" + "pointer": "/components/schemas/Bag/contentSchema" } } ], @@ -201,7 +244,7 @@ { "format": "openapi@3.1", "path": "content-vocabulary.yaml", - "hash": "b037c671833d74933ede0b2e0dfa2c22ed381955c1cfdec83963882ebf1db6a7" + "hash": "3ef4175a9903ee435b730ca49ade19bec243e6194599dcd221212ea6850fd81c" } ] } diff --git a/testdata/conformance/openapi/content-vocabulary.yaml b/testdata/conformance/openapi/content-vocabulary.yaml index 676a0eb6..df32f2f9 100644 --- a/testdata/conformance/openapi/content-vocabulary.yaml +++ b/testdata/conformance/openapi/content-vocabulary.yaml @@ -8,8 +8,8 @@ components: type: string contentEncoding: base64 contentMediaType: image/png - # contentSchema is a schema, not an encoding, so it has no IR home at any - # position and is kept verbatim beside the encoding that did lower. + # contentSchema is a schema, so it lowers like one: hoisted at its own + # pointer and reached from Encoding.Schema by ID, never a raw blob. Envelope: type: string contentMediaType: application/json @@ -17,10 +17,12 @@ components: type: object properties: id: {type: string} - # An object position has no Encoding field at all, so both keywords are kept. + # An object position has no Encoding field at all, so all three are kept. Bag: type: object contentEncoding: base64 contentMediaType: application/json + contentSchema: + type: object properties: a: {type: string} From 75394232ab8dd129638620da291491f1f2047a33 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Tue, 8 Sep 2026 00:13:56 +0300 Subject: [PATCH 2/6] docs(ir): say constraints never cross a $ref to a use site MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At a $ref use site the compiler merges the referent's documentation, deprecation and default onto the referencing Property/Parameter, and leaves its constraints where they were declared. The split was deliberate and tested but written down nowhere a consumer reads, so an empty Constraints at a use site could be read as "this value is unbounded" when it means "this position declared no bound". The split is kept, because the two halves are not the same kind of fact. An annotation is a single value one position may restate for another, so use-site precedence is the only sensible rule and applying it once in the compiler keeps every carrier alike. A bound is not: maxLength 64 on the referent and maxLength 100 beside the $ref are both in force and the narrower wins, so merging under use-site precedence would publish 100 as the whole truth and lose the bound the document enforces. What changes is that the rule is now stated where it is read: a new ir-design §12.2, the Constraints, Property.Constraints, Parameter.Constraints and TypeRef field docs, and the two lowering sites that implement it. An absent Constraints at a use site means that position declared no bound; the effective bound is its conjunction with every node reached from its TypeRef. param-ref-inheritance now declares a bound beside the $ref at both carriers, so the split is witnessed rather than merely absent: the use site's 100 lands on the carrier, the referent's 64 stays on the referent, and the case reddens if either is copied onto the other. Fixes #428 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- compilers/openapi/conformance_test.go | 18 ++++++- .../openapi/internal/operation/params.go | 4 ++ compilers/openapi/internal/schema/schema.go | 5 ++ docs/ir-design.md | 30 ++++++++++- ir/constraints.go | 10 ++++ ir/operation.go | 7 ++- ir/property.go | 7 ++- ir/typeref.go | 7 +++ .../openapi/param-ref-inheritance.golden.json | 52 ++++++++++++++++++- .../openapi/param-ref-inheritance.yaml | 11 +++- 10 files changed, 144 insertions(+), 7 deletions(-) diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index ae62cc31..95d9bed2 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -2150,6 +2150,12 @@ func assertParamQuerystring(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { // silent, and from the use site when it is not. Constraints inherit at neither // carrier, so the identical property is asserted beside it — a parameter must not // take more from a referent than a property does (GitHub #131). +// +// The bound the use site declares beside the $ref is asserted at both carriers +// too, against the referent's own: it is what makes the split observable rather +// than merely absent, and it is the case use-site precedence would get wrong, +// publishing 100 as the whole truth while the document enforces 64 (§12.2, +// GitHub #428). func assertParamRefInheritance(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { op, ok := opByName(doc, "listItems") require.True(t, ok) @@ -2169,6 +2175,10 @@ func assertParamRefInheritance(t *testing.T, doc *ir.Document, _ []ir.Diagnostic "...and a keyword the use site is silent about still inherits") require.NotNil(t, override.Default) assert.Equal(t, "9", override.Default.Str) + require.NotNil(t, override.Constraints, "a bound beside the $ref lands on the carrier") + require.NotNil(t, override.Constraints.MaxLength) + assert.Equal(t, int64(100), *override.Constraints.MaxLength, + "and it is the use site's own, not narrowed against the referent's here") holder, ok := doc.Types[namedID("Holder")].(*ir.Model) require.True(t, ok) @@ -2180,12 +2190,18 @@ func assertParamRefInheritance(t *testing.T, doc *ir.Document, _ []ir.Diagnostic assert.Equal(t, *cursor.Default, *prop.Default) assert.Nil(t, prop.Constraints, "neither carrier inherits the referent's constraints") + overrideProp, ok := propByWire(holder, "override") + require.True(t, ok) + assert.Equal(t, override.Constraints, overrideProp.Constraints, + "and a property keeps its own bound exactly as the parameter does") + decl, ok := doc.Types[namedID("Cursor")].(*ir.Scalar) require.True(t, ok) require.NotNil(t, decl.Constraints) require.NotNil(t, decl.Constraints.MaxLength) assert.Equal(t, int64(64), *decl.Constraints.MaxLength, - "a consumer that wants the bound reads it off the referent") + "a consumer that wants the bound reads it off the referent, and conjoins "+ + "it with the use site's: both are in force, and 64 is the narrower") } // assertHeaderContentSchema pins that both spellings of a header's type lower diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index 5c60d92f..c897ffa3 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -114,6 +114,10 @@ func fillParamType(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde // inherit from it still reach the parameter (ir-design §14, GitHub #131). // Constraints stay use-site-only, exactly as fillPropertyConstraints keeps // them: a parameter must not inherit more from a referent than a property does. +// The referent's bounds are not dropped, they are simply left where they were +// declared — bounds conjoin rather than override, so copying one down under +// use-site precedence would publish the wider bound as the whole truth +// (ir-design §12.2). func fillParamSchema(c lowering.Ctx, ts *compile.Types, param *ir.Parameter, js *oas3.JSONSchema[oas3.Referenceable], pointer string) []ir.Diagnostic { if js == nil || !js.IsSchema() { return nil diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index ee72f6f7..72c1b3c9 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1203,6 +1203,11 @@ func fillPropertyDefault(c lowering.Ctx, p *ir.Property, ref, tgt *oas3.Schema, // co-declared bound keyword that reached none of them, to the property itself. // ir.Property is the carrier at this position: a property's schema is read // through CarriedRef, so it hoists no node of its own to hold either. +// +// It reads ref alone and never the $ref target, which is why no tgt reaches it: +// bounds conjoin rather than override, so a referent's bound merged here under +// use-site precedence would publish the wider of the two as the whole truth. It +// stays on the node the reference points at instead (ir-design §12.2). func fillPropertyConstraints(c lowering.Ctx, p *ir.Property, ref *oas3.Schema, pointer string) []ir.Diagnostic { cons, diags := schemaConstraints(c, &p.Unmodeled, ref, pointer) if cons != nil { diff --git a/docs/ir-design.md b/docs/ir-design.md index c68af204..9c9e39fa 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1909,6 +1909,34 @@ and `Parameter.Constraints` are filled whether or not the schema hoisted a node, such a schema can hoist holds them: an object lowers to a `Model` and a formatted scalar to a `Scalar` carrying an `Encoding`, and neither has a field for the scalar bounds. +### 12.2 What a reference carries down, and what it does not + +A `$ref` divides the declarations at its target in two, and the line falls between annotations and +constraints. + +**Annotations merge onto the use site.** Documentation, deprecation, visibility and `default` are +read off the referent and written onto the referencing `Property`/`Parameter` with **use-site +precedence** (§14), field by field. Each is a single value that one position may restate for +another — a `description` beside the `$ref` says what *this* input is, replacing the target's — so +the two can never both be true at once, and any consumer reading both would need this precedence +rule anyway. Applying it once, in the compiler, is what keeps every carrier alike. + +**Constraints do not merge, and are never copied to a use site.** Bounds *conjoin*: `maxLength: 64` on the +referent and `maxLength: 100` beside the `$ref` are both in force, and the admitted value is the +narrower of the two. There is no precedence to apply — merging with use-site precedence would +publish `100` as the whole truth and lose the bound the document actually enforces. So +`Property.Constraints` and `Parameter.Constraints` hold what their own position declared and +nothing else, and every node reached through `TypeRef` keeps its own. + +The rule a consumer needs follows from that, and it is the reason this is written down rather than +left to be inferred from an empty struct: **an absent `Constraints` at a use site means that +position declared no bound, never that the value is unbounded.** The effective bound is the +conjunction of the use site's own `Constraints` with those of every node reached from its +`TypeRef`; a consumer that wants it — a validator emitter, a differ comparing two revisions — +resolves the reference and intersects. A differ that reads use sites alone sees no change when a +shared component's `maxLength` moves, because the change is at the component, which is the one +place it was declared and the one place it is recorded. + ## 13. Provenance & diagnostics ```go @@ -1945,7 +1973,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; per-status responses/default → Conditions + ranges, with the responses-map key as written → Response.Name.Hint and ErrorCase.Name.Hint alike, so `4XX` and `default` round-trip the spelling their range cannot state; an error response lowers exactly as a success one — its `headers` → ErrorCase.Headers and every media type of its `content` → ErrorCase.Payload.Contents, neither degraded and neither kept under Unmodeled; response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType`/`contentSchema` → `Encoding` on the scalar the position lowers to — the last as `Encoding.Schema`, a TypeRef to the decoded shape hoisted at its own pointer — and all three → Unmodeled (`no_ir_home`) at a position with no `Encoding` field, per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; per-status responses/default → Conditions + ranges, with the responses-map key as written → Response.Name.Hint and ErrorCase.Name.Hint alike, so `4XX` and `default` round-trip the spelling their range cannot state; an error response lowers exactly as a success one — its `headers` → ErrorCase.Headers and every media type of its `content` → ErrorCase.Payload.Contents, neither degraded and neither kept under Unmodeled; response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType`/`contentSchema` → `Encoding` on the scalar the position lowers to — the last as `Encoding.Schema`, a TypeRef to the decoded shape hoisted at its own pointer — and all three → Unmodeled (`no_ir_home`) at a position with no `Encoding` field, per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3 — constraints excepted, since bounds conjoin rather than override: each position keeps the ones it declared and none is copied to a use site (§12.2); a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled | diff --git a/ir/constraints.go b/ir/constraints.go index 338dabbd..4066fdfc 100644 --- a/ir/constraints.go +++ b/ir/constraints.go @@ -3,6 +3,16 @@ package ir // Constraints restricts the admissible values of a scalar, list, string, or // numeric type (ir-design §5.3). Numeric bounds are arbitrary-precision decimal // strings, never float64. +// +// Every Constraints is position-scoped: it holds what the position carrying it +// declared, and nothing is ever copied across a TypeRef. Bounds conjoin rather +// than override, so the effective restriction on a value is this struct +// together with the Constraints of every node reached from the position's +// TypeRef, and an absent Constraints means that position declared no bound — +// never that the value is unbounded (ir-design §12.2). Documentation, +// deprecation and Default are the other way round: a compiler merges them from +// a $ref's target onto the referencing carrier with use-site precedence, so a +// use site already carries those and resolves nothing to read them. type Constraints struct { // Min is the inclusive (or exclusive, per ExclusiveMin) lower numeric bound. Min *BigVal `json:"min,omitempty"` diff --git a/ir/operation.go b/ir/operation.go index 51510616..c1e2e092 100644 --- a/ir/operation.go +++ b/ir/operation.go @@ -77,7 +77,12 @@ type Parameter struct { Required bool `json:"required"` // Default is the parameter's default value. Default *Value `json:"default,omitempty"` - // Constraints restricts the parameter's admissible values. + // Constraints restricts the parameter's admissible values, and holds only + // what the parameter's own position declared. A bound on a $ref'd schema + // stays on the node Type points at and is never copied here, unlike Docs, + // Deprecation and Default, which merge from that target with use-site + // precedence: bounds conjoin rather than override, so nil means this + // position declared none, not that the value is unbounded (ir-design §12.2). Constraints *Constraints `json:"constraints,omitempty"` // ValueFrom derives the parameter's value from a location in the // outgoing/incoming message (AsyncAPI parameter location runtime diff --git a/ir/property.go b/ir/property.go index b9f5177b..78894a5a 100644 --- a/ir/property.go +++ b/ir/property.go @@ -88,7 +88,12 @@ type Property struct { Visibility Visibility `json:"visibility"` // Default is the property's default value. Default *Value `json:"default,omitempty"` - // Constraints restricts the property's admissible values. + // Constraints restricts the property's admissible values, and holds only + // what the property's own position declared. A bound on a $ref'd schema + // stays on the node Type points at and is never copied here, unlike Docs, + // Deprecation and Default, which merge from that target with use-site + // precedence: bounds conjoin rather than override, so nil means this + // position declared none, not that the value is unbounded (ir-design §12.2). Constraints *Constraints `json:"constraints,omitempty"` // Encoding overrides the property's wire encoding. Encoding *Encoding `json:"encoding,omitempty"` diff --git a/ir/typeref.go b/ir/typeref.go index 6b0bc56a..1366b667 100644 --- a/ir/typeref.go +++ b/ir/typeref.go @@ -5,6 +5,13 @@ package ir // not the target type, because the same type is nullable in one position and not // another; combined with Property.Required it yields the four distinct // required/optional × nullable/non-null states. +// +// A TypeRef carries no fact of the target's down with it. What a use site can +// read without resolving Target is only what a compiler already merged onto the +// carrier — a Property's or Parameter's Docs, Deprecation and Default, taken +// from a $ref's target with use-site precedence. Everything else the target +// declares, Constraints above all, is read from the target node itself +// (ir-design §12.2). type TypeRef struct { // Target identifies the referenced TypeDef in Document.Types. Target TypeID `json:"target"` diff --git a/testdata/conformance/openapi/param-ref-inheritance.golden.json b/testdata/conformance/openapi/param-ref-inheritance.golden.json index 68924546..e1329165 100644 --- a/testdata/conformance/openapi/param-ref-inheritance.golden.json +++ b/testdata/conformance/openapi/param-ref-inheritance.golden.json @@ -70,6 +70,12 @@ "list": null, "object": null }, + "constraints": { + "exclusiveMin": false, + "exclusiveMax": false, + "maxLength": 100, + "uniqueItems": false + }, "docs": { "summary": "Cursor", "description": "Cursor for this endpoint only." @@ -237,6 +243,50 @@ "source": 0, "pointer": "/components/schemas/Holder/properties/cursor" } + }, + { + "id": "p/openapi/components/schemas/Holder/properties/override", + "name": { + "source": "override", + "canonical": "override" + }, + "wireName": "override", + "type": { + "target": "t/openapi/components/schemas/Cursor", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "default": { + "kind": "string", + "str": "0", + "bytes": null, + "list": null, + "object": null + }, + "constraints": { + "exclusiveMin": false, + "exclusiveMax": false, + "maxLength": 100, + "uniqueItems": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": { + "summary": "Cursor", + "description": "Opaque pagination cursor." + }, + "deprecation": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Holder/properties/override" + } } ], "abstract": false, @@ -281,7 +331,7 @@ { "format": "openapi@3.1", "path": "param-ref-inheritance.yaml", - "hash": "ceaabaa7016e71d9190b69c6ae61fb9a8388ccfc6f7cb9069bd067d65fab5d2a" + "hash": "05b17aae85704e581a282689a5cdb4c3b157bb884d6a23035b1f44e2d13b7d42" } ] } diff --git a/testdata/conformance/openapi/param-ref-inheritance.yaml b/testdata/conformance/openapi/param-ref-inheritance.yaml index 98040de6..7caa3611 100644 --- a/testdata/conformance/openapi/param-ref-inheritance.yaml +++ b/testdata/conformance/openapi/param-ref-inheritance.yaml @@ -14,6 +14,10 @@ paths: $ref: '#/components/schemas/Cursor' description: Cursor for this endpoint only. default: "9" + # A bound is the one keyword the use site does not take over: this + # one lands here and the referent's 64 stays on the referent, both + # in force, since bounds conjoin rather than override. + maxLength: 100 responses: "200": description: ok @@ -26,9 +30,12 @@ components: deprecated: true default: "0" maxLength: 64 - # The property form of the same reference: what a parameter inherits, a - # property inherits identically, constraints included in neither. + # The property form of the same references: what a parameter inherits, a + # property inherits identically, and constraints cross to neither. Holder: type: object properties: cursor: {$ref: '#/components/schemas/Cursor'} + override: + $ref: '#/components/schemas/Cursor' + maxLength: 100 From 443bca82aa61c6576ebe81e5c381b2d1d13d58ff Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Tue, 8 Sep 2026 00:39:44 +0300 Subject: [PATCH 3/6] fix(ir)!: carry minimum and exclusiveMinimum as separate bounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In JSON Schema 2020-12 minimum and exclusiveMinimum are independent keywords that both apply; a schema may legally declare both, and the same holds on the upper side. ir.Constraints held one bound plus one exclusivity flag per side, so a co-declared pair had to be reconciled: the tighter keyword took the slot and the other was kept verbatim under Unmodeled as degraded_lowering. The loser was preserved, so nothing was lost outright. But a consumer comparing constraints across two revisions of a spec reads the fields, not the Unmodeled map: a revision that moved only the dropped keyword read as no change, and one that swapped which keyword was tighter read as a change of a different kind than the one that happened (#425). ir.Constraints now holds four bounds — Min, ExclusiveMin, Max, ExclusiveMax, each a *BigVal, each the keyword of the same name. A co-declared pair reaches two fields, keeps nothing beside them, and reports nothing: there is no degradation left to announce. The reconciliation, its exact-decimal tighter-of-two comparison, and its diagnostics are gone; merge adopts and compares each of the four the way it already did multipleOf. BREAKING CHANGE: ExclusiveMin and ExclusiveMax change from bool to *BigVal and their JSON keys gain omitempty, so `"exclusiveMin": false` no longer appears and an exclusive bound serializes as its literal rather than as a flag on `min`. The OpenAPI 3.0 spelling — a boolean modifying the minimum beside it — now lowers to the bound it means: `{minimum: 5, exclusiveMinimum: true}` becomes ExclusiveMin "5" with Min unset, which is what the 3.1 spelling of the same restriction produces, so a 3.0 document and its 3.1 translation no longer differ in the IR. A 3.0 modifier written with no bound to modify (invalid under draft-4, and unchecked by the loader) is kept verbatim under Unmodeled and reported as a warning rather than setting a flag over an absent bound. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- compilers/openapi/conformance_test.go | 69 ++-- .../openapi/constraints_internal_test.go | 27 +- .../internal/annotation/constraints.go | 229 ++++------- .../annotation/constraints_internal_test.go | 2 +- .../constraints_readers_internal_test.go | 387 +++++++----------- .../annotation/decimal_internal_test.go | 12 +- .../internal/merge/conflict_internal_test.go | 17 +- compilers/openapi/internal/merge/merge.go | 76 ++-- .../internal/merge/reconcile_internal_test.go | 65 ++- .../openapi/internal/operation/params_test.go | 72 ++-- .../openapi/internal/schema/compose_test.go | 9 +- .../openapi/internal/schema/schema_test.go | 165 +++++--- docs/ir-design.md | 6 +- ir/constraints.go | 25 +- ir/constraints_test.go | 13 +- ir/helpers_test.go | 12 +- .../allof-oneof-cooccurrence.golden.json | 2 - .../allof-ref-branch-siblings.golden.json | 2 - .../openapi/constraints.golden.json | 80 +--- testdata/conformance/openapi/constraints.yaml | 17 +- .../openapi/encoding-byte.golden.json | 4 - .../openapi/header-content-schema.golden.json | 6 - .../openapi/inline-annotations.golden.json | 10 - .../openapi/multipart-encoding.golden.json | 2 - .../openapi/numeric-precision.golden.json | 10 +- .../openapi/param-ref-inheritance.golden.json | 6 - .../openapi/scalar-format.golden.json | 4 - .../openapi/unhomed-keywords.golden.json | 4 - 28 files changed, 586 insertions(+), 747 deletions(-) diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 95d9bed2..d2e702f1 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -1510,62 +1510,63 @@ func assertConstraints(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { } // assertCoDeclaredBounds pins the 2020-12 rule that a side declaring both of -// its keywords keeps the tighter of the two: the property bounded below keeps -// its minimum, the one bounded above keeps its exclusiveMaximum, and each side -// names the keyword that did not reach ir.Constraints (GitHub #33). +// its keywords carries both: minimum and exclusiveMinimum are independent and +// conjunctive, ir.Constraints has a field for each, and neither is chosen over +// the other (GitHub #33, #425). // -// Both directions are here on purpose. A case where only the exclusive keyword -// survives passes just as well on the reader that always took it, so on its own -// it would say nothing about the fix. +// Both directions are here on purpose. The property bounded below has the +// inclusive keyword as its tighter bound and the one bounded above the +// exclusive one, so a reader that kept the tighter alone answers the two +// differently — and a reader that always kept the exclusive keyword passes the +// second on its own. func assertCoDeclaredBounds(t *testing.T, m *ir.Model, diags []ir.Diagnostic) { t.Helper() low, ok := propByWire(m, "atLeastTen") require.True(t, ok) require.NotNil(t, low.Constraints) require.NotNil(t, low.Constraints.Min) - assert.Equal(t, ir.BigVal("10"), *low.Constraints.Min, "minimum is the tighter bound") - assert.False(t, low.Constraints.ExclusiveMin, "and it is inclusive as written") + require.NotNil(t, low.Constraints.ExclusiveMin) + assert.Equal(t, ir.BigVal("10"), *low.Constraints.Min, "minimum as written") + assert.Equal(t, ir.BigVal("0"), *low.Constraints.ExclusiveMin, + "and the looser exclusiveMinimum beside it, not dropped for being implied") high, ok := propByWire(m, "underTen") require.True(t, ok) require.NotNil(t, high.Constraints) require.NotNil(t, high.Constraints.Max) - assert.Equal(t, ir.BigVal("10"), *high.Constraints.Max, "exclusiveMaximum is the tighter bound") - assert.True(t, high.Constraints.ExclusiveMax) + require.NotNil(t, high.Constraints.ExclusiveMax) + assert.Equal(t, ir.BigVal("100"), *high.Constraints.Max, "maximum as written") + assert.Equal(t, ir.BigVal("10"), *high.Constraints.ExclusiveMax, "and exclusiveMaximum beside it") - for _, want := range []string{"exclusiveMinimum, which it implies", "maximum, which it implies"} { - assert.True(t, slices.ContainsFunc(diags, func(d ir.Diagnostic) bool { - return strings.Contains(d.Message, want) - }), "the keyword ir.Constraints has no room for is named, not dropped in silence: %q", want) + for _, d := range diags { + assert.NotContains(t, d.Message, "exclusiveMinimum", + "a pair that reaches two fields is not a degradation to report") } } -// assertCoDeclaredBoundKept is the losslessness half of the same rule -// (GitHub #286): a keyword named only in a diagnostic reaches no field of the -// document a downstream stage reads, so {minimum: 10, exclusiveMinimum: 0} and -// {minimum: 10} lowered identically. It is kept verbatim on whichever carrier -// read it — the property here, the alias node a component's body reduces to -// below — beside the constraints it did not reach. +// assertCoDeclaredBoundKept is the losslessness half of the same rule: with a +// field per keyword there is nothing left over, so neither carrier keeps a bound +// verbatim. Nothing is restated beside constraints that hold it all — an entry +// there would give one bound two homes, and {minimum: 10, exclusiveMinimum: 0} +// is told from {minimum: 10} by the fields themselves (GitHub #286). func assertCoDeclaredBoundKept(t *testing.T, doc *ir.Document, m *ir.Model) { t.Helper() low, ok := propByWire(m, "atLeastTen") require.True(t, ok) - entry := unmodeledEntry(t, low.Unmodeled, "openapi:exclusiveMinimum") - assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) - assert.JSONEq(t, "0", string(entry.Value)) - assert.Equal(t, "/components/schemas/S/properties/atLeastTen/exclusiveMinimum", - entry.Provenance.Pointer) + assert.Empty(t, low.Unmodeled, "the property keeps nothing beside its constraints") high, ok := propByWire(m, "underTen") require.True(t, ok) - assert.JSONEq(t, "100", string(unmodeledEntry(t, high.Unmodeled, "openapi:maximum").Value), - "the inclusive keyword is the one kept where the exclusive bound is tighter") + assert.Empty(t, high.Unmodeled, "and neither does the side settled the other way") alias, ok := doc.Types[namedID("Bounded")].(*ir.Scalar) require.True(t, ok, "a component reducing to a shared primitive owns an alias node") require.NotNil(t, alias.Constraints) - assert.JSONEq(t, "0", string(unmodeledEntry(t, alias.Unmodeled, "openapi:exclusiveMinimum").Value), - "a node carries what its constraints had no room for, exactly as a property does") + require.NotNil(t, alias.Constraints.Min) + require.NotNil(t, alias.Constraints.ExclusiveMin) + assert.Equal(t, ir.BigVal("0"), *alias.Constraints.ExclusiveMin, + "a node carries both bounds, exactly as a property does") + assert.Empty(t, alias.Unmodeled) } // assertLengthAndCollectionBounds pins the non-numeric bounds: a string length @@ -1613,12 +1614,10 @@ func assertNumericPrecision(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { exclusive, ok := propByWire(m, "exclusive") require.True(t, ok) require.NotNil(t, exclusive.Constraints) - require.NotNil(t, exclusive.Constraints.Min) - require.NotNil(t, exclusive.Constraints.Max) - assert.True(t, exclusive.Constraints.ExclusiveMin) - assert.True(t, exclusive.Constraints.ExclusiveMax) - assert.Equal(t, ir.BigVal("0.5"), *exclusive.Constraints.Min) - assert.Equal(t, ir.BigVal("0.12345678901234567890123456789"), *exclusive.Constraints.Max) + require.NotNil(t, exclusive.Constraints.ExclusiveMin) + require.NotNil(t, exclusive.Constraints.ExclusiveMax) + assert.Equal(t, ir.BigVal("0.5"), *exclusive.Constraints.ExclusiveMin) + assert.Equal(t, ir.BigVal("0.12345678901234567890123456789"), *exclusive.Constraints.ExclusiveMax) // A default beyond float64 range is captured as a number, not a string. withDefault, ok := propByWire(m, "withDefault") diff --git a/compilers/openapi/constraints_internal_test.go b/compilers/openapi/constraints_internal_test.go index d2ffa1fa..4270f554 100644 --- a/compilers/openapi/constraints_internal_test.go +++ b/compilers/openapi/constraints_internal_test.go @@ -30,10 +30,15 @@ func TestConstraints_ExclusiveBoolean30(t *testing.T) { doc, diags := lowerSpec(t, spec) openapitest.RequireNoErrorDiags(t, diags) c := propConstraints(t, doc, "S", "n") - assert.True(t, c.ExclusiveMin) - assert.True(t, c.ExclusiveMax) - require.NotNil(t, c.Min) - assert.Equal(t, ir.BigVal("5"), *c.Min) + // The modifier is a spelling of the exclusive bound, so the literal it + // modified lands in the exclusive field and the inclusive one is left empty + // — the same constraints the 2020-12 spelling of "x > 5, x < 10" produces. + assert.Nil(t, c.Min, "the modified minimum does not also stay inclusive") + assert.Nil(t, c.Max) + require.NotNil(t, c.ExclusiveMin) + require.NotNil(t, c.ExclusiveMax) + assert.Equal(t, ir.BigVal("5"), *c.ExclusiveMin) + assert.Equal(t, ir.BigVal("10"), *c.ExclusiveMax) } func TestConstraints_ExclusiveNumeric31(t *testing.T) { @@ -49,12 +54,12 @@ func TestConstraints_ExclusiveNumeric31(t *testing.T) { doc, diags := lowerSpec(t, spec) openapitest.RequireNoErrorDiags(t, diags) c := propConstraints(t, doc, "S", "n") - assert.True(t, c.ExclusiveMin) - assert.True(t, c.ExclusiveMax) - require.NotNil(t, c.Min) - require.NotNil(t, c.Max) - assert.Equal(t, ir.BigVal("1.5"), *c.Min) - assert.Equal(t, ir.BigVal("9.5"), *c.Max) + assert.Nil(t, c.Min) + assert.Nil(t, c.Max) + require.NotNil(t, c.ExclusiveMin) + require.NotNil(t, c.ExclusiveMax) + assert.Equal(t, ir.BigVal("1.5"), *c.ExclusiveMin) + assert.Equal(t, ir.BigVal("9.5"), *c.ExclusiveMax) } func TestConstraints_MalformedNumericLiterals(t *testing.T) { @@ -174,7 +179,7 @@ func TestConstraints_ExclusiveWrongDialectForm(t *testing.T) { require.True(t, ok) for _, p := range m.Properties { if p.WireName == "n" && p.Constraints != nil { - assert.False(t, p.Constraints.ExclusiveMin, "wrong-form exclusive bound is not set") + assert.Nil(t, p.Constraints.ExclusiveMin, "wrong-form exclusive bound is not set") } } }) diff --git a/compilers/openapi/internal/annotation/constraints.go b/compilers/openapi/internal/annotation/constraints.go index 8acc5e39..587a1646 100644 --- a/compilers/openapi/internal/annotation/constraints.go +++ b/compilers/openapi/internal/annotation/constraints.go @@ -27,15 +27,17 @@ const ( // are List-owned and read elsewhere. A non-finite bound literal yields an // error-severity diag.NumericPrecision diagnostic and is skipped; nil is // returned when no constraint is present. exclusiveBoolean selects the -// exclusiveMinimum/exclusiveMaximum dialect (see applyExclusive), and under the -// 2020-12 one a side that declares both of its keywords is settled by -// reconcileBound rather than by whichever ran last. +// exclusiveMinimum/exclusiveMaximum dialect (see applyExclusive); under the +// 2020-12 one a side may declare both of its keywords, and both reach a field +// of their own, so neither is chosen over the other. // -// The keyword that reconciliation leaves out of ir.Constraints comes back as the -// second return, an ir.Unmodeled the caller merges into whichever carrier its -// reading position owns. pointer and srcIndex locate it, exactly as they locate -// what Read keeps. Everything else a schema says about its values reaches a -// field, so on all but a co-declared numeric bound that map is nil. +// A keyword that reaches no field comes back as the second return, an +// ir.Unmodeled the caller merges into whichever carrier its reading position +// owns. pointer and srcIndex locate it, exactly as they locate what Read keeps. +// One keyword can land there: a 3.0 exclusiveMinimum/exclusiveMaximum true with +// no bound beside it to make exclusive (see applyExclusiveFlag). Everything +// else a schema says about its values reaches a field, so that map is usually +// nil. // // It reads beside the other readers here for the reason they are here at all: // what a schema says about the values admitted at a position is read the same @@ -62,12 +64,12 @@ func Constraints(s *oas3.Schema, exclusiveBoolean bool, pointer string, srcIndex return c, residue.kept, diags } -// boundResidue is where a schema's bounds were written, and what became of the -// co-declared keywords that reached no field of ir.Constraints. +// boundResidue is where a schema's bounds were written, and what became of a +// bound keyword that reached no field of ir.Constraints. // -// One value serves both sides, so a schema co-declaring each of them leaves two -// entries here and each keyword survives — writing the map rather than adding to -// it would keep whichever side ran second. +// One value serves both sides, so a schema leaving residue on each of them +// leaves two entries here and each keyword survives — writing the map rather +// than adding to it would keep whichever side ran second. // // The keyword is recorded here rather than handed back for a caller to record, // so that the diagnostic naming it is written at the same statement that keeps @@ -82,19 +84,16 @@ type boundResidue struct { kept ir.Unmodeled } -// keepRedundant keeps the co-declared keyword that ir.Constraints has no room -// for, and returns the diagnostic reporting the pair. +// keepUnmodifiable keeps a 3.0 exclusive-bound modifier that had no bound to +// modify, and returns the diagnostic reporting it. // -// It writes back the literal already read rather than re-reading the keyword's -// raw node. The two produce the same bytes — RawFromNode renders a numeric -// scalar through the same value.NumericLiteral this bound came from — but only -// this one cannot fail, since BigVal's contract is that its text renders as a -// JSON number. That is what lets the message state the keyword is kept without -// a branch for the case where it was not. -func (b *boundResidue) keepRedundant(keptProp string, kept ir.BigVal, dropProp string, dropped ir.BigVal, compared bool) ir.Diagnostic { - PreserveInto(&b.kept, "openapi:"+dropProp, ir.RawValue(dropped), - ir.ReasonDegradedLowering, b.pointer+ids.Ptr(dropProp), b.srcIndex) - return redundantBoundDiag(keptProp, kept, dropProp, dropped, compared) +// The literal is the boolean the keyword was written as, which is the whole of +// what it said; there is no numeric bound here to write back, because the +// absence of one is the reason it is being kept at all. +func (b *boundResidue) keepUnmodifiable(inclProp, exclProp string) ir.Diagnostic { + PreserveInto(&b.kept, "openapi:"+exclProp, ir.RawValue("true"), + ir.ReasonDegradedLowering, b.pointer+ids.Ptr(exclProp), b.srcIndex) + return unmodifiableExclusiveDiag(inclProp, exclProp) } // numericBounds fills Min, Max, and MultipleOf from the raw minimum/maximum/ @@ -135,15 +134,16 @@ func boundLiteralDiag(prop, literal string, err error) ir.Diagnostic { } // applyExclusive handles exclusiveMinimum/exclusiveMaximum in both dialects: the -// 3.0 boolean arm flags the corresponding Min/Max as exclusive; the 2020-12 -// numeric arm (3.1/3.2) carries the bound value itself, read from the raw node to -// avoid the float64 trap, and hands it to reconcileBound, which decides how it -// meets any minimum/maximum declared beside it. side picks which of the two -// keywords is read, residue is where the reconciliation records the one that -// reaches no field, and exclusiveBoolean selects the dialect (true for 3.0). -// Because load suppresses the library's type-mismatch on these keywords, a -// value in the wrong form for the dialect is reported and dropped here rather -// than silently accepted. +// 3.0 boolean arm modifies the minimum/maximum written beside it (see +// applyExclusiveFlag); the 2020-12 numeric arm (3.1/3.2) carries the bound value +// itself, read from the raw node to avoid the float64 trap, and writes it to the +// side's own exclusive field, where it stands beside any minimum/maximum +// declared with it rather than in place of it. side picks which of the two +// keywords is read, residue is where the one keyword that can reach no field is +// recorded, and exclusiveBoolean selects the dialect (true for 3.0). Because +// load suppresses the library's type-mismatch on these keywords, a value in the +// wrong form for the dialect is reported and dropped here rather than silently +// accepted. func applyExclusive(c *ir.Constraints, s *oas3.Schema, side boundSide, residue *boundResidue, exclusiveBoolean bool) []ir.Diagnostic { ev, prop := s.GetExclusiveMaximum(), "exclusiveMaximum" if side == minBound { @@ -156,10 +156,7 @@ func applyExclusive(c *ir.Constraints, s *oas3.Schema, side boundSide, residue * return []ir.Diagnostic{exclusiveFormDiag(prop, exclusiveBoolean)} } if ev.IsLeft() { - if b := ev.GetLeft(); b != nil && *b { - setExclusiveFlag(c, side) - } - return nil + return applyExclusiveFlag(c, side, residue, ev.GetLeft()) } node := RawPropertyNode(s, prop) if node == nil { @@ -169,104 +166,55 @@ func applyExclusive(c *ir.Constraints, s *oas3.Schema, side boundSide, residue * if err != nil { return []ir.Diagnostic{boundLiteralDiag(prop, node.Value, err)} } - return reconcileBound(c, side, residue, v) + setExclusiveBound(c, side, &v) + return nil } -// reconcileBound settles one side's bound when the 2020-12 dialect declares -// both keywords for it: the inclusive minimum/maximum numericBounds has already -// put in c, and the exclusive bound excl read alongside it. +// applyExclusiveFlag reads the 3.0 boolean arm, where exclusiveMinimum is not a +// bound but a modifier of the minimum written beside it: "minimum: 5, +// exclusiveMinimum: true" is "x > 5", which is what ir.Constraints spells as +// ExclusiveMin. So the literal moves from the inclusive slot to the exclusive +// one and the inclusive slot is emptied — the 2020-12 spelling of the same +// restriction, not a lowering of it, and the only reading under which a 3.0 +// document and its 3.1 translation lower alike. // -// The two are independent and conjunctive there — "x >= m and x > e" — so the -// tighter of them is the effective bound and the other adds nothing. ir.Constraints -// holds one bound plus one exclusivity flag per side, so the tighter one is kept; -// taking the exclusive bound unconditionally, as this did before, published a -// constraint weaker than the source wherever minimum was the tighter (GitHub #33). +// A false modifier says the bound beside it is inclusive, which is where +// numericBounds already put it, so it moves nothing. // -// The discarded keyword is implied by the kept one, so no value the source -// admits or excludes changes. What would change is the record that the source -// spelled the bound twice, so it is kept verbatim on residue rather than left to a -// diagnostic message: a consumer reconstructing or diffing the source reads the -// document, not the diagnostics, and cannot otherwise tell -// {minimum: 10, exclusiveMinimum: 0} from {minimum: 10} (GitHub #286). -func reconcileBound(c *ir.Constraints, side boundSide, residue *boundResidue, excl ir.BigVal) []ir.Diagnostic { - incl, inclProp, exclProp := c.Max, "maximum", "exclusiveMaximum" - if side == minBound { - incl, inclProp, exclProp = c.Min, "minimum", "exclusiveMinimum" - } - if incl == nil { - setExclusiveBound(c, side, &excl) +// A true modifier with no bound beside it modifies nothing: draft-4 requires +// minimum wherever exclusiveMinimum appears, so such a schema is invalid, and +// there is no bound for the IR to make exclusive. Dropping it would be a +// declared keyword lost without a word, so it is kept verbatim under Unmodeled +// and reported. +func applyExclusiveFlag(c *ir.Constraints, side boundSide, residue *boundResidue, flag *bool) []ir.Diagnostic { + if flag == nil || !*flag { return nil } - - tighter, compared := inclusiveIsTighter(*incl, excl, side) - if tighter { - return []ir.Diagnostic{residue.keepRedundant(inclProp, *incl, exclProp, excl, compared)} + incl := inclusiveBound(c, side) + if *incl == nil { + inclProp, exclProp := boundProps(side) + return []ir.Diagnostic{residue.keepUnmodifiable(inclProp, exclProp)} } - - dropped := *incl - setExclusiveBound(c, side, &excl) - return []ir.Diagnostic{residue.keepRedundant(exclProp, excl, inclProp, dropped, compared)} + setExclusiveBound(c, side, *incl) + *incl = nil + return nil } -// inclusiveIsTighter reports whether the inclusive bound incl admits fewer -// values than the exclusive bound excl written on the same side, and whether -// the two could be compared at all. -// -// A minimum is tighter when it is the greater of the two, a maximum when it is -// the lesser; equal magnitudes are never tighter, which is what gives the -// exclusive bound the tie on both sides ("x >= 5 and x > 5" is "x > 5", -// "x <= 5 and x < 5" is "x < 5"). -// -// The comparison is exact and never rounds to float64: these are the literals -// BigVal exists to keep intact, so comparing them as floats would let a pair -// that differs past float64's precision — or one beyond its range — pick the -// wrong bound, reintroducing the defect this reconciliation exists to fix. It -// is also total over every magnitude a spec may legally write, which a rational -// is not: math/big will not build 1e1000001 as one, and a bound it cannot order -// is a bound it may silently widen. -// -// What it cannot order is a literal outside the decimal grammar, and there the -// caller keeps the exclusive bound and says the other may have been the tighter. -// No schema reaches that today — every bound comes through ir.NewBigVal, whose -// grammar is the narrower of the two — so it stands for the day that changes: -// a bound this cannot order is one that could be silently replaced by the looser -// of its pair, which is the defect this reconciliation exists to prevent. -func inclusiveIsTighter(incl, excl ir.BigVal, side boundSide) (tighter, compared bool) { - inclDec, inclOK := parseDecimalBound(incl) - exclDec, exclOK := parseDecimalBound(excl) - if !inclOK || !exclOK { - return false, false - } - order := compareDecimalBounds(inclDec, exclDec) - if order == 0 { - return false, true +// boundProps names the inclusive and exclusive keyword that bound one side. +func boundProps(side boundSide) (inclProp, exclProp string) { + if side == minBound { + return "minimum", "exclusiveMinimum" } - return (order > 0) == (side == minBound), true + return "maximum", "exclusiveMaximum" } -// redundantBoundDiag reports the co-declared 2020-12 bound that reached no -// field of ir.Constraints, naming both keywords and both exact literals so a -// reader can see which bound the IR carries without going back to the source. -// -// It states that the other keyword is kept verbatim because keepRedundant has -// already kept it, by a route with no failure to report. -// -// compared tells the two cases apart. When the magnitudes did compare, the kept -// bound is provably the tighter and the other is redundant, which costs the -// consumer nothing — hence info severity. When they did not, the kept bound is -// the exclusive one by fallback and may be the looser of the two, so the message -// says so and the severity rises to warning. -func redundantBoundDiag(keptProp string, kept ir.BigVal, dropProp string, dropped ir.BigVal, compared bool) ir.Diagnostic { - if !compared { - return diag.Newf(ir.SeverityWarning, diag.DegradedConstruct, ir.Provenance{}, - "%s %s and %s %s both bound this value but their magnitudes could not be compared; "+ - "kept %s as the bound, and %s, which may be the tighter of the two, verbatim under Unmodeled", - keptProp, kept, dropProp, dropped, keptProp, dropProp) +// inclusiveBound addresses the Min or Max slot of c, so that a caller reading +// one side can both read and clear it without repeating the side branch. +func inclusiveBound(c *ir.Constraints, side boundSide) **ir.BigVal { + if side == minBound { + return &c.Min } - return diag.Newf(ir.SeverityInfo, diag.DegradedConstruct, ir.Provenance{}, - "%s %s and %s %s both bound this value and the IR holds one bound per side; "+ - "kept %s as the tighter of the two, and %s, which it implies, verbatim under Unmodeled", - keptProp, kept, dropProp, dropped, keptProp, dropProp) + return &c.Max } // exclusiveFormDiag reports an exclusiveMinimum/exclusiveMaximum whose value form @@ -283,27 +231,30 @@ func exclusiveFormDiag(prop string, exclusiveBoolean bool) ir.Diagnostic { "%s must be %s in this OpenAPI dialect", prop, want) } -// setExclusiveFlag marks the low or high bound exclusive. -func setExclusiveFlag(c *ir.Constraints, side boundSide) { - if side == minBound { - c.ExclusiveMin = true - return - } - c.ExclusiveMax = true +// unmodifiableExclusiveDiag reports a 3.0 exclusive-bound modifier written +// without the bound it modifies. Draft-4 requires minimum wherever +// exclusiveMinimum appears (and maximum wherever exclusiveMaximum does), so the +// schema is invalid; but the keyword is one the loader hands to Morphic +// unchecked, and an invalid schema is still a schema whose text a consumer may +// need, so this is a warning over a kept construct rather than an error over a +// dropped one. +func unmodifiableExclusiveDiag(inclProp, exclProp string) ir.Diagnostic { + return diag.Newf(ir.SeverityWarning, diag.DegradedConstruct, ir.Provenance{}, + "%s is true with no %s beside it to make exclusive, so it bounds nothing; "+ + "kept verbatim under Unmodeled", exclProp, inclProp) } -// setExclusiveBound sets an exclusive numeric bound (2020-12 arm) on Min or Max, -// replacing whatever minimum/maximum put there. Only reconcileBound may call it, -// which is where the replacement is decided; calling it directly is the shape of -// GitHub #33. +// setExclusiveBound writes the exclusive bound of one side. It never touches +// the inclusive slot: the two keywords are independent, both apply where both +// are declared, and overwriting one with the other published a bound the source +// never wrote (GitHub #33) and hid a change to the overwritten one (GitHub +// #425). func setExclusiveBound(c *ir.Constraints, side boundSide, v *ir.BigVal) { if side == minBound { - c.Min = v - c.ExclusiveMin = true + c.ExclusiveMin = v return } - c.Max = v - c.ExclusiveMax = true + c.ExclusiveMax = v } // emptyConstraints reports whether c carries no scalar constraint set by @@ -311,7 +262,7 @@ func setExclusiveBound(c *ir.Constraints, side boundSide, v *ir.BigVal) { // Constraints populates must appear in this check; a // missing field silently leaks a non-nil *Constraints when it should be nil. func emptyConstraints(c *ir.Constraints) bool { - return c.Min == nil && c.Max == nil && !c.ExclusiveMin && !c.ExclusiveMax && + return c.Min == nil && c.Max == nil && c.ExclusiveMin == nil && c.ExclusiveMax == nil && c.MultipleOf == nil && c.Precision == nil && c.Scale == nil && c.MinLength == nil && c.MaxLength == nil && c.Pattern == "" && c.PatternMessage == "" && diff --git a/compilers/openapi/internal/annotation/constraints_internal_test.go b/compilers/openapi/internal/annotation/constraints_internal_test.go index 0d31c4c0..9e92fcce 100644 --- a/compilers/openapi/internal/annotation/constraints_internal_test.go +++ b/compilers/openapi/internal/annotation/constraints_internal_test.go @@ -27,6 +27,6 @@ func TestApplyExclusive_NumericWithoutRootNode(t *testing.T) { // The numeric arm is taken (2020-12 dialect, numeric value) but there is no raw // node to read the exact literal from, so nothing is set and no diagnostic. assert.Nil(t, diags) - assert.False(t, c.ExclusiveMin) + assert.Nil(t, c.ExclusiveMin) assert.Empty(t, residue.kept) } diff --git a/compilers/openapi/internal/annotation/constraints_readers_internal_test.go b/compilers/openapi/internal/annotation/constraints_readers_internal_test.go index 6c6c68eb..9c3f723c 100644 --- a/compilers/openapi/internal/annotation/constraints_readers_internal_test.go +++ b/compilers/openapi/internal/annotation/constraints_readers_internal_test.go @@ -124,13 +124,13 @@ func TestApplyExclusive_BothDialects(t *testing.T) { body string exclusiveBoolean bool wantMin, wantMax *ir.BigVal - wantExclMin bool - wantExclMax bool + wantExclMin *ir.BigVal + wantExclMax *ir.BigVal }{ { - name: "3.0 boolean flags the bound beside it", exclusiveBoolean: true, - body: "minimum: 1\nexclusiveMinimum: true\nmaximum: 9\nexclusiveMaximum: true\n", - wantMin: bigOf("1"), wantMax: bigOf("9"), wantExclMin: true, wantExclMax: true, + name: "3.0 boolean turns the bound beside it exclusive", exclusiveBoolean: true, + body: "minimum: 1\nexclusiveMinimum: true\nmaximum: 9\nexclusiveMaximum: true\n", + wantExclMin: bigOf("1"), wantExclMax: bigOf("9"), }, { name: "3.0 false leaves the bound inclusive", exclusiveBoolean: true, @@ -139,8 +139,8 @@ func TestApplyExclusive_BothDialects(t *testing.T) { }, { name: "2020-12 numeric carries the bound itself", exclusiveBoolean: false, - body: "exclusiveMinimum: 1\nexclusiveMaximum: 9\n", - wantMin: bigOf("1"), wantMax: bigOf("9"), wantExclMin: true, wantExclMax: true, + body: "exclusiveMinimum: 1\nexclusiveMaximum: 9\n", + wantExclMin: bigOf("1"), wantExclMax: bigOf("9"), }, } for _, tc := range tests { @@ -190,7 +190,8 @@ func TestApplyExclusive_TheWrongFormForTheDialectIsReported(t *testing.T) { assert.Contains(t, diags[0].Message, tc.wantSays) assert.Contains(t, diags[0].Message, "exclusiveMinimum") require.NotNil(t, got, "the sibling minimum is still read") - assert.False(t, got.ExclusiveMin, "the mismatched value sets no flag") + assert.Nil(t, got.ExclusiveMin, "the mismatched value sets no bound") + assert.Equal(t, bigOf("1"), got.Min, "and it stays inclusive, unmoved") }) } } @@ -208,128 +209,70 @@ func TestApplyExclusive_AMalformedNumericBoundIsReported(t *testing.T) { assert.Nil(t, got) } -// TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBounds pins the 2020-12 rule -// that a side's two keywords are independent and conjunctive, so one bound slot -// must hold the tighter of them. Keeping the looser is a constraint weaker than -// the source wrote, which is a wrong answer rather than an incomplete one -// (GitHub #33) — {minimum: 10, exclusiveMinimum: 0} once compiled to "> 0". +// TestConstraints_CoDeclaredBoundsBothReachAField pins the 2020-12 rule that a +// side's two keywords are independent and conjunctive: each is a restriction the +// source wrote, ir.Constraints has a field for each, and neither is chosen over +// the other. // -// The tie rows are the reason each side is spelled out rather than derived from -// the other: "x >= 5 and x > 5" is "x > 5" and "x <= 5 and x < 5" is "x < 5", so -// the exclusive bound wins a tie on both sides even though "tighter" runs the -// opposite way on each. +// The rows come in pairs that swap which keyword is the tighter while leaving +// the same two magnitudes on the side. One slot per side answers both rows of a +// pair with the tighter bound alone, so a consumer diffing two revisions of a +// spec across such a swap saw a change of a different kind than the one that +// happened — and a revision that moved only the looser keyword read as no change +// at all (GitHub #425). Two fields answer them differently, which is what these +// pairs are here to hold. // -// wantKept is the other half of the rule and the half a diagnostic cannot do -// (GitHub #286): the keyword the bound slot has no room for is a keyword the -// source wrote, so it comes back as an entry a carrier holds. Without it -// {minimum: 10, exclusiveMinimum: 0} and {minimum: 10} produce the same -// document, which is what lossless-by-default forbids. -// TestConstraints_BothSidesCoDeclaredKeepEachKeyword covers the two sides -// together, which the rows below cover only one at a time. -// -// One boundResidue serves both calls to applyExclusive, so the second side adds -// to what the first kept. Were it to write the map instead, the surviving entry -// would be whichever side ran second and the other keyword would go — silently, -// since a schema declaring all four is as valid as one declaring two. Every -// other case here declares one side, so none of them can tell the two apart. -// -// The two sides are deliberately settled opposite ways — the minimum loses to -// its exclusive keyword, the maximum wins over its own — so the entries come -// from both of reconcileBound's arms rather than twice from one. Both dropping -// the same keyword would leave the other arm's write untested in combination. -func TestConstraints_BothSidesCoDeclaredKeepEachKeyword(t *testing.T) { - t.Parallel() - _, kept, diags := Constraints(schemaFromYAML(t, `type: integer -minimum: 10 -exclusiveMinimum: 20 -maximum: 100 -exclusiveMaximum: 999 -`), false, "/p", 0) - - require.Len(t, kept, 2, "each side leaves the keyword it had no room for; got %v", kept) - for _, want := range []struct{ key, value, pointer string }{ - {"openapi:minimum", "10", "/p/minimum"}, - {"openapi:exclusiveMaximum", "999", "/p/exclusiveMaximum"}, - } { - entry, ok := kept[want.key] - require.True(t, ok, "%s survives the other side", want.key) - assert.Equal(t, want.value, string(entry.Value)) - assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) - assert.Equal(t, ir.Provenance{Pointer: want.pointer}, entry.Provenance) - } - assert.Len(t, diags, 2, "and each side reports its own pair") -} - -func TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBounds(t *testing.T) { +// Nothing is kept verbatim and nothing is reported: with both keywords in the +// document there is no residue to keep and no degradation to announce. +func TestConstraints_CoDeclaredBoundsBothReachAField(t *testing.T) { t.Parallel() tests := []struct { - name string - body string - want ir.Constraints - wantKept string - wantRaw string - wantSays []string + name string + body string + want ir.Constraints }{ { - name: "minimum is the tighter of the pair", - body: "minimum: 10\nexclusiveMinimum: 0\n", - want: ir.Constraints{Min: bigOf("10")}, - wantKept: "openapi:exclusiveMinimum", wantRaw: "0", - wantSays: []string{"minimum 10", "exclusiveMinimum 0", - "kept minimum as the tighter of the two, and exclusiveMinimum, " + - "which it implies, verbatim under Unmodeled"}, + name: "minimum is the tighter of the pair", + body: "minimum: 10\nexclusiveMinimum: 0\n", + want: ir.Constraints{Min: bigOf("10"), ExclusiveMin: bigOf("0")}, }, { - name: "exclusiveMinimum is the tighter of the pair", - body: "minimum: 0\nexclusiveMinimum: 10\n", - want: ir.Constraints{Min: bigOf("10"), ExclusiveMin: true}, - wantKept: "openapi:minimum", wantRaw: "0", - wantSays: []string{"exclusiveMinimum 10", "minimum 0", - "kept exclusiveMinimum as the tighter of the two, and minimum, " + - "which it implies, verbatim under Unmodeled"}, + name: "exclusiveMinimum is the tighter of the pair", + body: "minimum: 0\nexclusiveMinimum: 10\n", + want: ir.Constraints{Min: bigOf("0"), ExclusiveMin: bigOf("10")}, }, { - name: "equal minimums leave the exclusive one standing", - body: "minimum: 5\nexclusiveMinimum: 5\n", - want: ir.Constraints{Min: bigOf("5"), ExclusiveMin: true}, - wantKept: "openapi:minimum", wantRaw: "5", - wantSays: []string{"exclusiveMinimum 5", "minimum 5", "kept exclusiveMinimum as the tighter"}, + name: "equal minimums are two keywords, not one", + body: "minimum: 5\nexclusiveMinimum: 5\n", + want: ir.Constraints{Min: bigOf("5"), ExclusiveMin: bigOf("5")}, }, { - name: "maximum is the tighter of the pair", - body: "maximum: 10\nexclusiveMaximum: 100\n", - want: ir.Constraints{Max: bigOf("10")}, - wantKept: "openapi:exclusiveMaximum", wantRaw: "100", - wantSays: []string{"maximum 10", "exclusiveMaximum 100", "kept maximum as the tighter"}, + name: "maximum is the tighter of the pair", + body: "maximum: 10\nexclusiveMaximum: 100\n", + want: ir.Constraints{Max: bigOf("10"), ExclusiveMax: bigOf("100")}, }, { - name: "exclusiveMaximum is the tighter of the pair", - body: "maximum: 100\nexclusiveMaximum: 10\n", - want: ir.Constraints{Max: bigOf("10"), ExclusiveMax: true}, - wantKept: "openapi:maximum", wantRaw: "100", - wantSays: []string{"exclusiveMaximum 10", "maximum 100", "kept exclusiveMaximum as the tighter"}, + name: "exclusiveMaximum is the tighter of the pair", + body: "maximum: 100\nexclusiveMaximum: 10\n", + want: ir.Constraints{Max: bigOf("100"), ExclusiveMax: bigOf("10")}, }, { - name: "equal maximums leave the exclusive one standing", - body: "maximum: 5\nexclusiveMaximum: 5\n", - want: ir.Constraints{Max: bigOf("5"), ExclusiveMax: true}, - wantKept: "openapi:maximum", wantRaw: "5", - wantSays: []string{"exclusiveMaximum 5", "maximum 5", "kept exclusiveMaximum as the tighter"}, + name: "both sides co-declared keep all four keywords", + body: "minimum: 10\nexclusiveMinimum: 20\nmaximum: 100\nexclusiveMaximum: 999\n", + want: ir.Constraints{ + Min: bigOf("10"), ExclusiveMin: bigOf("20"), + Max: bigOf("100"), ExclusiveMax: bigOf("999"), + }, }, { - name: "a bound decided by a digit float64 cannot hold", - body: "minimum: 9007199254740993\nexclusiveMinimum: 9007199254740992\n", - want: ir.Constraints{Min: bigOf("9007199254740993")}, - wantKept: "openapi:exclusiveMinimum", wantRaw: "9007199254740992", - wantSays: []string{"minimum 9007199254740993", "exclusiveMinimum 9007199254740992", - "kept minimum as the tighter"}, + name: "a pair no float64 tells apart keeps both literals", + body: "minimum: 9007199254740993\nexclusiveMinimum: 9007199254740992\n", + want: ir.Constraints{Min: bigOf("9007199254740993"), ExclusiveMin: bigOf("9007199254740992")}, }, { - name: "one value spelled two ways is still a tie", - body: "minimum: 1e2\nexclusiveMinimum: 100\n", - want: ir.Constraints{Min: bigOf("100"), ExclusiveMin: true}, - wantKept: "openapi:minimum", wantRaw: "1e2", - wantSays: []string{"exclusiveMinimum 100", "minimum 1e2", "kept exclusiveMinimum as the tighter"}, + name: "one value spelled two ways stays two keywords", + body: "minimum: 1e2\nexclusiveMinimum: 100\n", + want: ir.Constraints{Min: bigOf("1e2"), ExclusiveMin: bigOf("100")}, }, } for _, tc := range tests { @@ -341,32 +284,40 @@ func TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBounds(t *testing.T) { if diff := cmp.Diff(tc.want, *got); diff != "" { t.Errorf("constraints (-want +got):\n%s", diff) } - entry, ok := kept[tc.wantKept] - require.True(t, ok, "the keyword no bound slot holds is kept verbatim; got %v", kept) - assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) - assert.Equal(t, tc.wantRaw, string(entry.Value), "its exact literal, not the bound that won") - assert.Equal(t, ir.Provenance{Source: 3, Pointer: "/p/" + strings.TrimPrefix(tc.wantKept, "openapi:")}, - entry.Provenance, "located at the keyword it came from") - assert.Len(t, kept, 1, "only the keyword the reconciliation left over") - - require.Len(t, diags, 1, "the keyword that did not reach the IR is reported") - assert.Equal(t, ir.SeverityInfo, diags[0].Severity) - assert.Equal(t, diag.DegradedConstruct, diags[0].Code) - for _, says := range tc.wantSays { - assert.Contains(t, diags[0].Message, says) - } + assert.Empty(t, kept, "every keyword written reaches a field of its own") + assert.Empty(t, diags, "so there is no degradation to report") }) } } -// TestReconcileBound_OneKeywordPerSideIsNotReconciled pins the silent path. A -// side that writes one keyword has nothing to reconcile, so announcing a -// dropped bound there would report a loss that did not happen — and it is the -// common case, which a diagnostic on every numeric schema would drown. -// -// It keeps nothing verbatim either: every keyword written here reaches a field -// of ir.Constraints, and an entry restating one would give a bound two homes. -func TestReconcileBound_OneKeywordPerSideIsNotReconciled(t *testing.T) { +// TestConstraints_ABoundNoFloatHoldsIsCarriedVerbatim pins that the fields hold +// the literal the source wrote at magnitudes nothing else here could carry. +// math/big will not build 1e2000000 as a rational and float64 has no room for it +// at all, so a lowering that reduced either bound to a number would have to +// round or fail; ir.NewBigVal keeps the text, and both keywords keep their own. +func TestConstraints_ABoundNoFloatHoldsIsCarriedVerbatim(t *testing.T) { + t.Parallel() + got, kept, diags := Constraints(schemaFromYAMLUnvalidated(t, + "type: number\nminimum: 1.0e2000000\nexclusiveMinimum: 5\nmaximum: 1e-1000001\nexclusiveMaximum: 5\n"), + false, "/p", 0) + + require.NotNil(t, got) + want := ir.Constraints{ + Min: bigOf("1.0e2000000"), ExclusiveMin: bigOf("5"), + Max: bigOf("1e-1000001"), ExclusiveMax: bigOf("5"), + } + if diff := cmp.Diff(want, *got); diff != "" { + t.Errorf("constraints (-want +got):\n%s", diff) + } + assert.Empty(t, kept) + assert.Empty(t, diags) +} + +// TestConstraints_OneKeywordPerSideKeepsNothing pins the ordinary case. Every +// keyword written reaches a field, so there is nothing to keep verbatim — an +// entry restating one would give a bound two homes — and nothing to report, +// which a diagnostic on every numeric schema would drown anyway. +func TestConstraints_OneKeywordPerSideKeepsNothing(t *testing.T) { t.Parallel() for _, body := range []string{ "minimum: 1\nmaximum: 9\n", @@ -384,141 +335,91 @@ func TestReconcileBound_OneKeywordPerSideIsNotReconciled(t *testing.T) { } } -// TestReconcileBound_ThreeZeroDialectPairIsUntouched pins the 3.0 arm against -// the 2020-12 fix. There exclusiveMinimum is a boolean modifier of the minimum -// beside it, so the two cannot be rival bounds and there is nothing to drop: -// reconciling them would invent a diagnostic and could discard the bound the -// flag modifies. Nothing is kept verbatim there either: both keywords reach a -// field, so there is no keyword left over to keep. -func TestReconcileBound_ThreeZeroDialectPairIsUntouched(t *testing.T) { - t.Parallel() - got, kept, diags := Constraints(schemaFromYAML(t, - "type: number\nminimum: 10\nexclusiveMinimum: true\nmaximum: 20\nexclusiveMaximum: true\n"), true, "/p", 0) - - require.NotNil(t, got) - assert.Empty(t, diags) - assert.Empty(t, kept) - want := ir.Constraints{Min: bigOf("10"), Max: bigOf("20"), ExclusiveMin: true, ExclusiveMax: true} - if diff := cmp.Diff(want, *got); diff != "" { - t.Errorf("constraints (-want +got):\n%s", diff) - } -} - -// TestApplyExclusive_ThreeZeroFlagsTheSideItWasReadFrom pins which side the 3.0 -// boolean arm marks exclusive. -// -// The case above declares the keyword on both sides, and every other 3.0 case -// here does too — where flagging the wrong side is symmetric, so a reader that -// crossed them over produces exactly the expected constraints. Only a schema -// exclusive on one side can tell the two apart. -func TestApplyExclusive_ThreeZeroFlagsTheSideItWasReadFrom(t *testing.T) { - t.Parallel() - got, kept, diags := Constraints(schemaFromYAML(t, - "type: number\nminimum: 10\nexclusiveMinimum: true\nmaximum: 20\n"), true, "/p", 0) - - require.NotNil(t, got) - assert.Empty(t, diags) - assert.Empty(t, kept) - want := ir.Constraints{Min: bigOf("10"), Max: bigOf("20"), ExclusiveMin: true} - if diff := cmp.Diff(want, *got); diff != "" { - t.Errorf("constraints (-want +got):\n%s", diff) - } -} - -// TestReconcileBound_AMagnitudeNoRationalHoldsStillCompares pins the exactness -// of the comparison at the size where the obvious way to make it gives out. -// math/big will not build 1e2000000 as a rational — the exponent is past its -// own limit for one — so reconciling through a rational had to fall back, and -// the fallback keeps the exclusive bound. Here that is the looser one: "> 5" -// where the source says ">= 1e2000000" is the wrong constraint GitHub #33 is -// about, in a rarer case and with a warning attached. +// TestApplyExclusiveFlag_ThreeZeroModifierMovesTheBound pins the 3.0 arm. There +// exclusiveMinimum is not a bound but a boolean modifying the minimum beside it, +// so "minimum: 10, exclusiveMinimum: true" is "x > 10" — which ir.Constraints +// spells as ExclusiveMin, not as Min plus something. The literal therefore moves +// into the exclusive field and the inclusive one is left empty: the 2020-12 +// spelling of the same restriction, so a 3.0 document and its 3.1 translation +// lower to the same constraints rather than to two documents that diff. // -// These magnitudes are legal in a spec and ir.NewBigVal keeps them, so the -// comparison has to reach them; the exponent alone separates the two bounds, -// and nothing here needs the million digits it stands for. -func TestReconcileBound_AMagnitudeNoRationalHoldsStillCompares(t *testing.T) { +// The maximum stays inclusive in the second case for the reason the first case +// cannot cover: flagging the wrong side is symmetric when both sides declare the +// modifier, so only a schema exclusive on one side can tell a crossed-over read +// from a correct one. +func TestApplyExclusiveFlag_ThreeZeroModifierMovesTheBound(t *testing.T) { t.Parallel() tests := []struct { - name string - body string - want ir.Constraints - wantKept string - wantRaw string - wantSays []string + name string + body string + want ir.Constraints }{ { - name: "a minimum too large for a rational is still the tighter", - body: "minimum: 1.0e2000000\nexclusiveMinimum: 5\n", - want: ir.Constraints{Min: bigOf("1.0e2000000")}, - wantKept: "openapi:exclusiveMinimum", wantRaw: "5", - wantSays: []string{"minimum 1.0e2000000", "exclusiveMinimum 5", "kept minimum as the tighter"}, + name: "both sides modified", + body: "minimum: 10\nexclusiveMinimum: true\nmaximum: 20\nexclusiveMaximum: true\n", + want: ir.Constraints{ExclusiveMin: bigOf("10"), ExclusiveMax: bigOf("20")}, + }, + { + name: "only the side that wrote the modifier moves", + body: "minimum: 10\nexclusiveMinimum: true\nmaximum: 20\n", + want: ir.Constraints{ExclusiveMin: bigOf("10"), Max: bigOf("20")}, }, { - name: "a maximum too small for one is the tighter on its side", - body: "maximum: 1e-1000001\nexclusiveMaximum: 5\n", - want: ir.Constraints{Max: bigOf("1e-1000001")}, - wantKept: "openapi:exclusiveMaximum", wantRaw: "5", - wantSays: []string{"maximum 1e-1000001", "exclusiveMaximum 5", "kept maximum as the tighter"}, + name: "a false modifier leaves the bound where it is", + body: "minimum: 10\nexclusiveMinimum: false\nmaximum: 20\nexclusiveMaximum: false\n", + want: ir.Constraints{Min: bigOf("10"), Max: bigOf("20")}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, kept, diags := Constraints(schemaFromYAMLUnvalidated(t, "type: number\n"+tc.body), false, "/p", 0) + got, kept, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), true, "/p", 0) require.NotNil(t, got) if diff := cmp.Diff(tc.want, *got); diff != "" { t.Errorf("constraints (-want +got):\n%s", diff) } - entry, ok := kept[tc.wantKept] - require.True(t, ok, "the keyword the bound slot has no room for; got %v", kept) - assert.Equal(t, tc.wantRaw, string(entry.Value)) - require.Len(t, diags, 1) - assert.Equal(t, ir.SeverityInfo, diags[0].Severity, "the pair did compare") - for _, says := range tc.wantSays { - assert.Contains(t, diags[0].Message, says) - } + assert.Empty(t, kept) + assert.Empty(t, diags) }) } } -// TestReconcileBound_ABoundNoDecimalReadingOrdersKeepsTheExclusiveOne pins the -// guard standing at this reader's boundary with ir.NewBigVal. +// TestApplyExclusiveFlag_AModifierWithNoBoundIsKeptAndReported pins the 3.0 +// modifier that modifies nothing. Draft-4 requires minimum wherever +// exclusiveMinimum appears, so the schema is invalid and there is no bound for +// the IR to make exclusive — but the loader hands these two keywords to Morphic +// unchecked, so dropping it here would lose a declared keyword with nothing +// said. It is kept verbatim at its own pointer and reported instead. // -// It is driven through reconcileBound rather than through a schema because no -// schema reaches it: every bound arrives via ir.NewBigVal, whose grammar -// TestBigValGrammarStaysWithinTheDecimalReading holds inside the one -// parseDecimalBound orders. The guard is what keeps a later widening of that -// grammar from widening a bound instead — a bound that cannot be ordered is one -// that could be silently replaced by the looser of the pair — so it keeps the -// exclusive bound and says the discarded one may have been the tighter, rather -// than claiming a comparison it never made. -func TestReconcileBound_ABoundNoDecimalReadingOrdersKeepsTheExclusiveOne(t *testing.T) { +// Both sides are declared at once because one boundResidue serves both calls to +// applyExclusive: were it to write the map rather than add to it, the surviving +// entry would be whichever side ran second, silently, since a schema writing +// both modifiers is exactly as valid (which is to say not) as one writing either. +func TestApplyExclusiveFlag_AModifierWithNoBoundIsKeptAndReported(t *testing.T) { t.Parallel() - c := &ir.Constraints{Min: bigOf("1p4")} - residue := boundResidue{pointer: "/p", srcIndex: 1} + got, kept, diags := Constraints(schemaFromYAML(t, + "type: number\nexclusiveMinimum: true\nexclusiveMaximum: true\n"), true, "/p", 3) - diags := reconcileBound(c, minBound, &residue, ir.BigVal("5")) + assert.Nil(t, got, "a modifier that bounds nothing leaves no constraint behind") + require.Len(t, kept, 2, "each side keeps its own modifier; got %v", kept) + for _, want := range []struct{ key, pointer string }{ + {"openapi:exclusiveMinimum", "/p/exclusiveMinimum"}, + {"openapi:exclusiveMaximum", "/p/exclusiveMaximum"}, + } { + entry, ok := kept[want.key] + require.True(t, ok, "%s survives the other side", want.key) + assert.Equal(t, "true", string(entry.Value), "the boolean is the whole of what it said") + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.Equal(t, ir.Provenance{Source: 3, Pointer: want.pointer}, entry.Provenance) + } - want := ir.Constraints{Min: bigOf("5"), ExclusiveMin: true} - if diff := cmp.Diff(want, *c); diff != "" { - t.Errorf("constraints (-want +got):\n%s", diff) + require.Len(t, diags, 2, "and each side reports its own") + for _, d := range diags { + assert.Equal(t, ir.SeverityWarning, d.Severity) + assert.Equal(t, diag.DegradedConstruct, d.Code) + assert.Contains(t, d.Message, "bounds nothing") } - require.Len(t, diags, 1) - assert.Equal(t, ir.SeverityWarning, diags[0].Severity, "the kept bound may be the looser one") - assert.Equal(t, diag.DegradedConstruct, diags[0].Code) - assert.Contains(t, diags[0].Message, "could not be compared") - assert.Contains(t, diags[0].Message, "minimum 1p4") - assert.Contains(t, diags[0].Message, "exclusiveMinimum 5") - - // The bound this reading cannot order is still the one the source wrote, so - // the fallback keeps it too — a bound replaced by one that may be looser is - // exactly the case a consumer needs to see the original of. The payload is - // the literal itself: not JSON here only because the fixture is a BigVal that - // breaks BigVal's own promise, which is the state irverify's raw-payload - // check exists to name. - entry, ok := residue.kept["openapi:minimum"] - require.True(t, ok, "the unordered bound is kept verbatim; got %v", residue.kept) - assert.Equal(t, "1p4", string(entry.Value)) - assert.Equal(t, ir.Provenance{Source: 1, Pointer: "/p/minimum"}, entry.Provenance) + assert.Contains(t, diags[0].Message, "exclusiveMinimum is true with no minimum beside it") + assert.Contains(t, diags[1].Message, "exclusiveMaximum is true with no maximum beside it") } diff --git a/compilers/openapi/internal/annotation/decimal_internal_test.go b/compilers/openapi/internal/annotation/decimal_internal_test.go index f2410c35..d4fbcf3b 100644 --- a/compilers/openapi/internal/annotation/decimal_internal_test.go +++ b/compilers/openapi/internal/annotation/decimal_internal_test.go @@ -132,14 +132,14 @@ func TestParseDecimalBound_DeclinesWhatIsNotADecimalLiteral(t *testing.T) { } // TestBigValGrammarStaysWithinTheDecimalReading pins the coupling that decides -// whether reconcileBound's incomparable guard is reachable: every literal +// whether BigValEqual's incomparable guard is reachable: every literal // ir.NewBigVal accepts must be one parseDecimalBound can order. // -// While it holds, no schema reaches that guard — which is why the test for it -// calls reconcileBound directly. The two grammars live in different packages -// and have already moved apart once, so nothing but this holds them together: -// when ir widens NewBigVal, a bound it now admits and this reader cannot order -// is a bound that would be silently replaced by the looser of its pair, and +// While it holds, no bound compiled from a schema reaches that guard. The two +// grammars live in different packages and have already moved apart once, so +// nothing but this holds them together: when ir widens NewBigVal, a bound it +// now admits and this reader cannot order is one whose disagreement with +// another spelling of the same magnitude would be reported as a conflict, and // that has to fail here rather than in a compiled document. // // The spellings NewBigVal refuses today are the load-bearing half of the diff --git a/compilers/openapi/internal/merge/conflict_internal_test.go b/compilers/openapi/internal/merge/conflict_internal_test.go index 087ee365..9c8cfa1a 100644 --- a/compilers/openapi/internal/merge/conflict_internal_test.go +++ b/compilers/openapi/internal/merge/conflict_internal_test.go @@ -59,8 +59,8 @@ func TestDifferentTypeKind_UnresolvableTargetIsNotAConflict(t *testing.T) { "an unresolvable target is not treated as a differing kind") } -// Both BigVal keywords rest on the same magnitude comparison, so both are -// driven here over the literals that comparison has to get right: one value +// Every BigVal keyword rests on the same magnitude comparison, so it is driven +// here over the literals that comparison has to get right: one value // under two spellings, and two values that genuinely differ — mostly at a // magnitude math/big will not build as a rational at all, with one in-range // row so a comparison that only handled the extremes would still be caught. @@ -91,10 +91,10 @@ func TestBigValConflictDetails_CompareMagnitudesAtAnyScale(t *testing.T) { b, err := ir.NewBigVal(tc.b) require.NoError(t, err, "%q is a literal a schema may write", tc.b) - _, boundOK := boundConflictDetail("minimum", &a, &b, false, false) + _, boundOK := bigValConflictDetail("minimum", &a, &b) assert.Equal(t, tc.wantConflict, boundOK, "minimum %s against %s", a, b) - _, multipleOK := multipleOfConflictDetail(&a, &b) + _, multipleOK := bigValConflictDetail("multipleOf", &a, &b) assert.Equal(t, tc.wantConflict, multipleOK, "multipleOf %s against %s", a, b) }) } @@ -208,9 +208,10 @@ func TestMergeConstraints_AdoptsEveryUnsetKeyword(t *testing.T) { // the spec-driven table tests in the compiler package. five, ten := int64(5), int64(10) minVal, maxVal, multipleOf := ir.BigVal("1"), ir.BigVal("9"), ir.BigVal("2") + exclMin, exclMax := ir.BigVal("0"), ir.BigVal("10") src := &ir.Constraints{ - Min: &minVal, ExclusiveMin: true, - Max: &maxVal, ExclusiveMax: true, + Min: &minVal, ExclusiveMin: &exclMin, + Max: &maxVal, ExclusiveMax: &exclMax, MultipleOf: &multipleOf, Precision: &ten, Scale: &five, @@ -227,9 +228,9 @@ func TestMergeConstraints_AdoptsEveryUnsetKeyword(t *testing.T) { merged := mergeConstraints(&ir.Constraints{}, src) require.NotNil(t, merged) assert.Same(t, src.Min, merged.Min) - assert.Equal(t, src.ExclusiveMin, merged.ExclusiveMin) + assert.Same(t, src.ExclusiveMin, merged.ExclusiveMin) assert.Same(t, src.Max, merged.Max) - assert.Equal(t, src.ExclusiveMax, merged.ExclusiveMax) + assert.Same(t, src.ExclusiveMax, merged.ExclusiveMax) assert.Same(t, src.MultipleOf, merged.MultipleOf) assert.Same(t, src.Precision, merged.Precision) assert.Same(t, src.Scale, merged.Scale) diff --git a/compilers/openapi/internal/merge/merge.go b/compilers/openapi/internal/merge/merge.go index 7d162411..2e8b593e 100644 --- a/compilers/openapi/internal/merge/merge.go +++ b/compilers/openapi/internal/merge/merge.go @@ -134,11 +134,13 @@ func (g *Merger) reconcileProperty(dst *ir.Property, src ir.Property) { // from src any keyword dst leaves unset (nil/""/false) — a keyword only one // branch constrains still applies to the merged field, so it is never dropped. // -// Min and Max are adopted together with their exclusivity flag: taking src.Min -// without src.ExclusiveMin would silently flip an exclusive "> 5" into an -// inclusive ">= 5". UniqueItems has no absent state to detect via cmp.Or, but -// under intersection a true from either branch is always correct, so adopting -// it via cmp.Or never wrongly downgrades dst from true to false. +// The four numeric bounds are four keywords, not two bounds with an +// exclusivity flag apiece, so each is adopted on its own: a branch declaring +// only exclusiveMinimum contributes it to a merged field whose minimum came +// from elsewhere, and neither displaces the other. UniqueItems has no absent +// state to detect via cmp.Or, but under intersection a true from either branch +// is always correct, so adopting it via cmp.Or never wrongly downgrades dst +// from true to false. func mergeConstraints(dst, src *ir.Constraints) *ir.Constraints { if dst == nil { return src @@ -146,12 +148,10 @@ func mergeConstraints(dst, src *ir.Constraints) *ir.Constraints { if src == nil { return dst } - if dst.Min == nil { - dst.Min, dst.ExclusiveMin = src.Min, src.ExclusiveMin - } - if dst.Max == nil { - dst.Max, dst.ExclusiveMax = src.Max, src.ExclusiveMax - } + dst.Min = cmp.Or(dst.Min, src.Min) + dst.Max = cmp.Or(dst.Max, src.Max) + dst.ExclusiveMin = cmp.Or(dst.ExclusiveMin, src.ExclusiveMin) + dst.ExclusiveMax = cmp.Or(dst.ExclusiveMax, src.ExclusiveMax) dst.MultipleOf = cmp.Or(dst.MultipleOf, src.MultipleOf) dst.Precision = cmp.Or(dst.Precision, src.Precision) dst.Scale = cmp.Or(dst.Scale, src.Scale) @@ -511,13 +511,15 @@ func constraintsConflict(a, b *ir.Constraints) (string, bool) { return "", false } checks := []func() (string, bool){ + func() (string, bool) { return bigValConflictDetail("minimum", a.Min, b.Min) }, func() (string, bool) { - return boundConflictDetail("minimum", a.Min, b.Min, a.ExclusiveMin, b.ExclusiveMin) + return bigValConflictDetail("exclusiveMinimum", a.ExclusiveMin, b.ExclusiveMin) }, + func() (string, bool) { return bigValConflictDetail("maximum", a.Max, b.Max) }, func() (string, bool) { - return boundConflictDetail("maximum", a.Max, b.Max, a.ExclusiveMax, b.ExclusiveMax) + return bigValConflictDetail("exclusiveMaximum", a.ExclusiveMax, b.ExclusiveMax) }, - func() (string, bool) { return multipleOfConflictDetail(a.MultipleOf, b.MultipleOf) }, + func() (string, bool) { return bigValConflictDetail("multipleOf", a.MultipleOf, b.MultipleOf) }, func() (string, bool) { return intConflictDetail("precision", a.Precision, b.Precision) }, func() (string, bool) { return intConflictDetail("scale", a.Scale, b.Scale) }, func() (string, bool) { return intConflictDetail("minLength", a.MinLength, b.MinLength) }, @@ -537,41 +539,23 @@ func constraintsConflict(a, b *ir.Constraints) (string, bool) { return "", false } -// boundConflictDetail reports whether two numeric bounds, each with its -// exclusivity flag, are both present and disagree in magnitude or in -// inclusive/exclusive sense, formatting the disagreement when they do. Such a -// disagreement is usually still individually satisfiable (minimum: 10 and -// exclusiveMinimum: 10 together just mean "> 10"), but it's diagnosed anyway: -// the merge keeps dst's bound (first declaration wins) over the true -// intersection, and the discarded bound is always the stricter one — staying -// silent would silently loosen the validation the spec intended. -func boundConflictDetail(keyword string, a, b *ir.BigVal, exclA, exclB bool) (string, bool) { - if a == nil || b == nil || (exclA == exclB && annotation.BigValEqual(*a, *b)) { - return "", false - } - return fmt.Sprintf("conflicting %s (%s and %s)", keyword, boundText(*a, exclA), boundText(*b, exclB)), true -} - -// boundText renders a numeric bound for a conflict detail, marking an -// exclusive bound so "conflicting minimum (10 and exclusive 10)" reads as the -// differing sense it is, not a duplicate magnitude. -func boundText(v ir.BigVal, exclusive bool) string { - if exclusive { - return "exclusive " + v.String() - } - return v.String() -} - -// multipleOfConflictDetail reports whether both branches pin multipleOf and pin -// it to different magnitudes, formatting the disagreement when they do. It is -// the one BigVal constraint with no exclusivity sense, so unlike a bound it -// compares by magnitude alone — the keyword is named here rather than passed -// because there is nothing else with that shape to compare. -func multipleOfConflictDetail(a, b *ir.BigVal) (string, bool) { +// bigValConflictDetail reports whether both branches pin the same +// arbitrary-precision keyword and pin it to different magnitudes, formatting +// the disagreement when they do. Every numeric keyword of ir.Constraints has +// this one shape — each of the four bounds states its own restriction, with no +// exclusivity sense to carry beside it — so one comparison serves them all, by +// magnitude, which is what keeps 10 and 10.0 from reading as a disagreement. +// +// A disagreement between two branches is usually still individually satisfiable +// (minimum: 10 in one and minimum: 20 in the other together just mean ">= 20"), +// but it's diagnosed anyway: the merge keeps dst's value (first declaration +// wins) over the true intersection, and the discarded one may be the stricter — +// staying silent would silently loosen the validation the spec intended. +func bigValConflictDetail(keyword string, a, b *ir.BigVal) (string, bool) { if a == nil || b == nil || annotation.BigValEqual(*a, *b) { return "", false } - return fmt.Sprintf("conflicting multipleOf (%s and %s)", a.String(), b.String()), true + return fmt.Sprintf("conflicting %s (%s and %s)", keyword, a.String(), b.String()), true } // intConflictDetail reports whether two optional integer bounds are both diff --git a/compilers/openapi/internal/merge/reconcile_internal_test.go b/compilers/openapi/internal/merge/reconcile_internal_test.go index 02b16ec9..7d8f2eea 100644 --- a/compilers/openapi/internal/merge/reconcile_internal_test.go +++ b/compilers/openapi/internal/merge/reconcile_internal_test.go @@ -299,60 +299,59 @@ func TestRecordRedeclarationConflict_ConstraintDisagreementIsReported(t *testing "and so is the discarded one") } -// TestBoundConflictDetail_ComparesMagnitudeAndSense pins both halves of a bound -// comparison. Equal magnitudes spelled differently must not read as a conflict, -// while the same magnitude under a differing exclusivity flag must — "> 10" and -// ">= 10" are different bounds, and the merge can only keep one. -func TestBoundConflictDetail_ComparesMagnitudeAndSense(t *testing.T) { +// TestBigValConflictDetail_ComparesByMagnitude pins the comparison every +// arbitrary-precision keyword goes through. Equal magnitudes spelled +// differently must not read as a conflict — 10 and 10.0 are one value, and +// reporting them would invent a disagreement the source never wrote — while +// differing magnitudes must, since the merge keeps one and drops the other. +// +// The keyword is a parameter, so the name in the message is the one the caller +// passed: the four bounds and multipleOf share this helper, and a hard-coded +// name would report every one of them as the same keyword. +func TestBigValConflictDetail_ComparesByMagnitude(t *testing.T) { t.Parallel() tests := []struct { - name string - a, b *ir.BigVal - exclA, exclB bool - want string + name string + keyword string + a, b *ir.BigVal + want string }{ - {name: "only one side bounds", a: bigVal("10"), exclA: true}, - {name: "neither side bounds"}, - {name: "the same magnitude, spelled differently", a: bigVal("10"), b: bigVal("10.0")}, + {name: "only one side bounds", keyword: "minimum", a: bigVal("10")}, + {name: "neither side bounds", keyword: "minimum"}, + { + name: "the same magnitude, spelled differently", + keyword: "minimum", a: bigVal("10"), b: bigVal("10.0"), + }, { - name: "differing magnitudes", a: bigVal("10"), b: bigVal("20"), + name: "differing magnitudes", keyword: "minimum", a: bigVal("10"), b: bigVal("20"), want: "conflicting minimum (10 and 20)", }, { - name: "the same magnitude, differing sense", a: bigVal("10"), b: bigVal("10"), exclB: true, - want: "conflicting minimum (10 and exclusive 10)", + name: "the exclusive bound reports under its own keyword", + keyword: "exclusiveMinimum", a: bigVal("10"), b: bigVal("20"), + want: "conflicting exclusiveMinimum (10 and 20)", + }, + { + name: "multipleOf shares the comparison", + keyword: "multipleOf", a: bigVal("3"), b: bigVal("5"), + want: "conflicting multipleOf (3 and 5)", }, { - name: "unparseable operands compare exactly", a: bigVal("nan"), b: bigVal("other"), + name: "unparseable operands compare exactly", + keyword: "minimum", a: bigVal("nan"), b: bigVal("other"), want: "conflicting minimum (nan and other)", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - detail, ok := boundConflictDetail("minimum", tc.a, tc.b, tc.exclA, tc.exclB) + detail, ok := bigValConflictDetail(tc.keyword, tc.a, tc.b) assert.Equal(t, tc.want != "", ok) assert.Equal(t, tc.want, detail) }) } } -// TestMultipleOfConflictDetail_ComparesByMagnitude pins the plain numeric -// comparison multipleOf goes through: 10 and 10.0 are one value, so reporting -// them as a conflict would invent a disagreement the source never wrote. -func TestMultipleOfConflictDetail_ComparesByMagnitude(t *testing.T) { - t.Parallel() - _, ok := multipleOfConflictDetail(bigVal("1e1"), bigVal("10")) - assert.False(t, ok, "equal magnitudes spelled differently do not conflict") - - _, ok = multipleOfConflictDetail(nil, bigVal("10")) - assert.False(t, ok, "a keyword only one branch sets is adopted, not a conflict") - - detail, ok := multipleOfConflictDetail(bigVal("3"), bigVal("5")) - assert.True(t, ok) - assert.Equal(t, "conflicting multipleOf (3 and 5)", detail) -} - // TestResolvePrimKind_EnumResolvesThroughItsValueType pins the enum case of the // resolution walk. An enum member can itself be a scalar of the kind being // redeclared, so it must answer with its value type rather than staying diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index b2d45a86..063a2b9a 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -1,7 +1,6 @@ package operation_test import ( - "strings" "testing" "github.com/speakeasy-api/openapi/validation" @@ -873,48 +872,71 @@ func TestParams_RefSiteKeywordsAreKeptOnTheParameter(t *testing.T) { assert.Equal(t, int64(3), *r.Constraints.MinLength) } -// TestParams_CoDeclaredBoundKeptOnTheParameter covers the parameter carrier for -// a 2020-12 side that declares both of its bound keywords (GitHub #286). -// ir.Constraints holds one bound per side, so one keyword reaches no field of -// the constraints the parameter carries and is kept verbatim beside them — -// otherwise {minimum: 10, exclusiveMinimum: 0} lowers to what {minimum: 10} -// does, at the one carrier ir.Parameter owns rather than a node. +// TestParams_CoDeclaredBoundsReachTheParameter covers the parameter carrier for +// a 2020-12 side that declares both of its bound keywords. Each is a keyword the +// source wrote and ir.Constraints has a field for each, so both reach the +// constraints the parameter holds and nothing is kept beside them — at the one +// carrier ir.Parameter owns rather than a node. // -// Both directions are here for the reason the property cases are: a row where -// the exclusive keyword is the one kept passes on a reader that always kept that -// one. -func TestParams_CoDeclaredBoundKeptOnTheParameter(t *testing.T) { +// The two rows swap which keyword is the tighter across the same magnitudes. A +// reader holding one bound per side answers both rows alike, which is what hid a +// change to the looser keyword from a consumer diffing two revisions (GitHub +// #425). +func TestParams_CoDeclaredBoundsReachTheParameter(t *testing.T) { t.Parallel() _, svc, diags := lowerServiceSpec(t, openapitest.PathsSpec( " /x:\n get:\n operationId: g\n parameters:\n"+ " - {name: low, in: query, schema: {type: integer, minimum: 10, exclusiveMinimum: 0}}\n"+ - " - {name: high, in: query, schema: {type: integer, maximum: 100, exclusiveMaximum: 5}}\n"+ + " - {name: high, in: query, schema: {type: integer, minimum: 0, exclusiveMinimum: 10}}\n"+ " - {name: plain, in: query, schema: {type: integer, minimum: 10}}\n"+ " responses: {\"204\": {description: ok}}\n")) openapitest.RequireNoErrorDiags(t, diags) params := paramsOf(t, svc) cases := []struct { - param, index, wantKept, wantRaw string + param, wantMin, wantExclMin string }{ - {param: "low", index: "0", wantKept: "openapi:exclusiveMinimum", wantRaw: "0"}, - {param: "high", index: "1", wantKept: "openapi:maximum", wantRaw: "100"}, + {param: "low", wantMin: "10", wantExclMin: "0"}, + {param: "high", wantMin: "0", wantExclMin: "10"}, } for _, tc := range cases { t.Run(tc.param, func(t *testing.T) { t.Parallel() - at := "/paths/~1x/get/parameters/" + tc.index + "/schema/" + - strings.TrimPrefix(tc.wantKept, "openapi:") - require.NotNil(t, params[tc.param].Constraints, "the tighter bound still reaches a field") - entry, ok := params[tc.param].Unmodeled[tc.wantKept] - require.True(t, ok, "%s is kept beside the constraints it did not reach; got %v", - tc.wantKept, params[tc.param].Unmodeled) - assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) - assert.JSONEq(t, tc.wantRaw, string(entry.Value)) - assert.Equal(t, at, entry.Provenance.Pointer, "located at the keyword itself") + c := params[tc.param].Constraints + require.NotNil(t, c, "both bounds reach the parameter's constraints") + require.NotNil(t, c.Min) + require.NotNil(t, c.ExclusiveMin) + assert.Equal(t, tc.wantMin, c.Min.String(), "minimum as written") + assert.Equal(t, tc.wantExclMin, c.ExclusiveMin.String(), "exclusiveMinimum as written, beside it") + assert.Empty(t, params[tc.param].Unmodeled, "with a field apiece there is nothing left to keep") }) } assert.Empty(t, params["plain"].Unmodeled, - "a side writing one keyword has it in a field, so nothing is restated beside it") + "and a side writing one keyword has it in a field, so nothing is restated beside it") +} + +// TestParams_ExclusiveModifierWithNoBoundIsKeptOnTheParameter covers the one +// bound keyword that still reaches no field, at the parameter carrier. A 3.0 +// exclusiveMinimum modifies the minimum beside it, so one written without a +// minimum modifies nothing and has no bound to become; dropping it would lose a +// declared keyword silently, so the parameter keeps it verbatim. +func TestParams_ExclusiveModifierWithNoBoundIsKeptOnTheParameter(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, openapitest.PathsSpecVer("3.0.3", + " /x:\n get:\n operationId: g\n parameters:\n"+ + " - {name: bare, in: query, schema: {type: integer, exclusiveMinimum: true}}\n"+ + " responses: {\"204\": {description: ok}}\n")) + params := paramsOf(t, svc) + + entry, ok := params["bare"].Unmodeled["openapi:exclusiveMinimum"] + require.True(t, ok, "kept beside the constraints it did not reach; got %v", params["bare"].Unmodeled) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, "true", string(entry.Value)) + assert.Equal(t, "/paths/~1x/get/parameters/0/schema/exclusiveMinimum", entry.Provenance.Pointer, + "located at the keyword itself") + assert.Contains(t, + openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityWarning, + "/paths/~1x/get/parameters/0/schema"), + "bounds nothing", "and reading it is what reports on it") } diff --git a/compilers/openapi/internal/schema/compose_test.go b/compilers/openapi/internal/schema/compose_test.go index 11a2b509..491625fd 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -2573,7 +2573,7 @@ func TestOneOf_CoDeclaredNotDistributedReasons(t *testing.T) { func TestUnionCombinators_CoDeclaredKeepsTheBoundsWrittenBesideIt(t *testing.T) { t.Parallel() three := int64(3) - ten, five := ir.BigVal("10"), ir.BigVal("5") + ten, five, zero := ir.BigVal("10"), ir.BigVal("5"), ir.BigVal("0") cases := []struct { name, schemas, unionKey string reason ir.UnmodeledReason @@ -2606,13 +2606,14 @@ func TestUnionCombinators_CoDeclaredKeepsTheBoundsWrittenBesideIt(t *testing.T) wantKept: []string{"openapi:anyOf"}, }, { + // Both bound keywords reach a field, so the union is the only + // entry on the node: a co-declared pair adds nothing beside it. name: "co-declared bounds beside a union", schemas: " A: {type: number, minimum: 10, exclusiveMinimum: 0, oneOf: [{minLength: 1}, {minLength: 2}]}\n", unionKey: "openapi:oneOf", reason: ir.ReasonValidationOnly, - want: ir.Constraints{Min: &ten}, - wantKept: []string{"openapi:exclusiveMinimum", "openapi:oneOf"}, - wantDiag: "kept minimum as the tighter of the two", + want: ir.Constraints{Min: &ten, ExclusiveMin: &zero}, + wantKept: []string{"openapi:oneOf"}, }, } for _, tc := range cases { diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index 2f4f6b14..a2d28ea0 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -1502,10 +1502,18 @@ func TestAllOf_ConstraintAndFormatConflictsDiagnosed(t *testing.T) { name, a, b, wantDetail string }{ { - name: "exclusive sense", + name: "minimum", a: "{type: number, minimum: 10}", - b: "{type: number, exclusiveMinimum: 10}", - wantDetail: "conflicting minimum (10 and exclusive 10)", + b: "{type: number, minimum: 20}", + wantDetail: "conflicting minimum (10 and 20)", + }, + { + // The exclusive bound is a keyword of its own, so it conflicts + // under its own name rather than as a differing sense of minimum. + name: "exclusiveMinimum", + a: "{type: number, exclusiveMinimum: 10}", + b: "{type: number, exclusiveMinimum: 20}", + wantDetail: "conflicting exclusiveMinimum (10 and 20)", }, { name: "pattern", @@ -1602,16 +1610,19 @@ func TestAllOf_CompatibleConstraintRedeclarationsStaySilent(t *testing.T) { }, }, { - name: "min and exclusiveMin adopted together", - a: "{type: number, multipleOf: 2}", + // minimum and exclusiveMinimum are two keywords, so a branch + // declaring one and a branch declaring the other intersect to a + // field carrying both — not to whichever the merge picked. + name: "minimum and exclusiveMinimum adopted side by side", + a: "{type: number, minimum: 1}", b: "{type: number, exclusiveMinimum: 5}", assertMerged: func(t *testing.T, c *ir.Constraints) { t.Helper() require.NotNil(t, c) - require.NotNil(t, c.Min, "the second branch's exclusiveMinimum is adopted as Min") - assert.Equal(t, "5", c.Min.String()) - assert.True(t, c.ExclusiveMin, - "ExclusiveMin travels with the adopted Min, not left at its false zero value") + require.NotNil(t, c.Min, "the first branch's minimum stays") + assert.Equal(t, "1", c.Min.String()) + require.NotNil(t, c.ExclusiveMin, "the second branch's exclusiveMinimum is adopted beside it") + assert.Equal(t, "5", c.ExclusiveMin.String()) }, }, {name: "equivalent multipleOf", a: "{type: number, multipleOf: 2}", b: "{type: number, multipleOf: 2.0}"}, @@ -4090,72 +4101,128 @@ func TestUnhomedKeywords_ElectedLoweringKeepsWhatItCannotRead(t *testing.T) { } } -// TestCoDeclaredBound_KeptOnTheCarrierThatReadIt pins the two carriers this -// package owns for a 2020-12 side that declares both of its bound keywords -// (GitHub #286). ir.Constraints holds one bound per side, so one keyword reaches -// no field of it, and without an entry beside those constraints -// {minimum: 10, exclusiveMinimum: 0} lowers to exactly what {minimum: 10} does. +// TestCoDeclaredBound_BothKeywordsReachTheCarriersConstraints pins the two +// carriers this package owns for a 2020-12 side that declares both of its bound +// keywords. The two keywords are independent and both apply, and ir.Constraints +// has a field for each, so both reach the constraints the carrier holds and +// neither is kept beside them. // -// Both directions run at both carriers. A case where the exclusive keyword is -// the one kept verbatim passes just as well on a reader that always kept that -// one, so on its own it would say nothing about which keyword the carrier holds. -func TestCoDeclaredBound_KeptOnTheCarrierThatReadIt(t *testing.T) { +// The rows are pairs that swap which of the two is the tighter while leaving the +// same magnitudes on the side. One bound slot answered both rows of a pair +// identically, which is what made a revision that moved only the looser keyword +// read as no change at all (GitHub #425). +func TestCoDeclaredBound_BothKeywordsReachTheCarriersConstraints(t *testing.T) { t.Parallel() doc, diags := parseFull(t, openapitest.ComponentSpec( " Alias: {type: integer, minimum: 10, exclusiveMinimum: 0}\n"+ - " Tight: {type: integer, maximum: 100, exclusiveMaximum: 5}\n"+ + " Tight: {type: integer, minimum: 0, exclusiveMinimum: 10}\n"+ " Holder:\n type: object\n properties:\n"+ - " low: {type: integer, minimum: 10, exclusiveMinimum: 0}\n"+ - " high: {type: integer, maximum: 100, exclusiveMaximum: 5}\n")) + " low: {type: integer, maximum: 100, exclusiveMaximum: 5}\n"+ + " high: {type: integer, maximum: 5, exclusiveMaximum: 100}\n")) openapitest.RequireNoErrorDiags(t, diags) tests := []struct { name string unmod ir.Unmodeled bound *ir.Constraints - wantKept string - wantRaw string - at string + read func(*ir.Constraints) (incl, excl *ir.BigVal) + wantIncl *ir.BigVal + wantExcl *ir.BigVal }{ { - name: "alias node keeps the exclusive bound the minimum implies", - unmod: typeByName(doc, "Alias").Common().Unmodeled, - bound: typeByName(doc, "Alias").(*ir.Scalar).Constraints, - wantKept: "openapi:exclusiveMinimum", wantRaw: "0", - at: "/components/schemas/Alias/exclusiveMinimum", + name: "alias node where the minimum is the tighter", + unmod: typeByName(doc, "Alias").Common().Unmodeled, + bound: typeByName(doc, "Alias").(*ir.Scalar).Constraints, + read: minSide, wantIncl: bigValOf("10"), wantExcl: bigValOf("0"), }, { - name: "alias node keeps the inclusive bound the exclusive one implies", - unmod: typeByName(doc, "Tight").Common().Unmodeled, - bound: typeByName(doc, "Tight").(*ir.Scalar).Constraints, - wantKept: "openapi:maximum", wantRaw: "100", - at: "/components/schemas/Tight/maximum", + name: "alias node where the exclusive minimum is the tighter", + unmod: typeByName(doc, "Tight").Common().Unmodeled, + bound: typeByName(doc, "Tight").(*ir.Scalar).Constraints, + read: minSide, wantIncl: bigValOf("0"), wantExcl: bigValOf("10"), }, { - name: "property keeps the exclusive bound the minimum implies", - unmod: propertyOf(t, doc, "Holder", "low").Unmodeled, - bound: propertyOf(t, doc, "Holder", "low").Constraints, - wantKept: "openapi:exclusiveMinimum", wantRaw: "0", - at: "/components/schemas/Holder/properties/low/exclusiveMinimum", + name: "property where the exclusive maximum is the tighter", + unmod: propertyOf(t, doc, "Holder", "low").Unmodeled, + bound: propertyOf(t, doc, "Holder", "low").Constraints, + read: maxSide, wantIncl: bigValOf("100"), wantExcl: bigValOf("5"), + }, + { + name: "property where the maximum is the tighter", + unmod: propertyOf(t, doc, "Holder", "high").Unmodeled, + bound: propertyOf(t, doc, "Holder", "high").Constraints, + read: maxSide, wantIncl: bigValOf("5"), wantExcl: bigValOf("100"), + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + require.NotNil(t, tc.bound, "both bounds reach ir.Constraints") + incl, excl := tc.read(tc.bound) + assert.Equal(t, tc.wantIncl, incl, "the inclusive keyword as written") + assert.Equal(t, tc.wantExcl, excl, "the exclusive keyword as written, beside it") + assert.Empty(t, tc.unmod, "with a field apiece there is nothing left to keep") + }) + } +} + +// minSide and maxSide read one side's pair of bounds off a Constraints, so one +// table can drive both sides through the same assertion. +func minSide(c *ir.Constraints) (incl, excl *ir.BigVal) { return c.Min, c.ExclusiveMin } +func maxSide(c *ir.Constraints) (incl, excl *ir.BigVal) { return c.Max, c.ExclusiveMax } + +// bigValOf is the *ir.BigVal a bound assertion compares against. +func bigValOf(v string) *ir.BigVal { + b := ir.BigVal(v) + return &b +} + +// TestExclusiveModifier_WithNoBoundIsKeptOnTheCarrierThatReadIt pins the one +// bound keyword that still reaches no field, at the two carriers this package +// owns. A 3.0 exclusiveMinimum is a modifier of the minimum beside it, so one +// written without a minimum modifies nothing — draft-4 forbids that schema, and +// the loader hands the keyword here unchecked. Dropping it would lose a declared +// keyword silently, so it is kept verbatim beside the constraints it did not +// reach. +func TestExclusiveModifier_WithNoBoundIsKeptOnTheCarrierThatReadIt(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, openapitest.ComponentSpecVer("3.0.3", + " Alias: {type: integer, exclusiveMinimum: true}\n"+ + " Holder:\n type: object\n properties:\n"+ + " low: {type: integer, exclusiveMaximum: true}\n")) + + tests := []struct { + name string + unmod ir.Unmodeled + wantKept string + at string + carrier string + }{ + { + name: "alias node", + unmod: typeByName(doc, "Alias").Common().Unmodeled, + wantKept: "openapi:exclusiveMinimum", + at: "/components/schemas/Alias/exclusiveMinimum", + carrier: "/components/schemas/Alias", }, { - name: "property keeps the inclusive bound the exclusive one implies", - unmod: propertyOf(t, doc, "Holder", "high").Unmodeled, - bound: propertyOf(t, doc, "Holder", "high").Constraints, - wantKept: "openapi:maximum", wantRaw: "100", - at: "/components/schemas/Holder/properties/high/maximum", + name: "property", + unmod: propertyOf(t, doc, "Holder", "low").Unmodeled, + wantKept: "openapi:exclusiveMaximum", + at: "/components/schemas/Holder/properties/low/exclusiveMaximum", + carrier: "/components/schemas/Holder/properties/low", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - require.NotNil(t, tc.bound, "the tighter bound still reaches ir.Constraints") entry, ok := tc.unmod[tc.wantKept] - require.True(t, ok, "%s is kept beside the constraints it did not reach; got %v", - tc.wantKept, tc.unmod) + require.True(t, ok, "%s is kept on the carrier that read it; got %v", tc.wantKept, tc.unmod) assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) - assert.JSONEq(t, tc.wantRaw, string(entry.Value)) - assert.Equal(t, tc.at, entry.Provenance.Pointer) + assert.JSONEq(t, "true", string(entry.Value)) + assert.Equal(t, tc.at, entry.Provenance.Pointer, "located at the keyword itself") + assert.Len(t, diagsAtPointer(diags, diag.DegradedConstruct, tc.carrier), 1, + "and reported once, at the schema that read it: %+v", diags) }) } } diff --git a/docs/ir-design.md b/docs/ir-design.md index 9c9e39fa..5057a61a 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -993,8 +993,10 @@ with storage and computation split. ```go type Constraints struct { // numeric — arbitrary-precision decimal strings, never float64 (TypeSpec Numeric lesson) - Min, Max *BigVal - ExclusiveMin, ExclusiveMax bool + Min, Max *BigVal // inclusive bounds (minimum / maximum) + ExclusiveMin, ExclusiveMax *BigVal // exclusive bounds (exclusiveMinimum / exclusiveMaximum); + // independent of Min/Max, not flags on them — a schema may + // declare both per side and both apply MultipleOf *BigVal Precision, Scale *int64 // decimal digit bounds (Avro decimal, XSD totalDigits/fractionDigits, // OData Edm.Decimal) diff --git a/ir/constraints.go b/ir/constraints.go index 4066fdfc..fd92b779 100644 --- a/ir/constraints.go +++ b/ir/constraints.go @@ -14,14 +14,27 @@ package ir // a $ref's target onto the referencing carrier with use-site precedence, so a // use site already carries those and resolves nothing to read them. type Constraints struct { - // Min is the inclusive (or exclusive, per ExclusiveMin) lower numeric bound. + // Min is the inclusive lower numeric bound (JSON Schema minimum): an + // admissible value is >= it. nil = this position declared none. Min *BigVal `json:"min,omitempty"` - // Max is the inclusive (or exclusive, per ExclusiveMax) upper numeric bound. + // Max is the inclusive upper numeric bound (JSON Schema maximum): an + // admissible value is <= it. nil = this position declared none. Max *BigVal `json:"max,omitempty"` - // ExclusiveMin makes Min an exclusive bound. - ExclusiveMin bool `json:"exclusiveMin"` - // ExclusiveMax makes Max an exclusive bound. - ExclusiveMax bool `json:"exclusiveMax"` + // ExclusiveMin is the exclusive lower numeric bound (JSON Schema + // exclusiveMinimum): an admissible value is > it. nil = this position + // declared none. + // + // It is a bound of its own rather than a flag on Min, because the two + // keywords are independent and conjunctive: a schema may declare both, both + // then apply, and the effective floor is whichever admits fewer values. One + // slot per side would have to keep that one and lower the other some other + // way, which is a change to the weaker keyword that a consumer diffing two + // revisions of a spec could not see at all (GitHub #425). + ExclusiveMin *BigVal `json:"exclusiveMin,omitempty"` + // ExclusiveMax is the exclusive upper numeric bound (JSON Schema + // exclusiveMaximum): an admissible value is < it. nil = this position + // declared none. It is independent of Max exactly as ExclusiveMin is of Min. + ExclusiveMax *BigVal `json:"exclusiveMax,omitempty"` // MultipleOf constrains the value to a multiple of this number. MultipleOf *BigVal `json:"multipleOf,omitempty"` // Precision bounds the total decimal digits (Avro decimal, XSD totalDigits, diff --git a/ir/constraints_test.go b/ir/constraints_test.go index b4109287..39d0112c 100644 --- a/ir/constraints_test.go +++ b/ir/constraints_test.go @@ -7,15 +7,16 @@ import ( ) // TestConstraints_JSONContract pins that every bound is a pointer (nil = -// unconstrained) except ExclusiveMin, ExclusiveMax, and UniqueItems, which -// are plain bools that always serialize — an unconstrained Constraints still -// asserts "not exclusive" and "not unique" as facts, not absences. It also -// pins that a fully populated Constraints round-trips with its BigVal decimal -// strings intact (no float64 anywhere in the IR). +// unconstrained) — the four numeric bounds alike, since exclusiveMinimum and +// exclusiveMaximum are bounds of their own rather than flags on minimum and +// maximum — leaving UniqueItems the one field that always serializes, because +// an unconstrained Constraints still asserts "not unique" as a fact rather than +// an absence. It also pins that a fully populated Constraints round-trips with +// its BigVal decimal strings intact (no float64 anywhere in the IR). func TestConstraints_JSONContract(t *testing.T) { t.Parallel() assertJSONContract(t, ir.Constraints{}, - `{"exclusiveMin":false,"exclusiveMax":false,"uniqueItems":false}`, + `{"uniqueItems":false}`, *populatedConstraints()) } diff --git a/ir/helpers_test.go b/ir/helpers_test.go index 113e8909..c5feca1b 100644 --- a/ir/helpers_test.go +++ b/ir/helpers_test.go @@ -314,6 +314,14 @@ func populatedConstraints() *ir.Constraints { if err != nil { panic(err) } + exclMinV, err := ir.NewBigVal("0") + if err != nil { + panic(err) + } + exclMaxV, err := ir.NewBigVal("101") + if err != nil { + panic(err) + } precision := int64(10) scale := int64(2) minLen := int64(1) @@ -325,8 +333,8 @@ func populatedConstraints() *ir.Constraints { return &ir.Constraints{ Min: &minV, Max: &maxV, - ExclusiveMin: true, - ExclusiveMax: true, + ExclusiveMin: &exclMinV, + ExclusiveMax: &exclMaxV, MultipleOf: &multV, Precision: &precision, Scale: &scale, diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json index cbc67588..75e7b04b 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json @@ -287,8 +287,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 3, "uniqueItems": false } diff --git a/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json b/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json index 7b3440c9..6b66caa3 100644 --- a/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json +++ b/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json @@ -48,8 +48,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "uniqueItems": false, "minProps": 3 } diff --git a/testdata/conformance/openapi/constraints.golden.json b/testdata/conformance/openapi/constraints.golden.json index 0c6fef2f..a4dfba39 100644 --- a/testdata/conformance/openapi/constraints.golden.json +++ b/testdata/conformance/openapi/constraints.golden.json @@ -36,8 +36,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minItems": 1, "maxItems": 5, "uniqueItems": true @@ -53,16 +51,6 @@ "anonymous": false, "docs": {}, "sensitive": false, - "unmodeled": { - "openapi:exclusiveMinimum": { - "reason": "degraded_lowering", - "value": 0, - "provenance": { - "source": 0, - "pointer": "/components/schemas/Bounded/exclusiveMinimum" - } - } - }, "provenance": { "source": 0, "pointer": "/components/schemas/Bounded" @@ -73,8 +61,7 @@ }, "constraints": { "min": "10", - "exclusiveMin": false, - "exclusiveMax": false, + "exclusiveMin": "0", "uniqueItems": false } }, @@ -113,8 +100,6 @@ "constraints": { "min": "0.30000000000000004", "max": "9007199254740993", - "exclusiveMin": false, - "exclusiveMax": false, "multipleOf": "0.1", "uniqueItems": false }, @@ -147,8 +132,7 @@ }, "constraints": { "min": "10", - "exclusiveMin": false, - "exclusiveMax": false, + "exclusiveMin": "0", "uniqueItems": false }, "flatten": false, @@ -156,16 +140,6 @@ "eventPayload": false, "secret": false, "docs": {}, - "unmodeled": { - "openapi:exclusiveMinimum": { - "reason": "degraded_lowering", - "value": 0, - "provenance": { - "source": 0, - "pointer": "/components/schemas/S/properties/atLeastTen/exclusiveMinimum" - } - } - }, "provenance": { "source": 0, "pointer": "/components/schemas/S/properties/atLeastTen" @@ -189,9 +163,8 @@ "none": false }, "constraints": { - "max": "10", - "exclusiveMin": false, - "exclusiveMax": true, + "max": "100", + "exclusiveMax": "10", "uniqueItems": false }, "flatten": false, @@ -199,16 +172,6 @@ "eventPayload": false, "secret": false, "docs": {}, - "unmodeled": { - "openapi:maximum": { - "reason": "degraded_lowering", - "value": 100, - "provenance": { - "source": 0, - "pointer": "/components/schemas/S/properties/underTen/maximum" - } - } - }, "provenance": { "source": 0, "pointer": "/components/schemas/S/properties/underTen" @@ -232,8 +195,6 @@ "none": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 2, "maxLength": 8, "uniqueItems": false @@ -277,8 +238,6 @@ } ], "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "uniqueItems": false, "minProps": 1, "maxProps": 4 @@ -334,40 +293,11 @@ "auth": null } ], - "diagnostics": [ - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "minimum 10 and exclusiveMinimum 0 both bound this value and the IR holds one bound per side; kept minimum as the tighter of the two, and exclusiveMinimum, which it implies, verbatim under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/components/schemas/S/properties/atLeastTen" - } - }, - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "exclusiveMaximum 10 and maximum 100 both bound this value and the IR holds one bound per side; kept exclusiveMaximum as the tighter of the two, and maximum, which it implies, verbatim under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/components/schemas/S/properties/underTen" - } - }, - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "minimum 10 and exclusiveMinimum 0 both bound this value and the IR holds one bound per side; kept minimum as the tighter of the two, and exclusiveMinimum, which it implies, verbatim under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/components/schemas/Bounded" - } - } - ], "sources": [ { "format": "openapi@3.1", "path": "constraints.yaml", - "hash": "421ee970477facfbd1d3d21838c66e21f37423d509929cd1d7504307a49db90a" + "hash": "26eb5b8535115a810b33386594ce36baafe0fc21f0bf7171d6cc923754a2ddaa" } ] } diff --git a/testdata/conformance/openapi/constraints.yaml b/testdata/conformance/openapi/constraints.yaml index 29970f41..a6524789 100644 --- a/testdata/conformance/openapi/constraints.yaml +++ b/testdata/conformance/openapi/constraints.yaml @@ -14,12 +14,13 @@ components: maximum: 9007199254740993 multipleOf: 0.1 # In 2020-12 the two keywords on a side are independent and both apply, - # so the effective bound is the tighter of them. Reading whichever came - # last published ">= 0" here and "< 100" below (GitHub #33). One bound - # slot per side means the other keyword reaches no field, so it is kept - # verbatim beside the constraints instead (GitHub #286) — without that, - # these two lower to exactly what `minimum: 10` and `exclusiveMaximum: - # 10` alone would. + # so ir.Constraints holds a field for each and each keeps the literal + # written. Reading whichever came last published ">= 0" here and "< 100" + # below (GitHub #33); keeping only the tighter left a change to the other + # keyword invisible to a consumer diffing two revisions (GitHub #425). + # The two sides are settled opposite ways — the inclusive bound is the + # tighter here, the exclusive one below — so neither field can be read + # off the other. atLeastTen: type: integer minimum: 10 @@ -37,8 +38,8 @@ components: uniqueItems: true items: {type: string} # A component whose body reduces to a shared primitive owns an alias node, - # the other carrier a co-declared bound can land on: the constraints go on - # the node, so the keyword they had no room for goes there too. + # the other carrier a co-declared pair can land on: both bounds go on the + # node's own constraints, exactly as they do on a property. Bounded: type: integer minimum: 10 diff --git a/testdata/conformance/openapi/encoding-byte.golden.json b/testdata/conformance/openapi/encoding-byte.golden.json index a86e4a85..c06231ca 100644 --- a/testdata/conformance/openapi/encoding-byte.golden.json +++ b/testdata/conformance/openapi/encoding-byte.golden.json @@ -36,8 +36,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 5, "maxLength": 9, "uniqueItems": false @@ -83,8 +81,6 @@ "none": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 5, "maxLength": 9, "uniqueItems": false diff --git a/testdata/conformance/openapi/header-content-schema.golden.json b/testdata/conformance/openapi/header-content-schema.golden.json index 40b0674c..98b99569 100644 --- a/testdata/conformance/openapi/header-content-schema.golden.json +++ b/testdata/conformance/openapi/header-content-schema.golden.json @@ -57,8 +57,6 @@ "none": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 4, "uniqueItems": false }, @@ -96,8 +94,6 @@ "none": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 4, "uniqueItems": false }, @@ -267,8 +263,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "pattern": "^r-[0-9]+$", "uniqueItems": false } diff --git a/testdata/conformance/openapi/inline-annotations.golden.json b/testdata/conformance/openapi/inline-annotations.golden.json index 4754e625..b2baf68b 100644 --- a/testdata/conformance/openapi/inline-annotations.golden.json +++ b/testdata/conformance/openapi/inline-annotations.golden.json @@ -37,8 +37,6 @@ }, "required": false, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 4, "uniqueItems": false }, @@ -104,8 +102,6 @@ "none": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 64, "uniqueItems": false }, @@ -196,8 +192,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 64, "uniqueItems": false } @@ -236,8 +230,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 3, "uniqueItems": false } @@ -262,8 +254,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 8192, "uniqueItems": false } diff --git a/testdata/conformance/openapi/multipart-encoding.golden.json b/testdata/conformance/openapi/multipart-encoding.golden.json index 7f4641e0..3dae8aa3 100644 --- a/testdata/conformance/openapi/multipart-encoding.golden.json +++ b/testdata/conformance/openapi/multipart-encoding.golden.json @@ -558,8 +558,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "pattern": "^[0-9a-f]{64}$", "uniqueItems": false } diff --git a/testdata/conformance/openapi/numeric-precision.golden.json b/testdata/conformance/openapi/numeric-precision.golden.json index c2281a27..44769852 100644 --- a/testdata/conformance/openapi/numeric-precision.golden.json +++ b/testdata/conformance/openapi/numeric-precision.golden.json @@ -168,8 +168,6 @@ "constraints": { "min": "1.8e308", "max": "123456789012345678901234567890", - "exclusiveMin": false, - "exclusiveMax": false, "multipleOf": "1e-30", "uniqueItems": false }, @@ -201,10 +199,8 @@ "none": false }, "constraints": { - "min": "0.5", - "max": "0.12345678901234567890123456789", - "exclusiveMin": true, - "exclusiveMax": true, + "exclusiveMin": "0.5", + "exclusiveMax": "0.12345678901234567890123456789", "uniqueItems": false }, "flatten": false, @@ -397,8 +393,6 @@ "constraints": { "min": "15", "max": "31", - "exclusiveMin": false, - "exclusiveMax": false, "multipleOf": "10", "uniqueItems": false }, diff --git a/testdata/conformance/openapi/param-ref-inheritance.golden.json b/testdata/conformance/openapi/param-ref-inheritance.golden.json index e1329165..86e43bf1 100644 --- a/testdata/conformance/openapi/param-ref-inheritance.golden.json +++ b/testdata/conformance/openapi/param-ref-inheritance.golden.json @@ -71,8 +71,6 @@ "object": null }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 100, "uniqueItems": false }, @@ -185,8 +183,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 64, "uniqueItems": false } @@ -269,8 +265,6 @@ "object": null }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "maxLength": 100, "uniqueItems": false }, diff --git a/testdata/conformance/openapi/scalar-format.golden.json b/testdata/conformance/openapi/scalar-format.golden.json index dc4cb80a..1141c1ae 100644 --- a/testdata/conformance/openapi/scalar-format.golden.json +++ b/testdata/conformance/openapi/scalar-format.golden.json @@ -36,8 +36,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 4, "uniqueItems": false }, @@ -78,8 +76,6 @@ "none": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 4, "uniqueItems": false }, diff --git a/testdata/conformance/openapi/unhomed-keywords.golden.json b/testdata/conformance/openapi/unhomed-keywords.golden.json index 4538a049..47aa1661 100644 --- a/testdata/conformance/openapi/unhomed-keywords.golden.json +++ b/testdata/conformance/openapi/unhomed-keywords.golden.json @@ -233,8 +233,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minItems": 3, "maxItems": 9, "uniqueItems": true @@ -523,8 +521,6 @@ "nullable": false }, "constraints": { - "exclusiveMin": false, - "exclusiveMax": false, "minLength": 3, "uniqueItems": false } From 49c9aba0161e28f6f35d65ec9ed338a3b0a3f7b3 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:49:16 +0300 Subject: [PATCH 4/6] fix(compilers/openapi): name what sits under contentSchema by its owner The contentSchema hoist added a structural position the outside-$ref namer did not know: structuralRole is the replay of the compile.SubHint call sites, and it was not extended, so an outside $ref reaching the position first named everything under it from the segment (content_schema_item) while the declaration named it from the owner (nested_content_item). The registry then depended on declaration order, and the harness said so. One case in structuralRole closes it. The two-order test that its doc named as the guard could not see the gap for any position: every row aimed the reference at the very node it asserted, which the declaration renames in either order (#372). Rows now say where the reference goes, and the row that pins this aims it above the node. The corpus gains the same shape, reference declared first, so the order oracle asks the question too; with the case reverted, the conformance golden, the two-order row and the in-repo harness sweep all redden. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- .../openapi/conformance_unmodeled_test.go | 14 +++ compilers/openapi/internal/schema/resolve.go | 11 +- .../openapi/internal/schema/schema_test.go | 37 ++++-- .../openapi/content-vocabulary.golden.json | 111 +++++++++++++++++- .../openapi/content-vocabulary.yaml | 16 +++ 5 files changed, 178 insertions(+), 11 deletions(-) diff --git a/compilers/openapi/conformance_unmodeled_test.go b/compilers/openapi/conformance_unmodeled_test.go index 801411c2..6c32a7e0 100644 --- a/compilers/openapi/conformance_unmodeled_test.go +++ b/compilers/openapi/conformance_unmodeled_test.go @@ -422,6 +422,20 @@ func assertContentVocabulary(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) assert.Equal(t, ir.ReasonNoIRHome, unmodeledEntry(t, bag.Unmodeled, key).Reason, "an object has no Encoding field, so %s is kept", key) } + + // The outside $ref reaches the contentSchema position first, and what is + // under it is still spelled from the declaration: the order-invariance oracle + // is what proves the two orders agree, and this is what says which spelling + // won (§4.3). + feed, ok := doc.Types[namedID("Feed")].(*ir.Scalar) + require.True(t, ok) + require.NotNil(t, feed.Encoding) + require.NotNil(t, feed.Encoding.Schema) + const contentItem = ir.TypeID("t/anon/components/schemas/Feed/contentSchema/items") + item, ok := doc.Types[contentItem] + require.True(t, ok, "the decoded array's item is hoisted at its own pointer") + assert.Equal(t, "feed_content_item", item.Common().Name.Hint, + "named from the enclosing declaration, not from the segment the reference offered") } // assertDialectKeywords pins the JSON Schema resource and dialect keywords as out diff --git a/compilers/openapi/internal/schema/resolve.go b/compilers/openapi/internal/schema/resolve.go index f1d5b6f2..bf4f7037 100644 --- a/compilers/openapi/internal/schema/resolve.go +++ b/compilers/openapi/internal/schema/resolve.go @@ -310,9 +310,12 @@ func structuralPointerHint(pointer string) (string, bool) { // structuralRole reports the role the structural lowering names the position at // the tail of segments by, and how many segments that position spells. The roles -// are the suffixes the four compile.SubHint call sites pass, and a change to one -// of them has to be made here too — TestInlinePosition_HintIsTheSameInBothOrders -// is what fails when they drift. +// are the suffixes the compile.SubHint call sites pass, and a change to one of +// them has to be made here too — TestInlinePosition_HintIsTheSameInBothOrders is +// what fails when they drift, provided its row aims the outside $ref above the +// node it asserts (see the refAt column there): a reference aimed at the node +// itself is renamed by the declaration in either order and cannot see a role +// missing here. // // segments holds at least two entries: its only caller reads the tail of a // pointer, which always starts with the empty segment before the first token, and @@ -324,6 +327,8 @@ func structuralRole(segments []string) (role string, consumed int, ok bool) { return "item", 1, true case "additionalProperties": return "value", 1, true + case "contentSchema": + return "content", 1, true } switch segments[len(segments)-2] { case "patternProperties": diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index a2d28ea0..ffbc82f6 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -2196,6 +2196,13 @@ func stolenPositions() []stolenPosition { // these after the keyword holding them ("items"), the pattern text ("^x") or the // slot ordinal ("0") — none of which distinguish the position from the same // position on any other schema. +// +// A row with no refAt aims the outside $ref at the node it asserts, and such a +// row cannot see a role missing from structuralRole: the declaration renames +// that very node in either order (#372), so both spellings agree on it whatever +// the reference called it. The collision surfaces one level below, where the +// subtree keeps the reference's name. A row meant to guard a role therefore +// aims the reference above the node it asserts, per refAt. func TestInlinePosition_HintIsTheSameInBothOrders(t *testing.T) { t.Parallel() for _, tc := range []struct { @@ -2203,28 +2210,44 @@ func TestInlinePosition_HintIsTheSameInBothOrders(t *testing.T) { owner string id ir.TypeID hint string + refAt ir.TypeID }{ {"items", " A: {type: array, items: " + openapitest.InlineProbeBody + "}\n", - "t/anon/components/schemas/A/items", "a_item"}, + "t/anon/components/schemas/A/items", "a_item", ""}, {"additionalProperties", " A: {type: object, additionalProperties: " + openapitest.InlineProbeBody + "}\n", - "t/anon/components/schemas/A/additionalProperties", "a_value"}, + "t/anon/components/schemas/A/additionalProperties", "a_value", ""}, {"patternProperties", " A: {type: object, patternProperties: {\"^x\": " + openapitest.InlineProbeBody + "}}\n", - "t/anon/components/schemas/A/patternProperties/^x", "a_pattern"}, + "t/anon/components/schemas/A/patternProperties/^x", "a_pattern", ""}, {"prefixItems", " A: {type: array, prefixItems: [" + openapitest.InlineProbeBody + "]}\n", - "t/anon/components/schemas/A/prefixItems/0", "a_0"}, + "t/anon/components/schemas/A/prefixItems/0", "a_0", ""}, // Nested, because the derivation replays the whole chain rather than one // step: the outside $ref used to name this "items", losing both levels. {"items under items", " A: {type: array, items: {type: array, items: " + openapitest.InlineProbeBody + "}}\n", - "t/anon/components/schemas/A/items/items", "a_item_item"}, + "t/anon/components/schemas/A/items/items", "a_item_item", ""}, // Rooted at a property rather than at the component, so the enclosing hint // the walk rebuilds from is the property's key. {"items under a property", " A: {type: object, properties: {p: {type: array, items: " + openapitest.InlineProbeBody + "}}}\n", - "t/anon/components/schemas/A/properties/p/items", "p_item"}, + "t/anon/components/schemas/A/properties/p/items", "p_item", ""}, + {"contentSchema", " A: {type: string, contentMediaType: application/json, contentSchema: " + + openapitest.InlineProbeBody + "}\n", + "t/anon/components/schemas/A/contentSchema", "a_content", ""}, + // The reference is aimed at contentSchema and the assertion at what is + // under it: pointed at the asserted node instead, this row passes with the + // contentSchema role removed, because the declaration renames that node + // itself. + {"items under contentSchema", " A: {type: string, contentMediaType: application/json, " + + "contentSchema: {type: array, items: " + openapitest.InlineProbeBody + "}}\n", + "t/anon/components/schemas/A/contentSchema/items", "a_content_item", + "t/anon/components/schemas/A/contentSchema"}, } { t.Run(tc.name, func(t *testing.T) { t.Parallel() - pos := stolenPosition{name: tc.name, owner: tc.owner, id: tc.id} + refAt := tc.refAt + if refAt == "" { + refAt = tc.id + } + pos := stolenPosition{name: tc.name, owner: tc.owner, id: refAt} for _, order := range []struct { name string refFirst bool diff --git a/testdata/conformance/openapi/content-vocabulary.golden.json b/testdata/conformance/openapi/content-vocabulary.golden.json index e546670a..c8179d6b 100644 --- a/testdata/conformance/openapi/content-vocabulary.golden.json +++ b/testdata/conformance/openapi/content-vocabulary.golden.json @@ -64,6 +64,70 @@ "positional": false, "inputOnly": false }, + "t/anon/components/schemas/Feed/contentSchema": { + "kind": "list", + "id": "t/anon/components/schemas/Feed/contentSchema", + "name": { + "hint": "feed_content" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Feed/contentSchema" + }, + "elem": { + "target": "t/anon/components/schemas/Feed/contentSchema/items", + "nullable": false + } + }, + "t/anon/components/schemas/Feed/contentSchema/items": { + "kind": "model", + "id": "t/anon/components/schemas/Feed/contentSchema/items", + "name": { + "hint": "feed_content_item" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Feed/contentSchema/items" + }, + "properties": [ + { + "id": "p/openapi/components/schemas/Feed/contentSchema/items/properties/id", + "name": { + "source": "id", + "canonical": "id" + }, + "wireName": "id", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Feed/contentSchema/items/properties/id" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, "t/openapi/components/schemas/Bag": { "kind": "model", "id": "t/openapi/components/schemas/Bag", @@ -139,6 +203,25 @@ "positional": false, "inputOnly": false }, + "t/openapi/components/schemas/Decoded": { + "kind": "scalar", + "id": "t/openapi/components/schemas/Decoded", + "name": { + "source": "Decoded", + "canonical": "decoded" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Decoded" + }, + "base": { + "target": "t/anon/components/schemas/Feed/contentSchema", + "nullable": false + } + }, "t/openapi/components/schemas/Envelope": { "kind": "scalar", "id": "t/openapi/components/schemas/Envelope", @@ -165,6 +248,32 @@ } } }, + "t/openapi/components/schemas/Feed": { + "kind": "scalar", + "id": "t/openapi/components/schemas/Feed", + "name": { + "source": "Feed", + "canonical": "feed" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Feed" + }, + "base": { + "target": "t/prim/string", + "nullable": false + }, + "encoding": { + "mediaType": "application/json", + "schema": { + "target": "t/anon/components/schemas/Feed/contentSchema", + "nullable": false + } + } + }, "t/openapi/components/schemas/Thumbnail": { "kind": "scalar", "id": "t/openapi/components/schemas/Thumbnail", @@ -244,7 +353,7 @@ { "format": "openapi@3.1", "path": "content-vocabulary.yaml", - "hash": "3ef4175a9903ee435b730ca49ade19bec243e6194599dcd221212ea6850fd81c" + "hash": "5866fcfbab242da99c5d4c259d1b45f85e7873d88924c822baf57edae5ed48da" } ] } diff --git a/testdata/conformance/openapi/content-vocabulary.yaml b/testdata/conformance/openapi/content-vocabulary.yaml index df32f2f9..ab0251ad 100644 --- a/testdata/conformance/openapi/content-vocabulary.yaml +++ b/testdata/conformance/openapi/content-vocabulary.yaml @@ -26,3 +26,19 @@ components: type: object properties: a: {type: string} + # An outside $ref can name the contentSchema position by pointer, interning + # the node the declaration owns before the declaration reaches it. Both + # namers have to spell what is under it the same way, so which one gets + # there first cannot decide the name (§4.3). Declared before its owner + # because that is the order that used to disagree. + Decoded: + $ref: '#/components/schemas/Feed/contentSchema' + Feed: + type: string + contentMediaType: application/json + contentSchema: + type: array + items: + type: object + properties: + id: {type: string} From c3649c5fe067311a9da1c4c79fa13f16822ed8a0 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:49:16 +0300 Subject: [PATCH 5/6] =?UTF-8?q?docs(ir-design):=20restore=20the=20=C2=A714?= =?UTF-8?q?=20clauses=20a=20stale=20paste=20dropped?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contentSchema commit rewrote the OpenAPI row from a copy taken before the ErrorCase fixes below it landed, and three unrelated facts went with it: the responses-map key reaching the IR neutralized rather than as written, the duplicate-status-key warning, and the payload hint derived from the declaration on both status sides. All three describe behaviour live on this branch. Word-diffed against the base, the row now differs by the two clauses this stack means to add and nothing else. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- docs/ir-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ir-design.md b/docs/ir-design.md index 5057a61a..7f8f114d 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1975,7 +1975,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; per-status responses/default → Conditions + ranges, with the responses-map key as written → Response.Name.Hint and ErrorCase.Name.Hint alike, so `4XX` and `default` round-trip the spelling their range cannot state; an error response lowers exactly as a success one — its `headers` → ErrorCase.Headers and every media type of its `content` → ErrorCase.Payload.Contents, neither degraded and neither kept under Unmodeled; response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType`/`contentSchema` → `Encoding` on the scalar the position lowers to — the last as `Encoding.Schema`, a TypeRef to the decoded shape hoisted at its own pointer — and all three → Unmodeled (`no_ir_home`) at a position with no `Encoding` field, per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3 — constraints excepted, since bounds conjoin rather than override: each position keeps the ones it declared and none is copied to a use site (§12.2); a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; per-status responses/default → Conditions + ranges, with the responses-map key as declared and then neutralized → Response.Name.Hint and ErrorCase.Name.Hint alike (`404`, `5_xx`, `default`), which records the spelling a range cannot state though only `default` survives neutralization unchanged; two keys resolving to one range — `4XX` beside `4xx` — are both kept and reported `openapi/duplicate-status-key`, since they reach the IR with one name and one condition; an error response lowers exactly as a success one — its `headers` → ErrorCase.Headers and every media type of its `content` → ErrorCase.Payload.Contents, neither degraded and neither kept under Unmodeled, and the payload's naming hint derived from the declaration pointer on both sides so that one `components/responses` entry mounted at a success and an error status interns one type whichever side reaches it first; response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType`/`contentSchema` → `Encoding` on the scalar the position lowers to — the last as `Encoding.Schema`, a TypeRef to the decoded shape hoisted at its own pointer — and all three → Unmodeled (`no_ir_home`) at a position with no `Encoding` field, per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3 — constraints excepted, since bounds conjoin rather than override: each position keeps the ones it declared and none is copied to a use site (§12.2); a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled | From 9cab8af688d4d5c05568bb1c4a59354a23ecb2f6 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:49:16 +0300 Subject: [PATCH 6/6] test(ir): hold every populated fixture to leaving no field zero populatedEncoding is the round-trip half of TestEncoding_JSONContract and did not set the new Schema field, so retagging it json:"-" left the package green. Set it, and add a reflective guard over every populated* struct fixture so the next added field cannot slip through the same way; a field a fixture leaves zero on purpose is listed against its reason and asserted in both directions. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- ir/constraints_test.go | 11 +++---- ir/fixtures_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++ ir/helpers_test.go | 1 + 3 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 ir/fixtures_test.go diff --git a/ir/constraints_test.go b/ir/constraints_test.go index 39d0112c..f3f95ada 100644 --- a/ir/constraints_test.go +++ b/ir/constraints_test.go @@ -20,11 +20,12 @@ func TestConstraints_JSONContract(t *testing.T) { *populatedConstraints()) } -// TestEncoding_JSONContract pins Encoding's omitempty contract — all three -// fields are optional, so a Property/Scalar with no encoding override -// marshals to an empty object rather than an explicit "no encoding" tag — -// and that a fully populated Encoding — name, a nested nullable WireType, and -// a media type — round-trips. +// TestEncoding_JSONContract pins Encoding's omitempty contract — every field +// is optional, so a Property/Scalar with no encoding override marshals to an +// empty object rather than an explicit "no encoding" tag — and that a fully +// populated Encoding — name, a nested nullable WireType, a media type, and a +// decoded Schema — round-trips. populatedEncoding is held to "fully" by +// TestPopulatedFixtures_LeaveNoFieldZero. func TestEncoding_JSONContract(t *testing.T) { t.Parallel() assertJSONContract(t, ir.Encoding{}, `{}`, *populatedEncoding()) diff --git a/ir/fixtures_test.go b/ir/fixtures_test.go new file mode 100644 index 00000000..560e7333 --- /dev/null +++ b/ir/fixtures_test.go @@ -0,0 +1,66 @@ +package ir_test + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestPopulatedFixtures_LeaveNoFieldZero holds every populated* fixture to the +// "every field non-zero" its doc comment claims. The fixtures are the round-trip +// half of the JSON-contract tests, and a field a fixture leaves zero is a field +// those tests say nothing about: Encoding.Schema shipped with its codec +// unasserted because populatedEncoding was not extended with it, and retagging +// the field json:"-" left the package green. +// +// Sums and maps are deliberately out: populatedValue sets only the payload its +// Kind selects, and a map fixture has no fields to leave zero. +func TestPopulatedFixtures_LeaveNoFieldZero(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + fixture any + // zeroOK names the fields a fixture leaves zero on purpose, against the + // reason. An entry is a claim a reviewer has to agree with, which is the + // point of spelling it here rather than dropping the fixture from the + // table: the rest of its fields stay covered. It is asserted in both + // directions, so an entry cannot outlive its reason. + zeroOK map[string]string + }{ + {"Naming", populatedNaming(), nil}, + {"Docs", populatedDocs(), nil}, + {"Provenance", populatedProvenance(), nil}, + {"Deprecation", populatedDeprecation(), nil}, + {"Availability", populatedAvailability(), nil}, + {"Constraints", populatedConstraints(), nil}, + {"Encoding", populatedEncoding(), nil}, + {"XMLHints", populatedXMLHints(), nil}, + {"TypeRef", populatedTypeRef(), nil}, + {"TypeCommon", populatedTypeCommon("t/openapi/components/schemas/User"), nil}, + {"Property", populatedProperty(), map[string]string{ + "EventPayload": "mutually exclusive with EventHeader, which the fixture sets; " + + "neither field carries omitempty, so both serialize either way", + }}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + rv := reflect.Indirect(reflect.ValueOf(tc.fixture)) + require.Equal(t, reflect.Struct, rv.Kind(), "fixture must be a struct or a pointer to one") + require.Positive(t, rv.NumField(), "a struct with no fields witnesses nothing") + for i := range rv.NumField() { + name := rv.Type().Field(i).Name + if why, exempt := tc.zeroOK[name]; exempt { + assert.Truef(t, rv.Field(i).IsZero(), + "%s.%s is listed as deliberately zero (%s) but the fixture sets it; drop the entry", + rv.Type().Name(), name, why) + continue + } + assert.Falsef(t, rv.Field(i).IsZero(), + "%s.%s is left zero, so the %s round-trip asserts nothing about it", + rv.Type().Name(), name, rv.Type().Name()) + } + }) + } +} diff --git a/ir/helpers_test.go b/ir/helpers_test.go index c5feca1b..9c64a291 100644 --- a/ir/helpers_test.go +++ b/ir/helpers_test.go @@ -356,6 +356,7 @@ func populatedEncoding() *ir.Encoding { Name: "rfc3339", WireType: &ir.TypeRef{Target: "t/prim/string", Nullable: true}, MediaType: "text/plain", + Schema: &ir.TypeRef{Target: "t/openapi/components/schemas/Decoded"}, } }