fix(compilers/openapi): keep bounds beside a preserved union (#363)

PR: #363

Author: OmarAlJarrah

compilers/openapi/conformance_test.go

@@ -759,6 +759,9 @@ func assertAllOfRequiredOnly(t *testing.T, doc *ir.Document, _ []ir.Diagnostic)
 // assertAllOfOneOfCooccurrence pins both halves of the co-declared composition
 // rule: §4.3 distributes a union whose branches all name referents, and §4.8
 // keeps one with an inline branch verbatim rather than distributing it halfway.
+// The verbatim half is covered over a model body, which owns a node already, and
+// over a scalar one, which does not — there the alias hoisted for the union
+// carries what the position wrote beside it too.
 // The outside reference to a branch pointer pins the third thing: a composed
 // variant is Morphic's own node, so it cannot be taken by, or take from, a $ref.
 func assertAllOfOneOfCooccurrence(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
@@ -780,6 +783,15 @@ func assertAllOfOneOfCooccurrence(t *testing.T, doc *ir.Document, _ []ir.Diagnos
 	require.True(t, ok, "and the union it could not absorb survives beside it")
 	assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason)
 
+	bounded, ok := doc.Types[namedID("BoundedKinds")].(*ir.Scalar)
+	require.True(t, ok, "a body that is not a model reduces to a shared primitive and hoists an alias")
+	entry, ok = bounded.Unmodeled["openapi:oneOf"]
+	require.True(t, ok, "which is the node the kept union sits on")
+	assert.Equal(t, ir.ReasonDegradedLowering, entry.Reason)
+	require.NotNil(t, bounded.Constraints, "and the bounds written beside the union sit on it too")
+	require.NotNil(t, bounded.Constraints.MinLength)
+	assert.Equal(t, int64(3), *bounded.Constraints.MinLength)
+
 	outsider, ok := doc.Types[namedID("Outsider")].(*ir.Model)
 	require.True(t, ok)
 	require.Len(t, outsider.Properties, 1)

compilers/openapi/internal/schema/compose_test.go

@@ -2330,6 +2330,90 @@ func TestOneOf_CoDeclaredNotDistributedReasons(t *testing.T) {
 		"each declined shape is reported once; got %+v", diags)
 }
 
+// TestUnionCombinators_CoDeclaredKeepsTheBoundsWrittenBesideIt pins that keeping
+// a union verbatim does not cost the position the value constraints written
+// beside it. The alias exists so the union attaches to a node this pointer owns
+// rather than to the shared primitive the body reduced to, and owning a node is
+// what stops hoistDeclarationHome hoisting the alias that would otherwise carry
+// the bounds — so this alias has to carry them itself, as every other hoist here
+// does (GitHub #343).
+//
+// Both reasons that keep a union hoist the same alias, and anyOf rides the same
+// path as oneOf, so each is covered. The last case pins what reading the bounds
+// also produces: the co-declared-bound reconciliation reports at a position
+// nothing used to read, and the keyword it cannot home is kept on the node the
+// bounds landed on. The key set is asserted whole — carrying the constraints
+// without that keyword leaves the diagnostic naming an entry the node lacks.
+func TestUnionCombinators_CoDeclaredKeepsTheBoundsWrittenBesideIt(t *testing.T) {
+	t.Parallel()
+	three := int64(3)
+	ten, five := ir.BigVal("10"), ir.BigVal("5")
+	cases := []struct {
+		name, schemas, unionKey string
+		reason                  ir.UnmodeledReason
+		want                    ir.Constraints
+		wantKept                []string
+		wantDiag                string
+	}{
+		{
+			name:     "a validation-only union",
+			schemas:  "    A: {type: string, minLength: 3, oneOf: [{minLength: 1}, {minLength: 2}]}\n",
+			unionKey: "openapi:oneOf",
+			reason:   ir.ReasonValidationOnly,
+			want:     ir.Constraints{MinLength: &three},
+			wantKept: []string{"openapi:oneOf"},
+		},
+		{
+			name:     "a union kept as a degraded lowering",
+			schemas:  "    A: {type: number, minimum: 10, multipleOf: 5, oneOf: [{type: string}, {type: integer}]}\n",
+			unionKey: "openapi:oneOf",
+			reason:   ir.ReasonDegradedLowering,
+			want:     ir.Constraints{Min: &ten, MultipleOf: &five},
+			wantKept: []string{"openapi:oneOf"},
+		},
+		{
+			name:     "an anyOf kept in place of a oneOf",
+			schemas:  "    A: {type: string, minLength: 3, anyOf: [{minLength: 1}, {minLength: 2}]}\n",
+			unionKey: "openapi:anyOf",
+			reason:   ir.ReasonValidationOnly,
+			want:     ir.Constraints{MinLength: &three},
+			wantKept: []string{"openapi:anyOf"},
+		},
+		{
+			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",
+		},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			doc, diags := lowerSpec(t, openapitest.ComponentSpec(tc.schemas))
+			openapitest.RequireNoErrorDiags(t, diags)
+
+			sc, ok := typeByName(doc, "A").(*ir.Scalar)
+			require.True(t, ok, "the preserved union hoists an alias over the shared primitive")
+			entry, ok := sc.Unmodeled[tc.unionKey]
+			require.True(t, ok, "and keeps the union on it")
+			assert.Equal(t, tc.reason, entry.Reason)
+			require.NotNil(t, sc.Constraints, "while keeping the bounds written beside it")
+			assert.Empty(t, cmp.Diff(tc.want, *sc.Constraints))
+			assert.Equal(t, tc.wantKept, unmodeledKeys(sc.Unmodeled),
+				"the bound keyword that reaches no Constraints field is kept on the same node")
+			if tc.wantDiag == "" {
+				return
+			}
+			assert.Contains(t,
+				openapitest.DiagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/components/schemas/A"),
+				tc.wantDiag, "reading the bounds is what reports on them")
+		})
+	}
+}
+
 // TestUnionCombinators_PassedOverBranchSetIsKept covers the preference nothing
 // used to record (GitHub #35). unionBranches takes oneOf whenever it is written
 // and falls back to anyOf only when it is not, so a schema declaring both lost

compilers/openapi/internal/schema/schema.go

@@ -428,13 +428,15 @@ func lowerBesideUnmodeledUnion(c lowering.Ctx, ts *compile.Types, anchors *Ancho
 	if got, _ := ts.Lookup(pointer); got != inner {
 		// 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.
-		//
-		// 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)
+		// shared primitive. The alias carries the position's value constraints for
+		// the reason hoistByteScalar records: owning the node is what stops
+		// hoistDeclarationHome hoisting the alias that would otherwise carry them.
+		// kept travels with them, so the co-declared bound keyword that reaches no
+		// Constraints field lands on the same node as the bounds it lost to.
+		var kept ir.Unmodeled
+		cons, consDiags := schemaConstraints(c, &kept, s, pointer)
+		diags = append(diags, consDiags...)
+		owner = internAlias(c, ts, pointer, hint, ir.TypeRef{Target: inner}, cons, kept)
 	}
 	return owner, append(diags, preserveUnionSiblings(c, ts, owner, s, pointer, reason, why)...)
 }

testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json

@@ -251,6 +251,48 @@
       "positional": false,
       "inputOnly": false
     },
+    "t/openapi/components/schemas/BoundedKinds": {
+      "kind": "scalar",
+      "id": "t/openapi/components/schemas/BoundedKinds",
+      "name": {
+        "source": "BoundedKinds",
+        "canonical": "bounded_kinds"
+      },
+      "anonymous": false,
+      "docs": {},
+      "sensitive": false,
+      "unmodeled": {
+        "openapi:oneOf": {
+          "reason": "degraded_lowering",
+          "value": [
+            {
+              "$ref": "#/components/schemas/A"
+            },
+            {
+              "type": "string"
+            }
+          ],
+          "provenance": {
+            "source": 0,
+            "pointer": "/components/schemas/BoundedKinds/oneOf"
+          }
+        }
+      },
+      "provenance": {
+        "source": 0,
+        "pointer": "/components/schemas/BoundedKinds"
+      },
+      "base": {
+        "target": "t/prim/string",
+        "nullable": false
+      },
+      "constraints": {
+        "exclusiveMin": false,
+        "exclusiveMax": false,
+        "minLength": 3,
+        "uniqueItems": false
+      }
+    },
     "t/openapi/components/schemas/Combo": {
       "kind": "union",
       "id": "t/openapi/components/schemas/Combo",
@@ -542,13 +584,22 @@
         "source": 0,
         "pointer": "/components/schemas/MixedKinds"
       }
+    },
+    {
+      "severity": "info",
+      "code": "openapi/degraded-construct",
+      "message": "oneOf/anyOf co-declared with structural keywords intersects with them, and the body is not a model, so it carries no composition to distribute into; union branches kept verbatim under Unmodeled",
+      "provenance": {
+        "source": 0,
+        "pointer": "/components/schemas/BoundedKinds"
+      }
     }
   ],
   "sources": [
     {
       "format": "openapi@3.1",
       "path": "allof-oneof-cooccurrence.yaml",
-      "hash": "580c2ffd45b8b24e175b4c5b7f1af6c237a920de7d1942d84b10a00ad2dc1e33"
+      "hash": "9deaf845e7a310c701113bd61e60a7e0d170cbf96860921816623667813e458f"
     }
   ]
 }

testdata/conformance/openapi/allof-oneof-cooccurrence.yaml

@@ -32,6 +32,17 @@ components:
       oneOf:
         - {$ref: '#/components/schemas/A'}
         - {type: string}
+    # The same conjunction over a body that is not a model. It reduces to the
+    # shared string primitive, so the position hoists an alias for the kept union
+    # to sit on — and that alias has to carry the bounds written here too, since
+    # owning a node is what stops the declaration-home fallback carrying them
+    # (GitHub #343).
+    BoundedKinds:
+      type: string
+      minLength: 3
+      oneOf:
+        - {$ref: '#/components/schemas/A'}
+        - {type: string}
     # A branch pointer denotes the branch schema, and a reference to it must get
     # that schema — not the variant Morphic composes for the same branch, which
     # is a node of its own with no pointer a $ref can name.