fix(compilers/openapi): warn on a 3.0 array without items

compilers/openapi/conformance_test.go

@@ -179,6 +179,7 @@ func conformanceCases() []conformanceCase {
 		{"enum-string", assertEnumString, []string{"enums-string"}},
 		{"enum-numeric", assertEnumNumeric, []string{"enums-numeric"}},
 		{"empty-enum", assertEmptyEnum, nil},
+		{"missing-array-items", assertMissingArrayItems, nil},
 		{"scalar-format", assertScalarFormat, []string{"custom-scalars"}},
 		{"encoding-byte", assertEncodingByte, []string{"encoding-hints"}},
 		{"content-vocabulary", assertContentVocabulary, []string{"encoding-hints"}},
@@ -1182,6 +1183,33 @@ func assertEmptyEnum(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
 		"and reports nothing")
 }
 
+// assertMissingArrayItems pins that a 3.0 schema writing type array and no
+// items is reported once, at its own pointer, wherever it sits and whatever it
+// lowers to: Choice is a union of literals (GitHub #650). Copy's $ref names
+// Holder's property coordinate and is declared first, so the property is
+// lowered twice. Holder's typed and empty arrays write items, so the list is
+// every report the document draws.
+func assertMissingArrayItems(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
+	var got []string
+	for _, d := range diags {
+		if d.Code == "openapi/missing-array-items" {
+			assert.Equal(t, ir.SeverityWarning, d.Severity, "at %s", d.Provenance.Pointer)
+			got = append(got, string(d.Provenance.Pointer))
+		}
+	}
+	slices.Sort(got)
+	assert.Equal(t, []string{
+		"/components/schemas/Bare",
+		"/components/schemas/Choice",
+		"/components/schemas/Holder/properties/tags",
+		"/paths/~1things/get/parameters/0/schema",
+	}, got)
+
+	bare, ok := doc.Types[namedID("Bare")].(*ir.List)
+	require.True(t, ok, "Bare still lowers to a List, got %T", doc.Types[namedID("Bare")])
+	assert.Equal(t, ir.TypeID("t/prim/any"), bare.Elem.Target, "over the top type")
+}
+
 func assertScalarFormat(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	h, ok := doc.Types[namedID("Holder")].(*ir.Model)
 	require.True(t, ok)

compilers/openapi/internal/diag/diag.go

@@ -351,6 +351,14 @@ const (
 	// Warning, not the error style gets: that is the parser's own validation
 	// refusal, whereas here the compiler has already kept the value.
 	InvalidLocationKeyword = "openapi/invalid-location-keyword"
+	// MissingArrayItems reports a schema in an OpenAPI 3.0 document that writes
+	// type array and no `items`, which 3.0 requires, at the schema's pointer. It
+	// lowers as it would in 3.1, where `items` is optional: a List's element is
+	// the top type. A warning rather than an error: nothing the document
+	// declared is dropped, and harness.Check stops at the first error
+	// diagnostic. A code of its own rather than DegradedConstruct, because
+	// nothing is kept in a weaker shape.
+	MissingArrayItems = "openapi/missing-array-items"
 )
 
 // Newf builds an ir.Diagnostic with a formatted message. It is the single

compilers/openapi/internal/diag/diag_test.go

@@ -148,7 +148,7 @@ func codes() []string {
 		diag.ReservedHeaderName, diag.UnpreservableConstruct,
 		diag.UnknownSchemaKeyword, diag.UnknownObjectKey, diag.UnknownKeyBudget,
 		diag.UnknownKeyUnreachable, diag.UnknownKeyEntryTaken,
-		diag.InvalidLocationKeyword,
+		diag.InvalidLocationKeyword, diag.MissingArrayItems,
 	}
 }
 

compilers/openapi/internal/lowering/lowering.go

@@ -349,19 +349,15 @@ func (c Ctx) DeclaresSchema(name string) bool { return c.schemas[name] }
 // caller, which is the one thing keeping it unexported was for.
 func (c Ctx) DeclaresAuth(id ir.AuthID) bool { _, ok := c.auth[id]; return ok }
 
-// ExclusiveBoundIsBoolean reports whether this document's dialect spells
-// exclusiveMinimum/exclusiveMaximum as a boolean modifier (OpenAPI 3.0) rather
-// than a numeric bound (the 2020-12 dialect of 3.1 and 3.2). An unrecognized
-// version defaults to the 2020-12 numeric form.
-//
-// It reads the document version, which makes it a question about the context
-// rather than about any schema — annotation.Constraints takes the answer as a
-// parameter precisely so the reader never has to ask it.
-//
-// The version is read through its accessor so a context with no document answers
-// like an unrecognized version rather than panicking, which is how the other two
-// readers behave on a zero value.
-func (c Ctx) ExclusiveBoundIsBoolean() bool {
+// Is30 reports whether this document's dialect is OpenAPI 3.0 rather than the
+// 2020-12 dialect of 3.1 and 3.2. A reading that differs between the two, such
+// as a boolean exclusiveMinimum or a required `items`, asks this rather than
+// parsing the version itself; `nullable` is still read in every dialect
+// (GitHub #632). An unrecognized version, and a context with no document,
+// answer false. It reads the entry document's version, whichever document
+// holds the schema, so a 3.1 file reached by $ref from a 3.0 root is read as
+// 3.0 (GitHub #839).
+func (c Ctx) Is30() bool {
 	minor, _ := load.SupportedMinor(c.Doc.GetOpenAPI())
 	return minor == "3.0"
 }
@@ -373,7 +369,7 @@ func (c Ctx) ExclusiveBoundIsBoolean() bool {
 // census must be told so (GitHub #615); one answer keeps the two agreeing. It
 // names a floor rather than one version, so a later dialect inherits the reads
 // rather than silently warning about its own fields again; an unrecognized
-// version answers false, as ExclusiveBoundIsBoolean does.
+// version answers false, as Is30 does.
 func (c Ctx) Since32() bool {
 	minor, ok := load.SupportedMinor(c.Doc.GetOpenAPI())
 	return ok && minor != "3.0" && minor != "3.1"

compilers/openapi/internal/lowering/lowering_test.go

@@ -161,23 +161,21 @@ func TestDeclaresAuth_TheZeroContextDeclaresNothing(t *testing.T) {
 	assert.False(t, c.DeclaresAuth(""))
 }
 
-// TestExclusiveBoundIsBoolean_TheZeroContextReadsAsNumeric completes the set the
-// two accessors above start: every reader on this type answers on a context with
-// nothing in it. This is the one that reaches through the document pointer, so
-// it is the one that would panic instead — and a test holding a Ctx literal is
-// not a hypothetical, it is how half this file drives the type.
-func TestExclusiveBoundIsBoolean_TheZeroContextReadsAsNumeric(t *testing.T) {
+// TestIs30_TheZeroContextReadsAs2020 pins that a context with no document
+// answers false rather than panicking: the accessor reaches through the
+// document pointer, and half this file drives the type as a Ctx literal.
+func TestIs30_TheZeroContextReadsAs2020(t *testing.T) {
 	t.Parallel()
 	var c lowering.Ctx
-	assert.False(t, c.ExclusiveBoundIsBoolean(),
-		"no document names no dialect, which falls back to the 2020-12 numeric form")
+	assert.False(t, c.Is30(),
+		"no document names no dialect, which falls back to 2020-12")
 }
 
-// TestExclusiveBoundIsBoolean_FollowsTheDialect pins which spelling of
-// exclusiveMinimum/exclusiveMaximum the document uses. Reading a 3.0 boolean as
-// a 2020-12 numeric bound (or the reverse) silently changes what constraint the
-// IR records, and neither form fails to parse as the other.
-func TestExclusiveBoundIsBoolean_FollowsTheDialect(t *testing.T) {
+// TestIs30_FollowsTheDeclaredVersion pins which documents read as OpenAPI 3.0.
+// A reading keyed on this answer goes wrong with it: reading 3.0 as 2020-12
+// turns a boolean exclusiveMinimum into a numeric bound and silences a missing
+// `items`, and the reverse warns on every 3.1 array that omits `items`.
+func TestIs30_FollowsTheDeclaredVersion(t *testing.T) {
 	t.Parallel()
 	tests := []struct {
 		version string
@@ -194,7 +192,7 @@ func TestExclusiveBoundIsBoolean_FollowsTheDialect(t *testing.T) {
 		t.Run(tc.version, func(t *testing.T) {
 			t.Parallel()
 			c := lowering.New(0, &soa.OpenAPI{OpenAPI: tc.version}, ir.SourceInfo{}, "", lowering.Limits{}, lowering.StreamingMedia{}, lowering.ExtensionPromotions{}, overlay.Origin{})
-			assert.Equal(t, tc.want, c.ExclusiveBoundIsBoolean())
+			assert.Equal(t, tc.want, c.Is30())
 		})
 	}
 }

compilers/openapi/internal/schema/schema.go

@@ -160,7 +160,7 @@ func ConstraintsOf(c lowering.Ctx, p *ir.Unmodeled, s *oas3.Schema, pointer json
 	if s == nil {
 		return nil, nil
 	}
-	cons, kept, diags := annotation.Constraints(s, c.ExclusiveBoundIsBoolean(), pointer, c.ProvenanceAt)
+	cons, kept, diags := annotation.Constraints(s, c.Is30(), pointer, c.ProvenanceAt)
 	*p = annotation.MergeUnmodeled(*p, kept)
 	return cons, stampConstraintDiags(c, diags, pointer)
 }
@@ -197,9 +197,12 @@ func internAlias(c lowering.Ctx, ts *compile.Types, s *oas3.Schema, pointer json
 // records the schema's own annotations on whatever node the lowering hoisted at
 // pointer. It is shared by Ref and by sub-schema hoisting
 // (resolveSchemaRef), which both reach a body only after peeling off any
-// leading $ref.
+// leading $ref. A rule about the declaration itself is checked here, above the
+// dispatch that picks what it lowers to (missingArrayItems).
 func schemaBody(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, schema *oas3.Schema, pointer jsontext.Pointer, hint string, home annotation.Home) (ir.TypeRef, []ir.Diagnostic) {
-	target, diags := lowerSchemaBody(c, ts, anchors, depth, schema, pointer, hint)
+	diags := missingArrayItems(c, schema, pointer)
+	target, bodyDiags := lowerSchemaBody(c, ts, anchors, depth, schema, pointer, hint)
+	diags = append(diags, bodyDiags...)
 	// A body's census ran inside lower(), against the node the walk actually
 	// built, which is the only thing that can answer it. A collapsed union built
 	// none, so it takes a $ref site's census, and its verdict homes an alias.
@@ -212,6 +215,24 @@ func schemaBody(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth i
 	return ref, append(diags, recordUnhomedKeywords(c, ts, ref.Target, schema, unhomed, collapsedShape, pointer)...)
 }
 
+// missingArrayItems reports a 3.0 schema that writes type array and no items,
+// which 3.0 requires and 2020-12 leaves optional (GitHub #650). The lowering
+// is the same in every dialect, so this is the only place the document is told.
+//
+// It reads the declaration, never what it lowered to, so an array that lowers
+// to a tuple, an enum or an allOf is held to the rule as a List is; that a 3.0
+// prefixItems lowers at all is GitHub #820. `items: {}` is silent. Neither a
+// $ref site, whose siblings 3.0 ignores, nor a subschema kept verbatim, such as
+// one under `not`, reaches it.
+func missingArrayItems(c lowering.Ctx, s *oas3.Schema, pointer jsontext.Pointer) []ir.Diagnostic {
+	if s.GetItems() != nil || !slices.Contains(s.GetType(), oas3.SchemaTypeArray) || !c.Is30() {
+		return nil
+	}
+	return []ir.Diagnostic{c.DiagAt(ir.SeverityWarning, diag.MissingArrayItems, pointer,
+		"OpenAPI 3.0 requires items when type is array, and this schema writes none, "+
+			"so only its other keywords constrain its elements")}
+}
+
 // hoistDeclarationHome gives a declaration a node of its own when its lowering
 // left it none and it wrote something only a node can hold
 // (declaresPositionScoped; GitHub #116), since a shared node must never carry

compilers/openapi/internal/schema/schema_test.go

@@ -386,16 +386,151 @@ func TestLower_ListConstraints(t *testing.T) {
 	assert.Equal(t, int64(1), *l.Constraints.MinItems)
 }
 
+// TestLower_ListWithoutItems pins that an array schema writing no items lowers
+// to a List over the top type in every dialect. 3.0's requirement is reported
+// (TestLower_MissingArrayItems) and never applied to the IR.
 func TestLower_ListWithoutItems(t *testing.T) {
 	t.Parallel()
-	// No `items` → schema.Ref(nil) → element is `any`.
-	doc, diags := lowerSpec(t, openapitest.ComponentSpec("    L: {type: array}\n"))
-	openapitest.RequireNoErrorDiags(t, diags)
-	l, ok := typeByName(doc, "L").(*ir.List)
+	for _, version := range []string{"3.0.3", "3.1.0", "3.2.0"} {
+		t.Run(version, func(t *testing.T) {
+			t.Parallel()
+			doc, diags := lowerSpec(t, openapitest.ComponentSpecVer(version, "    L: {type: array}\n"))
+			openapitest.RequireNoErrorDiags(t, diags)
+			l, ok := typeByName(doc, "L").(*ir.List)
+			require.True(t, ok)
+			assert.Equal(t, ir.TypeID("t/prim/any"), l.Elem.Target)
+		})
+	}
+}
+
+// TestLower_MissingArrayItems pins which declarations the 3.0 rule reports and
+// where. Every schema writing type array and no items is reported at its own
+// pointer, whatever it lowers to; a written items, a type that is not array, a
+// $ref site, a subschema kept verbatim and a 2020-12 document are not. Reports
+// are counted as the lowering returns them, before compile.Diags drops
+// repeats, and each row lowers each declaration once, so a doubled report
+// reddens.
+func TestLower_MissingArrayItems(t *testing.T) {
+	t.Parallel()
+	const holder = "    Holder:\n      type: object\n      properties:\n"
+	cases := []struct {
+		name    string
+		version string
+		schemas string
+		want    []string // report pointers, in order; nil when silent
+	}{
+		{name: "a bare array", version: "3.0.3",
+			schemas: "    L: {type: array}\n", want: []string{"/components/schemas/L"}},
+		{name: "a nullable array", version: "3.0.3",
+			schemas: "    L: {type: array, nullable: true}\n", want: []string{"/components/schemas/L"}},
+		{name: "a property", version: "3.0.3",
+			schemas: holder + "        tags: {type: array}\n", want: []string{"/components/schemas/Holder/properties/tags"}},
+		{name: "an array as another's items", version: "3.0.3",
+			schemas: "    L: {type: array, items: {type: array}}\n", want: []string{"/components/schemas/L/items"}},
+		{name: "a type list holding array", version: "3.0.3",
+			schemas: "    T: {type: [array, string]}\n", want: []string{"/components/schemas/T"}},
+		{name: "a tuple", version: "3.0.3",
+			schemas: "    L: {type: array, prefixItems: [{type: string}]}\n", want: []string{"/components/schemas/L"}},
+		{name: "an enum of arrays", version: "3.0.3",
+			schemas: "    L: {type: array, enum: [[1, 2], [3]]}\n", want: []string{"/components/schemas/L"}},
+		{name: "an empty enum", version: "3.0.3",
+			schemas: "    L: {type: array, enum: []}\n", want: []string{"/components/schemas/L"}},
+		{name: "a const", version: "3.0.3",
+			schemas: "    L: {type: array, const: [1]}\n", want: []string{"/components/schemas/L"}},
+		{name: "an allOf beside it", version: "3.0.3",
+			schemas: "    L: {type: array, allOf: [{minItems: 1}]}\n", want: []string{"/components/schemas/L"}},
+		{name: "a oneOf beside it", version: "3.0.3",
+			schemas: "    L: {type: array, oneOf: [{maxItems: 1}, {minItems: 3}]}\n", want: []string{"/components/schemas/L"}},
+		{name: "a $ref to it from a sibling", version: "3.0.3",
+			schemas: holder + "        l: {$ref: '#/components/schemas/L'}\n    L: {type: array}\n",
+			want:    []string{"/components/schemas/L"}},
+		{name: "items written empty is silent", version: "3.0.3",
+			schemas: "    L: {type: array, items: {}}\n"},
+		{name: "array keywords without a type are silent", version: "3.0.3",
+			schemas: "    L: {minItems: 1}\n"},
+		{name: "a $ref site's sibling type is silent", version: "3.0.3",
+			schemas: "    L: {$ref: '#/components/schemas/S', type: array}\n    S: {type: string}\n"},
+		{name: "an array kept verbatim under not is silent", version: "3.0.3",
+			schemas: "    L: {type: integer, not: {type: array}}\n"},
+		{name: "3.1 is silent", version: "3.1.0", schemas: "    L: {type: array}\n"},
+		{name: "3.2 is silent", version: "3.2.0", schemas: "    L: {type: array}\n"},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			l, loadDiags := loweredFor(t, openapitest.ComponentSpecVer(tc.version, tc.schemas))
+			raw := schema.LowerComponentSchemas(t.Context(), l.ctx, l.types, &l.anchors)
+			openapitest.RequireNoErrorDiags(t, append(loadDiags, raw...))
+			assert.Equal(t, tc.want, missingItemsAt(t, raw))
+		})
+	}
+}
+
+// TestLower_MissingArrayItemsLoweredTwiceWarnsOnce pins the path that lowers a
+// declaration twice: a $ref naming a coordinate inside another declaration (a
+// property here; an `items` or a parameter's schema takes the same path),
+// lowered before that declaration, lowers the body, and the declaration lowers
+// it again (Types.InternDeclared). The lowering reports each time, and
+// compile.Diags drops the identical repeat, so the caller sees one warning in
+// both orders.
+func TestLower_MissingArrayItemsLoweredTwiceWarnsOnce(t *testing.T) {
+	t.Parallel()
+	const (
+		ref    = "    A:\n      type: object\n      properties:\n        copy: {$ref: '#/components/schemas/Holder/properties/tags'}\n"
+		holder = "    Holder:\n      type: object\n      properties:\n        tags: {type: array}\n"
+		tags   = "/components/schemas/Holder/properties/tags"
+	)
+	cases := []struct {
+		name    string
+		schemas string
+		raw     int // reports the lowering returns before compile.Diags
+	}{
+		{name: "ref first", schemas: ref + holder, raw: 2},
+		{name: "declaration first", schemas: holder + ref, raw: 1},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			l, _ := loweredFor(t, openapitest.ComponentSpecVer("3.0.3", tc.schemas))
+			raw := schema.LowerComponentSchemas(t.Context(), l.ctx, l.types, &l.anchors)
+			require.Len(t, missingItemsAt(t, raw), tc.raw, "this order lowers the property that many times")
+
+			l.diags.AppendAll(raw)
+			assert.Equal(t, []string{tags}, missingItemsAt(t, l.diags.List()))
+		})
+	}
+}
+
+// TestCompile_MissingArrayItemsReachesTheDiagnostics drives the issue's exact
+// document (GitHub #650) through the public compiler: the warning is raised by
+// the schema lowering, and a caller only sees it if it survives the walk up to
+// Compile's own diagnostics.
+func TestCompile_MissingArrayItemsReachesTheDiagnostics(t *testing.T) {
+	t.Parallel()
+	doc, diags := parseFull(t, openapitest.ComponentSpecVer("3.0.3", "    NoItems: {type: array}\n"))
+	assert.Equal(t, []string{"/components/schemas/NoItems"}, missingItemsAt(t, diags),
+		"the warning reaches Compile's diagnostics, once")
+
+	l, ok := doc.Types[componentID("NoItems")].(*ir.List)
 	require.True(t, ok)
 	assert.Equal(t, ir.TypeID("t/prim/any"), l.Elem.Target)
 }
 
+// missingItemsAt returns the pointer of each missing-array-items report in
+// diags, in order, holding each to the warning severity the code carries.
+func missingItemsAt(t *testing.T, diags []ir.Diagnostic) []string {
+	t.Helper()
+	var at []string
+	for _, d := range diags {
+		if d.Code != diag.MissingArrayItems {
+			continue
+		}
+		assert.Equal(t, ir.SeverityWarning, d.Severity, "at %s", d.Provenance.Pointer)
+		at = append(at, string(d.Provenance.Pointer))
+	}
+	return at
+}
+
 func TestLower_ValidationOnlyKeywords(t *testing.T) {
 	t.Parallel()
 	spec := openapitest.ComponentSpec(`    V: