diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index bcf1229..cba3d9b 100644 --- a/compilers/openapi/conformance_test.go +++ b/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,6 +1752,33 @@ 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) { @@ -1712,9 +1786,41 @@ func assertWebhooks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { 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) { diff --git a/compilers/openapi/internal/diag/diag.go b/compilers/openapi/internal/diag/diag.go index 6f8f3a9..f159c9e 100644 --- a/compilers/openapi/internal/diag/diag.go +++ b/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 diff --git a/compilers/openapi/internal/diag/diag_test.go b/compilers/openapi/internal/diag/diag_test.go index 30aced4..2a3754f 100644 --- a/compilers/openapi/internal/diag/diag_test.go +++ b/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, } } diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index 3f75926..c202731 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/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 diff --git a/compilers/openapi/internal/operation/content_test.go b/compilers/openapi/internal/operation/content_test.go index fab75fb..4273d5d 100644 --- a/compilers/openapi/internal/operation/content_test.go +++ b/compilers/openapi/internal/operation/content_test.go @@ -1004,6 +1004,198 @@ func TestHeaders_DeprecationUnionsWithTheSchema(t *testing.T) { } } +// TestHeaders_SerializationKeywordsKept covers the two keywords a header object +// writes that ir.Property has no field for. explode decides whether a +// collection-valued header goes on the wire as one repeated field or one joined +// value, so dropping it left the IR unable to say how the header serializes — +// and it was dropped in silence, at both of lowerHeaders' callers. +// +// Both callers are exercised, because the loss belonged to the shared lowering +// rather than to the response position it was noticed at: a multipart part's +// per-encoding headers build the same ir.Property from the same function. +func TestHeaders_SerializationKeywordsKept(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ name, spec, at string }{ + { + name: "response header", + spec: pathsSpec(" /x:\n get:\n operationId: g\n responses:\n" + + " \"200\":\n description: ok\n headers:\n" + + " X-H:\n style: simple\n explode: true\n" + + " schema: {type: array, items: {type: string}}\n"), + at: "/paths/~1x/get/responses/200/headers/X-H", + }, + { + name: "multipart encoding header", + spec: pathsSpec(" /x:\n post:\n operationId: g\n requestBody:\n" + + " content:\n multipart/form-data:\n" + + " schema: {type: object, properties: {file: {type: string}}}\n" + + " encoding:\n file:\n headers:\n" + + " X-H:\n style: simple\n" + + " explode: true\n" + + " schema: {type: array, items: {type: string}}\n" + + " responses: {\"200\": {description: ok}}\n"), + at: "/paths/~1x/post/requestBody/content/multipart~1form-data/encoding/file/headers/X-H", + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, tc.spec) + requireNoErrorDiags(t, diags) + assertHeaderSerializationKept(t, headerAt(t, firstOp(t, svc)), diags, tc.at) + }) + } +} + +// headerAt returns the one header the fixtures above declare, whichever of the +// two positions carries it. +func headerAt(t *testing.T, op ir.Operation) ir.Property { + t.Helper() + if len(op.Responses) > 0 && len(op.Responses[0].Headers) > 0 { + return op.Responses[0].Headers[0] + } + require.NotNil(t, op.Request) + require.Len(t, op.Request.Contents, 1) + for _, pe := range op.Request.Contents[0].Encoding { + require.Len(t, pe.Headers, 1) + return pe.Headers[0] + } + t.Fatal("no header at either position") + return ir.Property{} +} + +// assertHeaderSerializationKept requires both keywords to be kept verbatim at +// their own coordinates under the header at `at`, each announced by a diagnostic. +func assertHeaderSerializationKept(t *testing.T, h ir.Property, diags []ir.Diagnostic, at string) { + t.Helper() + for key, want := range map[string]string{"style": `"simple"`, "explode": `true`} { + entry, ok := h.Unmodeled["openapi:"+key] + require.True(t, ok, "%s is kept", key) + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, want, string(entry.Value)) + assert.Equal(t, at+"/"+key, entry.Provenance.Pointer) + assertInfoDiagAt(t, diags, entry.Provenance.Pointer) + } +} + +// TestHeaders_SerializationKeywordsAbsentRecordNothing is the control for the +// test above: preservation keys off what the header declares, so a header that +// declares neither keyword records neither — rather than recording the OpenAPI +// defaults the accessors would hand back and calling them source facts. +func TestHeaders_SerializationKeywordsAbsentRecordNothing(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpec( + " /x:\n get:\n operationId: g\n responses:\n"+ + " \"200\":\n description: ok\n headers:\n"+ + " X-H: {schema: {type: string}}\n")) + requireNoErrorDiags(t, diags) + + h := firstOp(t, svc).Responses[0].Headers[0] + assert.NotContains(t, h.Unmodeled, "openapi:style") + assert.NotContains(t, h.Unmodeled, "openapi:explode") + assert.Empty(t, diags, "and nothing is announced about keywords the header never wrote") +} + +// TestHeaders_ReservedContentTypeEntryIsReported covers the headers-map half of +// the rule diag.ReservedHeaderName records. OpenAPI states "SHALL be ignored" +// for a reserved header name at three positions, not one: a header parameter +// (§4.8.12), a Content-Type entry in a response's headers map (§4.8.17), and a +// Content-Type entry in an encoding's (§4.8.15). Morphic lowers all three +// anyway, because dropping declared content is a loss and the choice belongs to +// an emitter — but doing that in silence is what leaves an emitter unable to +// tell such a header from any other, and generating one that restates the media +// type the position already owns. +// +// Both headers-map positions are exercised, because the rule belongs to the +// shared lowering rather than the response position: an encoding's per-part +// headers reach lowerHeaders from the other caller. +func TestHeaders_ReservedContentTypeEntryIsReported(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name, header, at string + reported bool + }{ + {"response Content-Type", "Content-Type", + "/paths/~1x/get/responses/200/headers/Content-Type", true}, + {"response lowercase spelling", "content-type", + "/paths/~1x/get/responses/200/headers/content-type", true}, + {"response ordinary header", "X-Trace", + "/paths/~1x/get/responses/200/headers/X-Trace", false}, + {"response Accept is not reserved here", "Accept", + "/paths/~1x/get/responses/200/headers/Accept", false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + _, svc, diags := lowerServiceSpec(t, pathsSpec( + " /x:\n get:\n operationId: g\n responses:\n"+ + " \"200\":\n description: ok\n headers:\n"+ + " "+tc.header+": {schema: {type: string}}\n")) + requireNoErrorDiags(t, diags) + + headers := firstOp(t, svc).Responses[0].Headers + require.Len(t, headers, 1, "the header lowers either way; nothing is dropped") + assert.Equal(t, tc.header, headers[0].WireName) + assert.Equal(t, tc.reported, hasDiagCodeAt(diags, diag.ReservedHeaderName, tc.at), + "reported at the map entry's own pointer") + }) + } +} + +// TestHeaders_ReservedContentTypeInEncodingIsReported is the same rule at the +// other caller of lowerHeaders: an encoding's headers map, which §4.8.15 says +// describes Content-Type separately and SHALL ignore an entry for it. +func TestHeaders_ReservedContentTypeInEncodingIsReported(t *testing.T) { + t.Parallel() + _, _, diags := lowerServiceSpec(t, pathsSpec( + " /x:\n post:\n operationId: g\n requestBody:\n"+ + " content:\n multipart/form-data:\n"+ + " schema: {type: object, properties: {file: {type: string}}}\n"+ + " encoding:\n file:\n headers:\n"+ + " Content-Type: {schema: {type: string}}\n"+ + " X-Other: {schema: {type: string}}\n"+ + " responses: {\"200\": {description: ok}}\n")) + requireNoErrorDiags(t, diags) + + base := "/paths/~1x/post/requestBody/content/multipart~1form-data/encoding/file/headers/" + assert.True(t, hasDiagCodeAt(diags, diag.ReservedHeaderName, base+"Content-Type")) + assert.False(t, hasDiagCodeAt(diags, diag.ReservedHeaderName, base+"X-Other"), + "the entry beside it is ordinary and says nothing") + assertHasCode(t, diags, diag.ReservedHeaderName, ir.SeverityWarning) +} + +// TestHeaders_ReservedNameIsTheKeyNotTheDeclaration pins which of two pointers +// the report lands on. The reserved thing is the key the header is mapped under, +// not the header object: one component $ref'd from a reserved key and an +// ordinary one is two declarations, and only the reserved key is reported — +// reporting at the shared declaration instead would collapse them into one. +func TestHeaders_ReservedNameIsTheKeyNotTheDeclaration(t *testing.T) { + t.Parallel() + _, _, diags := lowerServiceSpec(t, `openapi: 3.1.0 +info: {title: T, version: "1"} +components: + headers: + Shared: {schema: {type: string}} +paths: + /x: + get: + operationId: g + responses: + "200": + description: ok + headers: + Content-Type: {$ref: '#/components/headers/Shared'} + X-Other: {$ref: '#/components/headers/Shared'} +`) + requireNoErrorDiags(t, diags) + + base := "/paths/~1x/get/responses/200/headers/" + assert.True(t, hasDiagCodeAt(diags, diag.ReservedHeaderName, base+"Content-Type"), + "the reserved key is reported at its own use site") + assert.False(t, hasDiagCodeAt(diags, diag.ReservedHeaderName, base+"X-Other"), + "the other key sharing that declaration is not") + assert.False(t, hasDiagCodeAt(diags, diag.ReservedHeaderName, "/components/headers/Shared"), + "and nothing is reported at the declaration they share") +} + // TestSingleContentEntry_ReportsExtraMediaTypes covers the invalid document // OpenAPI forbids: a content-style header or parameter declaring more than one // media type. Only the first can lower — ir.Property and ir.Parameter each hold diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 49df478..3605ab4 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -173,6 +173,7 @@ func lowerWebhooks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde } op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) + diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) grp := groups.group("webhook", func() ir.OperationGroup { // A hint, not a source name: no document declares this group. The // compiler synthesizes it to hold webhook operations, exactly as it @@ -291,9 +292,46 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd if len(ext) > 0 { op.Unmodeled = ext } + // After the extensions assignment, which would otherwise overwrite the map. + diags = append(diags, applyOperationServers(c, &op, src, decl)...) return op, extra, append(diags, checkOperationIDUnique(c, operationIDs, op, mount)...) } +// applyOperationServers preserves an operation's own `servers` verbatim under +// Unmodeled, for the same reason applyPathServers preserves the path item's: +// §10 scopes servers by index list at service and channel, and ir.Operation has +// no such list yet, so the scoping is kept raw with an info diagnostic. +// +// It is the overriding half of the pair. OpenAPI says an Operation Object's +// servers override the Path Item Object's, so a document declaring both had the +// superseded list kept and the effective one dropped outright — an emitter +// reading the entry would route to the wrong host, and nothing said so +// (GitHub #39). +// +// The two are kept under separate keys because they are two declarations at two +// pointers, and one map key cannot hold both: writing them to a single key would +// make the surviving list depend on which lowering ran last, silently. The path +// item's keeps the plain `openapi:servers` it already shipped under, so this one +// names its own object rather than renaming what a golden already records. +// +// Unlike applyPathServers, this is called from lowerOperation rather than from +// each route: the operation is lowered in one place, so no route can be added +// later that forgets it — which is exactly how the path-item half came to be +// missing on two of its three routes. +func applyOperationServers(c lowering.Ctx, op *ir.Operation, src *soa.Operation, declPtr string) []ir.Diagnostic { + if len(src.GetServers()) == 0 { + return nil + } + kept, diags := schema.PreserveNode(c, &op.Unmodeled, "openapi:operationServers", + annotation.RawChildNode(src.GetRootNode(), "servers"), ir.ReasonNoIRHome, declPtr+ids.Ptr("servers")) + if !kept { + return diags + } + return append(diags, diag.Newf(ir.SeverityInfo, diag.DegradedConstruct, op.Provenance, + "operation servers kept under Unmodeled; an operation has no server-scope list to bind "+ + "them to, and these override any path-item servers kept beside them")) +} + // checkOperationIDUnique reports an operationId claimed by more than one // operation. OpenAPI requires it to be unique across the whole API, and the // resolver cannot see this shape: one path item declaring an operationId and @@ -343,6 +381,14 @@ func fillOperationDocs(d *ir.Docs, src *soa.Operation) { // lists (Service.Servers, Channel.Servers); ir.Operation just has no such list // yet, so the scoping is kept raw with an info diagnostic — a gap the IR can // close by adding one, hence ReasonNoIRHome rather than a boundary. +// +// Every route that lowers a path item reaches it: a path, a webhook, and a +// callback expression are the same object under three parents, and a document +// that overrides the server for one of the latter two was losing the override +// outright while the paths route reported it (GitHub #39). +// +// This is the path-item half of the pair; applyOperationServers keeps the +// operation's own list, which overrides this one, under its own key. func applyPathServers(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { if len(pi.GetServers()) == 0 { return nil @@ -473,10 +519,9 @@ func preserveErrorHeaders(c lowering.Ctx, ec *ir.ErrorCase, r *soa.Response, rpt } // fillErrorType lowers every content entry's schema into the type registry -// (nothing dropped) and points ErrorCase.Type at the first. When more than one -// media type exists, the full content map is preserved raw with an info -// diagnostic, since ErrorCase.Type holds a single model reference (ir-design -// §7.2 clarification). +// (nothing dropped) and points ErrorCase.Type at the first, then keeps the +// content map beside it, since ErrorCase.Type holds a single model reference +// (ir-design §7.2 clarification). func fillErrorType(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, ec *ir.ErrorCase, r *soa.Response, rptr string) []ir.Diagnostic { content := r.GetContent() if content == nil || content.Len() == 0 { @@ -492,18 +537,41 @@ func fillErrorType(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde first = false } } - if content.Len() > 1 { - kept, keptDiags := schema.PreserveNode(c, &ec.Unmodeled, "openapi:content", - annotation.RawChildNode(r.GetRootNode(), "content"), ir.ReasonNoIRHome, rptr+ids.Ptr("content")) - diags = append(diags, keptDiags...) - if kept { - diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, rptr, - "error response has multiple media types; full content map kept under Unmodeled")) - } + return append(diags, preserveErrorContent(c, ec, r, rptr, content.Len())...) +} + +// preserveErrorContent keeps an error response's content map verbatim under +// Unmodeled, whatever its arity. +// +// ir.ErrorCase holds a TypeRef and no media type at all, so one entry loses the +// media type it was keyed by just as surely as several lose the entries past the +// first: an error declared only as application/problem+json reached the IR +// indistinguishable from one declared as application/json. Only the multi-entry +// case used to be kept, which made the single-entry loss the quieter of two +// halves of one gap rather than a different kind of thing (GitHub #39). +// +// n is the entry count, and picks which of the two the diagnostic names, so a +// reader is told what was actually lost rather than a message covering both. +func preserveErrorContent(c lowering.Ctx, ec *ir.ErrorCase, r *soa.Response, rptr string, n int) []ir.Diagnostic { + kept, diags := schema.PreserveNode(c, &ec.Unmodeled, "openapi:content", + annotation.RawChildNode(r.GetRootNode(), "content"), ir.ReasonNoIRHome, rptr+ids.Ptr("content")) + if kept { + diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, rptr, + "%s", errorContentMessage(n))) } return diags } +// errorContentMessage names which loss the kept content map stands for: entries +// past the first when there are several, and the sole media type's own key when +// there is one. +func errorContentMessage(n int) string { + if n > 1 { + return "error response has multiple media types; full content map kept under Unmodeled" + } + return "error response media type has no ErrorCase home; content map kept under Unmodeled" +} + // lowerCallbacks lowers each callback expression's path-item operations as // Operations registered in the parent's group, and binds them to the parent via // HTTPBinding.Callbacks keyed by the runtime expression (ir-design §8.1). @@ -564,6 +632,7 @@ func lowerCallbackOps(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI } op, _, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) diags = append(diags, opDiags...) + diags = append(diags, applyPathServers(c, &op, pi, cb.decl)...) opIDs = append(opIDs, op.ID) ops = append(ops, op) } diff --git a/compilers/openapi/internal/operation/operations_internal_test.go b/compilers/openapi/internal/operation/operations_internal_test.go index 34c1f94..2004025 100644 --- a/compilers/openapi/internal/operation/operations_internal_test.go +++ b/compilers/openapi/internal/operation/operations_internal_test.go @@ -239,6 +239,18 @@ func TestApplyPathServers_WithoutRootNode(t *testing.T) { assert.Empty(t, diags) } +// TestApplyOperationServers_WithoutRootNode is the operation half of the test +// above: a declared list whose source node cannot be read keeps nothing, and +// announces nothing it did not keep. +func TestApplyOperationServers_WithoutRootNode(t *testing.T) { + t.Parallel() + l := newRawLowerer(&soa.OpenAPI{}) + op := &ir.Operation{} + diags := applyOperationServers(l.ctx, op, &soa.Operation{Servers: []*soa.Server{{URL: "https://x"}}}, "/paths/~1a/get") + assert.Nil(t, op.Unmodeled, "servers with no raw node are not preserved") + assert.Empty(t, diags) +} + func TestLowerTagDefs_NilEntrySkipped(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{Tags: []*soa.Tag{nil, {}}}) diff --git a/compilers/openapi/internal/operation/operations_test.go b/compilers/openapi/internal/operation/operations_test.go index 90da558..b621fdb 100644 --- a/compilers/openapi/internal/operation/operations_test.go +++ b/compilers/openapi/internal/operation/operations_test.go @@ -1237,9 +1237,10 @@ func TestDiag_SharedDeclarationReportsEachDefectOnce(t *testing.T) { assert.Equal(t, 1, n, "one defect, one diagnostic: %s", key) } - // Both defects still surface — de-duplication must not silence either. - assert.Equal(t, 2, countDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), - "the optional body and the homeless error headers are two distinct defects") + // Every defect still surfaces — de-duplication must not silence any of them. + assert.Equal(t, 3, countDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), + "the optional body, the homeless error headers and the homeless error media type "+ + "are three distinct defects") } // TestDiag_DistinctDefectsAtOnePointerBothSurvive is the control for the rule @@ -1430,3 +1431,243 @@ func TestOperation_DeprecatedIsCarried(t *testing.T) { assert.NotNil(t, findOp(t, doc, "getA").Deprecation, "the declared flag is carried") assert.Nil(t, findOp(t, doc, "postA").Deprecation, "and an operation that declares none has none") } + +// pathItemServersSpec declares the same `servers` override on each of the three +// path items a document can hold — a path, a webhook, and a callback expression +// — with a distinct URL apiece so a preserved list can be traced to the path +// item that wrote it. +const pathItemServersSpec = `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /p: + servers: [{url: 'https://path.example'}] + post: + operationId: postP + callbacks: + onEvent: + '{$request.body#/url}': + servers: [{url: 'https://callback.example'}] + post: + operationId: onEvent + responses: {"200": {description: ok}} + responses: {"200": {description: ok}} +webhooks: + hooked: + servers: [{url: 'https://webhook.example'}] + post: + operationId: onHook + responses: {"200": {description: ok}} +` + +// TestOperations_PathItemServersKeptOnEveryRoute pins that a path item's servers +// survive whichever of the three routes reaches the path item. +// +// A path item is one object with three parents, and only the `paths` walk called +// the preserve-plus-diagnostic path: a webhook or callback that overrode its +// delivery host lost the override with nothing said, while the identical +// declaration under `paths` was both kept and reported. Each route is asserted +// with its own URL, so a fix that preserved the wrong path item's list — or the +// enclosing one's — fails rather than passing on the shape alone. +func TestOperations_PathItemServersKeptOnEveryRoute(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathItemServersSpec) + requireNoErrorDiags(t, diags) + + // kept is where the preserved list is recorded (the servers keyword); + // reported is where the diagnostic is stamped (the operation itself). + for _, tc := range []struct{ op, url, kept, reported string }{ + {"postP", "https://path.example", "/paths/~1p/servers", "/paths/~1p/post"}, + {"onHook", "https://webhook.example", "/webhooks/hooked/servers", "/webhooks/hooked/post"}, + {"onEvent", "https://callback.example", + "/paths/~1p/post/callbacks/onEvent/{$request.body#~1url}/servers", + "/paths/~1p/post/callbacks/onEvent/{$request.body#~1url}/post"}, + } { + entry, ok := findOp(t, doc, tc.op).Unmodeled["openapi:servers"] + require.True(t, ok, "%s keeps its path item's servers", tc.op) + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, `[{"url":"`+tc.url+`"}]`, string(entry.Value), + "%s keeps the list its own path item declared", tc.op) + assert.Equal(t, tc.kept, entry.Provenance.Pointer) + assert.True(t, hasDiagCodeAt(diags, diag.DegradedConstruct, tc.reported), + "%s reports the degradation as the paths route already did", tc.op) + } +} + +// TestErrorCase_SingleMediaTypeKeepsContentMap pins the arity-independent half of +// error-content preservation. ir.ErrorCase holds a TypeRef and no media type, so +// an error declared only as application/problem+json reached the IR +// indistinguishable from one declared as application/json — while the same +// response with a second media type beside it was kept in full. One entry losing +// its key is the same loss as several losing all but the first. +func TestErrorCase_SingleMediaTypeKeepsContentMap(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathsSpec(` /x: + get: + operationId: getX + responses: + "200": {description: ok} + "404": + description: gone + content: + application/problem+json: + schema: {type: object} + "409": {description: conflict} +`)) + requireNoErrorDiags(t, diags) + errs := indexBy(findOp(t, doc, "getX").Errors, + func(ec ir.ErrorCase) int { return ec.Conditions.StatusCodes[0].From }) + + entry, ok := errs[404].Unmodeled["openapi:content"] + require.True(t, ok, "the single-entry content map is kept") + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, `{"application/problem+json":{"schema":{"type":"object"}}}`, string(entry.Value), + "the media type the map is keyed by is what would otherwise be lost") + assert.Equal(t, "/paths/~1x/get/responses/404/content", entry.Provenance.Pointer) + assert.Contains(t, + diagMessageAt(t, diags, diag.DegradedConstruct, ir.SeverityInfo, "/paths/~1x/get/responses/404"), + "media type has no ErrorCase home", + "the single-entry case names its own loss, not the multi-entry one") + + assert.NotContains(t, errs[409].Unmodeled, "openapi:content", + "an error response declaring no content keeps no content map") +} + +// operationServersSpec declares `servers` at both levels OpenAPI allows, on an +// operation of each of the three routes that lower one, with a distinct URL +// everywhere so a preserved list can be traced to the object that wrote it. The +// last two path items cover the single-level shapes. +const operationServersSpec = `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /both: + servers: [{url: 'https://pathitem.example'}] + post: + operationId: postBoth + servers: [{url: 'https://operation.example'}] + callbacks: + onEvent: + '{$request.body#/url}': + servers: [{url: 'https://cb-pathitem.example'}] + post: + operationId: onEvent + servers: [{url: 'https://cb-operation.example'}] + responses: {"200": {description: ok}} + responses: {"200": {description: ok}} + /operationOnly: + get: + operationId: getOperationOnly + servers: [{url: 'https://only.example'}] + responses: {"200": {description: ok}} + /pathItemOnly: + servers: [{url: 'https://pathonly.example'}] + get: + operationId: getPathItemOnly + responses: {"200": {description: ok}} +webhooks: + hooked: + servers: [{url: 'https://wh-pathitem.example'}] + post: + operationId: onHook + servers: [{url: 'https://wh-operation.example'}] + responses: {"200": {description: ok}} +` + +// TestOperations_OwnServersKeptBesideThePathItems pins the overriding half of +// the servers pair, on every route that lowers an operation. OpenAPI says an +// Operation Object's servers override the Path Item Object's, but only the path +// item's were read: a document declaring both kept the superseded list and +// dropped the effective one outright, so an emitter reading openapi:servers +// would route to a host the operation had replaced — and nothing reported it. +// +// All three routes are asserted because the path-item half of this same pair was +// missing on two of its three, and preserving from lowerOperation is what is +// claimed to make that unrepeatable. A single-route case cannot see a fix that +// skips the other two, which is the whole shape of GitHub #39 item 2. +// +// The two levels are asserted under separate keys because they are two +// declarations at two pointers. One key for both would make the surviving list +// depend on which lowering ran last, which no single-order test could see. +func TestOperations_OwnServersKeptBesideThePathItems(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, operationServersSpec) + requireNoErrorDiags(t, diags) + + // own/inherited are the operation's own list and its path item's; reported is + // where the operation's degradation is stamped (the operation itself). + for _, tc := range []struct{ op, own, ownAt, inherited, inheritedAt, reported string }{ + {"postBoth", + "https://operation.example", "/paths/~1both/post/servers", + "https://pathitem.example", "/paths/~1both/servers", "/paths/~1both/post"}, + {"onHook", + "https://wh-operation.example", "/webhooks/hooked/post/servers", + "https://wh-pathitem.example", "/webhooks/hooked/servers", "/webhooks/hooked/post"}, + {"onEvent", + "https://cb-operation.example", + "/paths/~1both/post/callbacks/onEvent/{$request.body#~1url}/post/servers", + "https://cb-pathitem.example", + "/paths/~1both/post/callbacks/onEvent/{$request.body#~1url}/servers", + "/paths/~1both/post/callbacks/onEvent/{$request.body#~1url}/post"}, + } { + op := findOp(t, doc, tc.op) + + own, ok := op.Unmodeled["openapi:operationServers"] + require.True(t, ok, "%s keeps its operation's own servers", tc.op) + assert.Equal(t, ir.ReasonNoIRHome, own.Reason) + assert.JSONEq(t, `[{"url":"`+tc.own+`"}]`, string(own.Value), + "%s keeps its own list, not its path item's", tc.op) + assert.Equal(t, tc.ownAt, own.Provenance.Pointer) + + inherited, ok := op.Unmodeled["openapi:servers"] + require.True(t, ok, "%s keeps its path item's list beside it, not replaced by it", tc.op) + assert.JSONEq(t, `[{"url":"`+tc.inherited+`"}]`, string(inherited.Value)) + assert.Equal(t, tc.inheritedAt, inherited.Provenance.Pointer, + "each entry keeps the coordinate of the object that declared it") + + assert.True(t, hasDiagCodeAt(diags, diag.DegradedConstruct, tc.reported), + "%s reports the operation's own list as the path item's already was", tc.op) + } +} + +// TestOperations_ServersKeysAreIndependent is the control for the test above: +// each key records only what its own object declared, so an operation declaring +// one level records that level alone rather than both. +func TestOperations_ServersKeysAreIndependent(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, operationServersSpec) + requireNoErrorDiags(t, diags) + + opOnly := findOp(t, doc, "getOperationOnly") + assert.Contains(t, opOnly.Unmodeled, "openapi:operationServers") + assert.NotContains(t, opOnly.Unmodeled, "openapi:servers", + "an operation whose path item declares none records none for it") + + pathOnly := findOp(t, doc, "getPathItemOnly") + assert.Contains(t, pathOnly.Unmodeled, "openapi:servers") + assert.NotContains(t, pathOnly.Unmodeled, "openapi:operationServers", + "and an operation declaring none of its own records none") +} + +// TestOperations_OwnServersSurviveBesideExtensions pins an ordering constraint +// inside lowerOperation that nothing else reaches. The operation's extensions +// are assigned to op.Unmodeled wholesale, so preserving the servers before that +// assignment discards them — a map replacement, not a merge. No other fixture +// declares both on one operation, so without this the constraint is a comment +// that a later edit can silently break. +func TestOperations_OwnServersSurviveBesideExtensions(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathsSpec(` /x: + get: + operationId: getX + servers: [{url: 'https://operation.example'}] + x-vendor: kept + responses: {"200": {description: ok}} +`)) + requireNoErrorDiags(t, diags) + + op := findOp(t, doc, "getX") + servers, ok := op.Unmodeled["openapi:operationServers"] + require.True(t, ok, "the servers survive the extensions assignment") + assert.JSONEq(t, `[{"url":"https://operation.example"}]`, string(servers.Value)) + assert.Contains(t, op.Unmodeled, "openapi:x-vendor", + "and the extensions survive beside them, so neither overwrote the other") +} diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index 45d1df1..e0875ee 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -1,6 +1,8 @@ package operation import ( + "strings" + oas3 "github.com/speakeasy-api/openapi/jsonschema/oas3" soa "github.com/speakeasy-api/openapi/openapi" @@ -60,9 +62,34 @@ func lowerParameter(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd AllowReserved: p.GetAllowReserved(), } diags := fillParamType(c, ts, anchors, ¶m, &binding, p, pptr, name) + diags = append(diags, reservedHeaderParamDiag(c, name, in, pptr)...) return param, binding, append(diags, fillParamDetail(c, ¶m, p, pptr)...) } +// reservedHeaderParamDiag reports a header parameter OpenAPI §4.8.12 reserves — +// one named Accept, Content-Type or Authorization, whose definition it says +// SHALL be ignored. The comparison is case-insensitive because HTTP field names +// are, so a parameter spelled "authorization" collides with the security scheme +// exactly as one spelled "Authorization" does. +// +// This is the parameter half of the rule, and reservedHeaderEntryDiag is the +// headers-map half. The parameter still lowers: see diag.ReservedHeaderName for +// why keeping it and reporting it is the choice, rather than dropping it here. +func reservedHeaderParamDiag(c lowering.Ctx, name string, in soa.ParameterIn, pptr string) []ir.Diagnostic { + if in != soa.ParameterInHeader { + return nil + } + for _, reserved := range []string{"Accept", "Content-Type", "Authorization"} { + if !strings.EqualFold(name, reserved) { + continue + } + return []ir.Diagnostic{c.DiagAt(ir.SeverityWarning, diag.ReservedHeaderName, pptr, + "header parameter %q is reserved: OpenAPI says a definition for %s SHALL be ignored, "+ + "so it is lowered as declared and left for the emitter to suppress", name, reserved)} + } + return nil +} + // fillParamType lowers a parameter's type from either its schema or, for a // content-style parameter, its single media-type entry (recording the media // type on the binding). Constraints come from that same schema position; @@ -247,7 +274,35 @@ func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr pExt, extDiags := schema.ExtensionsOf(c, p.GetExtensions(), pptr) diags = append(diags, extDiags...) param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, pExt) - return diags + return append(diags, preserveAllowEmptyValue(c, param, p, pptr)...) +} + +// preserveAllowEmptyValue keeps a parameter's allowEmptyValue flag. It says a +// query parameter may be sent with an empty value — a wire fact about how the +// parameter serializes, alongside style, explode and allowReserved, which +// ir.HTTPParamBinding does hold. It holds no field for this one, and nothing +// else on this path read the flag either, so a document declaring it lost it +// outright (GitHub #39). +// +// ReasonNoIRHome rather than a boundary, for the same reason as its neighbours: +// the IR can close the gap by adding the field. It is kept on ir.Parameter +// because that is the carrier at this position with an Unmodeled map at all — +// ir.HTTPParamBinding has none. +// +// A parameter that does not declare it records nothing: RawChildNode returns nil +// for an absent keyword and PreserveNode keeps nothing for a nil node. That is +// deliberately presence, not truth — allowEmptyValue: false is a declared fact +// too, and a compiler that kept only the true spelling would decide for the +// reader which declarations count. +func preserveAllowEmptyValue(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr string) []ir.Diagnostic { + at := pptr + ids.Ptr("allowEmptyValue") + kept, diags := schema.PreserveNode(c, ¶m.Unmodeled, "openapi:allowEmptyValue", + annotation.RawChildNode(p.GetRootNode(), "allowEmptyValue"), ir.ReasonNoIRHome, at) + if !kept { + return diags + } + return append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, at, + "parameter allowEmptyValue has no ir.HTTPParamBinding home; kept verbatim under Unmodeled")) } // resolveStyleExplode materializes a parameter's resolved serialization style diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index f9bb976..ab7feed 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -606,3 +606,88 @@ func TestParams_SchemaVisibilityKeptWhenTheSchemaOwnsANode(t *testing.T) { assertInfoDiagAt(t, diags, entry.Provenance.Pointer) } } + +// TestParams_AllowEmptyValueKept pins the last unread Parameter field. Style, +// explode, allowReserved and a content-style media type all reach +// ir.HTTPParamBinding; allowEmptyValue reached nothing, so a document declaring +// it compiled to IR identical to one that did not, and said nothing about it. +// +// The false spelling is asserted beside the true one because presence, not +// truth, is what is kept: a compiler recording only `true` would be deciding +// which declarations count. +func TestParams_AllowEmptyValueKept(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathsSpec(` /x: + get: + operationId: getX + parameters: + - {name: on, in: query, allowEmptyValue: true, schema: {type: string}} + - {name: off, in: query, allowEmptyValue: false, schema: {type: string}} + - {name: silent, in: query, schema: {type: string}} + responses: {"200": {description: ok}} +`)) + requireNoErrorDiags(t, diags) + params := indexBy(findOp(t, doc, "getX").Params, func(p ir.Parameter) string { return p.Name.Source }) + + for _, tc := range []struct{ name, want, index string }{ + {"on", `true`, "0"}, + {"off", `false`, "1"}, + } { + entry, ok := params[tc.name].Unmodeled["openapi:allowEmptyValue"] + require.True(t, ok, "%s keeps its declared flag", tc.name) + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, tc.want, string(entry.Value)) + assert.Equal(t, "/paths/~1x/get/parameters/"+tc.index+"/allowEmptyValue", + entry.Provenance.Pointer, "kept at the keyword's own coordinate") + assertInfoDiagAt(t, diags, entry.Provenance.Pointer) + } + + assert.NotContains(t, params["silent"].Unmodeled, "openapi:allowEmptyValue", + "a parameter that declares nothing records nothing") +} + +// TestParams_ReservedHeaderNamesAreReported pins a deviation the compiler takes +// knowingly. OpenAPI says a header parameter named Accept, Content-Type or +// Authorization SHALL be ignored; Morphic lowers it anyway, because dropping +// declared content is a loss and choosing between the two belongs to an emitter. +// What must not happen is doing that in silence, which is what it used to do: an +// emitter had no way to tell such a parameter from any other, and would generate +// one that fights the security scheme or the negotiated media type. +func TestParams_ReservedHeaderNamesAreReported(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + param string + in string + reported bool + }{ + {"authorization", "Authorization", "header", true}, + {"accept", "Accept", "header", true}, + {"content type", "Content-Type", "header", true}, + {"lowercase spelling", "authorization", "header", true}, + {"ordinary header", "X-Trace", "header", false}, + {"same name in query", "Accept", "query", false}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathsSpec(` /x: + get: + operationId: getX + parameters: + - {name: `+tc.param+`, in: `+tc.in+`, schema: {type: string}} + responses: {"200": {description: ok}} +`)) + requireNoErrorDiags(t, diags) + op := findOp(t, doc, "getX") + require.Len(t, op.Params, 1, "the parameter lowers either way; nothing is dropped") + assert.Equal(t, tc.param, op.Params[0].Name.Source) + + assert.Equal(t, tc.reported, + hasDiagCodeAt(diags, diag.ReservedHeaderName, "/paths/~1x/get/parameters/0"), + "reported at the parameter's own pointer") + if tc.reported { + assertHasCode(t, diags, diag.ReservedHeaderName, ir.SeverityWarning) + } + }) + } +} diff --git a/docs/ir-design.md b/docs/ir-design.md index 150e8b5..a72ceea 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1698,7 +1698,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and a multi-media error `content` map → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field); webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers` → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet); securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and its `content` map whatever its arity → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field: it holds one TypeRef and no media type, so one entry loses the key it was written under just as several lose all but the first); response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled | diff --git a/testdata/conformance/openapi/callbacks.golden.json b/testdata/conformance/openapi/callbacks.golden.json index d0b0242..c6805e1 100644 --- a/testdata/conformance/openapi/callbacks.golden.json +++ b/testdata/conformance/openapi/callbacks.golden.json @@ -109,6 +109,20 @@ } ] }, + "unmodeled": { + "openapi:servers": { + "reason": "no_ir_home", + "value": [ + { + "url": "https://callbacks.example.com" + } + ], + "provenance": { + "source": 0, + "pointer": "/paths/~1subscribe/post/callbacks/onEvent/{$request.body#~1callbackUrl}/servers" + } + } + }, "provenance": { "source": 0, "pointer": "/paths/~1subscribe/post/callbacks/onEvent/{$request.body#~1callbackUrl}/post" @@ -133,11 +147,22 @@ "auth": null } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "path-item servers kept under Unmodeled; an operation has no server-scope list to bind them to", + "provenance": { + "source": 0, + "pointer": "/paths/~1subscribe/post/callbacks/onEvent/{$request.body#~1callbackUrl}/post" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "callbacks.yaml", - "hash": "d8d2488e77b7f921dd60e133f70c926d72ab3c3c521ca11aea82511bc8731171" + "hash": "108b2bab248050864d2156a13cc0a0d3367f2ff3101bc15a6ef33b60216cca8b" } ] } diff --git a/testdata/conformance/openapi/callbacks.yaml b/testdata/conformance/openapi/callbacks.yaml index 1b6beaf..f5bfc00 100644 --- a/testdata/conformance/openapi/callbacks.yaml +++ b/testdata/conformance/openapi/callbacks.yaml @@ -10,6 +10,9 @@ paths: callbacks: onEvent: '{$request.body#/callbackUrl}': + # A callback path item carries servers like any other path item. + servers: + - url: https://callbacks.example.com post: operationId: onEvent responses: diff --git a/testdata/conformance/openapi/component-reuse.golden.json b/testdata/conformance/openapi/component-reuse.golden.json index fbae76a..f5394bd 100644 --- a/testdata/conformance/openapi/component-reuse.golden.json +++ b/testdata/conformance/openapi/component-reuse.golden.json @@ -324,6 +324,27 @@ }, "docs": { "description": "anything else" + }, + "unmodeled": { + "openapi:content": { + "reason": "no_ir_home", + "value": { + "application/json": { + "schema": { + "properties": { + "message": { + "type": "string" + } + }, + "type": "object" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/components/responses/Failure/content" + } + } } } ], @@ -684,6 +705,17 @@ "auth": null } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "error response media type has no ErrorCase home; content map kept under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/components/responses/Failure" + } + } + ], "sources": [ { "format": "openapi@3.1", diff --git a/testdata/conformance/openapi/header-content-schema.golden.json b/testdata/conformance/openapi/header-content-schema.golden.json index 99afc81..30bbf40 100644 --- a/testdata/conformance/openapi/header-content-schema.golden.json +++ b/testdata/conformance/openapi/header-content-schema.golden.json @@ -149,6 +149,51 @@ "source": 0, "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-Ref" } + }, + { + "id": "p/openapi/paths/~1reports/get/responses/200/headers/X-Report-List", + "name": { + "source": "X-Report-List", + "canonical": "x_report_list" + }, + "wireName": "X-Report-List", + "type": { + "target": "t/anon/paths/~1reports/get/responses/200/headers/X-Report-List/schema", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "unmodeled": { + "openapi:explode": { + "reason": "no_ir_home", + "value": true, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-List/explode" + } + }, + "openapi:style": { + "reason": "no_ir_home", + "value": "simple", + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-List/style" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-List" + } } ], "docs": { @@ -185,6 +230,24 @@ } ], "types": { + "t/anon/paths/~1reports/get/responses/200/headers/X-Report-List/schema": { + "kind": "list", + "id": "t/anon/paths/~1reports/get/responses/200/headers/X-Report-List/schema", + "name": { + "hint": "X-Report-List" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-List/schema" + }, + "elem": { + "target": "t/prim/string", + "nullable": false + } + }, "t/openapi/components/schemas/ReportID": { "kind": "scalar", "id": "t/openapi/components/schemas/ReportID", @@ -233,11 +296,31 @@ "auth": null } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "header style has no ir.Property home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-List/style" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "header explode has no ir.Property home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/responses/200/headers/X-Report-List/explode" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "header-content-schema.yaml", - "hash": "17ace78b333229b8c03cfdc4c1b40ccbefdab45603653c131de122cce2bd7a87" + "hash": "3ef8d9fedce7f77b8f6e14d9d58250555df19ef1e95391a4969110a12c4d99c4" } ] } diff --git a/testdata/conformance/openapi/header-content-schema.yaml b/testdata/conformance/openapi/header-content-schema.yaml index 18ef0f1..c2d1b1a 100644 --- a/testdata/conformance/openapi/header-content-schema.yaml +++ b/testdata/conformance/openapi/header-content-schema.yaml @@ -33,6 +33,16 @@ paths: content: application/json: schema: {$ref: '#/components/schemas/ReportID'} + # A header may state how its value serializes. explode is a real wire + # fact for a collection-valued header — one repeated field versus one + # comma-joined value — and ir.Property has no field for either it or + # style, so both are kept verbatim rather than dropped. + X-Report-List: + style: simple + explode: true + schema: + type: array + items: {type: string} components: schemas: ReportID: diff --git a/testdata/conformance/openapi/param-styles.golden.json b/testdata/conformance/openapi/param-styles.golden.json index c89f7be..e5c36fa 100644 --- a/testdata/conformance/openapi/param-styles.golden.json +++ b/testdata/conformance/openapi/param-styles.golden.json @@ -62,13 +62,35 @@ "required": false, "docs": {} }, + { + "name": { + "source": "bare", + "canonical": "bare" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {}, + "unmodeled": { + "openapi:allowEmptyValue": { + "reason": "no_ir_home", + "value": true, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/3/allowEmptyValue" + } + } + } + }, { "name": { "source": "payload", "canonical": "payload" }, "type": { - "target": "t/anon/paths/~1search/get/parameters/3/content/application~1json/schema", + "target": "t/anon/paths/~1search/get/parameters/4/content/application~1json/schema", "nullable": false }, "required": false, @@ -139,6 +161,14 @@ "explode": true, "allowReserved": true }, + { + "param": "bare", + "location": "query", + "wireName": "bare", + "style": "form", + "explode": true, + "allowReserved": false + }, { "param": "payload", "location": "query", @@ -262,9 +292,9 @@ "positional": false, "inputOnly": false }, - "t/anon/paths/~1search/get/parameters/3/content/application~1json/schema": { + "t/anon/paths/~1search/get/parameters/4/content/application~1json/schema": { "kind": "model", - "id": "t/anon/paths/~1search/get/parameters/3/content/application~1json/schema", + "id": "t/anon/paths/~1search/get/parameters/4/content/application~1json/schema", "name": { "hint": "payload" }, @@ -273,7 +303,7 @@ "sensitive": false, "provenance": { "source": 0, - "pointer": "/paths/~1search/get/parameters/3/content/application~1json/schema" + "pointer": "/paths/~1search/get/parameters/4/content/application~1json/schema" }, "abstract": false, "positional": false, @@ -349,11 +379,22 @@ "auth": null } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "parameter allowEmptyValue has no ir.HTTPParamBinding home; kept verbatim under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/3/allowEmptyValue" + } + } + ], "sources": [ { "format": "openapi@3.1", "path": "param-styles.yaml", - "hash": "e8ff405aa817f069496ca1c8e55355c1caf83bc4014866b4b90f96390a9dc43c" + "hash": "d755ab33399018c9636fce7ec5cf4a807be8f9eb5b14ad2406d014261c7e6d46" } ] } diff --git a/testdata/conformance/openapi/param-styles.yaml b/testdata/conformance/openapi/param-styles.yaml index ba3fa55..e190de4 100644 --- a/testdata/conformance/openapi/param-styles.yaml +++ b/testdata/conformance/openapi/param-styles.yaml @@ -16,6 +16,9 @@ paths: explode: true schema: {type: object} - {name: path, in: query, allowReserved: true, schema: {type: string}} + # allowReserved's neighbour, and the one ir.HTTPParamBinding has no field + # for: kept verbatim so the declaration survives somewhere. + - {name: bare, in: query, allowEmptyValue: true, schema: {type: string}} - name: payload in: query content: diff --git a/testdata/conformance/openapi/per-status-errors.golden.json b/testdata/conformance/openapi/per-status-errors.golden.json index 378b94f..f587672 100644 --- a/testdata/conformance/openapi/per-status-errors.golden.json +++ b/testdata/conformance/openapi/per-status-errors.golden.json @@ -60,6 +60,22 @@ "fault": "client", "docs": { "description": "not found" + }, + "unmodeled": { + "openapi:content": { + "reason": "no_ir_home", + "value": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Err" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/404/content" + } + } } }, { @@ -197,6 +213,17 @@ "auth": null } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "error response media type has no ErrorCase home; content map kept under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/get/responses/404" + } + } + ], "sources": [ { "format": "openapi@3.1", diff --git a/testdata/conformance/openapi/unwitnessed.golden.txt b/testdata/conformance/openapi/unwitnessed.golden.txt index 53a9aea..0ef3569 100644 --- a/testdata/conformance/openapi/unwitnessed.golden.txt +++ b/testdata/conformance/openapi/unwitnessed.golden.txt @@ -43,7 +43,6 @@ EnumMember.Unmodeled EnumMember.WireName ErrorCase.Retryable ErrorCase.Throttling -ErrorCase.Unmodeled ErrorExample.Content ErrorExample.Type EventInfo.ContentType diff --git a/testdata/conformance/openapi/webhooks.golden.json b/testdata/conformance/openapi/webhooks.golden.json index 59eaff8..a050faa 100644 --- a/testdata/conformance/openapi/webhooks.golden.json +++ b/testdata/conformance/openapi/webhooks.golden.json @@ -81,6 +81,32 @@ } ] }, + "unmodeled": { + "openapi:operationServers": { + "reason": "no_ir_home", + "value": [ + { + "url": "https://hooks-override.example.com" + } + ], + "provenance": { + "source": 0, + "pointer": "/webhooks/newPet/post/servers" + } + }, + "openapi:servers": { + "reason": "no_ir_home", + "value": [ + { + "url": "https://hooks.example.com" + } + ], + "provenance": { + "source": 0, + "pointer": "/webhooks/newPet/servers" + } + } + }, "provenance": { "source": 0, "pointer": "/webhooks/newPet/post" @@ -133,13 +159,31 @@ "source": 0, "pointer": "/webhooks/newPet/post/requestBody" } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "operation servers kept under Unmodeled; an operation has no server-scope list to bind them to, and these override any path-item servers kept beside them", + "provenance": { + "source": 0, + "pointer": "/webhooks/newPet/post" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "path-item servers kept under Unmodeled; an operation has no server-scope list to bind them to", + "provenance": { + "source": 0, + "pointer": "/webhooks/newPet/post" + } } ], "sources": [ { "format": "openapi@3.1", "path": "webhooks.yaml", - "hash": "32abadfcd65ac8530ddedcf4780f90471b3ce5d1f670384f3c87a5100edd1a96" + "hash": "a1a8c6abd2eddc50abf4aaa27edf7c634a26b068f2c13fdfb0a6dbe8a8d2ef83" } ] } diff --git a/testdata/conformance/openapi/webhooks.yaml b/testdata/conformance/openapi/webhooks.yaml index 75c1d4f..d90a492 100644 --- a/testdata/conformance/openapi/webhooks.yaml +++ b/testdata/conformance/openapi/webhooks.yaml @@ -3,8 +3,18 @@ info: {title: Webhooks, version: "1.0.0"} paths: {} webhooks: newPet: + # A webhook path item may override the server the delivery is addressed to, + # exactly as a path may. An operation has no server-scope list, so this is + # kept verbatim — the point being that it is kept at all. + servers: + - url: https://hooks.example.com post: operationId: onNewPet + # And the operation may override the path item's in turn — OpenAPI says + # this one wins. Both are kept, under separate keys: one key for two + # declarations would leave the survivor depending on lowering order. + servers: + - url: https://hooks-override.example.com requestBody: content: application/json: diff --git a/testdata/golden/openapi/petstore.golden.json b/testdata/golden/openapi/petstore.golden.json index f8dfdc5..3393444 100644 --- a/testdata/golden/openapi/petstore.golden.json +++ b/testdata/golden/openapi/petstore.golden.json @@ -97,6 +97,22 @@ }, "docs": { "description": "Unexpected error" + }, + "unmodeled": { + "openapi:content": { + "reason": "no_ir_home", + "value": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/get/responses/default/content" + } + } } } ], @@ -196,6 +212,22 @@ "fault": "client", "docs": { "description": "Not found" + }, + "unmodeled": { + "openapi:content": { + "reason": "no_ir_home", + "value": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post/responses/404/content" + } + } } } ], @@ -789,6 +821,26 @@ } } ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "error response media type has no ErrorCase home; content map kept under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/get/responses/default" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "error response media type has no ErrorCase home; content map kept under Unmodeled", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post/responses/404" + } + } + ], "sources": [ { "format": "openapi@3.1",