diff --git a/internal/codegen/decode/main.go b/internal/codegen/decode/main.go index 446dc030..a00d5b64 100644 --- a/internal/codegen/decode/main.go +++ b/internal/codegen/decode/main.go @@ -494,29 +494,45 @@ func (x *{{.Name}}) decodeFromMap(m map[string]any) { if arr, ok := m["{{.JSONKey}}"].([]any); ok { x.{{.FieldName}} = make([]*{{.ElemType}}, 0, len(arr)) for _, item := range arr { +{{- if eq .ElemType "Schema"}} + if elem := decodeSchemaValue(item); elem != nil { + x.{{.FieldName}} = append(x.{{.FieldName}}, elem) + } +{{- else}} if sub, ok := item.(map[string]any); ok { elem := new({{.ElemType}}) elem.decodeFromMap(sub) x.{{.FieldName}} = append(x.{{.FieldName}}, elem) } +{{- end}} } } {{- else if eq .Strategy "oas_ptr"}} +{{- if eq .ElemType "Schema"}} + x.{{.FieldName}} = decodeSchemaValue(m["{{.JSONKey}}"]) +{{- else}} if sub, ok := m["{{.JSONKey}}"].(map[string]any); ok { x.{{.FieldName}} = new({{.ElemType}}) x.{{.FieldName}}.decodeFromMap(sub) } +{{- end}} {{- else if eq .Strategy "string_or_object"}} x.{{.FieldName}} = decode{{.ElemType}}(m["{{.JSONKey}}"]) {{- else if eq .Strategy "oas_map"}} if sub, ok := m["{{.JSONKey}}"].(map[string]any); ok { x.{{.FieldName}} = make(map[string]*{{.ElemType}}, len(sub)) for k, v := range sub { +{{- if eq .ElemType "Schema"}} + if elem := decodeSchemaValue(v); elem != nil { + x.{{.FieldName}}[k] = elem + } +{{- else}} if vm, ok := v.(map[string]any); ok { elem := new({{.ElemType}}) elem.decodeFromMap(vm) x.{{.FieldName}}[k] = elem } +{{- end}} } } {{- else if eq .Strategy "string_map"}} diff --git a/internal/codegen/deepcopy/main.go b/internal/codegen/deepcopy/main.go index af971562..b4840cc2 100644 --- a/internal/codegen/deepcopy/main.go +++ b/internal/codegen/deepcopy/main.go @@ -196,6 +196,10 @@ var typeConfigs = []TypeConfig{ {Name: "MinContains", Type: "*int", CopyMethod: "prim_pointer"}, {Name: "MaxProperties", Type: "*int", CopyMethod: "prim_pointer"}, {Name: "MinProperties", Type: "*int", CopyMethod: "prim_pointer"}, + // BoolForm is not a spec field, but it is still a pointer: without + // an entry here `*out = *in` would alias the pointee between the + // original and the copy. + {Name: "BoolForm", Type: "*bool", CopyMethod: "prim_pointer"}, // Struct pointer fields {Name: "Discriminator", Type: "*Discriminator", CopyMethod: "pointer"}, {Name: "XML", Type: "*XML", CopyMethod: "pointer"}, diff --git a/internal/driftguard/marshal_test.go b/internal/driftguard/marshal_test.go index ffcf0680..c71b23f8 100644 --- a/internal/driftguard/marshal_test.go +++ b/internal/driftguard/marshal_test.go @@ -33,6 +33,13 @@ var marshalExclusions = map[string]map[string]string{ // 2.0 bare string, which is why this case is checked before decoding. "StringForm": "selects the OAS 2.0 bare-string form", }, + "Schema": { + // Same shape as StringForm, one level up: setting it re-spells the whole + // Schema as a bare boolean, so the output is the scalar `true` or `false` + // and there is no object left to decode. The value it carries is not lost — + // it *is* the output — so this exclusion covers the key, not the meaning. + "BoolForm": "re-spells the schema as a bare boolean", + }, } // marshalSubjects pairs each type carrying a hand-built MarshalJSON with a fresh diff --git a/internal/schemautil/bool_schema_hash_test.go b/internal/schemautil/bool_schema_hash_test.go new file mode 100644 index 00000000..13af43eb --- /dev/null +++ b/internal/schemautil/bool_schema_hash_test.go @@ -0,0 +1,103 @@ +package schemautil + +import ( + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/erraggy/oastools/parser" +) + +// A boolean schema has two representations in the parser model. In a +// schema-or-bool field such as Items it stays a raw bool, because the promotion +// step passes bools through untouched; in a *Schema-typed field it arrives as a +// Schema with BoolForm set. They mean the same thing, so they must hash the +// same — otherwise deduplication sorts equivalent schemas into different +// buckets and never compares them. +func TestHashBoolSchemaRepresentationsAgree(t *testing.T) { + tests := []struct { + name string + raw *parser.Schema + wrap *parser.Schema + }{ + { + name: "items true", + raw: &parser.Schema{Type: "array", Items: true}, + wrap: &parser.Schema{Type: "array", Items: parser.NewBoolSchema(true)}, + }, + { + name: "items false", + raw: &parser.Schema{Type: "array", Items: false}, + wrap: &parser.Schema{Type: "array", Items: parser.NewBoolSchema(false)}, + }, + { + name: "additionalProperties true", + raw: &parser.Schema{Type: "object", AdditionalProperties: true}, + wrap: &parser.Schema{Type: "object", AdditionalProperties: parser.NewBoolSchema(true)}, + }, + { + name: "additionalProperties false", + raw: &parser.Schema{Type: "object", AdditionalProperties: false}, + wrap: &parser.Schema{Type: "object", AdditionalProperties: parser.NewBoolSchema(false)}, + }, + { + name: "additionalItems true", + raw: &parser.Schema{Type: "array", AdditionalItems: true}, + wrap: &parser.Schema{Type: "array", AdditionalItems: parser.NewBoolSchema(true)}, + }, + { + name: "additionalItems false", + raw: &parser.Schema{Type: "array", AdditionalItems: false}, + wrap: &parser.Schema{Type: "array", AdditionalItems: parser.NewBoolSchema(false)}, + }, + { + name: "unevaluatedProperties true", + raw: &parser.Schema{Type: "object", UnevaluatedProperties: true}, + wrap: &parser.Schema{Type: "object", UnevaluatedProperties: parser.NewBoolSchema(true)}, + }, + { + name: "unevaluatedItems false", + raw: &parser.Schema{Type: "array", UnevaluatedItems: false}, + wrap: &parser.Schema{Type: "array", UnevaluatedItems: parser.NewBoolSchema(false)}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, NewSchemaHasher().Hash(tt.raw), NewSchemaHasher().Hash(tt.wrap), + "the raw bool and BoolForm representations must hash alike") + }) + } +} + +// The two boolean values are opposite schemas, so they must not collide — in +// either representation, and at the top level as well as nested. +func TestHashBoolSchemaValuesDiffer(t *testing.T) { + tests := []struct { + name string + a, b *parser.Schema + }{ + { + name: "top-level true and false", + a: parser.NewBoolSchema(true), + b: parser.NewBoolSchema(false), + }, + { + name: "raw items true and false", + a: &parser.Schema{Type: "array", Items: true}, + b: &parser.Schema{Type: "array", Items: false}, + }, + { + name: "a boolean schema and an empty object schema", + a: parser.NewBoolSchema(true), + b: &parser.Schema{}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.NotEqual(t, NewSchemaHasher().Hash(tt.a), NewSchemaHasher().Hash(tt.b), + "distinct schemas must not share a deduplication bucket") + }) + } +} diff --git a/internal/schemautil/hash.go b/internal/schemautil/hash.go index a4da3207..45ef4ffb 100644 --- a/internal/schemautil/hash.go +++ b/internal/schemautil/hash.go @@ -62,6 +62,15 @@ func (h *SchemaHasher) hashSchema(hasher hash.Hash64, schema *parser.Schema) { h.visited[ptr] = true defer func() { h.visited[ptr] = false }() + // The bare-boolean form has no keywords, so it hashes on its value alone. + // `true` and `false` are opposite schemas and must not share a bucket with + // each other or with an object schema, or deduplication groups them and the + // deep comparison never gets to reject the merge. + if b, ok := schema.IsBool(); ok { + h.writeBoolSchema(hasher, b) + return + } + // Hash $ref if present. JSON Schema 2020-12 allows keywords alongside $ref, so // this records the reference and keeps going: returning here made // {$ref: X, default: 1} and {$ref: X, default: 2} hash alike. @@ -80,32 +89,7 @@ func (h *SchemaHasher) hashSchema(hasher hash.Hash64, schema *parser.Schema) { h.writeString(hasher, "pattern:") h.writeString(hasher, schema.Pattern) - // Enum (order matters). Length-framed: unframed, ["ab"] and ["a", "b"] both - // hash as "enum:ab". - if len(schema.Enum) > 0 { - h.writeString(hasher, "enum:") - for _, v := range schema.Enum { - h.writeLabeled(hasher, "v", fmt.Sprintf("%v", v)) - } - } - - // Const - if schema.Const != nil { - h.writeString(hasher, "const:") - h.writeString(hasher, fmt.Sprintf("%v", schema.Const)) - } - - // Required (sort for order-independent comparison). Length-framed for the same - // reason as Enum. - if len(schema.Required) > 0 { - h.writeString(hasher, "required:") - sorted := make([]string, len(schema.Required)) - copy(sorted, schema.Required) - sort.Strings(sorted) - for _, r := range sorted { - h.writeLabeled(hasher, "r", r) - } - } + h.hashEnumConstRequired(hasher, schema) // Properties (sorted by key for deterministic hashing) if len(schema.Properties) > 0 { @@ -364,14 +348,22 @@ func (h *SchemaHasher) hashSchemaOrBool(hasher hash.Hash64, v any) { case *parser.Schema: h.hashSchema(hasher, val) case bool: - if val { - h.writeString(hasher, "true") - } else { - h.writeString(hasher, "false") - } + // Same encoding hashSchema uses for Schema.BoolForm. A boolean schema + // has two representations — a raw bool here, a *Schema with BoolForm + // set in a *Schema-typed position — and they mean the same thing, so + // they must hash the same. Writing a bare "true" here put `items: true` + // and `items: NewBoolSchema(true)` in different buckets. + h.writeBoolSchema(hasher, val) } } +// writeBoolSchema writes the bare-boolean schema form. Shared by hashSchema and +// hashSchemaOrBool so the two representations cannot drift apart. +func (h *SchemaHasher) writeBoolSchema(hasher hash.Hash64, v bool) { + h.writeString(hasher, "boolschema:") + h.writeString(hasher, strconv.FormatBool(v)) +} + // hashIdentity hashes the JSON Schema identity and dialect keywords. They decide // which schema a $ref or $dynamicRef resolves to and which vocabulary validates // it, so two schemas differing here are not interchangeable however alike their @@ -413,6 +405,38 @@ func (h *SchemaHasher) hashIdentity(hasher hash.Hash64, schema *parser.Schema) { // default is an annotation in JSON Schema terms, but two schemas that default // differently generate different code and fill payloads differently, so // consolidating them is not safe. collectionFormat decides a wire format outright. +// hashEnumConstRequired hashes the three value-set keywords. Extracted from +// hashSchema purely to keep that function under the complexity limit; the write +// order is unchanged, so hashes computed before and after the extraction match. +func (h *SchemaHasher) hashEnumConstRequired(hasher hash.Hash64, schema *parser.Schema) { + // Enum (order matters). Length-framed: unframed, ["ab"] and ["a", "b"] both + // hash as "enum:ab". + if len(schema.Enum) > 0 { + h.writeString(hasher, "enum:") + for _, v := range schema.Enum { + h.writeLabeled(hasher, "v", fmt.Sprintf("%v", v)) + } + } + + // Const + if schema.Const != nil { + h.writeString(hasher, "const:") + h.writeString(hasher, fmt.Sprintf("%v", schema.Const)) + } + + // Required (sort for order-independent comparison). Length-framed for the same + // reason as Enum. + if len(schema.Required) > 0 { + h.writeString(hasher, "required:") + sorted := make([]string, len(schema.Required)) + copy(sorted, schema.Required) + sort.Strings(sorted) + for _, r := range sorted { + h.writeLabeled(hasher, "r", r) + } + } +} + func (h *SchemaHasher) hashValueSemantics(hasher hash.Hash64, schema *parser.Schema) { if schema.Default != nil { h.writeLabeled(hasher, "default", fmt.Sprintf("%v", schema.Default)) diff --git a/joiner/bool_schema_test.go b/joiner/bool_schema_test.go new file mode 100644 index 00000000..9d278c92 --- /dev/null +++ b/joiner/bool_schema_test.go @@ -0,0 +1,221 @@ +package joiner + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/erraggy/oastools/parser" +) + +// TestCompareBoolSchemas covers the bare-boolean schema form through the +// joiner's own comparison, which is a separate implementation from +// parser.Schema.Equals and is the one semantic deduplication actually consults. +// +// The pair that matters is `true` against `false`: they are opposite schemas, +// so calling them equivalent would let deduplication merge a schema that +// accepts everything with one that accepts nothing. +func TestCompareBoolSchemas(t *testing.T) { + tests := []struct { + name string + left *parser.Schema + right *parser.Schema + equivalent bool + }{ + {"true is equivalent to true", parser.NewBoolSchema(true), parser.NewBoolSchema(true), true}, + {"false is equivalent to false", parser.NewBoolSchema(false), parser.NewBoolSchema(false), true}, + {"true is not equivalent to false", parser.NewBoolSchema(true), parser.NewBoolSchema(false), false}, + {"false is not equivalent to true", parser.NewBoolSchema(false), parser.NewBoolSchema(true), false}, + { + name: "true is not equivalent to an object schema", + left: parser.NewBoolSchema(true), + right: &parser.Schema{Type: "string"}, + }, + { + name: "an object schema is not equivalent to false", + left: &parser.Schema{Type: "string"}, + right: parser.NewBoolSchema(false), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for _, mode := range []EquivalenceMode{EquivalenceModeDeep, EquivalenceModeShallow} { + result := CompareSchemas(tt.left, tt.right, mode) + assert.Equal(t, tt.equivalent, result.Equivalent, + "mode %s: differences: %v", mode, result.Differences) + } + }) + } +} + +// TestBoolSchemaIsNotEmpty guards the early return that made the comparison +// above unreachable. isEmptySchema treated a schema carrying only BoolForm as +// empty, so CompareSchemasWithOptions reported two identical `true` schemas as +// non-equivalent — and did it with an empty Differences slice, because nothing +// had been compared. +func TestBoolSchemaIsNotEmpty(t *testing.T) { + assert.False(t, isEmptySchema(parser.NewBoolSchema(true)), + "`true` accepts every instance deliberately; it is not an empty schema") + assert.False(t, isEmptySchema(parser.NewBoolSchema(false)), + "`false` rejects every instance deliberately; it is not an empty schema") + assert.True(t, isEmptySchema(&parser.Schema{}), + "an object schema with no keywords is still empty") +} + +// TestCompareBoolSchemasReportsADifference checks the result shape, not just the +// verdict. A non-equivalent result with no differences recorded tells a caller +// nothing about why, which is exactly what the empty-schema early return +// produced before. +func TestCompareBoolSchemasReportsADifference(t *testing.T) { + result := CompareSchemas(parser.NewBoolSchema(true), parser.NewBoolSchema(false), EquivalenceModeDeep) + + assert.False(t, result.Equivalent) + assert.NotEmpty(t, result.Differences, "a mismatch should say what differed") +} + +// TestBoolSchemaDifferenceValuesArePrintable guards what a reported difference +// actually says. Callers format LeftValue and RightValue with %v, and the two +// values naturally available at these sites do not survive it: a *bool prints +// as a pointer address, and a *Schema prints as a full struct dump. Either way +// the reader loses the one fact the difference exists to convey. +// +// The side that is not a boolean schema is recorded as nil, matching how the +// other comparators leave a value absent rather than inventing one. +func TestBoolSchemaDifferenceValuesArePrintable(t *testing.T) { + tests := []struct { + name string + left, right *parser.Schema + wantLeft any + wantRight any + }{ + { + name: "top-level true and false", + left: parser.NewBoolSchema(true), + right: parser.NewBoolSchema(false), + wantLeft: true, + wantRight: false, + }, + { + name: "top-level boolean and object", + left: parser.NewBoolSchema(true), + right: &parser.Schema{Type: "string"}, + wantLeft: true, + wantRight: nil, + }, + { + name: "items raw and wrapped mismatch", + left: &parser.Schema{Items: true}, + right: &parser.Schema{Items: parser.NewBoolSchema(false)}, + wantLeft: true, + wantRight: false, + }, + { + name: "items boolean and object", + left: &parser.Schema{Items: true}, + right: &parser.Schema{Items: &parser.Schema{Type: "string"}}, + wantLeft: true, + wantRight: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CompareSchemas(tt.left, tt.right, EquivalenceModeDeep) + require.Len(t, result.Differences, 1) + + difference := result.Differences[0] + assertBoolOrNil(t, tt.wantLeft, difference.LeftValue) + assertBoolOrNil(t, tt.wantRight, difference.RightValue) + }) + } +} + +// assertBoolOrNil requires a SchemaDifference value to hold a plain bool, or +// nil for a side that is not a boolean schema. +// +// The type is the subject here, not the value: a *bool or a *Schema would +// carry the right information and still be useless to a caller, since these +// fields exist to be displayed. Asserting it explicitly says so, rather than +// relying on assert.Equal's type-strictness to catch it as a side effect. +func assertBoolOrNil(t *testing.T, want, got any) { + t.Helper() + if want == nil { + assert.Nil(t, got) + return + } + require.IsType(t, false, got, "must be a plain bool, not the pointer it was read from") + assert.Equal(t, want, got) +} + +// TestCompareNestedBoolSchemas covers boolean schemas below the top level. +// Checking only at the entry point left every nested position unguarded: +// `{p: true}` and `{p: false}` compared equal, because the field-by-field +// comparison finds nothing set on either side and a boolean carries no fields. +func TestCompareNestedBoolSchemas(t *testing.T) { + object := func(property *parser.Schema) *parser.Schema { + return &parser.Schema{ + Type: "object", + Properties: map[string]*parser.Schema{"p": property}, + } + } + + tests := []struct { + name string + left *parser.Schema + right *parser.Schema + equivalent bool + }{ + {"nested true and true", object(parser.NewBoolSchema(true)), object(parser.NewBoolSchema(true)), true}, + {"nested false and false", object(parser.NewBoolSchema(false)), object(parser.NewBoolSchema(false)), true}, + {"nested true and false", object(parser.NewBoolSchema(true)), object(parser.NewBoolSchema(false)), false}, + {"nested boolean and object", object(parser.NewBoolSchema(true)), object(&parser.Schema{Type: "string"}), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CompareSchemas(tt.left, tt.right, EquivalenceModeDeep) + assert.Equal(t, tt.equivalent, result.Equivalent, "differences: %v", result.Differences) + }) + } +} + +// TestCompareSchemaOrBoolRepresentations covers the any-typed schema-or-bool +// fields, where a boolean can arrive either as a raw bool — what the decoders +// leave there — or as a *Schema with BoolForm set, which is what building one +// programmatically produces. The two mean the same thing. +// +// Three separate functions compare these fields (compareSchemaOrBool, +// compareItemsSchemas, comparePolymorphicSchemas), so each field below is +// covered rather than trusting one to stand for the rest. +func TestCompareSchemaOrBoolRepresentations(t *testing.T) { + wrapped := parser.NewBoolSchema + + tests := []struct { + name string + left, right *parser.Schema + equivalent bool + }{ + {"items raw and wrapped true", &parser.Schema{Items: true}, &parser.Schema{Items: wrapped(true)}, true}, + {"items raw true and wrapped false", &parser.Schema{Items: true}, &parser.Schema{Items: wrapped(false)}, false}, + {"items both raw true", &parser.Schema{Items: true}, &parser.Schema{Items: true}, true}, + {"items raw true and raw false", &parser.Schema{Items: true}, &parser.Schema{Items: false}, false}, + {"items boolean and object", &parser.Schema{Items: true}, &parser.Schema{Items: &parser.Schema{Type: "string"}}, false}, + + {"additionalProperties raw and wrapped", &parser.Schema{AdditionalProperties: true}, &parser.Schema{AdditionalProperties: wrapped(true)}, true}, + {"additionalProperties raw true and false", &parser.Schema{AdditionalProperties: true}, &parser.Schema{AdditionalProperties: false}, false}, + + {"additionalItems raw and wrapped", &parser.Schema{AdditionalItems: true}, &parser.Schema{AdditionalItems: wrapped(true)}, true}, + + {"unevaluatedProperties raw and wrapped", &parser.Schema{UnevaluatedProperties: true}, &parser.Schema{UnevaluatedProperties: wrapped(true)}, true}, + {"unevaluatedItems raw true and wrapped false", &parser.Schema{UnevaluatedItems: true}, &parser.Schema{UnevaluatedItems: wrapped(false)}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := CompareSchemas(tt.left, tt.right, EquivalenceModeDeep) + assert.Equal(t, tt.equivalent, result.Equivalent, "differences: %v", result.Differences) + }) + } +} diff --git a/joiner/equivalence.go b/joiner/equivalence.go index 08bc4a87..e880484f 100644 --- a/joiner/equivalence.go +++ b/joiner/equivalence.go @@ -161,6 +161,15 @@ func isEmptySchema(s *parser.Schema) bool { return false } + // A bare-boolean schema is the opposite of empty: `false` rejects every + // instance and `true` accepts every instance, both deliberately. Without + // this, CompareSchemasWithOptions took the empty-schema early return and + // reported two identical `true` schemas as non-equivalent — with an empty + // Differences slice, since nothing had actually been compared. + if s.BoolForm != nil { + return false + } + // Basic type constraints if s.Type != nil { return false @@ -399,6 +408,14 @@ func CompareSchemasWithOptions(left, right *parser.Schema, opts CompareOptions) // Use stack-based path builder to minimize allocations path := &comparePath{segments: make([]string, 0, 8)} + // A bare-boolean operand settles the comparison on its own. compareDeep + // repeats this for nested positions; running it here too is what covers + // shallow mode, which does not recurse. + if !equalBoolForms(left, right, path, &result) { + result.Equivalent = len(result.Differences) == 0 + return result + } + if opts.Mode == EquivalenceModeShallow { compareShallow(left, right, path, &result, compareDocs) } else { @@ -752,6 +769,96 @@ func compareShallow(left, right *parser.Schema, path *comparePath, result *Equiv compareCommonFields(left, right, path, result, compareDocs) } +// equalBoolForms compares two schemas when either is the bare-boolean form, +// recording a difference when they disagree. It reports whether comparison +// should continue: false means one or both sides were boolean and the verdict +// is settled, since a boolean schema has no other fields to compare. +// +// `true` and `false` are opposite schemas — one accepts every instance, the +// other none — and neither is equivalent to an object schema. +func equalBoolForms(left, right *parser.Schema, path *comparePath, result *EquivalenceResult) bool { + leftBool, leftIsBool := left.IsBool() + rightBool, rightIsBool := right.IsBool() + if !leftIsBool && !rightIsBool { + return true + } + if leftIsBool && rightIsBool && leftBool == rightBool { + return false + } + result.Differences = append(result.Differences, SchemaDifference{ + Path: path.String(), + LeftValue: boolDifferenceValue(leftBool, leftIsBool), + RightValue: boolDifferenceValue(rightBool, rightIsBool), + Description: "boolean schema form mismatch", + }) + return false +} + +// boolDifferenceValue renders one side of a boolean-form mismatch for a +// SchemaDifference. Callers format LeftValue and RightValue with %v, and +// neither of the values available here survives that: a *bool prints as a +// pointer address, and a *Schema prints as a full struct dump. Both hide the +// one thing the difference exists to report. +// +// nil marks the side that is not a boolean schema at all, matching how the +// other comparators leave a value absent rather than inventing one. +func boolDifferenceValue(value, ok bool) any { + if !ok { + return nil + } + return value +} + +// compareBoolOperands handles the bare-boolean form for the any-typed +// schema-or-bool fields, recording a difference when the two sides disagree. +// It reports whether the comparison is settled, which it is as soon as either +// side is boolean: a boolean schema has no other fields. +// +// Shared by the three functions that compare these fields — compareSchemaOrBool, +// compareItemsSchemas and comparePolymorphicSchemas — so the check cannot be +// added to one and forgotten in the others. Pass an empty field when the caller +// has already pushed the path segment. +func compareBoolOperands(field string, left, right any, path *comparePath, result *EquivalenceResult) bool { + leftBool, leftIsBool := boolSchemaOperand(left) + rightBool, rightIsBool := boolSchemaOperand(right) + if !leftIsBool && !rightIsBool { + return false + } + if leftIsBool && rightIsBool && leftBool == rightBool { + return true + } + + description := "boolean value mismatch" + if field != "" { + path.push(field) + defer path.pop() + description = field + " " + description + } + result.Differences = append(result.Differences, SchemaDifference{ + Path: path.String(), + LeftValue: boolDifferenceValue(leftBool, leftIsBool), + RightValue: boolDifferenceValue(rightBool, rightIsBool), + Description: description, + }) + return true +} + +// boolSchemaOperand reports the boolean a schema-or-bool operand represents, in +// either of the two representations it can arrive in: a raw bool, which is what +// the decoders leave in these any-typed fields, or a *Schema with BoolForm set, +// which is what a caller building one programmatically produces. They mean the +// same thing and must compare equal. +func boolSchemaOperand(v any) (value bool, ok bool) { + switch t := v.(type) { + case bool: + return t, true + case *parser.Schema: + return t.IsBool() + default: + return false, false + } +} + // compareDeep recursively compares all schema properties func compareDeep(left, right *parser.Schema, path *comparePath, result *EquivalenceResult, visited map[pointerPair]bool, compareDocs bool) { // Everything below dereferences both operands, and a nested schema can @@ -771,6 +878,15 @@ func compareDeep(left, right *parser.Schema, path *comparePath, result *Equivale return } + // The bare-boolean form has no keywords, so it is compared by value and none + // of the field-by-field comparison below applies. Checked here rather than + // only at the top-level entry point because every nested schema position + // routes through this function: without it, `{p: true}` and `{p: false}` + // compared equal, since compareCommonFields finds nothing set on either side. + if !equalBoolForms(left, right, path, result) { + return + } + // Check for circular references pair := pointerPair{ left: reflect.ValueOf(left).Pointer(), @@ -1212,6 +1328,10 @@ func compareItemsSchemas(left, right any, path *comparePath, result *Equivalence return } + if compareBoolOperands("items", left, right, path, result) { + return + } + // Both schemas leftSchema, leftIsSchema := left.(*parser.Schema) rightSchema, rightIsSchema := right.(*parser.Schema) @@ -1222,23 +1342,6 @@ func compareItemsSchemas(left, right any, path *comparePath, result *Equivalence return } - // Both booleans - leftBool, leftIsBool := left.(bool) - rightBool, rightIsBool := right.(bool) - if leftIsBool && rightIsBool { - if leftBool != rightBool { - path.push("items") - result.Differences = append(result.Differences, SchemaDifference{ - Path: path.String(), - LeftValue: leftBool, - RightValue: rightBool, - Description: "items boolean value mismatch", - }) - path.pop() - } - return - } - // Type mismatch path.push("items") result.Differences = append(result.Differences, SchemaDifference{ @@ -1278,6 +1381,10 @@ func compareSchemaOrBool(field string, left, right any, path *comparePath, resul return } + if compareBoolOperands(field, left, right, path, result) { + return + } + // Both schemas leftSchema, leftIsSchema := left.(*parser.Schema) rightSchema, rightIsSchema := right.(*parser.Schema) @@ -1290,23 +1397,6 @@ func compareSchemaOrBool(field string, left, right any, path *comparePath, resul return } - // Both booleans - leftBool, leftIsBool := left.(bool) - rightBool, rightIsBool := right.(bool) - if leftIsBool && rightIsBool { - if leftBool != rightBool { - path.push(field) - result.Differences = append(result.Differences, SchemaDifference{ - Path: path.String(), - LeftValue: leftBool, - RightValue: rightBool, - Description: field + " boolean value mismatch", - }) - path.pop() - } - return - } - // Type mismatch path.push(field) result.Differences = append(result.Differences, SchemaDifference{ @@ -1335,6 +1425,10 @@ func comparePolymorphicSchemas(left, right any, path *comparePath, result *Equiv return } + if compareBoolOperands("", left, right, path, result) { + return + } + // Both schemas leftSchema, leftIsSchema := left.(*parser.Schema) rightSchema, rightIsSchema := right.(*parser.Schema) @@ -1343,21 +1437,6 @@ func comparePolymorphicSchemas(left, right any, path *comparePath, result *Equiv return } - // Both booleans - leftBool, leftIsBool := left.(bool) - rightBool, rightIsBool := right.(bool) - if leftIsBool && rightIsBool { - if leftBool != rightBool { - result.Differences = append(result.Differences, SchemaDifference{ - Path: path.String(), - LeftValue: leftBool, - RightValue: rightBool, - Description: "boolean value mismatch", - }) - } - return - } - // Type mismatch result.Differences = append(result.Differences, SchemaDifference{ Path: path.String(), diff --git a/parser/bool_schema_test.go b/parser/bool_schema_test.go new file mode 100644 index 00000000..1d0ca390 --- /dev/null +++ b/parser/bool_schema_test.go @@ -0,0 +1,218 @@ +package parser + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.yaml.in/yaml/v4" +) + +// boolSchemaSpecs is the same document in both source formats. parser keeps +// separate YAML and JSON decode paths, so anything asserted about decoding has +// to be asserted twice or it covers half the surface. +var boolSchemaSpecs = map[string]string{ + "yaml": ` +openapi: 3.2.0 +info: + title: API + version: 1.0.0 +components: + schemas: + anything: true + nothing: false + object: {} + nested: + type: object + properties: + p: true + q: false +`, + "json": `{ + "openapi": "3.2.0", + "info": {"title": "API", "version": "1.0.0"}, + "components": { + "schemas": { + "anything": true, + "nothing": false, + "object": {}, + "nested": { + "type": "object", + "properties": {"p": true, "q": false} + } + } + } +}`, +} + +// TestBoolSchemaDecodes covers the bare-boolean schema form that JSON Schema +// 2020-12 allows and OAS 3.1+ adopts. `true` accepts anything, `false` accepts +// nothing, and both are legal wherever a Schema Object is expected. +func TestBoolSchemaDecodes(t *testing.T) { + for _, format := range []string{"yaml", "json"} { + t.Run(format, func(t *testing.T) { + result, err := New().ParseBytes([]byte(boolSchemaSpecs[format])) + require.NoError(t, err) + doc, ok := result.OAS3Document() + require.True(t, ok, "expected an OAS3 document") + + checks := []struct { + name string + schema *Schema + wantValue bool + wantBool bool + }{ + {"anything", doc.Components.Schemas["anything"], true, true}, + {"nothing", doc.Components.Schemas["nothing"], false, true}, + // An empty object is a schema that constrains nothing. It is not + // the boolean `true`, and must not be reported as one. + {"object", doc.Components.Schemas["object"], false, false}, + {"nested.properties.p", doc.Components.Schemas["nested"].Properties["p"], true, true}, + {"nested.properties.q", doc.Components.Schemas["nested"].Properties["q"], false, true}, + } + for _, c := range checks { + require.NotNil(t, c.schema, "%s: schema was dropped", c.name) + gotValue, gotBool := c.schema.IsBool() + assert.Equal(t, c.wantBool, gotBool, "%s: IsBool ok", c.name) + assert.Equal(t, c.wantValue, gotValue, "%s: IsBool value", c.name) + } + }) + } +} + +// TestBoolSchemaSurvivesResolveRefs covers the third decode path. decodeFromMap +// is map-driven, so before decodeSchemaValue existed it dropped a boolean value +// silently — the schema simply was not there, with no error to say so. +func TestBoolSchemaSurvivesResolveRefs(t *testing.T) { + p := New() + p.ResolveRefs = true + + result, err := p.ParseBytes([]byte(boolSchemaSpecs["yaml"])) + require.NoError(t, err) + doc, ok := result.OAS3Document() + require.True(t, ok, "expected an OAS3 document") + + assert.Len(t, doc.Components.Schemas, 4, "a boolean schema was dropped") + for name, want := range map[string]bool{"anything": true, "nothing": false} { + schema := doc.Components.Schemas[name] + require.NotNil(t, schema, "%s: dropped by the ResolveRefs decode path", name) + got, isBool := schema.IsBool() + assert.True(t, isBool, "%s: not reported as a boolean schema", name) + assert.Equal(t, want, got, "%s: wrong boolean value", name) + } +} + +// TestBoolSchemaRoundTrips checks that a boolean schema serializes back as the +// bare scalar. Emitting `{}` instead would silently rewrite it into a different +// schema — one that constrains nothing, rather than `false`, which permits +// nothing. +func TestBoolSchemaRoundTrips(t *testing.T) { + tests := []struct { + name string + schema *Schema + wantYAML string + wantJSON string + }{ + {"true", NewBoolSchema(true), "true\n", "true"}, + {"false", NewBoolSchema(false), "false\n", "false"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotYAML, err := yaml.Marshal(tt.schema) + require.NoError(t, err) + assert.Equal(t, tt.wantYAML, string(gotYAML)) + + gotJSON, err := json.Marshal(tt.schema) + require.NoError(t, err) + assert.Equal(t, tt.wantJSON, string(gotJSON)) + }) + } +} + +// TestBoolSchemaEquality guards the pair that matters most: `true` and `false` +// are opposite schemas. Treating BoolForm as spelling — the way +// Discriminator.StringForm is treated — would make them compare equal and let +// semantic deduplication merge them. +func TestBoolSchemaEquality(t *testing.T) { + tests := []struct { + name string + a, b *Schema + equal bool + }{ + {"true equals true", NewBoolSchema(true), NewBoolSchema(true), true}, + {"false equals false", NewBoolSchema(false), NewBoolSchema(false), true}, + {"true does not equal false", NewBoolSchema(true), NewBoolSchema(false), false}, + {"true does not equal empty object", NewBoolSchema(true), &Schema{}, false}, + {"false does not equal empty object", NewBoolSchema(false), &Schema{}, false}, + {"empty object does not equal true", &Schema{}, NewBoolSchema(true), false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.equal, tt.a.Equals(tt.b)) + }) + } +} + +// TestIsBool covers the accessor's own contract, including the nil receiver it +// documents. The two returns are independent: (false, false) is an ordinary +// object schema, while (false, true) is the boolean schema `false`. +func TestIsBool(t *testing.T) { + tests := []struct { + name string + schema *Schema + wantValue bool + wantOK bool + }{ + {"nil receiver", nil, false, false}, + {"empty object schema", &Schema{}, false, false}, + {"populated object schema", &Schema{Type: "string"}, false, false}, + {"boolean true", NewBoolSchema(true), true, true}, + {"boolean false", NewBoolSchema(false), false, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotValue, gotOK := tt.schema.IsBool() + assert.Equal(t, tt.wantOK, gotOK) + assert.Equal(t, tt.wantValue, gotValue) + }) + } +} + +// TestBoolSchemaDeepCopyDoesNotAlias guards the pointer. BoolForm is a *bool, +// so a struct copy without an explicit deep copy would share the pointee +// between the original and the copy. +func TestBoolSchemaDeepCopyDoesNotAlias(t *testing.T) { + original := NewBoolSchema(true) + clone := original.DeepCopy() + + value, ok := clone.IsBool() + require.True(t, ok, "clone is not a boolean schema") + require.True(t, value, "clone has the wrong value") + require.NotSame(t, original.BoolForm, clone.BoolForm, + "DeepCopy shares the BoolForm pointer with the original") + + *clone.BoolForm = false + originalValue, _ := original.IsBool() + assert.True(t, originalValue, "mutating the clone changed the original") +} + +// TestQuotedTrueIsNotABoolSchema guards the tag check. In YAML a quoted "true" +// is a string scalar, which is not a schema at all — so it must not be silently +// accepted as the boolean form. +func TestQuotedTrueIsNotABoolSchema(t *testing.T) { + spec := ` +openapi: 3.2.0 +info: + title: API + version: 1.0.0 +components: + schemas: + quoted: "true" +` + _, err := New().ParseBytes([]byte(spec)) + assert.Error(t, err, "a string where a schema is expected should not parse") +} diff --git a/parser/conformance_relaxed_test.go b/parser/conformance_relaxed_test.go index 72fbe1c1..6f58d3ab 100644 --- a/parser/conformance_relaxed_test.go +++ b/parser/conformance_relaxed_test.go @@ -3,6 +3,9 @@ package parser import ( "strings" "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // hasErrorContaining reports whether any parse error mentions substr. @@ -130,19 +133,17 @@ info: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result, err := New().ParseBytes([]byte(tt.spec)) - if err != nil { - t.Fatalf("ParseBytes: %v", err) - } - got := hasErrorContaining(result.Errors, tt.errSubst) - if tt.wantErr && !got { - t.Errorf("want error containing %q, got errors: %v", tt.errSubst, result.Errors) - } - if !tt.wantErr { - // Neither form of the root requirement may fire. - if hasErrorContaining(result.Errors, wantMsg) || hasErrorContaining(result.Errors, want30Msg) { - t.Errorf("want no root-requirement error, got: %v", result.Errors) - } + require.NoError(t, err) + if tt.wantErr { + assert.True(t, hasErrorContaining(result.Errors, tt.errSubst), + "want error containing %q, got errors: %v", tt.errSubst, result.Errors) + return } + // Neither form of the root requirement may fire. + assert.False(t, hasErrorContaining(result.Errors, wantMsg), + "want no root-requirement error, got: %v", result.Errors) + assert.False(t, hasErrorContaining(result.Errors, want30Msg), + "want no root-requirement error, got: %v", result.Errors) }) } } @@ -234,13 +235,9 @@ paths: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result, err := New().ParseBytes([]byte(tt.spec)) - if err != nil { - t.Fatalf("ParseBytes: %v", err) - } - got := hasErrorContaining(result.Errors, wantMsg) - if got != tt.wantErr { - t.Errorf("responses-required error = %v, want %v; errors: %v", got, tt.wantErr, result.Errors) - } + require.NoError(t, err) + assert.Equal(t, tt.wantErr, hasErrorContaining(result.Errors, wantMsg), + "responses-required error presence; errors: %v", result.Errors) }) } } @@ -316,12 +313,8 @@ paths: // the test should fail so the move is a decision rather than a // side effect. _, err := New().ParseBytes([]byte(tt.spec)) - if err == nil { - t.Fatalf("want a hard ParseBytes error containing %q, got nil", want) - } - if !strings.Contains(err.Error(), want) { - t.Errorf("want error containing %q, got: %v", want, err) - } + require.Error(t, err, "want a hard ParseBytes error containing %q", want) + assert.Contains(t, err.Error(), want) }) } } diff --git a/parser/decode_helpers.go b/parser/decode_helpers.go index b3f09171..e8d57850 100644 --- a/parser/decode_helpers.go +++ b/parser/decode_helpers.go @@ -133,6 +133,27 @@ func mapGetBoolPtr(m map[string]any, key string) *bool { return nil } +// decodeSchemaValue decodes one Schema from the generic map representation, +// accepting both the object form and the bare-boolean form that JSON Schema +// 2020-12 allows wherever a schema is expected (see Schema.BoolForm). +// +// It returns nil for anything else, including an absent key. This is the +// decodeFromMap counterpart of the bool handling in Schema.UnmarshalYAML and +// Schema.UnmarshalJSON; all three paths must agree, or a boolean schema +// survives one route and vanishes on another. +func decodeSchemaValue(v any) *Schema { + switch t := v.(type) { + case bool: + return NewBoolSchema(t) + case map[string]any: + s := new(Schema) + s.decodeFromMap(t) + return s + default: + return nil + } +} + // mapGetStringMap extracts a map[string]string from m[key]. func mapGetStringMap(m map[string]any, key string) map[string]string { v, ok := m[key] diff --git a/parser/decode_schema_value_test.go b/parser/decode_schema_value_test.go new file mode 100644 index 00000000..287296cf --- /dev/null +++ b/parser/decode_schema_value_test.go @@ -0,0 +1,123 @@ +package parser + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// decodeSchemaValue is the seam the decode generator emits for every Schema +// position, so it is exercised directly here rather than only through a parsed +// document. The generator uses it in three shapes — a singular *Schema field, a +// []*Schema slice and a map[string]*Schema — and the ResolveRefs path is the +// only one that reaches any of them. +func TestDecodeSchemaValue(t *testing.T) { + tests := []struct { + name string + input any + wantNil bool + wantBool bool + wantIsBool bool + wantType any + }{ + {name: "boolean true", input: true, wantBool: true, wantIsBool: true}, + {name: "boolean false", input: false, wantBool: false, wantIsBool: true}, + {name: "object schema", input: map[string]any{"type": "string"}, wantType: "string"}, + {name: "empty object schema", input: map[string]any{}}, + // Anything that is neither a schema object nor a boolean is not a + // schema. Returning nil lets the caller omit the entry rather than + // inventing an empty one. + {name: "string is not a schema", input: "true", wantNil: true}, + {name: "number is not a schema", input: 1, wantNil: true}, + {name: "nil", input: nil, wantNil: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := decodeSchemaValue(tt.input) + if tt.wantNil { + assert.Nil(t, got) + return + } + require.NotNil(t, got) + value, isBool := got.IsBool() + assert.Equal(t, tt.wantIsBool, isBool) + assert.Equal(t, tt.wantBool, value) + assert.Equal(t, tt.wantType, got.Type) + }) + } +} + +// TestDecodeFromMapSchemaShapes drives the three generated shapes through the +// ResolveRefs path, which is the route that reaches decodeFromMap. Before +// decodeSchemaValue existed, each of these dropped a boolean silently. +func TestDecodeFromMapSchemaShapes(t *testing.T) { + spec := ` +openapi: 3.2.0 +info: + title: API + version: 1.0.0 +components: + schemas: + mapShape: true + ptrShape: + not: false + sliceShape: + allOf: + - true + - type: string + - false + defsShape: + $defs: + inner: true +` + p := New() + p.ResolveRefs = true + + result, err := p.ParseBytes([]byte(spec)) + require.NoError(t, err) + doc, ok := result.OAS3Document() + require.True(t, ok, "expected an OAS3 document") + + schemas := doc.Components.Schemas + require.Len(t, schemas, 4, "a schema was dropped by decodeFromMap") + + t.Run("map value", func(t *testing.T) { + value, isBool := schemas["mapShape"].IsBool() + assert.True(t, isBool) + assert.True(t, value) + }) + + t.Run("singular pointer field", func(t *testing.T) { + require.NotNil(t, schemas["ptrShape"].Not, "not: false was dropped") + value, isBool := schemas["ptrShape"].Not.IsBool() + assert.True(t, isBool) + assert.False(t, value) + }) + + t.Run("slice element", func(t *testing.T) { + allOf := schemas["sliceShape"].AllOf + require.Len(t, allOf, 3, "a boolean member of allOf was dropped") + + first, isBool := allOf[0].IsBool() + assert.True(t, isBool) + assert.True(t, first) + + _, middleIsBool := allOf[1].IsBool() + assert.False(t, middleIsBool, "the object member should not be a boolean schema") + assert.Equal(t, "string", allOf[1].Type) + + last, isBool := allOf[2].IsBool() + assert.True(t, isBool) + assert.False(t, last) + }) + + t.Run("nested map value", func(t *testing.T) { + inner := schemas["defsShape"].Defs["inner"] + require.NotNil(t, inner, "$defs entry was dropped") + value, isBool := inner.IsBool() + assert.True(t, isBool) + assert.True(t, value) + }) +} diff --git a/parser/schema.go b/parser/schema.go index 0c358c2d..33765429 100644 --- a/parser/schema.go +++ b/parser/schema.go @@ -98,6 +98,40 @@ type Schema struct { // Extension fields // Extra captures specification extensions (fields starting with "x-") Extra map[string]any `yaml:",inline" json:"-"` + + // BoolForm reports that this schema came from — and should be written back + // as — a bare boolean rather than an object. JSON Schema 2020-12 allows + // `true` (accept anything) and `false` (accept nothing) wherever a schema is + // expected, and OAS 3.1+ adopts that dialect wholesale, so a Components + // entry may legally be `MySchema: true`. + // + // nil is the ordinary object form. Non-nil carries the boolean's value, so + // `true` and `false` stay distinguishable — they are opposite schemas, not a + // present/absent pair. + // + // This is spelling, not meaning, so it is excluded from JSON and YAML the + // same way Discriminator.StringForm is. Every other field is meaningless + // alongside it and is dropped when serializing with BoolForm set: a boolean + // schema has no keywords by definition. + // + // Only valid for OAS 3.1+. The validator rejects it for 3.0 and 2.0, which + // predate the 2020-12 dialect. + BoolForm *bool `yaml:"-" json:"-"` +} + +// IsBool reports whether the schema is the bare-boolean form, and its value. +// The two returns are independent: (false, false) is an ordinary object schema, +// while (false, true) is the boolean schema `false`, which accepts nothing. +func (s *Schema) IsBool() (value bool, ok bool) { + if s == nil || s.BoolForm == nil { + return false, false + } + return *s.BoolForm, true +} + +// NewBoolSchema returns a schema in the bare-boolean form. +func NewBoolSchema(v bool) *Schema { + return &Schema{BoolForm: &v} } // Discriminator represents a discriminator for polymorphism. diff --git a/parser/schema_equals.go b/parser/schema_equals.go index ce7facdd..5bd715f5 100644 --- a/parser/schema_equals.go +++ b/parser/schema_equals.go @@ -42,6 +42,17 @@ func (s *Schema) equalsWithVisited(other *Schema, visited map[schemaPair]bool) b } visited[pair] = true + // The bare-boolean form is compared by value, not treated as spelling. + // Unlike Discriminator.StringForm — where both spellings mean the same + // thing — `true` and `false` are opposite schemas, and neither is equal to + // an object schema. A boolean schema has no other fields, so this is the + // whole comparison when either side is one. + sv, sok := s.IsBool() + ov, ook := other.IsBool() + if sok || ook { + return sok && ook && sv == ov + } + // Group 1: Boolean fields (cheapest comparisons first) if s.ReadOnly != other.ReadOnly { return false diff --git a/parser/schema_json.go b/parser/schema_json.go index 7cb03e8c..7048ad40 100644 --- a/parser/schema_json.go +++ b/parser/schema_json.go @@ -13,6 +13,12 @@ import ( // into the top-level JSON object, as Go's encoding/json doesn't support // inline maps like yaml:",inline". func (s *Schema) MarshalJSON() ([]byte, error) { + // The bare-boolean form has no keywords, so it round-trips as the scalar it + // came from rather than as an object. See Schema.BoolForm. + if b, ok := s.IsBool(); ok { + return json.Marshal(b) + } + // Fast path: no Extra fields, use standard marshaling if len(s.Extra) == 0 { type Alias Schema @@ -96,6 +102,15 @@ func (s *Schema) MarshalJSON() ([]byte, error) { // UnmarshalJSON implements custom JSON unmarshaling for Schema. // This captures unknown fields (specification extensions like x-*) in the Extra map. func (s *Schema) UnmarshalJSON(data []byte) error { + // A schema may be a bare boolean in JSON Schema 2020-12, which OAS 3.1+ + // adopts. Unmarshaling a scalar into the struct alias below fails, so catch + // it first and record the spelling. See Schema.BoolForm. + var b bool + if err := json.Unmarshal(data, &b); err == nil { + *s = Schema{BoolForm: &b} + return nil + } + type Alias Schema if err := json.Unmarshal(data, (*Alias)(s)); err != nil { return err diff --git a/parser/schema_yaml.go b/parser/schema_yaml.go index 9e2b144a..f91e4f5c 100644 --- a/parser/schema_yaml.go +++ b/parser/schema_yaml.go @@ -18,6 +18,15 @@ import ( // Schema.UnmarshalJSON does the same job for JSON, and decodeFromMap for the // ResolveRefs path. All three must agree on the decoded types. func (s *Schema) UnmarshalYAML(node *yaml.Node) error { + // A schema may be a bare boolean in JSON Schema 2020-12, which OAS 3.1+ + // adopts — `true` accepts anything, `false` accepts nothing. Decoding a + // scalar into the struct alias below fails, so catch it first and record + // the spelling. See Schema.BoolForm. + if b, ok := boolSchemaNode(node); ok { + *s = Schema{BoolForm: &b} + return nil + } + type Alias Schema var alias Alias if err := node.Decode(&alias); err != nil { @@ -116,6 +125,24 @@ func childValueNode(node *yaml.Node, key string) *yaml.Node { // decoded document — yaml.Unmarshal hands an Unmarshaler the document node // rather than its content — and an AliasNode when the schema was written as // `*anchor`. Returns nil when no node is left to unwrap. +// boolSchemaNode reports whether a node is the bare-boolean schema form, and +// its value. Anchors are followed first so `schema: *alwaysValid` is classified +// by what it points at rather than by the alias node. +// +// Only a genuine `!!bool` counts. A quoted "true" is a string scalar and is not +// a boolean schema, so tag is checked rather than the raw value. +func boolSchemaNode(node *yaml.Node) (bool, bool) { + node = unwrapSchemaNode(node) + if node == nil || node.Kind != yaml.ScalarNode || node.Tag != "!!bool" { + return false, false + } + var b bool + if err := node.Decode(&b); err != nil { + return false, false + } + return b, true +} + func unwrapSchemaNode(node *yaml.Node) *yaml.Node { for node != nil { switch node.Kind { @@ -198,6 +225,21 @@ func yamlKindName(k yaml.Kind) string { } } +// MarshalYAML implements custom YAML marshaling for Schema. +// +// When BoolForm is set the bare-boolean form is emitted, so `MySchema: true` +// is not silently rewritten into the empty object `MySchema: {}` — which is a +// different schema, and one that constrains nothing rather than the `false` +// case that permits nothing. Every other field is dropped, since a boolean +// schema has no keywords. +func (s *Schema) MarshalYAML() (any, error) { + if b, ok := s.IsBool(); ok { + return b, nil + } + type Alias Schema + return (*Alias)(s), nil +} + // MarshalYAML implements custom YAML marshaling for Discriminator. // // When StringForm is set the OAS 2.0 bare-string form is emitted, so a 2.0 diff --git a/parser/zz_generated_decode.go b/parser/zz_generated_decode.go index ef165ed8..0fdf19c2 100644 --- a/parser/zz_generated_decode.go +++ b/parser/zz_generated_decode.go @@ -12,9 +12,7 @@ func (x *Components) decodeFromMap(m map[string]any) { if sub, ok := m["schemas"].(map[string]any); ok { x.Schemas = make(map[string]*Schema, len(sub)) for k, v := range sub { - if vm, ok := v.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(vm) + if elem := decodeSchemaValue(v); elem != nil { x.Schemas[k] = elem } } @@ -200,10 +198,7 @@ func (x *Header) decodeFromMap(m map[string]any) { x.Deprecated, _ = m["deprecated"].(bool) x.Style, _ = m["style"].(string) x.Explode = mapGetBoolPtr(m, "explode") - if sub, ok := m["schema"].(map[string]any); ok { - x.Schema = new(Schema) - x.Schema.decodeFromMap(sub) - } + x.Schema = decodeSchemaValue(m["schema"]) x.Example = m["example"] if sub, ok := m["examples"].(map[string]any); ok { x.Examples = make(map[string]*Example, len(sub)) @@ -317,10 +312,7 @@ func (x *Link) decodeFromMap(m map[string]any) { } func (x *MediaType) decodeFromMap(m map[string]any) { - if sub, ok := m["schema"].(map[string]any); ok { - x.Schema = new(Schema) - x.Schema.decodeFromMap(sub) - } + x.Schema = decodeSchemaValue(m["schema"]) x.Example = m["example"] if sub, ok := m["examples"].(map[string]any); ok { x.Examples = make(map[string]*Example, len(sub)) @@ -342,10 +334,7 @@ func (x *MediaType) decodeFromMap(m map[string]any) { } } } - if sub, ok := m["itemSchema"].(map[string]any); ok { - x.ItemSchema = new(Schema) - x.ItemSchema.decodeFromMap(sub) - } + x.ItemSchema = decodeSchemaValue(m["itemSchema"]) if sub, ok := m["itemEncoding"].(map[string]any); ok { x.ItemEncoding = new(Encoding) x.ItemEncoding.decodeFromMap(sub) @@ -380,9 +369,7 @@ func (x *OAS2Document) decodeFromMap(m map[string]any) { if sub, ok := m["definitions"].(map[string]any); ok { x.Definitions = make(map[string]*Schema, len(sub)) for k, v := range sub { - if vm, ok := v.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(vm) + if elem := decodeSchemaValue(v); elem != nil { x.Definitions[k] = elem } } @@ -590,10 +577,7 @@ func (x *Parameter) decodeFromMap(m map[string]any) { x.Style, _ = m["style"].(string) x.Explode = mapGetBoolPtr(m, "explode") x.AllowReserved, _ = m["allowReserved"].(bool) - if sub, ok := m["schema"].(map[string]any); ok { - x.Schema = new(Schema) - x.Schema.decodeFromMap(sub) - } + x.Schema = decodeSchemaValue(m["schema"]) x.Example = m["example"] if sub, ok := m["examples"].(map[string]any); ok { x.Examples = make(map[string]*Example, len(sub)) @@ -772,10 +756,7 @@ func (x *Response) decodeFromMap(m map[string]any) { } } x.Summary, _ = m["summary"].(string) - if sub, ok := m["schema"].(map[string]any); ok { - x.Schema = new(Schema) - x.Schema.decodeFromMap(sub) - } + x.Schema = decodeSchemaValue(m["schema"]) if sub, ok := m["examples"].(map[string]any); ok { x.Examples = sub } @@ -808,9 +789,7 @@ func (x *Schema) decodeFromMap(m map[string]any) { if arr, ok := m["prefixItems"].([]any); ok { x.PrefixItems = make([]*Schema, 0, len(arr)) for _, item := range arr { - if sub, ok := item.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(sub) + if elem := decodeSchemaValue(item); elem != nil { x.PrefixItems = append(x.PrefixItems, elem) } } @@ -820,18 +799,13 @@ func (x *Schema) decodeFromMap(m map[string]any) { x.MaxItems = mapGetIntPtr(m, "maxItems") x.MinItems = mapGetIntPtr(m, "minItems") x.UniqueItems, _ = m["uniqueItems"].(bool) - if sub, ok := m["contains"].(map[string]any); ok { - x.Contains = new(Schema) - x.Contains.decodeFromMap(sub) - } + x.Contains = decodeSchemaValue(m["contains"]) x.MaxContains = mapGetIntPtr(m, "maxContains") x.MinContains = mapGetIntPtr(m, "minContains") if sub, ok := m["properties"].(map[string]any); ok { x.Properties = make(map[string]*Schema, len(sub)) for k, v := range sub { - if vm, ok := v.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(vm) + if elem := decodeSchemaValue(v); elem != nil { x.Properties[k] = elem } } @@ -839,9 +813,7 @@ func (x *Schema) decodeFromMap(m map[string]any) { if sub, ok := m["patternProperties"].(map[string]any); ok { x.PatternProperties = make(map[string]*Schema, len(sub)) for k, v := range sub { - if vm, ok := v.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(vm) + if elem := decodeSchemaValue(v); elem != nil { x.PatternProperties[k] = elem } } @@ -849,41 +821,25 @@ func (x *Schema) decodeFromMap(m map[string]any) { x.AdditionalProperties = decodeSchemaOrBool(m["additionalProperties"]) x.UnevaluatedProperties = decodeSchemaOrBool(m["unevaluatedProperties"]) x.Required = mapGetStringSlice(m, "required") - if sub, ok := m["propertyNames"].(map[string]any); ok { - x.PropertyNames = new(Schema) - x.PropertyNames.decodeFromMap(sub) - } + x.PropertyNames = decodeSchemaValue(m["propertyNames"]) x.MaxProperties = mapGetIntPtr(m, "maxProperties") x.MinProperties = mapGetIntPtr(m, "minProperties") x.DependentRequired = mapGetDependentRequired(m, "dependentRequired") if sub, ok := m["dependentSchemas"].(map[string]any); ok { x.DependentSchemas = make(map[string]*Schema, len(sub)) for k, v := range sub { - if vm, ok := v.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(vm) + if elem := decodeSchemaValue(v); elem != nil { x.DependentSchemas[k] = elem } } } - if sub, ok := m["if"].(map[string]any); ok { - x.If = new(Schema) - x.If.decodeFromMap(sub) - } - if sub, ok := m["then"].(map[string]any); ok { - x.Then = new(Schema) - x.Then.decodeFromMap(sub) - } - if sub, ok := m["else"].(map[string]any); ok { - x.Else = new(Schema) - x.Else.decodeFromMap(sub) - } + x.If = decodeSchemaValue(m["if"]) + x.Then = decodeSchemaValue(m["then"]) + x.Else = decodeSchemaValue(m["else"]) if arr, ok := m["allOf"].([]any); ok { x.AllOf = make([]*Schema, 0, len(arr)) for _, item := range arr { - if sub, ok := item.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(sub) + if elem := decodeSchemaValue(item); elem != nil { x.AllOf = append(x.AllOf, elem) } } @@ -891,9 +847,7 @@ func (x *Schema) decodeFromMap(m map[string]any) { if arr, ok := m["anyOf"].([]any); ok { x.AnyOf = make([]*Schema, 0, len(arr)) for _, item := range arr { - if sub, ok := item.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(sub) + if elem := decodeSchemaValue(item); elem != nil { x.AnyOf = append(x.AnyOf, elem) } } @@ -901,17 +855,12 @@ func (x *Schema) decodeFromMap(m map[string]any) { if arr, ok := m["oneOf"].([]any); ok { x.OneOf = make([]*Schema, 0, len(arr)) for _, item := range arr { - if sub, ok := item.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(sub) + if elem := decodeSchemaValue(item); elem != nil { x.OneOf = append(x.OneOf, elem) } } } - if sub, ok := m["not"].(map[string]any); ok { - x.Not = new(Schema) - x.Not.decodeFromMap(sub) - } + x.Not = decodeSchemaValue(m["not"]) x.Nullable, _ = m["nullable"].(bool) x.Discriminator = decodeDiscriminator(m["discriminator"]) x.ReadOnly, _ = m["readOnly"].(bool) @@ -929,10 +878,7 @@ func (x *Schema) decodeFromMap(m map[string]any) { x.Format, _ = m["format"].(string) x.ContentEncoding, _ = m["contentEncoding"].(string) x.ContentMediaType, _ = m["contentMediaType"].(string) - if sub, ok := m["contentSchema"].(map[string]any); ok { - x.ContentSchema = new(Schema) - x.ContentSchema.decodeFromMap(sub) - } + x.ContentSchema = decodeSchemaValue(m["contentSchema"]) x.CollectionFormat, _ = m["collectionFormat"].(string) x.ID, _ = m["$id"].(string) x.Anchor, _ = m["$anchor"].(string) @@ -943,9 +889,7 @@ func (x *Schema) decodeFromMap(m map[string]any) { if sub, ok := m["$defs"].(map[string]any); ok { x.Defs = make(map[string]*Schema, len(sub)) for k, v := range sub { - if vm, ok := v.(map[string]any); ok { - elem := new(Schema) - elem.decodeFromMap(vm) + if elem := decodeSchemaValue(v); elem != nil { x.Defs[k] = elem } } diff --git a/parser/zz_generated_deepcopy.go b/parser/zz_generated_deepcopy.go index 6406fc21..ce4ef345 100644 --- a/parser/zz_generated_deepcopy.go +++ b/parser/zz_generated_deepcopy.go @@ -1180,6 +1180,11 @@ func (in *Schema) DeepCopyInto(out *Schema) { *out.MinProperties = *in.MinProperties } + if in.BoolForm != nil { + out.BoolForm = new(bool) + *out.BoolForm = *in.BoolForm + } + if in.Discriminator != nil { out.Discriminator = in.Discriminator.DeepCopy() } diff --git a/validator/bool_schema_test.go b/validator/bool_schema_test.go new file mode 100644 index 00000000..407a25ba --- /dev/null +++ b/validator/bool_schema_test.go @@ -0,0 +1,125 @@ +package validator + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestBoolSchemaVersionGate covers the version applicability of the bare-boolean +// schema form. JSON Schema 2020-12 allows `true` and `false` wherever a schema is +// expected, and OAS 3.1 adopted that dialect wholesale. OAS 3.0 is based on an +// earlier draft where a schema is always an object. +// +// The parser accepts the form regardless of version — a Schema Object is decoded +// before the document version is known to it — so this check is the only thing +// standing between a 3.0 document and a silently accepted boolean schema. Same +// division of labour as the discriminator dialects. +func TestBoolSchemaVersionGate(t *testing.T) { + const wantMsg = "Boolean schemas" + + tests := []struct { + name string + spec string + wantErr bool + }{ + { + name: "3.1 accepts a boolean component schema", + spec: ` +openapi: 3.1.0 +info: + title: API + version: 1.0.0 +components: + schemas: + anything: true +`, + }, + { + name: "3.2 accepts a boolean component schema", + spec: ` +openapi: 3.2.0 +info: + title: API + version: 1.0.0 +components: + schemas: + nothing: false +`, + }, + { + name: "3.0 rejects a boolean component schema", + spec: ` +openapi: 3.0.3 +info: + title: API + version: 1.0.0 +paths: {} +components: + schemas: + anything: true +`, + wantErr: true, + }, + { + name: "2.0 rejects a boolean definition", + spec: ` +swagger: "2.0" +info: + title: API + version: 1.0.0 +paths: {} +definitions: + anything: true +`, + wantErr: true, + }, + { + name: "3.0 rejects a boolean nested in properties", + spec: ` +openapi: 3.0.3 +info: + title: API + version: 1.0.0 +paths: {} +components: + schemas: + Pet: + type: object + properties: + anything: true +`, + wantErr: true, + }, + { + name: "3.1 accepts a boolean nested in properties", + spec: ` +openapi: 3.1.0 +info: + title: API + version: 1.0.0 +components: + schemas: + Pet: + type: object + properties: + anything: true +`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := validateSpec(t, tt.spec) + assert.Equal(t, tt.wantErr, resultHasMessage(result, wantMsg), + "boolean-schema error presence; errors: %v", result.Errors) + + // The accepting cases assert full validity, not merely the absence + // of this one message — otherwise they would pass on a document + // that is invalid for some unrelated reason. + if !tt.wantErr { + assert.True(t, result.Valid, "document should validate clean; errors: %v", result.Errors) + } + }) + } +} diff --git a/validator/conformance_items_test.go b/validator/conformance_items_test.go index 93a048a2..eb4c45e8 100644 --- a/validator/conformance_items_test.go +++ b/validator/conformance_items_test.go @@ -4,6 +4,9 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/erraggy/oastools/parser" ) @@ -11,13 +14,9 @@ import ( func validateSpec(t *testing.T, spec string) *ValidationResult { t.Helper() parseResult, err := parser.New().ParseBytes([]byte(spec)) - if err != nil { - t.Fatalf("ParseBytes: %v", err) - } + require.NoError(t, err, "ParseBytes") result, err := New().ValidateParsed(*parseResult) - if err != nil { - t.Fatalf("ValidateParsed: %v", err) - } + require.NoError(t, err, "ValidateParsed") return result } @@ -103,9 +102,8 @@ components: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := validateSpec(t, tt.spec) - if resultHasMessage(result, wantAbsent) { - t.Errorf("no OAS version requires 'items' on an array Schema Object, got: %v", result.Errors) - } + assert.False(t, resultHasMessage(result, wantAbsent), + "no OAS version requires 'items' on an array Schema Object, got: %v", result.Errors) }) } } @@ -289,9 +287,8 @@ responses: for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { result := validateSpec(t, tt.spec) - if got := resultHasMessage(result, tt.want); got != tt.wantErr { - t.Errorf("message %q present = %v, want %v; errors: %v", tt.want, got, tt.wantErr, result.Errors) - } + assert.Equal(t, tt.wantErr, resultHasMessage(result, tt.want), + "message %q presence; errors: %v", tt.want, result.Errors) }) } } @@ -318,9 +315,8 @@ paths: description: ok ` result := validateSpec(t, spec) - if resultHasMessage(result, "must have 'items' defined") { - t.Errorf("OAS 3.x has no items-required rule, got: %v", result.Errors) - } + assert.False(t, resultHasMessage(result, "must have 'items' defined"), + "OAS 3.x has no items-required rule, got: %v", result.Errors) } // TestOAS2ItemsNestingDepthBounded guards the depth bound on the Items chain. @@ -347,7 +343,6 @@ parameters: } result := validateSpec(t, sb.String()) - if !resultHasMessage(result, "Items nesting depth") { - t.Errorf("want a depth-bound error, got: %v", result.Errors) - } + assert.True(t, resultHasMessage(result, "Items nesting depth"), + "want a depth-bound error, got: %v", result.Errors) } diff --git a/validator/schema.go b/validator/schema.go index 9450c717..6e75ad98 100644 --- a/validator/schema.go +++ b/validator/schema.go @@ -43,6 +43,13 @@ func (v *Validator) validateSchemaWithVisited(schema *parser.Schema, path string } visited[schema] = true + // A bare-boolean schema has no keywords, so it is the whole check when + // present — nothing below applies to it. + if _, isBool := schema.IsBool(); isBool { + v.validateBoolSchemaVersion(schema, path, result) + return + } + // Check for excessive nesting depth to prevent resource exhaustion if depth > maxSchemaNestingDepth { v.addError(result, path, @@ -194,6 +201,30 @@ func (v *Validator) validateSchemaTypeConstraints(schema *parser.Schema, path st } } +// validateBoolSchemaVersion rejects the bare-boolean schema form for the +// versions that predate it. +// +// JSON Schema 2020-12 allows `true` and `false` wherever a schema is expected, +// and OAS 3.1 adopted that dialect wholesale. OAS 3.0 is based on an earlier +// draft where a schema is always an object, and OAS 2.0 more so. The parser +// accepts the form regardless of version — a Schema Object is decoded before +// the document version is known to it — which makes this check the only thing +// standing between a 3.0 document and a silently accepted boolean schema. +// +// The same division of labour as validateDiscriminatorForm. +func (v *Validator) validateBoolSchemaVersion(schema *parser.Schema, path string, result *ValidationResult) { + // An unrecognized version says nothing about which forms are legal. + if !v.oasVersion.IsValid() || v.oasVersion >= parser.OASVersion310 { + return + } + b, _ := schema.IsBool() + v.addError(result, path, + fmt.Sprintf("Boolean schemas (%t) require OpenAPI 3.1 or later; in this version a schema must be an object", b), + withSpecRef(getJSONSchemaRef()), + withValue(b), + ) +} + // isOAS30x reports whether the given version is in the OAS 3.0.x family, // where "null" is not a valid schema type. func isOAS30x(version parser.OASVersion) bool {