diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d9..1e09245 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -1128,12 +1128,13 @@ func assertConstraints(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { assertLengthAndCollectionBounds(t, doc, m) assertCoDeclaredBounds(t, m, diags) + assertCoDeclaredBoundKept(t, doc, m) } // 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 the IR (GitHub #33). +// names the keyword that did not reach ir.Constraints (GitHub #33). // // 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 @@ -1154,13 +1155,41 @@ func assertCoDeclaredBounds(t *testing.T, m *ir.Model, diags []ir.Diagnostic) { assert.Equal(t, ir.BigVal("10"), *high.Constraints.Max, "exclusiveMaximum is the tighter bound") assert.True(t, high.Constraints.ExclusiveMax) - for _, want := range []string{"dropped exclusiveMinimum", "dropped maximum"} { + 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) - }), "a keyword the IR does not carry is reported, not dropped in silence: %q", want) + }), "the keyword ir.Constraints has no room for is named, not dropped in silence: %q", want) } } +// 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. +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) + + 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") + + 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") +} + // assertLengthAndCollectionBounds pins the non-numeric bounds: a string length // pair on the declaring property, and a collection bound on the List the array // position hoisted, which is the node that describes the collection. diff --git a/compilers/openapi/internal/annotation/constraints.go b/compilers/openapi/internal/annotation/constraints.go index ebf5dba..8b96267 100644 --- a/compilers/openapi/internal/annotation/constraints.go +++ b/compilers/openapi/internal/annotation/constraints.go @@ -4,6 +4,7 @@ import ( oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" "github.com/dexpace/morphic/compilers/openapi/internal/diag" + "github.com/dexpace/morphic/compilers/openapi/internal/ids" "github.com/dexpace/morphic/compilers/openapi/internal/value" "github.com/dexpace/morphic/ir" ) @@ -19,28 +20,63 @@ import ( // 2020-12 one a side that declares both of its keywords is settled by // reconcileBound rather than by whichever ran last. // +// 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. +// // 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 // way whoever asks, and none of it needs the lowering walk. Which dialect // applies is the caller's to decide — that is a fact about the document, not // about the schema, and it is the one thing this reader will not go and find. -func Constraints(s *oas3.Schema, exclusiveBoolean bool) (*ir.Constraints, []ir.Diagnostic) { +func Constraints(s *oas3.Schema, exclusiveBoolean bool, pointer string, srcIndex int) (*ir.Constraints, ir.Unmodeled, []ir.Diagnostic) { if s == nil { - return nil, nil + return nil, nil, nil } c := &ir.Constraints{} + site := boundSite{pointer: pointer, srcIndex: srcIndex} diags := numericBounds(c, s) - diags = append(diags, applyExclusive(c, s, true, exclusiveBoolean)...) - diags = append(diags, applyExclusive(c, s, false, exclusiveBoolean)...) + diags = append(diags, applyExclusive(c, s, &site, true, exclusiveBoolean)...) + diags = append(diags, applyExclusive(c, s, &site, false, exclusiveBoolean)...) c.MinLength = s.MinLength c.MaxLength = s.MaxLength c.Pattern = s.GetPattern() c.MinProps = s.MinProperties c.MaxProps = s.MaxProperties if emptyConstraints(c) { - return nil, diags + return nil, site.kept, diags } - return c, diags + return c, site.kept, diags +} + +// boundSite is where a schema's bounds were written, and what became of the +// co-declared keyword that reached no field of ir.Constraints. +// +// 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 +// it. Announcing a preservation from anywhere else is how a message comes to +// claim one that never happened (GitHub #144). +type boundSite struct { + pointer string + srcIndex int + kept ir.Unmodeled +} + +// keepRedundant keeps the co-declared keyword that ir.Constraints has no room +// for, and returns the diagnostic reporting the pair. +// +// 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 *boundSite) 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) } // numericBounds fills Min, Max, and MultipleOf from the raw minimum/maximum/ @@ -88,7 +124,7 @@ func boundLiteralDiag(prop, literal string, err error) ir.Diagnostic { // 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, isMin, exclusiveBoolean bool) []ir.Diagnostic { +func applyExclusive(c *ir.Constraints, s *oas3.Schema, site *boundSite, isMin, exclusiveBoolean bool) []ir.Diagnostic { ev, prop := s.GetExclusiveMaximum(), "exclusiveMaximum" if isMin { ev, prop = s.GetExclusiveMinimum(), "exclusiveMinimum" @@ -113,7 +149,7 @@ func applyExclusive(c *ir.Constraints, s *oas3.Schema, isMin, exclusiveBoolean b if err != nil { return []ir.Diagnostic{boundLiteralDiag(prop, node.Value, err)} } - return reconcileBound(c, isMin, v) + return reconcileBound(c, site, isMin, v) } // reconcileBound settles one side's bound when the 2020-12 dialect declares @@ -126,14 +162,13 @@ func applyExclusive(c *ir.Constraints, s *oas3.Schema, isMin, exclusiveBoolean b // taking the exclusive bound unconditionally, as this did before, published a // constraint weaker than the source wherever minimum was the tighter (GitHub #33). // -// The discarded keyword is implied by the kept one, so no value the source admits -// or excludes changes — but it is still a keyword the source wrote and the IR does -// not carry, so it is named in a diagnostic rather than dropped in silence. It is -// not also preserved verbatim: Constraints has no Unmodeled channel, and its -// callers route what it returns to different carriers — a property, a parameter -// and a hoisted alias node — so opening one is a change of its own, tracked in -// GitHub #286 rather than made here. -func reconcileBound(c *ir.Constraints, isMin bool, excl ir.BigVal) []ir.Diagnostic { +// 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 site 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, site *boundSite, isMin bool, excl ir.BigVal) []ir.Diagnostic { incl, inclProp, exclProp := c.Max, "maximum", "exclusiveMaximum" if isMin { incl, inclProp, exclProp = c.Min, "minimum", "exclusiveMinimum" @@ -145,12 +180,12 @@ func reconcileBound(c *ir.Constraints, isMin bool, excl ir.BigVal) []ir.Diagnost tighter, compared := inclusiveIsTighter(*incl, excl, isMin) if tighter { - return []ir.Diagnostic{redundantBoundDiag(inclProp, *incl, exclProp, excl, compared)} + return []ir.Diagnostic{site.keepRedundant(inclProp, *incl, exclProp, excl, compared)} } dropped := *incl setExclusiveBound(c, isMin, &excl) - return []ir.Diagnostic{redundantBoundDiag(exclProp, excl, inclProp, dropped, compared)} + return []ir.Diagnostic{site.keepRedundant(exclProp, excl, inclProp, dropped, compared)} } // inclusiveIsTighter reports whether the inclusive bound incl admits fewer @@ -189,25 +224,28 @@ func inclusiveIsTighter(incl, excl ir.BigVal, isMin bool) (tighter, compared boo return (order > 0) == isMin, true } -// redundantBoundDiag reports the co-declared 2020-12 bound that did not reach -// the IR, naming both keywords and both exact literals so a reader can see what -// was dropped without going back to the source. +// 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 dropped one 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. +// 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 and dropped %s, which may be the tighter of the two", + "kept %s as the bound, and %s, which may be the tighter of the two, verbatim under Unmodeled", keptProp, kept, dropProp, dropped, keptProp, dropProp) } 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 dropped %s, which it implies", + "kept %s as the tighter of the two, and %s, which it implies, verbatim under Unmodeled", keptProp, kept, dropProp, dropped, keptProp, dropProp) } diff --git a/compilers/openapi/internal/annotation/constraints_internal_test.go b/compilers/openapi/internal/annotation/constraints_internal_test.go index 6484004..9219c0d 100644 --- a/compilers/openapi/internal/annotation/constraints_internal_test.go +++ b/compilers/openapi/internal/annotation/constraints_internal_test.go @@ -12,8 +12,9 @@ import ( func TestConstraints_NilSchema(t *testing.T) { t.Parallel() - c, diags := Constraints(nil, false) + c, kept, diags := Constraints(nil, false, "/p", 0) assert.Nil(t, c) + assert.Nil(t, kept) assert.Nil(t, diags) } func TestApplyExclusive_NumericWithoutRootNode(t *testing.T) { @@ -21,9 +22,11 @@ func TestApplyExclusive_NumericWithoutRootNode(t *testing.T) { f := 5.0 s := &oas3.Schema{ExclusiveMinimum: &values.EitherValue[bool, bool, float64, float64]{Right: &f}} c := &ir.Constraints{} - diags := applyExclusive(c, s, true, false) + site := boundSite{pointer: "/p"} + diags := applyExclusive(c, s, &site, true, false) // 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.Empty(t, site.kept) } diff --git a/compilers/openapi/internal/annotation/constraints_readers_internal_test.go b/compilers/openapi/internal/annotation/constraints_readers_internal_test.go index 805375c..ed1c4ec 100644 --- a/compilers/openapi/internal/annotation/constraints_readers_internal_test.go +++ b/compilers/openapi/internal/annotation/constraints_readers_internal_test.go @@ -50,7 +50,7 @@ func TestConstraints_ReadsEveryScalarKeyword(t *testing.T) { s := schemaFromYAML(t, "type: string\nminimum: 1\nmaximum: 9\nmultipleOf: 3\n"+ "minLength: 2\nmaxLength: 8\npattern: '^a'\nminProperties: 1\nmaxProperties: 4\n") - got, diags := Constraints(s, false) + got, _, diags := Constraints(s, false, "/p", 0) require.Empty(t, diags) require.NotNil(t, got) @@ -70,11 +70,11 @@ func TestConstraints_ReadsEveryScalarKeyword(t *testing.T) { // wrote. func TestConstraints_NothingDeclaredIsNilNotEmpty(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: string\n"), false) + got, _, diags := Constraints(schemaFromYAML(t, "type: string\n"), false, "/p", 0) assert.Nil(t, got) assert.Empty(t, diags) - got, diags = Constraints(nil, false) + got, _, diags = Constraints(nil, false, "/p", 0) assert.Nil(t, got) assert.Nil(t, diags) } @@ -86,7 +86,7 @@ func TestConstraints_KeepsTheExactLiteral(t *testing.T) { t.Parallel() s := schemaFromYAML(t, "type: number\nminimum: 9007199254740993\nmaximum: 0.30000000000000004\n") - got, diags := Constraints(s, false) + got, _, diags := Constraints(s, false, "/p", 0) require.Empty(t, diags) require.NotNil(t, got) @@ -103,7 +103,7 @@ func TestNumericBounds_AMalformedLiteralIsReportedNotDropped(t *testing.T) { for _, keyword := range []string{"minimum", "maximum", "multipleOf"} { t.Run(keyword, func(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: number\n"+keyword+": .inf\n"), false) + got, _, diags := Constraints(schemaFromYAML(t, "type: number\n"+keyword+": .inf\n"), false, "/p", 0) require.Len(t, diags, 1) assert.Equal(t, ir.SeverityError, diags[0].Severity) @@ -146,7 +146,7 @@ func TestApplyExclusive_BothDialects(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), tc.exclusiveBoolean) + got, _, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), tc.exclusiveBoolean, "/p", 0) require.Empty(t, diags) require.NotNil(t, got) @@ -182,7 +182,7 @@ func TestApplyExclusive_TheWrongFormForTheDialectIsReported(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), tc.exclusiveBoolean) + got, _, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), tc.exclusiveBoolean, "/p", 0) require.Len(t, diags, 1) assert.Equal(t, ir.SeverityError, diags[0].Severity) @@ -200,7 +200,7 @@ func TestApplyExclusive_TheWrongFormForTheDialectIsReported(t *testing.T) { // has the same way to fail as minimum and maximum do. func TestApplyExclusive_AMalformedNumericBoundIsReported(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: number\nexclusiveMaximum: .inf\n"), false) + got, _, diags := Constraints(schemaFromYAML(t, "type: number\nexclusiveMaximum: .inf\n"), false, "/p", 0) require.Len(t, diags, 1) assert.Equal(t, diag.NumericPrecision, diags[0].Code) @@ -218,80 +218,101 @@ func TestApplyExclusive_AMalformedNumericBoundIsReported(t *testing.T) { // 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. +// +// 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. func TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBounds(t *testing.T) { t.Parallel() tests := []struct { name string body string want ir.Constraints + wantKept string + wantRaw string wantSays []string }{ { - name: "minimum is the tighter of the pair", - body: "minimum: 10\nexclusiveMinimum: 0\n", - want: ir.Constraints{Min: bigOf("10")}, + 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", "dropped exclusiveMinimum"}, + "kept minimum as the tighter of the two, and exclusiveMinimum, " + + "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("10"), ExclusiveMin: true}, + 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", "dropped minimum"}, + "kept exclusiveMinimum as the tighter of the two, and minimum, " + + "which it implies, verbatim under Unmodeled"}, }, { - name: "equal minimums leave the exclusive one standing", - body: "minimum: 5\nexclusiveMinimum: 5\n", - want: ir.Constraints{Min: bigOf("5"), ExclusiveMin: true}, - wantSays: []string{"exclusiveMinimum 5", "minimum 5", - "kept exclusiveMinimum as the tighter", "dropped minimum"}, + 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: "maximum is the tighter of the pair", - body: "maximum: 10\nexclusiveMaximum: 100\n", - want: ir.Constraints{Max: bigOf("10")}, - wantSays: []string{"maximum 10", "exclusiveMaximum 100", - "kept maximum as the tighter", "dropped exclusiveMaximum"}, + 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: "exclusiveMaximum is the tighter of the pair", - body: "maximum: 100\nexclusiveMaximum: 10\n", - want: ir.Constraints{Max: bigOf("10"), ExclusiveMax: true}, - wantSays: []string{"exclusiveMaximum 10", "maximum 100", - "kept exclusiveMaximum as the tighter", "dropped maximum"}, + 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: "equal maximums leave the exclusive one standing", - body: "maximum: 5\nexclusiveMaximum: 5\n", - want: ir.Constraints{Max: bigOf("5"), ExclusiveMax: true}, - wantSays: []string{"exclusiveMaximum 5", "maximum 5", - "kept exclusiveMaximum as the tighter", "dropped maximum"}, + 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: "a bound decided by a digit float64 cannot hold", - body: "minimum: 9007199254740993\nexclusiveMinimum: 9007199254740992\n", - want: ir.Constraints{Min: bigOf("9007199254740993")}, + 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", "dropped exclusiveMinimum"}, + "kept minimum as the tighter"}, }, { - name: "one value spelled two ways is still a tie", - body: "minimum: 1e2\nexclusiveMinimum: 100\n", - want: ir.Constraints{Min: bigOf("100"), ExclusiveMin: true}, - wantSays: []string{"exclusiveMinimum 100", "minimum 1e2", - "kept exclusiveMinimum as the tighter", "dropped minimum"}, + 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"}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), false) + got, kept, diags := Constraints(schemaFromYAML(t, "type: number\n"+tc.body), false, "/p", 3) 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 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) @@ -306,6 +327,9 @@ func TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBounds(t *testing.T) { // 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) { t.Parallel() for _, body := range []string{ @@ -315,10 +339,11 @@ func TestReconcileBound_OneKeywordPerSideIsNotReconciled(t *testing.T) { } { t.Run(body, func(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, "type: number\n"+body), false) + got, kept, diags := Constraints(schemaFromYAML(t, "type: number\n"+body), false, "/p", 0) require.NotNil(t, got) assert.Empty(t, diags) + assert.Empty(t, kept) }) } } @@ -327,14 +352,16 @@ func TestReconcileBound_OneKeywordPerSideIsNotReconciled(t *testing.T) { // 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. +// 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, diags := Constraints(schemaFromYAML(t, - "type: number\nminimum: 10\nexclusiveMinimum: true\nmaximum: 20\nexclusiveMaximum: true\n"), true) + 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) @@ -350,11 +377,12 @@ func TestReconcileBound_ThreeZeroDialectPairIsUntouched(t *testing.T) { // exclusive on one side can tell the two apart. func TestApplyExclusive_ThreeZeroFlagsTheSideItWasReadFrom(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAML(t, - "type: number\nminimum: 10\nexclusiveMinimum: true\nmaximum: 20\n"), true) + 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) @@ -378,32 +406,37 @@ func TestReconcileBound_AMagnitudeNoRationalHoldsStillCompares(t *testing.T) { name string body string want ir.Constraints + wantKept string + wantRaw string wantSays []string }{ { - 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")}, - wantSays: []string{"minimum 1.0e2000000", "exclusiveMinimum 5", - "kept minimum as the tighter", "dropped exclusiveMinimum"}, + 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: "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")}, - wantSays: []string{"maximum 1e-1000001", "exclusiveMaximum 5", - "kept maximum as the tighter", "dropped exclusiveMaximum"}, + 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"}, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - got, diags := Constraints(schemaFromYAMLUnvalidated(t, "type: number\n"+tc.body), false) + got, kept, diags := Constraints(schemaFromYAMLUnvalidated(t, "type: number\n"+tc.body), false, "/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 { @@ -427,8 +460,9 @@ func TestReconcileBound_AMagnitudeNoRationalHoldsStillCompares(t *testing.T) { func TestReconcileBound_ABoundNoDecimalReadingOrdersKeepsTheExclusiveOne(t *testing.T) { t.Parallel() c := &ir.Constraints{Min: bigOf("1p4")} + site := boundSite{pointer: "/p", srcIndex: 1} - diags := reconcileBound(c, true, ir.BigVal("5")) + diags := reconcileBound(c, &site, true, ir.BigVal("5")) want := ir.Constraints{Min: bigOf("5"), ExclusiveMin: true} if diff := cmp.Diff(want, *c); diff != "" { @@ -440,4 +474,15 @@ func TestReconcileBound_ABoundNoDecimalReadingOrdersKeepsTheExclusiveOne(t *test 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 := site.kept["openapi:minimum"] + require.True(t, ok, "the unordered bound is kept verbatim; got %v", site.kept) + assert.Equal(t, "1p4", string(entry.Value)) + assert.Equal(t, ir.Provenance{Source: 1, Pointer: "/p/minimum"}, entry.Provenance) } diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index e0875ee..0ab4a5c 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -136,8 +136,12 @@ func fillParamSchema(c lowering.Ctx, ts *compile.Types, param *ir.Parameter, js tgt := resolve.TargetSchema(js, s) diags := fillParamDefault(c, param, s, tgt, pointer) - cons, consDiags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean()) + // The co-declared bound keyword ir.Constraints has no field for is kept on + // the parameter, the carrier at this position, exactly as a property keeps + // its own (GitHub #286). + cons, kept, consDiags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean(), pointer, c.SrcIndex) diags = append(diags, schema.StampConstraintDiags(c, consDiags, pointer)...) + param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, kept) if cons != nil { param.Constraints = cons } diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index ab7feed..b33ca60 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -1,6 +1,7 @@ package operation_test import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -691,3 +692,49 @@ func TestParams_ReservedHeaderNamesAreReported(t *testing.T) { }) } } + +// 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. +// +// 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) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, 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: plain, in: query, schema: {type: integer, minimum: 10}}\n"+ + " responses: {\"204\": {description: ok}}\n")) + requireNoErrorDiags(t, diags) + params := paramsOf(t, svc) + + cases := []struct { + param, index, wantKept, wantRaw string + }{ + {param: "low", index: "0", wantKept: "openapi:exclusiveMinimum", wantRaw: "0"}, + {param: "high", index: "1", wantKept: "openapi:maximum", wantRaw: "100"}, + } + 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") + }) + } + + assert.Empty(t, params["plain"].Unmodeled, + "a side writing one keyword has it in a field, so nothing is restated beside it") +} diff --git a/compilers/openapi/internal/schema/resolve.go b/compilers/openapi/internal/schema/resolve.go index a340714..1999eb7 100644 --- a/compilers/openapi/internal/schema/resolve.go +++ b/compilers/openapi/internal/schema/resolve.go @@ -151,9 +151,10 @@ func hoistSubSchema(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, dep if owned, ok := ts.Lookup(pointer); ok { return owned, true, diags } - cons, consDiags := schemaConstraints(c, s.Node, pointer) + var kept ir.Unmodeled + cons, consDiags := schemaConstraints(c, &kept, s.Node, pointer) diags = append(diags, consDiags...) - id := internAlias(c, ts, pointer, hint, ref, cons) + id := internAlias(c, ts, pointer, hint, ref, cons, kept) // As in lowerComponentSchema: this alias is the first node the pointer owns, // so the annotations Ref had nowhere to put now have a home. return id, true, append(diags, attachDeclaredAnnotations(c, ts, anchors, s.Node, pointer)...) diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb..8a38413 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -56,9 +56,10 @@ func lowerComponentSchema(c lowering.Ctx, ts *compile.Types, anchors *AnchorInde if _, owned := ts.Lookup(pointer); owned { return diags } - cons, consDiags := schemaConstraints(c, s.Node, pointer) + var kept ir.Unmodeled + cons, consDiags := schemaConstraints(c, &kept, s.Node, pointer) diags = append(diags, consDiags...) - internAlias(c, ts, pointer, name, ref, cons) + internAlias(c, ts, pointer, name, ref, cons, kept) // This alias is the first node the pointer owns, so the annotations // schemaBody had nowhere to put now have a home. if s.Node != nil { @@ -153,11 +154,17 @@ func recordResidue(c lowering.Ctx, common *ir.TypeCommon, s *oas3.Schema, pointe // internal sub-schema (hoistSubSchema) — so a scalar that aliases a shared // primitive never drops the constraints it carried, including a bound written // beside a $ref, which constrains the position it is written at. -func schemaConstraints(c lowering.Ctx, s *oas3.Schema, pointer string) (*ir.Constraints, []ir.Diagnostic) { +// +// p is the Unmodeled map of the same carrier the constraints are about to land +// on. A co-declared numeric bound leaves one keyword with no field of +// ir.Constraints to reach, and it is kept there — beside the constraints it did +// not reach, wherever those go (GitHub #286). +func schemaConstraints(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, pointer string) (*ir.Constraints, []ir.Diagnostic) { if s == nil { return nil, nil } - cons, diags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean()) + cons, kept, diags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean(), pointer, c.SrcIndex) + *p = annotation.MergeUnmodeled(*p, kept) return cons, StampConstraintDiags(c, diags, pointer) } @@ -176,15 +183,16 @@ func StampConstraintDiags(c lowering.Ctx, diags []ir.Diagnostic, pointer string) // component (or a sibling-carrying schema) whose body lowered to a shared or // referenced target still owns a resolvable node at its own TypeID. Any value // constraints the schema carried are attached so a scalar component never drops -// them. +// them, and kept holds what those constraints had no field for — the co-declared +// bound keyword schemaConstraints read alongside them. func internAlias(c lowering.Ctx, ts *compile.Types, pointer, hint string, - target ir.TypeRef, constraints *ir.Constraints, + target ir.TypeRef, constraints *ir.Constraints, kept ir.Unmodeled, ) ir.TypeID { return internNode(c, ts, pointer, hint, func(common ir.TypeCommon) ir.TypeDef { base := target + common.Unmodeled = annotation.MergeUnmodeled(common.Unmodeled, kept) return &ir.Scalar{TypeCommon: common, Base: &base, Constraints: constraints} }) - } // schemaBody lowers a concrete (non-reference) schema body to a TypeRef and @@ -227,8 +235,9 @@ func hoistDeclarationHome(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, ref if id, owned := ts.Lookup(pointer); owned { return ir.TypeRef{Target: id, Nullable: ref.Nullable}, nil } - cons, diags := schemaConstraints(c, s, pointer) - id := internAlias(c, ts, pointer, hint, ref, cons) + var kept ir.Unmodeled + cons, diags := schemaConstraints(c, &kept, s, pointer) + id := internAlias(c, ts, pointer, hint, ref, cons, kept) return ir.TypeRef{Target: id, Nullable: ref.Nullable}, diags } @@ -381,7 +390,12 @@ func lowerBesideUnmodeledUnion(c lowering.Ctx, ts *compile.Types, anchors *Ancho // The structural body reduced to a shared/aliased target; hoist an alias // so the preserved union attaches to a node this pointer owns, never to a // shared primitive. - owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, nil) + // + // Alone among the alias hoists this one reads no constraints, so the + // position's bounds — and with them the co-declared keyword kept beside + // them — reach no field here. That is GitHub #343, deliberately left as + // it was rather than settled as a side effect of the keyword's own fix. + owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, nil, nil) } return owner, append(diags, preserveUnionSiblings(c, ts, owner, s, pointer, reason, why)...) } @@ -648,9 +662,10 @@ func preserveUnhomedKeywords(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, } owner := id if got, _ := ts.Lookup(pointer); got != id { - cons, consDiags := schemaConstraints(c, s, pointer) + var kept ir.Unmodeled + cons, consDiags := schemaConstraints(c, &kept, s, pointer) diags = append(diags, consDiags...) - owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: id}, cons) + owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: id}, cons, kept) } diags = append(diags, recordUnhomedKeywords(c, ts, owner, s, unhomed, td.Kind(), pointer)...) return owner, append(diags, recordSkippedFamilies(c, ts, owner, s, d, pointer)...) @@ -784,7 +799,7 @@ func lowerUnion(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i func lowerModel(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 { - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) m := &ir.Model{TypeCommon: common, Constraints: cons} diags = append(diags, fillModelProperties(c, ts, anchors, depth, m, s, pointer)...) @@ -930,14 +945,16 @@ func fillPropertyDefault(c lowering.Ctx, p *ir.Property, ref, tgt *oas3.Schema, return nil } -// fillPropertyConstraints attaches the property's scalar constraints and stamps -// each constraint diagnostic with the property's provenance. +// fillPropertyConstraints attaches the property's scalar constraints, and the +// 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. func fillPropertyConstraints(c lowering.Ctx, p *ir.Property, ref *oas3.Schema, pointer string) []ir.Diagnostic { - cons, diags := annotation.Constraints(ref, c.ExclusiveBoundIsBoolean()) + cons, diags := schemaConstraints(c, &p.Unmodeled, ref, pointer) if cons != nil { p.Constraints = cons } - return StampConstraintDiags(c, diags, pointer) + return diags } // attachDeclaredAnnotations records every annotation s declares on the type @@ -1091,7 +1108,7 @@ func hoistByteScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer, enc, encDiags := scalarEncoding(c, s, "base64", &common, pointer) diags = append(diags, encDiags...) enc.WireType = &wire - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) return &ir.Scalar{ TypeCommon: common, @@ -1111,7 +1128,7 @@ func hoistFormatScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, base i baseRef := ts.PrimRef(base) enc, encDiags := scalarEncoding(c, s, format, &common, pointer) diags = append(diags, encDiags...) - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) return &ir.Scalar{ TypeCommon: common, @@ -1133,7 +1150,7 @@ func hoistContentScalar(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, prim base := ts.PrimRef(prim) enc, encDiags := scalarEncoding(c, s, "", &common, pointer) diags = append(diags, encDiags...) - cons, consDiags := schemaConstraints(c, s, pointer) + cons, consDiags := schemaConstraints(c, &common.Unmodeled, s, pointer) diags = append(diags, consDiags...) return &ir.Scalar{ TypeCommon: common, diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index e1b560a..0062857 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -91,9 +91,11 @@ func TestSchemaConstraints_NonSchemaInputs(t *testing.T) { annotation.SchemaOf(oas3.NewJSONSchemaFromBool(true)), annotation.SchemaOf(oas3.NewJSONSchemaFromReference("#/components/schemas/Other")), } { - cons, diags := schemaConstraints(l.ctx, js, "/p") + var kept ir.Unmodeled + cons, diags := schemaConstraints(l.ctx, &kept, js, "/p") assert.Nil(t, cons) assert.Empty(t, diags) + assert.Empty(t, kept) } } @@ -107,9 +109,11 @@ func TestSchemaConstraints_EmptyRefSchema(t *testing.T) { l := newRawLowerer(&soa.OpenAPI{}) emptyRef := references.Reference("") js := oas3.NewJSONSchemaFromSchema[oas3.Referenceable](&oas3.Schema{Ref: &emptyRef}) - cons, diags := schemaConstraints(l.ctx, annotation.SchemaOf(js), "/p") + var kept ir.Unmodeled + cons, diags := schemaConstraints(l.ctx, &kept, annotation.SchemaOf(js), "/p") assert.Nil(t, cons) assert.Empty(t, diags) + assert.Empty(t, kept) } func TestResolveSchemaRef_ReusesInternedSubSchema(t *testing.T) { diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index f4b1c91..7a74357 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -3563,3 +3563,88 @@ func TestCoDeclaredFamily_UnpreservableIsNotAnnounced(t *testing.T) { assert.Empty(t, preservationClaims(diags), "nothing was written under Unmodeled, so nothing may announce that it was") } + +// 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. +// +// 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) { + t.Parallel() + doc, diags := parseFull(t, componentSpec( + " Alias: {type: integer, minimum: 10, exclusiveMinimum: 0}\n"+ + " Tight: {type: integer, maximum: 100, exclusiveMaximum: 5}\n"+ + " Holder:\n type: object\n properties:\n"+ + " low: {type: integer, minimum: 10, exclusiveMinimum: 0}\n"+ + " high: {type: integer, maximum: 100, exclusiveMaximum: 5}\n")) + requireNoErrorDiags(t, diags) + + tests := []struct { + name string + unmod ir.Unmodeled + bound *ir.Constraints + wantKept string + wantRaw string + at string + }{ + { + 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 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: "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 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", + }, + } + 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) + assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason) + assert.JSONEq(t, tc.wantRaw, string(entry.Value)) + assert.Equal(t, tc.at, entry.Provenance.Pointer) + }) + } +} + +// TestCoDeclaredBound_ASingleKeywordKeepsNothing is the other half of the case +// above: a side writing one keyword has it in a field, so an entry restating it +// would give one bound two homes and make the two source shapes indistinguishable +// in the opposite direction. +func TestCoDeclaredBound_ASingleKeywordKeepsNothing(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, componentSpec( + " Alias: {type: integer, minimum: 10}\n"+ + " Holder: {type: object, properties: {low: {type: integer, exclusiveMinimum: 0}}}\n")) + requireNoErrorDiags(t, diags) + + assert.Empty(t, typeByName(doc, "Alias").Common().Unmodeled) + assert.Empty(t, propertyOf(t, doc, "Holder", "low").Unmodeled) +} diff --git a/testdata/conformance/openapi/constraints.golden.json b/testdata/conformance/openapi/constraints.golden.json index ea750ff..0c6fef2 100644 --- a/testdata/conformance/openapi/constraints.golden.json +++ b/testdata/conformance/openapi/constraints.golden.json @@ -43,6 +43,41 @@ "uniqueItems": true } }, + "t/openapi/components/schemas/Bounded": { + "kind": "scalar", + "id": "t/openapi/components/schemas/Bounded", + "name": { + "source": "Bounded", + "canonical": "bounded" + }, + "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" + }, + "base": { + "target": "t/prim/integer", + "nullable": false + }, + "constraints": { + "min": "10", + "exclusiveMin": false, + "exclusiveMax": false, + "uniqueItems": false + } + }, "t/openapi/components/schemas/S": { "kind": "model", "id": "t/openapi/components/schemas/S", @@ -121,6 +156,16 @@ "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" @@ -154,6 +199,16 @@ "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" @@ -283,7 +338,7 @@ { "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 dropped exclusiveMinimum, which it implies", + "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" @@ -292,18 +347,27 @@ { "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 dropped maximum, which it implies", + "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": "3153e80d3265e6f1d00a35b4955110a7d9b95b79984f4556f6e81edcc23801e7" + "hash": "421ee970477facfbd1d3d21838c66e21f37423d509929cd1d7504307a49db90a" } ] } diff --git a/testdata/conformance/openapi/constraints.yaml b/testdata/conformance/openapi/constraints.yaml index ad2e148..29970f4 100644 --- a/testdata/conformance/openapi/constraints.yaml +++ b/testdata/conformance/openapi/constraints.yaml @@ -16,7 +16,10 @@ components: # 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 is reported rather than kept. + # 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. atLeastTen: type: integer minimum: 10 @@ -33,3 +36,10 @@ components: maxItems: 5 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. + Bounded: + type: integer + minimum: 10 + exclusiveMinimum: 0