fix(compilers/openapi): stop silently dropping operation details

compilers/openapi/conformance_test.go

@@ -1388,6 +1388,8 @@ func assertParamStyles(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 		"reserved characters passing through unescaped is a wire fact, not a style")
 	assert.False(t, q.AllowReserved, "and the default is to escape them")
 
+	assertAllowEmptyValueKept(t, op)
+
 	// The path item's shared parameter merges into both of its operations and
 	// interns its schema once, at the path item's own pointer (issue #36).
 	clearSearch, ok := opByName(doc, "clearSearch")
@@ -1402,6 +1404,22 @@ func assertParamStyles(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 		"the shared schema is hoisted at the path item's own pointer, not a per-operation one")
 }
 
+// assertAllowEmptyValueKept covers the one serialization flag ir.HTTPParamBinding
+// has no field for. Style, explode, allowReserved and the content-style media
+// type all land on the binding above; allowEmptyValue used to be read nowhere at
+// all, so a document declaring it produced IR indistinguishable from one that
+// did not (GitHub #39). It is kept on the logical Parameter, which is the
+// carrier at this position with an Unmodeled map.
+func assertAllowEmptyValueKept(t *testing.T, op ir.Operation) {
+	t.Helper()
+	bare, ok := paramByName(op, "bare")
+	require.True(t, ok, "the allowEmptyValue parameter still lowers")
+	entry, ok := bare.Unmodeled["openapi:allowEmptyValue"]
+	require.True(t, ok, "and its declared flag is kept beside it")
+	assert.Equal(t, ir.ReasonNoIRHome, entry.Reason)
+	assert.JSONEq(t, `true`, string(entry.Value))
+}
+
 // assertParamRefInheritance pins ir-design §14 at a parameter whose schema is a
 // $ref: docs, deprecation and default come from the referent when the use site is
 // silent, and from the use site when it is not. Constraints inherit at neither
@@ -1458,7 +1476,7 @@ func assertHeaderContentSchema(t *testing.T, doc *ir.Document, diags []ir.Diagno
 	require.True(t, ok)
 	require.Len(t, op.Responses, 1)
 	headers := op.Responses[0].Headers
-	require.Len(t, headers, 3)
+	require.Len(t, headers, 4)
 
 	bySchema, ok := headerByWire(headers, "X-Report-Schema")
 	require.True(t, ok)
@@ -1485,9 +1503,36 @@ func assertHeaderContentSchema(t *testing.T, doc *ir.Document, diags []ir.Diagno
 	assert.Equal(t, namedID("ReportID"), byRef.Type.Target,
 		"a $ref under content resolves to the named component, not to an anonymous copy")
 
+	assertHeaderStyleAndExplodeKept(t, headers)
 	assertNoErrorDiags(t, diags)
 }
 
+// assertHeaderStyleAndExplodeKept covers the header keywords ir.Property has no
+// field for. A header's explode decides whether a collection value is written as
+// one repeated field or one joined value, so dropping it left the IR unable to
+// say how the header goes on the wire — and it was dropped without a word
+// (GitHub #39). Kept verbatim, since the IR can close the gap by adding fields.
+func assertHeaderStyleAndExplodeKept(t *testing.T, headers []ir.Property) {
+	t.Helper()
+	list, ok := headerByWire(headers, "X-Report-List")
+	require.True(t, ok)
+
+	for key, want := range map[string]string{
+		"openapi:style":   `"simple"`,
+		"openapi:explode": `true`,
+	} {
+		entry, found := list.Unmodeled[key]
+		require.True(t, found, "%s is kept", key)
+		assert.Equal(t, ir.ReasonNoIRHome, entry.Reason)
+		assert.JSONEq(t, want, string(entry.Value))
+	}
+
+	plain, ok := headerByWire(headers, "X-Report-Schema")
+	require.True(t, ok)
+	assert.NotContains(t, plain.Unmodeled, "openapi:explode",
+		"a header that declares neither keyword records neither")
+}
+
 // headerByWire returns the response header with the given wire name.
 func headerByWire(headers []ir.Property, wire string) (ir.Property, bool) {
 	for _, h := range headers {
@@ -1691,10 +1736,12 @@ func assertPerStatusErrors(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	require.True(t, ok)
 	require.Len(t, op.Responses, 1, "the 2xx success response")
 	faults := map[string]ir.StatusRange{}
+	byRange := map[ir.StatusRange]ir.ErrorCase{}
 	var sawDefault bool
 	for _, ec := range op.Errors {
 		require.Len(t, ec.Conditions.StatusCodes, 1)
 		rng := ec.Conditions.StatusCodes[0]
+		byRange[rng] = ec
 		if rng.From == 0 && rng.To == 0 {
 			sawDefault = true
 			assert.Empty(t, ec.Fault, "the default catch-all is unclassified")
@@ -1705,16 +1752,75 @@ func assertPerStatusErrors(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	assert.Equal(t, ir.StatusRange{From: 404, To: 404}, faults["client"])
 	assert.Equal(t, ir.StatusRange{From: 500, To: 599}, faults["server"])
 	assert.True(t, sawDefault, "the default response becomes a catch-all error case")
+
+	assertErrorMediaTypeKept(t, byRange)
+}
+
+// assertErrorMediaTypeKept covers what ir.ErrorCase cannot say. It holds one
+// TypeRef and no media type, so an error declared as application/problem+json
+// reached the IR indistinguishable from one declared as application/json — the
+// single-entry half of a gap whose multi-entry half was already kept, which is
+// why it read as a deliberate asymmetry rather than a loss (GitHub #39). Both
+// halves are now the same rule.
+//
+// The 5XX case is the control: an error response with no content at all keeps
+// nothing, so the entry marks a declaration rather than appearing on every error.
+func assertErrorMediaTypeKept(t *testing.T, byRange map[ir.StatusRange]ir.ErrorCase) {
+	t.Helper()
+	notFound, ok := byRange[ir.StatusRange{From: 404, To: 404}]
+	require.True(t, ok)
+	entry, ok := notFound.Unmodeled["openapi:content"]
+	require.True(t, ok, "a single-media error keeps the map that names its media type")
+	assert.Equal(t, ir.ReasonNoIRHome, entry.Reason)
+	assert.JSONEq(t,
+		`{"application/json":{"schema":{"$ref":"#/components/schemas/Err"}}}`, string(entry.Value))
+
+	serverErr, ok := byRange[ir.StatusRange{From: 500, To: 599}]
+	require.True(t, ok)
+	assert.NotContains(t, serverErr.Unmodeled, "openapi:content",
+		"an error response declaring no content keeps no content map")
 }
 
 func assertWebhooks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	op, ok := opByName(doc, "onNewPet")
 	require.True(t, ok)
 	require.Len(t, op.Bindings.HTTP, 1)
 	assert.True(t, op.Bindings.HTTP[0].IsWebhook, "webhook operation carries IsWebhook")
+	assertPathItemServersKept(t, op, "https://hooks.example.com")
+	assertOwnServersKeptBesideThem(t, op, "https://hooks-override.example.com")
 	assertWebhookGroupIsAHint(t, doc)
 }
 
+// assertOwnServersKeptBesideThem pins the overriding half of the servers pair in
+// the corpus, so the two-order oracle and the JSON round-trip see it. OpenAPI
+// says an operation's own servers override its path item's, so keeping only the
+// path item's recorded the superseded list and dropped the effective one.
+//
+// The two keys are asserted together because the hazard is that they collapse
+// into one: a single key holding both would leave the surviving list depending
+// on which lowering ran last, which only a two-order diff can see.
+func assertOwnServersKeptBesideThem(t *testing.T, op ir.Operation, url string) {
+	t.Helper()
+	entry, ok := op.Unmodeled["openapi:operationServers"]
+	require.True(t, ok, "the operation's own servers are kept beside its path item's")
+	assert.Equal(t, ir.ReasonNoIRHome, entry.Reason)
+	assert.JSONEq(t, `[{"url":"`+url+`"}]`, string(entry.Value))
+	assert.NotEqual(t, entry.Value, op.Unmodeled["openapi:servers"].Value,
+		"and the two keys hold different declarations, not one overwriting the other")
+}
+
+// assertPathItemServersKept pins that a path item's servers survive whichever
+// parent the path item hangs from. The preserve-plus-diagnostic path used to be
+// reached only from the `paths` walk, so the same override written on a webhook
+// or a callback disappeared with nothing said (GitHub #39).
+func assertPathItemServersKept(t *testing.T, op ir.Operation, url string) {
+	t.Helper()
+	entry, ok := op.Unmodeled["openapi:servers"]
+	require.True(t, ok, "the path item's servers are kept on the operation")
+	assert.Equal(t, ir.ReasonNoIRHome, entry.Reason)
+	assert.JSONEq(t, `[{"url":"`+url+`"}]`, string(entry.Value))
+}
+
 // assertWebhookGroupIsAHint pins which of the two things the group's name is.
 // The compiler synthesizes the group to hold webhook operations, so no document
 // declares it and Naming.Source — the spelling the source used — is the wrong
@@ -1745,8 +1851,11 @@ func assertCallbacks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {
 	require.Len(t, op.Bindings.HTTP[0].Callbacks, 1)
 	assert.Equal(t, "{$request.body#/callbackUrl}", op.Bindings.HTTP[0].Callbacks[0].Expression)
 	assert.NotEmpty(t, op.Bindings.HTTP[0].Callbacks[0].Operations)
-	_, ok = opByName(doc, "onEvent")
-	assert.True(t, ok, "the callback operation is registered alongside its parent")
+	cb, ok := opByName(doc, "onEvent")
+	require.True(t, ok, "the callback operation is registered alongside its parent")
+	assertPathItemServersKept(t, cb, "https://callbacks.example.com")
+	assert.NotContains(t, op.Unmodeled, "openapi:servers",
+		"the callback's own servers stay on the callback, not on the parent")
 }
 
 func assertDeprecation(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) {

compilers/openapi/internal/diag/diag.go

@@ -143,6 +143,35 @@ const (
 	// operation, which OpenAPI forbids. A path item mounted at two paths is the
 	// shape that reaches this without the document repeating the id in source.
 	DuplicateOperationID = "openapi/duplicate-operation-id"
+	// ReservedHeaderName reports a header declaration OpenAPI says SHALL be
+	// ignored, because the name restates something the protocol layer already
+	// owns. The specification states the rule at three positions, and this code
+	// covers all three rather than the one it was first noticed at:
+	//
+	//   - §4.8.12, a parameter with `in: header` named Accept, Content-Type or
+	//     Authorization — content negotiation, the request body's media type, the
+	//     security scheme's credential;
+	//   - §4.8.17, a Content-Type entry in a response's `headers` map, whose
+	//     media type the response's own `content` map already names;
+	//   - §4.8.15, a Content-Type entry in an encoding's `headers` map, which the
+	//     encoding's own `contentType` describes separately.
+	//
+	// The compiler keeps the declaration in every case, because dropping declared
+	// content is a loss and choosing between the two is an emitter's call, not a
+	// compiler's (invariant 2). The diagnostic is what makes the deviation from
+	// the SHALL visible, so an emitter can suppress the declaration rather than
+	// generate one that fights what it collides with (GitHub #39).
+	//
+	// Unconditional, and deliberately not behind an Options switch: invariant 6
+	// governs what is *inferred*, and nothing here is. The names are fixed by the
+	// specification, the comparison is against a declared name, and the document
+	// lowers byte-for-byte the same whether or not this fires — so there is no
+	// inference to mark Inferred and no semantics to disable. Warning rather than
+	// info because the document really did write something the spec says has no
+	// effect; error is wrong twice over, since the document is well-formed and
+	// harness.Check stops at the first error diagnostic, which would hide every
+	// later finding in the same spec.
+	ReservedHeaderName = "openapi/reserved-header-name"
 	// UnpreservableConstruct reports a construct that reached the IR in no form at
 	// all: the compiler had no field to model it and its source node could not be
 	// converted to JSON either, so Unmodeled could not hold it. It is an error

compilers/openapi/internal/diag/diag_test.go

@@ -136,7 +136,7 @@ func codes() []string {
 		diag.CompositionLowering, diag.DynamicRefExpanded, diag.ConflictingRedecl,
 		diag.DisjointVisibility,
 		diag.AliasAmplification, diag.UnattachableRequired, diag.InternalInvariant,
-		diag.DuplicateOperationID, diag.UnpreservableConstruct,
+		diag.DuplicateOperationID, diag.ReservedHeaderName, diag.UnpreservableConstruct,
 	}
 }
 

compilers/openapi/internal/operation/content.go

@@ -355,11 +355,37 @@ func lowerHeaders(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex
 		}
 		p, headerDiags := lowerHeader(c, ts, anchors, h, name, hptr, hdecl)
 		diags = append(diags, headerDiags...)
+		diags = append(diags, reservedHeaderEntryDiag(c, name, hptr)...)
 		out = append(out, p)
 	}
 	return out, diags
 }
 
+// reservedHeaderEntryDiag reports a headers-map entry OpenAPI says SHALL be
+// ignored. Both maps this lowering serves reserve Content-Type and nothing else:
+// a response's (§4.8.17), whose media type its own `content` map already names,
+// and an encoding's (§4.8.15), which the encoding's own `contentType` describes
+// separately. The comparison is case-insensitive because HTTP field names are.
+//
+// It reports at the entry's own pointer rather than the declaration's, because
+// the reserved thing is the key the header is mapped under, not the header
+// object: two keys $ref'ing one component are two declarations, and only the one
+// spelled Content-Type is reserved. That is the opposite choice from
+// preserveHeaderSerialization, which keeps keywords the header object itself
+// writes and so records them at the declaration.
+//
+// This is the headers-map half of the rule; reservedHeaderParamDiag is the
+// parameter half. The header still lowers: see diag.ReservedHeaderName for why
+// keeping it and reporting it is the choice, rather than dropping it here.
+func reservedHeaderEntryDiag(c lowering.Ctx, name, hptr string) []ir.Diagnostic {
+	if !strings.EqualFold(name, "Content-Type") {
+		return nil
+	}
+	return []ir.Diagnostic{c.DiagAt(ir.SeverityWarning, diag.ReservedHeaderName, hptr,
+		"header %q is reserved: OpenAPI says a Content-Type entry in a headers map SHALL be "+
+			"ignored, so it is lowered as declared and left for the emitter to suppress", name)}
+}
+
 // lowerHeader lowers one header entry into a Property. Its schema goes through
 // schema.FillPropertyDetail like a model property's: a header schema declares
 // docs, constraints, xml, examples and validation-only keywords the same way,
@@ -385,7 +411,36 @@ func lowerHeader(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex,
 		p.Encoding = &ir.Encoding{MediaType: mediaType}
 	}
 	diags = append(diags, schema.FillPropertyDetail(c, ts, anchors, &p, js, schemaPtr)...)
-	return p, append(diags, applyHeaderAnnotations(c, &p, h, hdecl)...)
+	diags = append(diags, applyHeaderAnnotations(c, &p, h, hdecl)...)
+	return p, append(diags, preserveHeaderSerialization(c, &p, h, hdecl)...)
+}
+
+// preserveHeaderSerialization keeps the two serialization controls a header
+// object declares. OpenAPI §4.8.21 lets a header write `style` and `explode`,
+// and explode governs how an array or object header value is written on the
+// wire — a declared wire fact rather than a hint — but ir.Property has a field
+// for neither. ir.PartEncoding does, and that is a multipart part's own config,
+// not a header's; ir.Encoding, the one thing hanging off a Property here, names
+// a value-encoding scheme rather than a parameter style. So they are kept
+// verbatim instead of dropped, with ReasonNoIRHome since the IR can close the
+// gap by adding the fields, exactly as a parameter's xml hints are kept.
+//
+// A header that declares neither records nothing: RawChildNode returns nil for
+// an absent keyword and PreserveNode keeps nothing for a nil node.
+func preserveHeaderSerialization(c lowering.Ctx, p *ir.Property, h *soa.Header, hdecl string) []ir.Diagnostic {
+	var diags []ir.Diagnostic
+	for _, keyword := range []string{"style", "explode"} {
+		at := hdecl + ids.Ptr(keyword)
+		kept, keptDiags := schema.PreserveNode(c, &p.Unmodeled, "openapi:"+keyword,
+			annotation.RawChildNode(h.GetRootNode(), keyword), ir.ReasonNoIRHome, at)
+		diags = append(diags, keptDiags...)
+		if !kept {
+			continue
+		}
+		diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at,
+			"header %s has no ir.Property home; kept verbatim under Unmodeled", keyword))
+	}
+	return diags
 }
 
 // headerSchema returns the schema a header declares, the pointer that schema sits