fix(compilers/openapi): keep unquoted YAML dates as string values (#103)

PR: #103

Author: OmarAlJarrah

compilers/openapi/compose.go

@@ -457,31 +457,25 @@ func (l *lowerer) enumAsUnion(s *oas3.Schema, common ir.TypeCommon, pointer, hin
 	}
 }
 
-// hoistLiteral hoists a single node as a Literal type at its own pointer. It is
+// hoistLiteral hoists a single node as a Literal type at its own pointer, or
+// falls back to the schemaless top type when the node is structurally
+// unconvertible — never a Literal whose Value lies about being null. It is
 // the single entry point for lowering both a bare `const` schema (pointer may
 // be a top-level component, so internNode's typeIDForPointer keeps that
 // component's stable named ID) and each individual member of a heterogeneous
 // enum (enumAsUnion, always an anonymous sub-pointer).
 func (l *lowerer) hoistLiteral(node values.Value, pointer, hint string) ir.TypeID {
 	return l.internNode(pointer, hint, func(common ir.TypeCommon) ir.TypeDef {
-		return &ir.Literal{
-			TypeCommon: common,
-			Value:      l.valueOrNull(node, pointer),
+		val, err := valueFromNode(node)
+		if err != nil {
+			l.diag(ir.SeverityWarning, codeDegradedConstruct, pointer,
+				"unconvertible value lowered as the top type: %s", err.Error())
+			return &ir.Any{TypeCommon: common}
 		}
+		return &ir.Literal{TypeCommon: common, Value: val}
 	})
 }
 
-// valueOrNull converts a node to an ir.Value, emitting a diagnostic and using
-// null when the node is structurally unconvertible.
-func (l *lowerer) valueOrNull(node values.Value, pointer string) ir.Value {
-	val, err := valueFromNode(node)
-	if err != nil {
-		l.diag(ir.SeverityWarning, codeDegradedConstruct, pointer, "value: %s", err.Error())
-		return ir.Value{Kind: ir.ValueNull}
-	}
-	return val
-}
-
 // enumValueType picks an Enum's ValueType from the schema's declared scalar
 // type, falling back to the kind inferred from its members.
 func enumValueType(s *oas3.Schema, kind ir.ValueKind) ir.PrimKind {

compilers/openapi/compose_test.go

@@ -715,6 +715,101 @@ func TestConst_BecomesLiteral(t *testing.T) {
 	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "fixed"}, k.Value)
 }
 
+func TestHoistLiteral_UnconvertibleConstBecomesAny(t *testing.T) {
+	t.Parallel()
+	// A custom tag is structurally unconvertible (no scalarValue case resolves
+	// it), forcing hoistLiteral's fallback. Before the fix this silently
+	// produced a Literal asserting the value is null, which the spec never said.
+	spec := componentSpec("    K:\n      const: !foo bar\n")
+	doc, diags := lowerSpec(t, spec)
+	k, ok := doc.Types[componentID("K")].(*ir.Any)
+	require.True(t, ok, "an unconvertible const hoists the schemaless top type at its own pointer")
+	assert.Equal(t, ir.KindAny, k.Kind())
+	for _, td := range doc.Types {
+		_, isLiteral := td.(*ir.Literal)
+		assert.False(t, isLiteral, "no Literal is produced anywhere; nothing lies about the value being null")
+	}
+	require.Equal(t, 1, countDiagsAt(diags, codeDegradedConstruct, ir.SeverityWarning),
+		"exactly one warning fires for the unconvertible value")
+	d, ok := firstDegradedWarning(diags)
+	require.True(t, ok)
+	assert.Equal(t, "/components/schemas/K", d.Provenance.Pointer)
+}
+
+func TestEnumAsUnion_UnconvertibleMemberBecomesAny(t *testing.T) {
+	t.Parallel()
+	// The convertible member ("ok") must still hoist a real Literal; only the
+	// genuinely unconvertible member ("!foo bar") falls back to the top type.
+	spec := componentSpec("    M:\n      enum: [ok, !foo bar]\n")
+	doc, diags := lowerSpec(t, spec)
+	u, ok := doc.Types[componentID("M")].(*ir.Union)
+	require.True(t, ok, "heterogeneous enum still lowers to a union of literals")
+	require.Len(t, u.Variants, 2)
+
+	member0, ok := doc.Types[u.Variants[0].Type.Target].(*ir.Literal)
+	require.True(t, ok, "the convertible member still hoists a Literal")
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "ok"}, member0.Value)
+
+	member1, ok := doc.Types[u.Variants[1].Type.Target].(*ir.Any)
+	require.True(t, ok, "the unconvertible member hoists the schemaless top type, not a lying null Literal")
+	assert.Equal(t, ir.KindAny, member1.Kind())
+
+	require.Equal(t, 1, countDiagsAt(diags, codeDegradedConstruct, ir.SeverityWarning),
+		"exactly one warning for the unconvertible member, distinct from the heterogeneous-enum info diagnostic")
+	d, ok := firstDegradedWarning(diags)
+	require.True(t, ok)
+	assert.Equal(t, "/components/schemas/M/enum/1", d.Provenance.Pointer)
+}
+
+func TestEnum_UnquotedDatesStayClosedEnum(t *testing.T) {
+	t.Parallel()
+	// The exact issue repro: YAML 1.1 resolves an unquoted date to !!timestamp.
+	// Before the fix, scalarValue had no case for it, so both enum members
+	// failed to convert, enumMembers bailed to enumAsUnion, and valueOrNull
+	// turned every member into a null literal — the actual dates never
+	// survived. A component-level default (as in the repro) is never lowered
+	// onto anything by itself (only properties/params read one), so it
+	// contributes no diagnostic either way; see
+	// TestProperty_UnquotedDateDefaultPreserved for the default path.
+	spec := componentSpec(`    D:
+      type: string
+      format: date
+      default: 2021-01-01
+      enum: [2021-01-01, 2022-02-02]
+`)
+	doc, diags := lowerSpec(t, spec)
+	assert.Empty(t, diags, "no diagnostics at all: the dates convert cleanly")
+	e, ok := doc.Types[componentID("D")].(*ir.Enum)
+	require.True(t, ok, "D stays a closed Enum, never degrades to a Union of literals")
+	assert.True(t, e.Closed)
+	assert.Equal(t, ir.PrimString, e.ValueType)
+	require.Len(t, e.Members, 2)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, e.Members[0].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2022-02-02"}, e.Members[1].Value)
+}
+
+func TestProperty_UnquotedDateDefaultPreserved(t *testing.T) {
+	t.Parallel()
+	// The repro's default sits at the component level, which nothing lowers by
+	// itself; this covers the path that actually surfaces the bug in practice —
+	// a date default declared on an object property.
+	spec := componentSpec(`    S:
+      type: object
+      properties:
+        d:
+          type: string
+          format: date
+          default: 2021-01-01
+`)
+	doc, diags := lowerSpec(t, spec)
+	assert.Empty(t, diags, "no diagnostics at all: the date default converts cleanly")
+	m, ok := doc.Types[componentID("S")].(*ir.Model)
+	require.True(t, ok)
+	require.Len(t, m.Properties, 1)
+	require.NotNil(t, m.Properties[0].Default)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, *m.Properties[0].Default)
+}
+
 func TestAllOf_DiscriminatorHierarchy(t *testing.T) {
 	t.Parallel()
 	spec := componentSpecVer("3.2.0", `    Pet:

compilers/openapi/conformance_test.go

@@ -49,6 +49,7 @@ func TestConformance(t *testing.T) {
 		{"nullable-30", assertNullable30},
 		{"nullable-31-ref", assertNullable31Ref},
 		{"defaults", assertDefaults},
+		{"yaml-timestamp-scalars", assertYAMLTimestampScalars},
 		{"constraints", assertConstraints},
 		{"numeric-precision", assertNumericPrecision},
 		{"readonly-writeonly", assertReadOnlyWriteOnly},
@@ -354,6 +355,45 @@ func assertDefaults(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	assert.Equal(t, ir.BigVal("9007199254740993"), m.Properties[0].Default.Num)
 }
 
+// assertYAMLTimestampScalars covers a YAML 1.1 quirk: an unquoted date like
+// 2021-01-01 resolves to tag !!timestamp, not !!str. It must survive as the
+// literal string everywhere OpenAPI's JSON data model can carry one — enum,
+// const, a property default, a schema-level example, and a media-type
+// example — with nothing dropped or degraded to null.
+func assertYAMLTimestampScalars(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) {
+	assert.Empty(t, diags, "every unquoted date converts cleanly; nothing is dropped or degraded")
+
+	d, ok := doc.Types[namedID("D")].(*ir.Enum)
+	require.True(t, ok, "D stays a closed Enum of the real dates, not a union of null literals")
+	assert.Equal(t, ir.PrimString, d.ValueType)
+	require.Len(t, d.Members, 2)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, d.Members[0].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2022-02-02"}, d.Members[1].Value)
+
+	k, ok := doc.Types[namedID("K")].(*ir.Literal)
+	require.True(t, ok, "K's const hoists a real Literal, not the schemaless top type")
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, k.Value)
+
+	s, ok := doc.Types[namedID("S")].(*ir.Model)
+	require.True(t, ok)
+	require.Len(t, s.Properties, 1)
+	prop := s.Properties[0]
+	require.NotNil(t, prop.Default, "the property default is preserved")
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, *prop.Default)
+	require.Len(t, prop.Examples, 1, "the schema-level example is preserved")
+	require.NotNil(t, prop.Examples[0].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, *prop.Examples[0].Value)
+
+	op, ok := opByName(doc, "getItem")
+	require.True(t, ok)
+	require.NotNil(t, op.Responses[0].Payload)
+	require.Len(t, op.Responses[0].Payload.Contents, 1)
+	mediaExamples := op.Responses[0].Payload.Contents[0].Examples
+	require.Len(t, mediaExamples, 1, "the media-type example is preserved")
+	require.NotNil(t, mediaExamples[0].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "2021-01-01"}, *mediaExamples[0].Value)
+}
+
 func assertConstraints(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	m, ok := doc.Types[namedID("S")].(*ir.Model)
 	require.True(t, ok)
@@ -597,6 +637,18 @@ func assertExamples(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	require.True(t, ok)
 	require.Len(t, m.Properties, 1)
 	assert.Len(t, m.Properties[0].Examples, 2)
+
+	// The plural `examples` map, in both spellings: an inline entry and one
+	// written as a $ref, which must resolve to the referenced component's value.
+	op, ok := opByName(doc, "getItem")
+	require.True(t, ok)
+	require.Len(t, op.Responses[0].Payload.Contents, 1)
+	ex := op.Responses[0].Payload.Contents[0].Examples
+	require.Len(t, ex, 2, "both entries lower, in source order")
+	require.NotNil(t, ex[0].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "hello"}, *ex[0].Value)
+	require.NotNil(t, ex[1].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "world"}, *ex[1].Value)
 }
 
 func assertDocsSummaryDesc(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {

compilers/openapi/content.go

@@ -42,7 +42,7 @@ func (l *lowerer) lowerContent(mt string, media *soa.MediaType, pointer, hint st
 		MediaType: mt,
 		Type:      l.schemaRef(media.GetSchema(), mediaPtr+ptr("schema"), hint),
 	}
-	if ex := l.mediaExamples(media); len(ex) > 0 {
+	if ex := l.mediaExamples(media, mediaPtr); len(ex) > 0 {
 		c.Examples = ex
 	}
 	switch {
@@ -155,38 +155,49 @@ func (l *lowerer) lowerHeaders(headers *sequencedmap.Map[string, *soa.Referenced
 }
 
 // mediaExamples lowers a media type's single and plural example values.
-func (l *lowerer) mediaExamples(media *soa.MediaType) []ir.Example {
-	return l.exampleList(media.GetExample(), media.GetExamples())
+func (l *lowerer) mediaExamples(media *soa.MediaType, pointer string) []ir.Example {
+	return l.exampleList(media.GetExample(), media.GetExamples(), pointer)
 }
 
 // exampleList lowers a single example node and a plural example map into value
-// examples, in source order; unconvertible nodes are skipped.
-func (l *lowerer) exampleList(single *yaml.Node, plural *sequencedmap.Map[string, *soa.ReferencedExample]) []ir.Example {
+// examples, in source order. An unconvertible node is skipped with a warning
+// diagnostic rather than silently; an entry carrying no value node at all — an
+// externalValue, or a summary-only stub — is still skipped without one.
+func (l *lowerer) exampleList(single *yaml.Node, plural *sequencedmap.Map[string, *soa.ReferencedExample], pointer string) []ir.Example {
 	var out []ir.Example
 	if single != nil {
-		if v, err := valueFromNode(single); err == nil {
-			out = append(out, ir.Example{Value: &v})
-		}
+		out = l.appendExample(out, single, pointer, "example")
 	}
 	if plural == nil {
 		return out
 	}
-	for _, re := range plural.All() {
-		ex := resolveRef[soa.Example](re)
-		if ex == nil {
-			continue
-		}
-		node := ex.GetValue()
-		if node == nil {
-			continue
-		}
-		if v, err := valueFromNode(node); err == nil {
-			out = append(out, ir.Example{Value: &v})
-		}
+	for name, re := range plural.All() {
+		out = l.appendPluralExample(out, re, pointer, name)
 	}
 	return out
 }
 
+// appendPluralExample lowers one named entry of a plural `examples` map. An
+// entry written as a $ref holds no value of its own — the value lives in the
+// referenced component — so its diagnostic is stamped at the reference site
+// rather than at a `value` node this entry never had; an inline entry is
+// stamped at its own `value`. Only this hop is de-referenced: an enclosing
+// $ref'd response or parameter is already flattened into pointer.
+func (l *lowerer) appendPluralExample(out []ir.Example, re *soa.ReferencedExample, pointer, name string) []ir.Example {
+	ex := resolveRef[soa.Example](re)
+	if ex == nil {
+		return out
+	}
+	node := ex.GetValue()
+	if node == nil {
+		return out
+	}
+	if re.IsReference() {
+		return l.appendExample(out, node, pointer, "examples", name)
+	}
+	return l.appendExample(out, node, pointer, "examples", name, "value")
+}
+
 // lowerRequestBody lowers an operation's request body onto op.Request and the
 // binding's RequestContentTypes. The IR expresses body optionality via presence,
 // so a non-required body stays present with its optionality preserved under

compilers/openapi/content_test.go

@@ -443,6 +443,86 @@ func TestContent_ExampleWithoutValueSkipped(t *testing.T) {
 	assert.Empty(t, c.Examples, "an example without a value is skipped")
 }
 
+func TestContent_UnconvertibleExamplesDiagnosed(t *testing.T) {
+	t.Parallel()
+	// Covers both the single 3.0-style `example` and one entry of the plural
+	// 3.1-style `examples` map — each carries a custom, structurally
+	// unconvertible tag, and each conversion failure must be diagnosed rather
+	// than discarded silently.
+	spec := pathsSpec(`  /items:
+    get:
+      operationId: getItem
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema: {type: string}
+              example: !foo bar
+              examples:
+                one: {value: !foo baz}
+`)
+	_, svc, diags := lowerServiceSpec(t, spec)
+	op := firstOp(t, svc)
+	require.Len(t, op.Responses, 1)
+	c := op.Responses[0].Payload.Contents[0]
+	assert.Empty(t, c.Examples, "both unconvertible examples are skipped, not appended")
+
+	require.Equal(t, 2, countDiagsAt(diags, codeDegradedConstruct, ir.SeverityWarning))
+	pointers := map[string]bool{}
+	for _, d := range diags {
+		if d.Code == codeDegradedConstruct && d.Severity == ir.SeverityWarning {
+			pointers[d.Provenance.Pointer] = true
+			assert.Contains(t, d.Message, "example:")
+		}
+	}
+	const base = "/paths/~1items/get/responses/200/content/application~1json"
+	assert.True(t, pointers[base+"/example"], "the single example's pointer")
+	assert.True(t, pointers[base+"/examples/one/value"], "the plural example's pointer, keyed by its name")
+}
+
+func TestContent_RefdExampleDiagnosedAtReferenceSite(t *testing.T) {
+	t.Parallel()
+	// An `examples` entry written as a $ref holds no value of its own — the
+	// value lives in the referenced component — so `.../examples/<name>/value`
+	// would name a location the source never had. The diagnostic belongs at the
+	// reference site. The convertible sibling pins that a $ref'd example still
+	// lowers normally.
+	spec := `openapi: 3.1.0
+info: {title: T, version: "1.0.0"}
+paths:
+  /items:
+    get:
+      operationId: getItem
+      responses:
+        "200":
+          description: ok
+          content:
+            application/json:
+              schema: {type: string}
+              examples:
+                good: {$ref: '#/components/examples/Good'}
+                bad: {$ref: '#/components/examples/Bad'}
+components:
+  examples:
+    Good: {value: fine}
+    Bad: {value: !foo baz}
+`
+	_, svc, diags := lowerServiceSpec(t, spec)
+	op := firstOp(t, svc)
+	c := op.Responses[0].Payload.Contents[0]
+	require.Len(t, c.Examples, 1, "the convertible $ref'd example still lowers")
+	require.NotNil(t, c.Examples[0].Value)
+	assert.Equal(t, ir.Value{Kind: ir.ValueString, Str: "fine"}, *c.Examples[0].Value)
+
+	require.Equal(t, 1, countDiagsAt(diags, codeDegradedConstruct, ir.SeverityWarning))
+	d, ok := firstDegradedWarning(diags)
+	require.True(t, ok)
+	assert.Equal(t, "/paths/~1items/get/responses/200/content/application~1json/examples/bad",
+		d.Provenance.Pointer, "the reference site, not a /value the source never had")
+	assert.Contains(t, d.Message, "example:")
+}
+
 func TestContentTypeKeys_Nil(t *testing.T) {
 	t.Parallel()
 	assert.Nil(t, contentTypeKeys(nil))

compilers/openapi/diag.go

@@ -40,8 +40,12 @@ const (
 	// value form is wrong for the document's dialect (a boolean under 2020-12, or
 	// a number under 3.0) — see exclusiveFormDiag.
 	codeExclusiveBoundForm = "openapi/invalid-exclusive-bound"
-	// codeDegradedConstruct reports a construct preserved raw because the IR
-	// has no structural home for it.
+	// codeDegradedConstruct reports a construct the compiler could not carry
+	// into the IR as written: preserved raw for want of a structural home,
+	// lowered to a weaker shape (a heterogeneous enum as a union, an
+	// unconvertible value as the top type), or — for an annotation like a
+	// default or example — dropped. It marks the lossy lowerings the compiler
+	// reports, not a guarantee that every lossy lowering is reported.
 	codeDegradedConstruct = "openapi/degraded-construct"
 	// codeConflictingRedecl reports that inline allOf branches redeclare one
 	// field with values that disagree: an incompatible target type (string vs.