fix(compilers/openapi): keep the tighter of co-declared bounds

compilers/openapi/conformance_test.go

@@ -1103,7 +1103,7 @@ func assertRawPreservedDates(t *testing.T, doc *ir.Document) {
 	}
 }
 
-func assertConstraints(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
+func assertConstraints(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
 	m, ok := doc.Types[namedID("S")].(*ir.Model)
 	require.True(t, ok)
 	ratio, ok := propByWire(m, "ratio")
@@ -1127,6 +1127,38 @@ func assertConstraints(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	assert.Equal(t, int64(4), *m.Constraints.MaxProps)
 
 	assertLengthAndCollectionBounds(t, doc, m)
+	assertCoDeclaredBounds(t, m, diags)
+}
+
+// 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).
+//
+// 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.
+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")
+
+	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)
+
+	for _, want := range []string{"dropped exclusiveMinimum", "dropped maximum"} {
+		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)
+	}
 }
 
 // assertLengthAndCollectionBounds pins the non-numeric bounds: a string length

compilers/openapi/internal/annotation/constraints.go

@@ -15,7 +15,9 @@ import (
 // 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).
+// 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.
 //
 // 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
@@ -81,8 +83,9 @@ 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 sets the exclusive flag. exclusiveBoolean selects
-// the dialect (true for 3.0). Because load suppresses the library's type-mismatch
+// avoid the float64 trap, and hands it to reconcileBound, which decides how it
+// meets any minimum/maximum declared beside it. 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, isMin, exclusiveBoolean bool) []ir.Diagnostic {
@@ -110,8 +113,102 @@ func applyExclusive(c *ir.Constraints, s *oas3.Schema, isMin, exclusiveBoolean b
 	if err != nil {
 		return []ir.Diagnostic{boundLiteralDiag(prop, node.Value, err)}
 	}
-	setExclusiveBound(c, isMin, &v)
-	return nil
+	return reconcileBound(c, isMin, v)
+}
+
+// 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.
+//
+// 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).
+//
+// 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 {
+	incl, inclProp, exclProp := c.Max, "maximum", "exclusiveMaximum"
+	if isMin {
+		incl, inclProp, exclProp = c.Min, "minimum", "exclusiveMinimum"
+	}
+	if incl == nil {
+		setExclusiveBound(c, isMin, &excl)
+		return nil
+	}
+
+	tighter, compared := inclusiveIsTighter(*incl, excl, isMin)
+	if tighter {
+		return []ir.Diagnostic{redundantBoundDiag(inclProp, *incl, exclProp, excl, compared)}
+	}
+
+	dropped := *incl
+	setExclusiveBound(c, isMin, &excl)
+	return []ir.Diagnostic{redundantBoundDiag(exclProp, excl, inclProp, dropped, compared)}
+}
+
+// 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, isMin bool) (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
+	}
+	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.
+//
+// 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.
+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",
+			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",
+		keptProp, kept, dropProp, dropped, keptProp, dropProp)
 }
 
 // exclusiveFormDiag reports an exclusiveMinimum/exclusiveMaximum whose value form
@@ -137,7 +234,10 @@ func setExclusiveFlag(c *ir.Constraints, isMin bool) {
 	c.ExclusiveMax = true
 }
 
-// setExclusiveBound sets an exclusive numeric bound (2020-12 arm) on Min or Max.
+// 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.
 func setExclusiveBound(c *ir.Constraints, isMin bool, v *ir.BigVal) {
 	if isMin {
 		c.Min = v