From a7735024b66d445b2a5762af3b1bdd31b554e517 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 5 Sep 2026 20:19:25 +0300 Subject: [PATCH 1/9] feat(ir)!: give Payload a Required field for body optionality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Request-body optionality survived only as an inverted sentinel: the OpenAPI compiler wrote Payload.Unmodeled["openapi:required"] = false when a body was not required and wrote nothing when it was, so recovering the fact meant knowing an OpenAPI-specific key and reading its absence as true. A consumer that reads typed fields alone saw every body as required — 563 times across GitHub's and Stripe's published specs. ir/unmodeled.go grades no_ir_home as "a gap expected to close, not a boundary", and this is that gap. ir.Payload now carries Required *bool. The pointer is the point: a format that expresses body optionality treats an unstated body as optional, so folding "the format is silent" onto the same value as "the document says no" would lose the distinction a non-OpenAPI compiler needs. Response and message payloads leave it nil, because only a request body can be omitted. The OpenAPI compiler always sets it, since OpenAPI's own default makes an undeclared `required` mean false rather than unstated, and it no longer writes the openapi:required entry or the info diagnostic that announced the degradation — the fact is modeled now, so neither describes anything. ir-design.md is normative on the field shapes, so §7.2's Payload and §14's OpenAPI lowering summary are updated with it. BREAKING CHANGE: a consumer reading Payload.Unmodeled["openapi:required"] must read Payload.Required instead; the Unmodeled entry and its openapi/degraded-construct info diagnostic are no longer emitted. The per-reason reachability test moves its no_ir_home witness to a parameter's allowEmptyValue, which still has no typed home. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- .../openapi/internal/operation/content.go | 16 ++--- .../internal/operation/content_test.go | 66 +++++++++++++++---- .../internal/operation/operations_test.go | 13 ++-- compilers/openapi/unpreservable_test.go | 14 ++-- docs/ir-design.md | 9 ++- ir/operation.go | 8 +++ ir/operation_test.go | 30 ++++++++- .../openapi/component-reuse.golden.json | 3 +- .../openapi/extensions-x.golden.json | 1 + .../conformance/openapi/file-body.golden.json | 22 +------ .../inline-hoist-positions.golden.json | 9 ++- .../openapi/multipart-encoding.golden.json | 60 +---------------- .../openapi/path-item-operations.golden.json | 22 +------ .../openapi/streaming-media-31.golden.json | 20 +----- .../conformance/openapi/webhooks.golden.json | 20 +----- testdata/golden/openapi/petstore.golden.json | 3 +- 16 files changed, 139 insertions(+), 177 deletions(-) diff --git a/compilers/openapi/internal/operation/content.go b/compilers/openapi/internal/operation/content.go index 11f32dff..70a172ee 100644 --- a/compilers/openapi/internal/operation/content.go +++ b/compilers/openapi/internal/operation/content.go @@ -723,9 +723,10 @@ func appendValuelessExample(c lowering.Ctx, out []ir.Example, proto ir.Example, } // lowerRequestBody lowers an operation's request body onto op.Request and the -// binding's RequestContentTypes. The IR expresses body optionality via presence, -// so a non-required body stays present with its optionality preserved under -// Unmodeled plus one info diagnostic (ir-design §7.2 clarification). opDeclPtr +// binding's RequestContentTypes. Body optionality lands on Payload.Required, +// always set here because OpenAPI always states it — an undeclared `required` +// means false by the specification's own default, not silence, so leaving the +// field nil would report the format as unable to express optionality. opDeclPtr // is the operation's own declaration pointer, so a $ref'd body interns its // content once at its component pointer rather than once per mount site // (issue #107) — and under the component's name, since the operationId hint @@ -739,13 +740,8 @@ func lowerRequestBody(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorI if payload == nil { return diags } - if !rb.GetRequired() { - schema.Preserve(c, &payload.Unmodeled, "openapi:required", ir.RawValue("false"), - ir.ReasonNoIRHome, bodyPtr+ids.Ptr("required")) - - diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.DegradedConstruct, bodyPtr, - "request body is not required; optionality kept under Unmodeled")) - } + required := rb.GetRequired() + payload.Required = &required // soa.RequestBody exposes no GetExtensions at this library version, so the // field is read directly — as XMLHints already reads its own. Both reads sit // after the payload guard because ir.Payload is the body's only carrier: a diff --git a/compilers/openapi/internal/operation/content_test.go b/compilers/openapi/internal/operation/content_test.go index c1206eec..19ffc157 100644 --- a/compilers/openapi/internal/operation/content_test.go +++ b/compilers/openapi/internal/operation/content_test.go @@ -293,17 +293,59 @@ func TestContent_NonRequiredRequestBody(t *testing.T) { openapitest.RequireNoErrorDiags(t, diags) op := openapitest.FirstOp(t, svc) require.NotNil(t, op.Request, "a non-required body still lowers to a present Payload") - raw, ok := op.Request.Unmodeled["openapi:required"] - require.True(t, ok, "body optionality kept under Unmodeled") - assert.Equal(t, "false", string(raw.Value)) - assert.Equal(t, ir.ReasonNoIRHome, raw.Reason) - found := false + require.NotNil(t, op.Request.Required, "OpenAPI always states body optionality") + assert.False(t, *op.Request.Required) + assert.NotContains(t, op.Request.Unmodeled, "openapi:required", + "the typed field carries the fact, so no sentinel is written beside it") for _, d := range diags { - if d.Severity == ir.SeverityInfo && strings.Contains(d.Message, "request body") { - found = true - } + assert.NotContains(t, d.Message, "request body", + "a typed fact is not a degraded construct") } - assert.True(t, found, "non-required body emits one info diagnostic") +} + +// TestContent_RequiredRequestBody is TestContent_NonRequiredRequestBody's other +// arm: `required: true` must reach the same field rather than being encoded as +// the sentinel's absence, which is what made a consumer read every body alike. +func TestContent_RequiredRequestBody(t *testing.T) { + t.Parallel() + spec := openapitest.PathsSpec(` /must: + post: + operationId: must + requestBody: + required: true + content: + application/json: {schema: {type: object, properties: {n: {type: string}}}} + responses: {"200": {description: ok}} +`) + _, svc, diags := lowerServiceSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + op := openapitest.FirstOp(t, svc) + require.NotNil(t, op.Request) + require.NotNil(t, op.Request.Required) + assert.True(t, *op.Request.Required) +} + +// TestContent_ResponsePayloadStatesNoOptionality pins the third state: only a +// request body can be omitted, so a response Payload leaves Required nil and a +// consumer reading it as "false" would be inventing a fact. +func TestContent_ResponsePayloadStatesNoOptionality(t *testing.T) { + t.Parallel() + spec := openapitest.PathsSpec(` /get: + get: + operationId: getThing + responses: + "200": + description: ok + content: + application/json: {schema: {type: object, properties: {n: {type: string}}}} +`) + _, svc, diags := lowerServiceSpec(t, spec) + openapitest.RequireNoErrorDiags(t, diags) + op := openapitest.FirstOp(t, svc) + require.Len(t, op.Responses, 1) + require.NotNil(t, op.Responses[0].Payload) + assert.Nil(t, op.Responses[0].Payload.Required, + "a response body has no optionality to state") } func TestContent_ArrayMultipartPartMulti(t *testing.T) { @@ -405,10 +447,10 @@ func TestContent_FullPipeline(t *testing.T) { doc, diags := parseFull(t, contentSpec) upload := openapitest.FindOp(t, doc, "upload") - // Non-required body preserved as present with optionality under Unmodeled. + // Non-required body preserved as present, optionality on the typed field. require.NotNil(t, upload.Request) - _, hasReq := upload.Request.Unmodeled["openapi:required"] - assert.True(t, hasReq, "non-required optionality preserved") + require.NotNil(t, upload.Request.Required, "optionality preserved") + assert.False(t, *upload.Request.Required) // Multipart encoding: comma-split content types, header, style/explode, file flag. hb := upload.Bindings.HTTP[0] diff --git a/compilers/openapi/internal/operation/operations_test.go b/compilers/openapi/internal/operation/operations_test.go index 6fbe1343..04211af1 100644 --- a/compilers/openapi/internal/operation/operations_test.go +++ b/compilers/openapi/internal/operation/operations_test.go @@ -1222,7 +1222,7 @@ func TestResponses_RefdErrorAndDefaultInternAtDeclaration(t *testing.T) { } } -const sharedOptionalBodySpec = `openapi: 3.1.0 +const sharedDefectiveBodySpec = `openapi: 3.1.0 info: {title: T, version: "1"} paths: /a: @@ -1243,7 +1243,7 @@ components: required: false content: application/json: - schema: {type: object, properties: {n: {type: string}}} + schema: {type: string, required: [n]} responses: Err: description: err @@ -1256,13 +1256,14 @@ components: // TestDiag_SharedDeclarationReportsEachDefectOnce pins the consequence of // lowering a referenced component at its declaration: both operations reach the -// same optional body and the same header-bearing error response, so each defect +// same request body — whose scalar schema carries a `required` the lowered node +// has no field for — and the same header-bearing error response, so each defect // now has one pointer and one message. Reported per use site they would arrive // as byte-identical copies — nothing a reader could act on twice — and a // component shared by twenty operations would repeat each line twenty times. func TestDiag_SharedDeclarationReportsEachDefectOnce(t *testing.T) { t.Parallel() - _, diags := parseFull(t, sharedOptionalBodySpec) + _, diags := parseFull(t, sharedDefectiveBodySpec) seen := map[string]int{} for _, d := range diags { @@ -1274,8 +1275,8 @@ func TestDiag_SharedDeclarationReportsEachDefectOnce(t *testing.T) { // Every defect still surfaces — de-duplication must not silence any of them. assert.Equal(t, 3, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), - "the optional body, the homeless error headers and the homeless error media type "+ - "are three distinct defects") + "the body schema's homeless required, the homeless error headers and the homeless "+ + "error media type are three distinct defects") } // TestDiag_DistinctDefectsAtOnePointerBothSurvive is the control for the rule diff --git a/compilers/openapi/unpreservable_test.go b/compilers/openapi/unpreservable_test.go index 8bd341d7..778f3d10 100644 --- a/compilers/openapi/unpreservable_test.go +++ b/compilers/openapi/unpreservable_test.go @@ -263,6 +263,8 @@ paths: /p: post: operationId: p + parameters: + - {name: q, in: query, allowEmptyValue: true, schema: {type: string}} requestBody: content: {application/json: {schema: {type: string}}} responses: {"204": {description: ok}} @@ -280,11 +282,13 @@ paths: seen[entry.Reason] = true } } - // The one no_ir_home site reachable from a minimal document: a requestBody - // that omits `required`, which the IR has no field for (§14). - body := openapitest.FirstOp(t, svc).Request - require.NotNil(t, body, "the operation must own a request payload") - for _, entry := range body.Unmodeled { + // The no_ir_home witness is the query parameter's allowEmptyValue, which + // ir.HTTPParamBinding holds no field for (§14). It replaced the requestBody + // one when Payload.Required landed, so this reason now rides on a lowering + // that still has no typed home rather than on one that just grew one. + params := openapitest.FirstOp(t, svc).Params + require.Len(t, params, 1, "the operation must own the allowEmptyValue parameter") + for _, entry := range params[0].Unmodeled { seen[entry.Reason] = true } diff --git a/docs/ir-design.md b/docs/ir-design.md index 97c8193d..4d6a4bfd 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1201,6 +1201,13 @@ type Parameter struct { type Payload struct { Contents []Content // one per media type / message schema — all kept + Required *bool // true = the body must be sent, false = it may be omitted; + // nil = the source format does not express body optionality. + // Three states, not two: a format that expresses it treats an + // unstated body as optional, so folding that onto nil would make + // "the format is silent" read as "the document says no". + // Response and message payloads leave it nil — only a request + // body can be omitted Unmodeled Unmodeled } @@ -1894,7 +1901,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; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — 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; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; 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 and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; 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; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); 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 property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; 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), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$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; `oneOf` beside `anyOf`, where oneOf wins; and a parameter's or header's `schema` beside its `content`, where content wins, since a media-type entry names both a schema and the media type serializing it and the IR models both — 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, `requestBody.required` → Payload.Required, always set since OpenAPI's own default makes an undeclared `required` mean false rather than unstated; 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; an Encoding Object's `allowReserved` → Content.Unmodeled (`no_ir_home`) under `openapi:encoding//allowReserved` or `openapi:itemEncoding/allowReserved`, ir.PartEncoding holding `style` and `explode` beside it but no field for this one, and read before the entry's emptiness is judged so an entry declaring nothing else is not dropped with the empty PartEncoding it lowers to; 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 and ErrorCase.Unmodeled alike (`no_ir_home`, promotable later): the two are lowerings of one Response Object, so a construct kept on only the success one makes a declaration survive or vanish on nothing but its status code; 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; a path item's own `summary`/`description`, at the same three mounts → Operation.Unmodeled under `openapi:pathItemSummary`/`openapi:pathItemDescription` (`no_ir_home`) rather than merged into Docs: ir.Docs holds the operation's own pair and a path item's documents the path, so merging would need a precedence rule and would attach documentation the operation's author never wrote — an inference, which §6 places in policy rather than in a lowering; every operation a path item declares — the fixed method fields, 3.2 `query`, and 3.2 `additionalOperations` keyed by method — → an Operation apiece, mounted at its own pointer, with the `additionalOperations` key used verbatim as HTTPBinding.Method since OpenAPI reads a method name case-sensitively, and a key naming no method at all lowered as declared + `invalid-method-key` warning (the binding is unusable, but dropping the entry would lose every operation it declares); 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 property redeclared across branches with a type the merge drops → the discarded `TypeRef` verbatim Unmodeled (`degraded_lowering`) under `openapi:conflicting-redeclaration` per §4.8, keyed by the losing declaration's pointer, with the `openapi/conflicting-redeclaration` diagnostic only where the two are unsatisfiable, and nullability intersecting rather than dropping where the targets agree; 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), read at every object that admits one: an object lowering to a node with a map of its own keeps them unscoped there, and one lowering to no node of its own is keyed by the path from its carrier down to it — on the document, `openapi:info/x-*`, `openapi:info/contact/x-*`, `openapi:info/license/x-*`, `openapi:externalDocs/x-*`, `openapi:components/x-*`, `openapi:tags//x-*`, `openapi:tags//externalDocs/x-*`; on the service, `openapi:paths/x-*`; on each operation the path item's `openapi:pathItem/x-*` plus `openapi:responses/x-*` and `openapi:externalDocs/x-*`; on the HTTP binding, `openapi:callbacks//x-*`; on the content, `openapi:encoding//x-*` and `openapi:itemEncoding/x-*`, `` and `` alike escaped RFC 6901 style so a document-chosen name stays one segment (§12); on the schema's type, `openapi:xml/x-*`, `openapi:discriminator/x-*`, `openapi:externalDocs/x-*`; on the scheme, `openapi:flows/x-*` — since several such objects reach one map and an unscoped key would leave the survivor to lowering order (§12); a Link Object's own ride inside the verbatim `links` entry rather than taking a key beside it; `$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/ir/operation.go b/ir/operation.go index 571c32d4..5bce24fe 100644 --- a/ir/operation.go +++ b/ir/operation.go @@ -100,6 +100,14 @@ type Parameter struct { type Payload struct { // Contents holds one entry per media type / message schema — all kept. Contents []Content `json:"contents,omitempty"` + // Required states whether the message may be omitted: true = the body must + // be sent, false = it is optional. nil = the source format does not express + // body optionality at all, which is why this is a pointer — for a format + // that does, an unstated body is optional, and collapsing that onto nil + // would make "the format is silent" indistinguishable from "the document + // says no". A response or message payload leaves it nil: only a request + // body can be omitted. + Required *bool `json:"required,omitempty"` // Unmodeled holds source constructs the IR does not model, kept verbatim. Unmodeled Unmodeled `json:"unmodeled,omitempty"` } diff --git a/ir/operation_test.go b/ir/operation_test.go index a42efeb8..7d7ff698 100644 --- a/ir/operation_test.go +++ b/ir/operation_test.go @@ -155,20 +155,48 @@ func TestParameter_JSONContract(t *testing.T) { }) } -// TestPayload_JSONContract pins Payload's omitempty contract (both fields are +// TestPayload_JSONContract pins Payload's omitempty contract (every field is // optional) and that a Payload with multiple media-type contents round-trips, // all kept per the "no primary-response selection" invariant. func TestPayload_JSONContract(t *testing.T) { t.Parallel() + required := true assertJSONContract(t, ir.Payload{}, `{}`, ir.Payload{ Contents: []ir.Content{ {MediaType: "application/json", Type: populatedTypeRef()}, {MediaType: "application/xml", Type: populatedTypeRef()}, }, + Required: &required, Unmodeled: populatedUnmodeled(), }) } +// TestPayload_RequiredIsTriState pins the reason Required is a pointer: the +// three states must survive the wire as three, so a consumer never has to read +// a missing key as a value. omitempty on a *bool drops only nil, so an optional +// body still says so out loud instead of looking like a format that cannot +// express optionality at all. +func TestPayload_RequiredIsTriState(t *testing.T) { + t.Parallel() + yes, no := true, false + for _, tc := range []struct { + name string + in *bool + want string + }{ + {"unstated", nil, `{}`}, + {"optional", &no, `{"required":false}`}, + {"mandatory", &yes, `{"required":true}`}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + payload := ir.Payload{Required: tc.in} + assertZeroValueShape(t, payload, tc.want) + assertRoundTrip(t, payload) + }) + } +} + // TestContent_JSONContract pins Content's omitempty contract — Type carries // no omitempty, every other field is optional — and that a fully populated // Content — item schema for sequential streaming, per-part encodings, and diff --git a/testdata/conformance/openapi/component-reuse.golden.json b/testdata/conformance/openapi/component-reuse.golden.json index 504132ab..a6dbf8e5 100644 --- a/testdata/conformance/openapi/component-reuse.golden.json +++ b/testdata/conformance/openapi/component-reuse.golden.json @@ -248,7 +248,8 @@ "nullable": false } } - ] + ], + "required": true }, "responses": [ { diff --git a/testdata/conformance/openapi/extensions-x.golden.json b/testdata/conformance/openapi/extensions-x.golden.json index 14ed9713..d0cd0a75 100644 --- a/testdata/conformance/openapi/extensions-x.golden.json +++ b/testdata/conformance/openapi/extensions-x.golden.json @@ -429,6 +429,7 @@ } } ], + "required": true, "unmodeled": { "openapi:x-mark": { "reason": "vendor_extension", diff --git a/testdata/conformance/openapi/file-body.golden.json b/testdata/conformance/openapi/file-body.golden.json index 5445d7b8..61c17561 100644 --- a/testdata/conformance/openapi/file-body.golden.json +++ b/testdata/conformance/openapi/file-body.golden.json @@ -41,16 +41,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/paths/~1blob/put/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -154,17 +145,6 @@ "auth": null } ], - "diagnostics": [ - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/paths/~1blob/put/requestBody" - } - } - ], "sources": [ { "format": "openapi@3.1", diff --git a/testdata/conformance/openapi/inline-hoist-positions.golden.json b/testdata/conformance/openapi/inline-hoist-positions.golden.json index 685d2e6e..732190be 100644 --- a/testdata/conformance/openapi/inline-hoist-positions.golden.json +++ b/testdata/conformance/openapi/inline-hoist-positions.golden.json @@ -48,7 +48,8 @@ "nullable": false } } - ] + ], + "required": true }, "responses": [ { @@ -164,7 +165,8 @@ "nullable": false } } - ] + ], + "required": true }, "responses": [ { @@ -230,7 +232,8 @@ "nullable": false } } - ] + ], + "required": true }, "responses": [ { diff --git a/testdata/conformance/openapi/multipart-encoding.golden.json b/testdata/conformance/openapi/multipart-encoding.golden.json index 11ee8d9a..7f4641e0 100644 --- a/testdata/conformance/openapi/multipart-encoding.golden.json +++ b/testdata/conformance/openapi/multipart-encoding.golden.json @@ -108,16 +108,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/paths/~1upload/post/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -196,16 +187,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/paths/~1upload-composed/post/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -282,16 +264,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/paths/~1submit/post/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -713,24 +686,6 @@ } ], "diagnostics": [ - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/paths/~1upload/post/requestBody" - } - }, - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/paths/~1upload-composed/post/requestBody" - } - }, { "severity": "info", "code": "openapi/degraded-construct", @@ -739,15 +694,6 @@ "source": 0, "pointer": "/paths/~1submit/post/requestBody/content/application~1x-www-form-urlencoded/encoding/ids/allowReserved" } - }, - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/paths/~1submit/post/requestBody" - } } ], "sources": [ diff --git a/testdata/conformance/openapi/path-item-operations.golden.json b/testdata/conformance/openapi/path-item-operations.golden.json index 6cdfeaa1..225c171e 100644 --- a/testdata/conformance/openapi/path-item-operations.golden.json +++ b/testdata/conformance/openapi/path-item-operations.golden.json @@ -131,16 +131,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/paths/~1index/query/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -401,17 +392,6 @@ "auth": null } ], - "diagnostics": [ - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/paths/~1index/query/requestBody" - } - } - ], "sources": [ { "format": "openapi@3.2", diff --git a/testdata/conformance/openapi/streaming-media-31.golden.json b/testdata/conformance/openapi/streaming-media-31.golden.json index ef217fd2..3e71332a 100644 --- a/testdata/conformance/openapi/streaming-media-31.golden.json +++ b/testdata/conformance/openapi/streaming-media-31.golden.json @@ -35,16 +35,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/paths/~1ingest/post/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -528,15 +519,6 @@ } ], "diagnostics": [ - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/paths/~1ingest/post/requestBody" - } - }, { "severity": "info", "code": "openapi/degraded-construct", diff --git a/testdata/conformance/openapi/webhooks.golden.json b/testdata/conformance/openapi/webhooks.golden.json index 5377aa9c..30a109d7 100644 --- a/testdata/conformance/openapi/webhooks.golden.json +++ b/testdata/conformance/openapi/webhooks.golden.json @@ -35,16 +35,7 @@ } } ], - "unmodeled": { - "openapi:required": { - "reason": "no_ir_home", - "value": false, - "provenance": { - "source": 0, - "pointer": "/webhooks/newPet/post/requestBody/required" - } - } - } + "required": false }, "responses": [ { @@ -151,15 +142,6 @@ } ], "diagnostics": [ - { - "severity": "info", - "code": "openapi/degraded-construct", - "message": "request body is not required; optionality kept under Unmodeled", - "provenance": { - "source": 0, - "pointer": "/webhooks/newPet/post/requestBody" - } - }, { "severity": "info", "code": "openapi/degraded-construct", diff --git a/testdata/golden/openapi/petstore.golden.json b/testdata/golden/openapi/petstore.golden.json index 3393444f..d88aba09 100644 --- a/testdata/golden/openapi/petstore.golden.json +++ b/testdata/golden/openapi/petstore.golden.json @@ -164,7 +164,8 @@ "nullable": false } } - ] + ], + "required": true }, "responses": [ { From 0abefbb7784a8f48a9df08f5dc0461c547a6f75e Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 5 Sep 2026 20:36:34 +0300 Subject: [PATCH 2/9] feat(ir)!: give Parameter a Provenance and promote its x-sunset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ir.Parameter was the last lowered node carrying no Provenance, and two things followed from that. A parameter's vendor extensions were stranded. ir-design §12 rule 4 says a node with no provenance is not promoted into, because a promotion that cannot be marked Inferred cannot be audited — so the parameter position was the one ir.Deprecation carrier PromoteDeprecation was not wired at, and a deprecated parameter's x-sunset sat unread beside an empty Deprecation. It is wired now, and extension-promotion.yaml gains the parameter row so the sweep fails at that carrier rather than being covered by a neighbour. Parameter origin was erased. mergeParameters merges a path item's parameters into every operation on the path, and nothing afterwards recorded that a given parameter was inherited rather than declared. The stamp uses the pointer internal/operation already threads per parameter for the interning fix (#36, #107): an operation's own entry points under that operation, a $ref'd one at the component it names, and a path-item one at the path item — one declaration named by every operation that inherits it, which is what tells the two apart. BREAKING CHANGE: ir.Parameter gains a Provenance field, serialized without omitempty like every other node's. Every golden carrying a parameter moves, and a consumer decoding the IR sees a new object on each one. Closes #423 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- .../openapi/internal/operation/params.go | 17 +- .../openapi/internal/operation/params_test.go | 53 +++++ compilers/openapi/promotion_test.go | 9 + docs/ir-design.md | 13 +- ir/operation.go | 6 + ir/operation_test.go | 13 +- .../codeclared-schema-content.golden.json | 8 + .../openapi/component-reuse.golden.json | 12 +- .../openapi/deprecation.golden.json | 6 +- .../conformance/openapi/examples.golden.json | 6 +- .../openapi/extension-promotion.golden.json | 53 ++++- .../openapi/extension-promotion.yaml | 7 + .../openapi/extensions-x.golden.json | 8 + .../openapi/http-binding.golden.json | 6 +- .../openapi/inline-annotations.golden.json | 4 + .../inline-hoist-positions.golden.json | 6 +- .../openapi/neutral-naming.golden.json | 12 +- .../openapi/nullable-enum-31.golden.json | 6 +- .../openapi/param-querystring.golden.json | 18 +- .../openapi/param-ref-inheritance.golden.json | 12 +- .../openapi/param-style-matrix.golden.json | 192 +++++++++++++++--- .../openapi/param-styles.golden.json | 40 +++- .../openapi/param-xml-residue.golden.json | 8 + .../openapi/response-links.golden.json | 6 +- testdata/golden/openapi/petstore.golden.json | 12 +- 25 files changed, 460 insertions(+), 73 deletions(-) diff --git a/compilers/openapi/internal/operation/params.go b/compilers/openapi/internal/operation/params.go index 71098d1f..5c60d92f 100644 --- a/compilers/openapi/internal/operation/params.go +++ b/compilers/openapi/internal/operation/params.go @@ -49,8 +49,9 @@ func lowerParameters(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIn func lowerParameter(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, p *soa.Parameter, pptr string) (ir.Parameter, ir.HTTPParamBinding, []ir.Diagnostic) { name, in := p.GetName(), p.GetIn() param := ir.Parameter{ - Name: compile.NamingFor(name), - Required: p.GetRequired() || in == soa.ParameterInPath, + Name: compile.NamingFor(name), + Required: p.GetRequired() || in == soa.ParameterInPath, + Provenance: c.ProvenanceAt(pptr), } style, explode := resolveStyleExplode(p, in) binding := ir.HTTPParamBinding{ @@ -263,11 +264,10 @@ func paramHoldsResidue(keyword string) bool { // schema-derived annotations fillParamSchema already recorded rather than // erasing them with an unset value. // -// It is the one carrier of an ir.Deprecation that does not promote a vendor -// extension into it: ir.Parameter has no Provenance, so there is nowhere to -// record that the field was read by a heuristic, and ir-design §12's promotion -// rules require that before the reading. Giving Parameter a provenance is a -// change to that document, not to this file (GitHub #252). +// The extension promotion runs last, after the parameter's own extensions have +// been preserved: PromoteDeprecation reads the kept Unmodeled entries rather +// than the source node, so a parameter whose x-* keys are not in the map yet +// has nothing to promote from (GitHub #423). func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr string) []ir.Diagnostic { if d := p.GetDescription(); d != "" { param.Docs.Description = d @@ -283,7 +283,8 @@ func fillParamDetail(c lowering.Ctx, param *ir.Parameter, p *soa.Parameter, pptr diags = append(diags, extDiags...) param.Unmodeled = annotation.MergeUnmodeled(param.Unmodeled, pExt) diags = append(diags, annotation.UnknownKeysIn(¶m.Unmodeled, p, c.SrcIndex, pptr)...) - return append(diags, preserveAllowEmptyValue(c, param, p, pptr)...) + diags = append(diags, preserveAllowEmptyValue(c, param, p, pptr)...) + return append(diags, c.PromoteDeprecation(param.Unmodeled, param.Deprecation, ¶m.Provenance)...) } // preserveAllowEmptyValue keeps a parameter's allowEmptyValue flag. It says a diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index 81e41ca1..f4e1c38f 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -332,6 +332,59 @@ func TestParams_ComponentRefSharedAcrossOperationsInternsOnce(t *testing.T) { assert.False(t, fabricatedB, "no fabricated per-operation ID for /b") } +const paramProvenanceSpec = `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /pets/{petId}: + parameters: + - {name: petId, in: path, required: true, schema: {type: string}} + get: + operationId: getPet + parameters: + - {name: fields, in: query, schema: {type: string}} + - {$ref: '#/components/parameters/Page'} + responses: {"200": {description: ok}} + delete: + operationId: deletePet + responses: {"200": {description: ok}} +components: + parameters: + Page: {name: page, in: query, schema: {type: integer}} +` + +// TestParams_ProvenanceIsTheDeclaringPosition pins where a parameter says it +// came from (GitHub #423). The three positions a parameter can be written at +// each answer differently, and the merge is why: an operation's own entry sits +// under that operation, a $ref'd one under the component it names, and a +// path-item one under the path item — the last shared by every operation on the +// path, which is what tells an inherited parameter from a declared one. +func TestParams_ProvenanceIsTheDeclaringPosition(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, paramProvenanceSpec) + openapitest.RequireNoErrorDiags(t, diags) + getPet := openapitest.FindOp(t, doc, "getPet") + deletePet := openapitest.FindOp(t, doc, "deletePet") + byName := openapitest.IndexBy(getPet.Params, func(p ir.Parameter) string { return p.Name.Source }) + require.Len(t, byName, 3, "two declared plus the inherited path-item one") + + assert.Equal(t, "/paths/~1pets~1{petId}/get/parameters/0", byName["fields"].Provenance.Pointer, + "an operation's own entry is declared under that operation") + assert.Equal(t, "/components/parameters/Page", byName["page"].Provenance.Pointer, + "a $ref'd entry is declared at the component it names, not at the use site") + + const pathItem = "/paths/~1pets~1{petId}/parameters/0" + assert.Equal(t, pathItem, byName["petId"].Provenance.Pointer, + "an inherited entry keeps the path item's pointer rather than the operation it merged into") + require.Len(t, deletePet.Params, 1) + assert.Equal(t, pathItem, deletePet.Params[0].Provenance.Pointer, + "and both operations on the path name the one declaration, not one pointer each") + + for name, p := range byName { + assert.Equal(t, 0, p.Provenance.Source, "%s addresses the compiled source", name) + assert.Empty(t, p.Provenance.Inferred, "%s is declared, not inferred", name) + } +} + const componentContentParamRefSpec = `openapi: 3.1.0 info: {title: T, version: "1"} paths: diff --git a/compilers/openapi/promotion_test.go b/compilers/openapi/promotion_test.go index 75052743..32f135b0 100644 --- a/compilers/openapi/promotion_test.go +++ b/compilers/openapi/promotion_test.go @@ -36,6 +36,8 @@ func promotionCarriers(t *testing.T, doc *ir.Document) map[string]promotionCarri require.Len(t, op.Responses, 1) require.Len(t, op.Responses[0].Headers, 1) header := op.Responses[0].Headers[0] + require.Len(t, op.Params, 1) + param := op.Params[0] model, ok := doc.Types[namedID("Old")].(*ir.Model) require.True(t, ok) @@ -47,6 +49,7 @@ func promotionCarriers(t *testing.T, doc *ir.Document) map[string]promotionCarri return map[string]promotionCarrier{ "operation": {op.Deprecation, op.Provenance, op.Unmodeled}, + "parameter": {param.Deprecation, param.Provenance, param.Unmodeled}, "header": {header.Deprecation, header.Provenance, header.Unmodeled}, "type": {model.Deprecation, model.Provenance, model.Unmodeled}, "property": {prop.Deprecation, prop.Provenance, prop.Unmodeled}, @@ -61,6 +64,7 @@ func promotionCarriers(t *testing.T, doc *ir.Document) map[string]promotionCarri func assertExtensionPromotion(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { want := map[string]string{ "operation": "use getY instead", + "parameter": "use filter instead", "header": "header goes away", "type": "replaced by New", "property": "field goes away", @@ -82,6 +86,11 @@ func assertExtensionPromotion(t *testing.T, doc *ir.Document, diags []ir.Diagnos assert.Equal(t, "1.2.0", op.Deprecation.Since) assert.Equal(t, "2.0.0", op.Deprecation.RemovalVersion) + require.Len(t, op.Params, 1) + require.NotNil(t, op.Params[0].Deprecation) + assert.Equal(t, "3.0.0", op.Params[0].Deprecation.RemovalVersion, + "the parameter's own x-sunset reaches its own removal version, not the operation's") + assertPromotionDeclined(t, doc, diags) } diff --git a/docs/ir-design.md b/docs/ir-design.md index 4d6a4bfd..2cd2632f 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1196,6 +1196,10 @@ type Parameter struct { Availability *Availability Examples []Example Unmodeled Unmodeled + Provenance Provenance // the parameter's own declaration; a parameter merged into several + // operations (an OpenAPI path-item parameter) points at that one + // declaration, not at the operation it was merged into, which is + // what tells an inherited parameter from a declared one // NOTE: no location here — path/query/header is HTTP-binding detail (§8.1) } @@ -1824,10 +1828,11 @@ this from `Unmodeled` and no two derive it differently: 3. **The node records that it was inferred**, in its own `Provenance.Inferred`, naming the heuristic. `Inferred` holds one string and a node can be reached by more than one heuristic, so the names are listed rather than overwritten, and a name already listed is not repeated. -4. **A node with no `Provenance` is not promoted into.** `Parameter` is today's instance: it - carries a `Deprecation` and no provenance, so a promotion there could not satisfy rule 3, and a - heuristic that cannot be audited is worse than an empty field. Giving such a node a provenance - is a change to this document, and the promotion follows it rather than preceding it. +4. **A node with no `Provenance` is not promoted into.** A node carrying a `Deprecation` and no + provenance could not satisfy rule 3, and a heuristic that cannot be audited is worse than an + empty field. Giving such a node a provenance is a change to this document, and the promotion + follows it rather than preceding it — which is the order `Parameter` went through: it was the + instance this rule named until it gained the `Provenance` §7.2 now gives it. A value the mapped field cannot hold — anything but text, for the three `Deprecation` members — is reported and not coerced, since the document means something else by the key. diff --git a/ir/operation.go b/ir/operation.go index 5bce24fe..efe119cc 100644 --- a/ir/operation.go +++ b/ir/operation.go @@ -93,6 +93,12 @@ type Parameter struct { Examples []Example `json:"examples,omitempty"` // Unmodeled holds source constructs the IR does not model, kept verbatim. Unmodeled Unmodeled `json:"unmodeled,omitempty"` + // Provenance records where the parameter was declared. A parameter shared by + // several operations — a path-item parameter in OpenAPI, merged into every + // operation on the path — points at its own single declaration rather than + // at the operation it was merged into, so a consumer can tell an inherited + // parameter from one the operation declares. + Provenance Provenance `json:"provenance"` } // Payload is the body/message content of a request, response, or message diff --git a/ir/operation_test.go b/ir/operation_test.go index 7d7ff698..2f13f2bd 100644 --- a/ir/operation_test.go +++ b/ir/operation_test.go @@ -130,13 +130,15 @@ func TestPageStrategy_Constants(t *testing.T) { } // TestParameter_JSONContract pins Parameter's omitempty contract — Name, -// Type, Required, and Docs carry no omitempty since every parameter has a -// naming, a type, a required flag, and a docs object; everything else is -// optional — and that a fully populated Parameter round-trips. +// Type, Required, Docs, and Provenance carry no omitempty since every parameter +// has a naming, a type, a required flag, a docs object, and a declaring +// position; everything else is optional — and that a fully populated Parameter +// round-trips. func TestParameter_JSONContract(t *testing.T) { t.Parallel() assertJSONContract(t, ir.Parameter{}, - `{"name":{},"type":{"target":"","nullable":false},"required":false,"docs":{}}`, + `{"name":{},"type":{"target":"","nullable":false},"required":false,"docs":{},`+ + `"provenance":{"source":0}}`, ir.Parameter{ Name: populatedNaming(), Type: populatedTypeRef(), @@ -151,7 +153,8 @@ func TestParameter_JSONContract(t *testing.T) { {Name: "ex1", Value: &ir.Value{Kind: ir.ValueNumber, Num: ir.BigVal("1")}}, {Name: "ex2", Value: &ir.Value{Kind: ir.ValueNumber, Num: ir.BigVal("2")}}, }, - Unmodeled: populatedUnmodeled(), + Unmodeled: populatedUnmodeled(), + Provenance: populatedProvenance(), }) } diff --git a/testdata/conformance/openapi/codeclared-schema-content.golden.json b/testdata/conformance/openapi/codeclared-schema-content.golden.json index d390209d..abb44b28 100644 --- a/testdata/conformance/openapi/codeclared-schema-content.golden.json +++ b/testdata/conformance/openapi/codeclared-schema-content.golden.json @@ -48,6 +48,10 @@ "pointer": "/paths/~1x/get/parameters/0/schema" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/parameters/0" } }, { @@ -72,6 +76,10 @@ "pointer": "/paths/~1x/get/parameters/1/schema" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/parameters/1" } } ], diff --git a/testdata/conformance/openapi/component-reuse.golden.json b/testdata/conformance/openapi/component-reuse.golden.json index a6dbf8e5..fed21dbd 100644 --- a/testdata/conformance/openapi/component-reuse.golden.json +++ b/testdata/conformance/openapi/component-reuse.golden.json @@ -35,7 +35,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/parameters/Sort" + } } ], "responses": [ @@ -142,7 +146,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/components/parameters/Sort" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/deprecation.golden.json b/testdata/conformance/openapi/deprecation.golden.json index 46d4ae81..fe0e51e7 100644 --- a/testdata/conformance/openapi/deprecation.golden.json +++ b/testdata/conformance/openapi/deprecation.golden.json @@ -38,7 +38,11 @@ }, "required": false, "docs": {}, - "deprecation": {} + "deprecation": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1old/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/examples.golden.json b/testdata/conformance/openapi/examples.golden.json index 20dfd940..041b4fe6 100644 --- a/testdata/conformance/openapi/examples.golden.json +++ b/testdata/conformance/openapi/examples.golden.json @@ -48,7 +48,11 @@ "object": null } } - ] + ], + "provenance": { + "source": 0, + "pointer": "/paths/~1items/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/extension-promotion.golden.json b/testdata/conformance/openapi/extension-promotion.golden.json index 76707a19..b9f40b8b 100644 --- a/testdata/conformance/openapi/extension-promotion.golden.json +++ b/testdata/conformance/openapi/extension-promotion.golden.json @@ -30,6 +30,47 @@ "since": "1.2.0", "removalVersion": "2.0.0" }, + "params": [ + { + "name": { + "source": "legacy", + "canonical": "legacy" + }, + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "docs": {}, + "deprecation": { + "message": "use filter instead", + "removalVersion": "3.0.0" + }, + "unmodeled": { + "openapi:x-deprecated-reason": { + "reason": "vendor_extension", + "value": "use filter instead", + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/parameters/0/x-deprecated-reason" + } + }, + "openapi:x-sunset": { + "reason": "vendor_extension", + "value": "3.0.0", + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/parameters/0/x-sunset" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1x/get/parameters/0", + "inferred": "extension-promotion" + } + } + ], "responses": [ { "name": { @@ -100,6 +141,16 @@ "method": "GET", "uriTemplate": "/x", "sharedRoute": false, + "paramBindings": [ + { + "param": "legacy", + "location": "query", + "wireName": "legacy", + "style": "form", + "explode": true, + "allowReserved": false + } + ], "checksumRequired": false, "isWebhook": false } @@ -383,7 +434,7 @@ { "format": "openapi@3.1", "path": "extension-promotion.yaml", - "hash": "1bade65b585c75be141198f5312404923b56b7fbcf36105d20f13e26da63c52f" + "hash": "aa16a7bd98de256e0feb2bf240903d08db7f651a6b802255d7d89a98a69bcadd" } ] } diff --git a/testdata/conformance/openapi/extension-promotion.yaml b/testdata/conformance/openapi/extension-promotion.yaml index 1455012b..30e78be2 100644 --- a/testdata/conformance/openapi/extension-promotion.yaml +++ b/testdata/conformance/openapi/extension-promotion.yaml @@ -8,6 +8,13 @@ paths: x-deprecated-reason: use getY instead x-deprecated-since: "1.2.0" x-sunset: "2.0.0" + parameters: + - name: legacy + in: query + deprecated: true + x-deprecated-reason: use filter instead + x-sunset: "3.0.0" + schema: {type: string} responses: "200": description: ok diff --git a/testdata/conformance/openapi/extensions-x.golden.json b/testdata/conformance/openapi/extensions-x.golden.json index d0cd0a75..e4affc33 100644 --- a/testdata/conformance/openapi/extensions-x.golden.json +++ b/testdata/conformance/openapi/extensions-x.golden.json @@ -71,6 +71,10 @@ "pointer": "/paths/~1widgets/parameters/0/x-mark" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/parameters/0" } } ], @@ -359,6 +363,10 @@ "pointer": "/paths/~1widgets/parameters/0/x-mark" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets/parameters/0" } } ], diff --git a/testdata/conformance/openapi/http-binding.golden.json b/testdata/conformance/openapi/http-binding.golden.json index 7c54ea3b..d8d1d27b 100644 --- a/testdata/conformance/openapi/http-binding.golden.json +++ b/testdata/conformance/openapi/http-binding.golden.json @@ -36,7 +36,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1items~1{id}/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/inline-annotations.golden.json b/testdata/conformance/openapi/inline-annotations.golden.json index 6c00f950..4754e625 100644 --- a/testdata/conformance/openapi/inline-annotations.golden.json +++ b/testdata/conformance/openapi/inline-annotations.golden.json @@ -54,6 +54,10 @@ "pointer": "/paths/~1codes/get/parameters/0/schema/x-facet" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1codes/get/parameters/0" } } ], diff --git a/testdata/conformance/openapi/inline-hoist-positions.golden.json b/testdata/conformance/openapi/inline-hoist-positions.golden.json index 732190be..4eb9d3a6 100644 --- a/testdata/conformance/openapi/inline-hoist-positions.golden.json +++ b/testdata/conformance/openapi/inline-hoist-positions.golden.json @@ -36,7 +36,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1orders/post/parameters/0" + } } ], "request": { diff --git a/testdata/conformance/openapi/neutral-naming.golden.json b/testdata/conformance/openapi/neutral-naming.golden.json index 9eceb455..a071dd27 100644 --- a/testdata/conformance/openapi/neutral-naming.golden.json +++ b/testdata/conformance/openapi/neutral-naming.golden.json @@ -37,7 +37,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets~1{widget.id}/get/parameters/0" + } }, { "name": { @@ -49,7 +53,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1widgets~1{widget.id}/get/parameters/1" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/nullable-enum-31.golden.json b/testdata/conformance/openapi/nullable-enum-31.golden.json index c87377d2..ec8ce1a3 100644 --- a/testdata/conformance/openapi/nullable-enum-31.golden.json +++ b/testdata/conformance/openapi/nullable-enum-31.golden.json @@ -36,7 +36,11 @@ "nullable": true }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1pick/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/param-querystring.golden.json b/testdata/conformance/openapi/param-querystring.golden.json index 5e4d992a..84e1774f 100644 --- a/testdata/conformance/openapi/param-querystring.golden.json +++ b/testdata/conformance/openapi/param-querystring.golden.json @@ -36,7 +36,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports/get/parameters/0" + } } ], "responses": [ @@ -103,7 +107,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports~1summary/get/parameters/0" + } } ], "responses": [ @@ -171,7 +179,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1reports~1raw/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/param-ref-inheritance.golden.json b/testdata/conformance/openapi/param-ref-inheritance.golden.json index 37bd069d..68924546 100644 --- a/testdata/conformance/openapi/param-ref-inheritance.golden.json +++ b/testdata/conformance/openapi/param-ref-inheritance.golden.json @@ -47,7 +47,11 @@ "summary": "Cursor", "description": "Opaque pagination cursor." }, - "deprecation": {} + "deprecation": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1items/get/parameters/0" + } }, { "name": { @@ -70,7 +74,11 @@ "summary": "Cursor", "description": "Cursor for this endpoint only." }, - "deprecation": {} + "deprecation": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1items/get/parameters/1" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/param-style-matrix.golden.json b/testdata/conformance/openapi/param-style-matrix.golden.json index e1a0ea94..de4ecae0 100644 --- a/testdata/conformance/openapi/param-style-matrix.golden.json +++ b/testdata/conformance/openapi/param-style-matrix.golden.json @@ -36,7 +36,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/0" + } }, { "name": { @@ -48,7 +52,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/1" + } }, { "name": { @@ -60,7 +68,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/2" + } }, { "name": { @@ -72,7 +84,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/3" + } }, { "name": { @@ -84,7 +100,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/4" + } }, { "name": { @@ -96,7 +116,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/5" + } }, { "name": { @@ -108,7 +132,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/6" + } }, { "name": { @@ -120,7 +148,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/7" + } }, { "name": { @@ -132,7 +164,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/8" + } }, { "name": { @@ -144,7 +180,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/9" + } }, { "name": { @@ -156,7 +196,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/10" + } }, { "name": { @@ -168,7 +212,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/11" + } }, { "name": { @@ -180,7 +228,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/12" + } }, { "name": { @@ -192,7 +244,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/13" + } }, { "name": { @@ -204,7 +260,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/14" + } }, { "name": { @@ -216,7 +276,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/15" + } }, { "name": { @@ -228,7 +292,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/16" + } }, { "name": { @@ -240,7 +308,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/17" + } }, { "name": { @@ -252,7 +324,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/18" + } }, { "name": { @@ -264,7 +340,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/19" + } }, { "name": { @@ -276,7 +356,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/20" + } }, { "name": { @@ -288,7 +372,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/21" + } }, { "name": { @@ -300,7 +388,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/22" + } }, { "name": { @@ -312,7 +404,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/23" + } }, { "name": { @@ -324,7 +420,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/24" + } }, { "name": { @@ -336,7 +436,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/25" + } }, { "name": { @@ -348,7 +452,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/26" + } }, { "name": { @@ -360,7 +468,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/27" + } }, { "name": { @@ -372,7 +484,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/28" + } }, { "name": { @@ -384,7 +500,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/29" + } }, { "name": { @@ -396,7 +516,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1matrix~1{pathMatrixExplode}~1{pathMatrixNoExplode}~1{pathMatrixDefaultExplode}~1{pathLabelExplode}~1{pathLabelNoExplode}~1{pathLabelDefaultExplode}~1{pathSimpleExplode}~1{pathSimpleNoExplode}~1{pathSimpleDefaultExplode}~1{pathDefaulted}/get/parameters/30" + } } ], "responses": [ @@ -704,7 +828,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1whole-query/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/param-styles.golden.json b/testdata/conformance/openapi/param-styles.golden.json index acb0ad12..b7edbc64 100644 --- a/testdata/conformance/openapi/param-styles.golden.json +++ b/testdata/conformance/openapi/param-styles.golden.json @@ -36,7 +36,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/0" + } }, { "name": { @@ -48,7 +52,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/1" + } }, { "name": { @@ -60,7 +68,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/2" + } }, { "name": { @@ -82,6 +94,10 @@ "pointer": "/paths/~1search/get/parameters/3/allowEmptyValue" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/3" } }, { @@ -94,7 +110,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/get/parameters/4" + } }, { "name": { @@ -106,7 +126,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/parameters/0" + } } ], "responses": [ @@ -215,7 +239,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1search/parameters/0" + } } ], "responses": [ diff --git a/testdata/conformance/openapi/param-xml-residue.golden.json b/testdata/conformance/openapi/param-xml-residue.golden.json index 92a85607..d7c5c761 100644 --- a/testdata/conformance/openapi/param-xml-residue.golden.json +++ b/testdata/conformance/openapi/param-xml-residue.golden.json @@ -49,6 +49,10 @@ "pointer": "/paths/~1docs/get/parameters/0/schema/xml" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1docs/get/parameters/0" } }, { @@ -74,6 +78,10 @@ "pointer": "/paths/~1docs/get/parameters/1/content/application~1xml/schema/xml" } } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1docs/get/parameters/1" } } ], diff --git a/testdata/conformance/openapi/response-links.golden.json b/testdata/conformance/openapi/response-links.golden.json index 57bcfa4e..d1e2f8ce 100644 --- a/testdata/conformance/openapi/response-links.golden.json +++ b/testdata/conformance/openapi/response-links.golden.json @@ -134,7 +134,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1orders~1{orderId}/get/parameters/0" + } } ], "responses": [ diff --git a/testdata/golden/openapi/petstore.golden.json b/testdata/golden/openapi/petstore.golden.json index d88aba09..f4b17b49 100644 --- a/testdata/golden/openapi/petstore.golden.json +++ b/testdata/golden/openapi/petstore.golden.json @@ -49,7 +49,11 @@ "nullable": false }, "required": false, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/get/parameters/0" + } } ], "responses": [ @@ -277,7 +281,11 @@ "nullable": false }, "required": true, - "docs": {} + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets~1{petId}/get/parameters/0" + } } ], "responses": [ From 62b00a177c2d6a1ee694078dab0930008c31f795 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Tue, 8 Sep 2026 01:13:34 +0300 Subject: [PATCH 3/9] docs(ir-design): name rule 4's live instances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit §12's fourth promotion rule read as if it had none left: `Parameter` was the instance it named, and the sentence recording that `Parameter` has since gained a `Provenance` left the rule with nothing to point at. `Variant` (§4.4) and `EnumMember` (§4.5) each still carry a `Deprecation` with no provenance of their own, so the rule governs them today. Name them. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SYeBgDsskwnyitLgPGCPn1 --- docs/ir-design.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/ir-design.md b/docs/ir-design.md index 2cd2632f..ae919aa5 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1831,8 +1831,10 @@ this from `Unmodeled` and no two derive it differently: 4. **A node with no `Provenance` is not promoted into.** A node carrying a `Deprecation` and no provenance could not satisfy rule 3, and a heuristic that cannot be audited is worse than an empty field. Giving such a node a provenance is a change to this document, and the promotion - follows it rather than preceding it — which is the order `Parameter` went through: it was the - instance this rule named until it gained the `Provenance` §7.2 now gives it. + follows it rather than preceding it — which is the order `Parameter` went through, and it held + this rule's only named instance until it gained the `Provenance` §7.2 now gives it. `Variant` + (§4.4) and `EnumMember` (§4.5) are the instances today: each carries a `Deprecation` and no + provenance of its own, so no key maps into either until one of them gains one. A value the mapped field cannot hold — anything but text, for the three `Deprecation` members — is reported and not coerced, since the document means something else by the key. From 054951f82122f63850a663250b3c74ec159823fe Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:34:56 +0300 Subject: [PATCH 4/9] docs(ir): say what Parameter.Provenance records for a $ref'd entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The field's GoDoc, the §7.2 sketch and the params test all said the pointer tells an inherited parameter from a declared one. That holds only for an entry written inline: a referenced entry lowers at the component it names from either mount, so a path-item $ref and an operation-level $ref land on one pointer and the mount site is not recorded. Say exactly that, so a consumer reading the pointer for inherited-vs-declared knows which entries answer. Payload.Required's lead clause was inverted against its own mapping and the §7.2 sketch; it now reads the way the sketch does. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- .../openapi/internal/operation/params_test.go | 4 +++- docs/ir-design.md | 9 +++++--- ir/operation.go | 21 +++++++++++-------- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/compilers/openapi/internal/operation/params_test.go b/compilers/openapi/internal/operation/params_test.go index f4e1c38f..b2d45a86 100644 --- a/compilers/openapi/internal/operation/params_test.go +++ b/compilers/openapi/internal/operation/params_test.go @@ -357,7 +357,9 @@ components: // each answer differently, and the merge is why: an operation's own entry sits // under that operation, a $ref'd one under the component it names, and a // path-item one under the path item — the last shared by every operation on the -// path, which is what tells an inherited parameter from a declared one. +// path. Only the inline entries tell an inherited parameter from a declared +// one: a $ref'd entry lands on its component from either mount, and the mount +// site is not recorded. func TestParams_ProvenanceIsTheDeclaringPosition(t *testing.T) { t.Parallel() doc, diags := parseFull(t, paramProvenanceSpec) diff --git a/docs/ir-design.md b/docs/ir-design.md index ae919aa5..e6d17d37 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1198,8 +1198,10 @@ type Parameter struct { Unmodeled Unmodeled Provenance Provenance // the parameter's own declaration; a parameter merged into several // operations (an OpenAPI path-item parameter) points at that one - // declaration, not at the operation it was merged into, which is - // what tells an inherited parameter from a declared one + // declaration, not at the operation it was merged into. For a + // referenced entry that is the component it names, and the mount + // site is not recorded — so inherited-vs-declared is readable off + // the pointer only for an entry written inline // NOTE: no location here — path/query/header is HTTP-binding detail (§8.1) } @@ -1211,7 +1213,8 @@ type Payload struct { // unstated body as optional, so folding that onto nil would make // "the format is silent" read as "the document says no". // Response and message payloads leave it nil — only a request - // body can be omitted + // body can be omitted — and pass/validate reports one set + // anywhere else (ir/payload-required-outside-request) Unmodeled Unmodeled } diff --git a/ir/operation.go b/ir/operation.go index efe119cc..ade6e359 100644 --- a/ir/operation.go +++ b/ir/operation.go @@ -96,8 +96,10 @@ type Parameter struct { // Provenance records where the parameter was declared. A parameter shared by // several operations — a path-item parameter in OpenAPI, merged into every // operation on the path — points at its own single declaration rather than - // at the operation it was merged into, so a consumer can tell an inherited - // parameter from one the operation declares. + // at the operation it was merged into. For a referenced entry the + // declaration is the component it names, and the mount site is not + // recorded: whether a parameter was inherited or declared by the operation + // is therefore readable off the pointer only for an entry written inline. Provenance Provenance `json:"provenance"` } @@ -106,13 +108,14 @@ type Parameter struct { type Payload struct { // Contents holds one entry per media type / message schema — all kept. Contents []Content `json:"contents,omitempty"` - // Required states whether the message may be omitted: true = the body must - // be sent, false = it is optional. nil = the source format does not express - // body optionality at all, which is why this is a pointer — for a format - // that does, an unstated body is optional, and collapsing that onto nil - // would make "the format is silent" indistinguishable from "the document - // says no". A response or message payload leaves it nil: only a request - // body can be omitted. + // Required states whether the message must be sent: true = the body is + // mandatory, false = it may be omitted. nil = the source format does not + // express body optionality at all, which is why this is a pointer — for a + // format that does, an unstated body is optional, and collapsing that onto + // nil would make "the format is silent" indistinguishable from "the + // document says no". A response or message payload leaves it nil: only a + // request body can be omitted, and pass/validate reports one that is set + // anywhere else (ir/payload-required-outside-request). Required *bool `json:"required,omitempty"` // Unmodeled holds source constructs the IR does not model, kept verbatim. Unmodeled Unmodeled `json:"unmodeled,omitempty"` From 32cde549ed377eb85065a37a4bd409ad5a333620 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:34:56 +0300 Subject: [PATCH 5/9] feat(pass): report a Payload.Required set outside a request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ir.Payload says only a request body can be omitted and a response or message payload leaves Required nil, and nothing enforced it: a document carrying the field on a response passed every oracle, which is the reading GitHub #421 was filed about — an emitter rendering "required" off the boolean prints it on a response. The Payload-bearing fields checkEncodingKeys named by hand now live in one walk, forEachPayload, so the carrier guard covers every check built on it and a carrier added to the IR reaches both. The new check reports ir/payload-required-outside-request at severity error, with a hand-built fixture per carrier since no compiler produces the shape. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- pass/validate.go | 66 ++++++++++++++++++++++----- pass/validate_carriers_test.go | 16 +++---- pass/validate_encoding_test.go | 83 ++++++++++++++++++++++++---------- 3 files changed, 121 insertions(+), 44 deletions(-) diff --git a/pass/validate.go b/pass/validate.go index 53793351..dff5fc0d 100644 --- a/pass/validate.go +++ b/pass/validate.go @@ -29,6 +29,7 @@ func Validate(doc *ir.Document) []ir.Diagnostic { diags = append(diags, checkServerIndices(doc)...) diags = append(diags, checkResponseIndices(doc)...) diags = append(diags, checkEncodingKeys(doc)...) + diags = append(diags, checkPayloadRequired(doc)...) diags = append(diags, checkPropIDRefs(doc)...) diags = append(diags, checkDiscriminators(doc)...) diags = append(diags, checkDuplicateWireNames(doc)...) @@ -232,35 +233,78 @@ func checkPropIDRefs(doc *ir.Document) []ir.Diagnostic { // (see the package doc): a key addressing nothing is a broken reference, so a // second checker growing this check adopts the code rather than forcing a rename. // Only this pass reports it today. +func checkEncodingKeys(doc *ir.Document) []ir.Diagnostic { + var diags []ir.Diagnostic + forEachPayload(doc, func(site payloadSite) { + diags = appendEncodingKeyDiags(diags, doc, site.payload, site.where) + }) + return diags +} + +// checkPayloadRequired reports a Payload.Required set anywhere but on a request. +// +// Only a request body can be omitted, so ir.Payload defines the field for that +// one position and says a response or message payload leaves it nil. Set there +// it states something no exchange can honour, and an emitter that renders +// "required" off the boolean prints it on a response — the reading GitHub #421 +// was filed about. No compiler produces the shape today; this is the rule's +// guard rather than the repair of a lowering, and an error rather than a +// warning because the document is wrong, not merely lossy. +// +// The code carries the ir/ namespace for the reason checkEncodingKeys does: it +// names the defect, not the finder. Only this pass reports it today. +func checkPayloadRequired(doc *ir.Document) []ir.Diagnostic { + var diags []ir.Diagnostic + forEachPayload(doc, func(site payloadSite) { + if site.request || site.payload.Required == nil { + return + } + diags = append(diags, diag(ir.SeverityError, "ir/payload-required-outside-request", + fmt.Sprintf("payload at %s sets required, which only a request body can state", site.where), + site.where)) + }) + return diags +} + +// payloadSite is one Payload a document carries: the node, where it hangs, and +// whether that position is a request — the one place Payload.Required is +// defined. +type payloadSite struct { + payload *ir.Payload + where string + request bool +} + +// forEachPayload calls fn once per Payload the document carries, skipping the +// positions that hold none. // // The fields that carry a Payload are named here — Operation.Request, // Response.Payload and Message.Payload — because nothing in a Payload's Go type // says who owns one, so a new one has to be added by hand. That coupling is // guarded: TestEncodingCarriers_NameEveryPayloadFieldInTheIR // (validate_carriers_test.go) walks the IR for Payload-bearing fields and fails -// the moment one of them is not walked here. -func checkEncodingKeys(doc *ir.Document) []ir.Diagnostic { - var diags []ir.Diagnostic +// the moment one of them is not walked here, and every check built on this walk +// reaches a carrier the day it is added. +func forEachPayload(doc *ir.Document, fn func(payloadSite)) { forEachOperation(doc, func(op ir.Operation) { - diags = appendEncodingKeyDiags(diags, doc, op.Request, string(op.ID)+"/request") + if op.Request != nil { + fn(payloadSite{payload: op.Request, where: string(op.ID) + "/request", request: true}) + } for i, r := range op.Responses { - at := fmt.Sprintf("%s/responses/%d", op.ID, i) - diags = appendEncodingKeyDiags(diags, doc, r.Payload, at) + if r.Payload != nil { + fn(payloadSite{payload: r.Payload, where: fmt.Sprintf("%s/responses/%d", op.ID, i)}) + } } }) for _, id := range sortedKeys(doc.Messages) { msg := doc.Messages[id] - diags = appendEncodingKeyDiags(diags, doc, &msg.Payload, string(id)) + fn(payloadSite{payload: &msg.Payload, where: string(id)}) } - return diags } // appendEncodingKeyDiags appends to dst a diagnostic per unresolvable encoding // key in each of the payload's contents; where locates the payload's owner. func appendEncodingKeyDiags(dst []ir.Diagnostic, doc *ir.Document, payload *ir.Payload, where string) []ir.Diagnostic { - if payload == nil { - return dst - } for i, c := range payload.Contents { if len(c.Encoding) == 0 { continue diff --git a/pass/validate_carriers_test.go b/pass/validate_carriers_test.go index bd478354..74143fcf 100644 --- a/pass/validate_carriers_test.go +++ b/pass/validate_carriers_test.go @@ -37,18 +37,18 @@ const ( // TestEncodingCarriers_NameEveryPayloadFieldInTheIR fails when the IR declares a // field carrying an ir.Payload that encodingCarriers does not name. // -// checkEncodingKeys reaches every Content by naming the Payload-bearing fields by +// forEachPayload reaches every Payload by naming the Payload-bearing fields by // hand, because nothing in a Payload's Go type says who owns one. Naming them // costs a coupling the compiler cannot check, and this is what checks it: a -// fourth carrier added to the IR would otherwise be walked by neither the check -// nor the cases below, its encoding keys resolved against nothing, with the whole -// suite green. +// fourth carrier added to the IR would otherwise be walked by neither the checks +// built on the walk nor the cases below, its encoding keys resolved against +// nothing and its Required unjudged, with the whole suite green. // // The guard holds both lists at once, in two steps. Here it holds // encodingCarriers against the IR; TestValidate_EncodingKeyAddressesNoProperty -// then holds checkEncodingKeys against encodingCarriers, by requiring a -// diagnostic from every entry. So a carrier added to the IR reddens this test, -// and adding it here reddens that one until checkEncodingKeys walks it too. +// then holds forEachPayload against encodingCarriers, by requiring a diagnostic +// from every entry. So a carrier added to the IR reddens this test, and adding +// it here reddens that one until forEachPayload walks it too. func TestEncodingCarriers_NameEveryPayloadFieldInTheIR(t *testing.T) { t.Parallel() carriers := encodingCarriers() @@ -63,7 +63,7 @@ func TestEncodingCarriers_NameEveryPayloadFieldInTheIR(t *testing.T) { "nothing and proves nothing about the ones encodingCarriers names") assert.Empty(t, cmp.Diff(found, listed), "encodingCarriers must name every ir field that carries an ir.Payload, once each "+ - "(-declared +listed); a new one also has to be walked by checkEncodingKeys") + "(-declared +listed); a new one also has to be walked by forEachPayload") } // payloadFields returns "Owner.Field", sorted, for every struct field the IR diff --git a/pass/validate_encoding_test.go b/pass/validate_encoding_test.go index de4d153d..e1f72507 100644 --- a/pass/validate_encoding_test.go +++ b/pass/validate_encoding_test.go @@ -17,34 +17,36 @@ func multipartContent(enc map[ir.PropID]ir.PartEncoding) ir.Content { return ir.Content{MediaType: "multipart/form-data", Type: ir.TypeRef{Target: "t/m"}, Encoding: enc} } -// encodingCarrier is one field that carries a Payload — and so an Encoding map — -// named as the ir field it is, and paired with the location a diagnostic about -// its first content must point at. +// multipartPayload is a payload of one multipartContent carrying enc. +func multipartPayload(enc map[ir.PropID]ir.PartEncoding) ir.Payload { + return ir.Payload{Contents: []ir.Content{multipartContent(enc)}} +} + +// encodingCarrier is one field that carries a Payload, named as the ir field it +// is, paired with the location a diagnostic about the payload must point at, +// and saying whether the position is a request — the one place +// Payload.Required is defined. type encodingCarrier struct { - field string - at string - plant func(doc *ir.Document, enc map[ir.PropID]ir.PartEncoding) + field string + at string + request bool + plant func(doc *ir.Document, payload ir.Payload) } -// encodingCarriers enumerates the Payload-bearing fields checkEncodingKeys walks. +// encodingCarriers enumerates the Payload-bearing fields forEachPayload walks. // TestEncodingCarriers_NameEveryPayloadFieldInTheIR holds this list against the -// IR and the cases below hold checkEncodingKeys against this list, so a carrier -// added to the IR has to reach both. +// IR and the cases below hold the checks built on the walk against this list, +// so a carrier added to the IR has to reach both. func encodingCarriers() []encodingCarrier { return []encodingCarrier{ - {"Operation.Request", "op/request/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { - requestContent(d).Encoding = enc + {"Operation.Request", "op/request", true, func(d *ir.Document, p ir.Payload) { + firstOp(d).Request = &p }}, - {"Response.Payload", "op/responses/0/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { - firstOp(d).Responses = []ir.Response{{ - Name: ir.Naming{Source: "ok"}, - Payload: &ir.Payload{Contents: []ir.Content{multipartContent(enc)}}, - }} + {"Response.Payload", "op/responses/0", false, func(d *ir.Document, p ir.Payload) { + firstOp(d).Responses = []ir.Response{{Name: ir.Naming{Source: "ok"}, Payload: &p}} }}, - {"Message.Payload", "msg/a/contents/0", func(d *ir.Document, enc map[ir.PropID]ir.PartEncoding) { - putMessage(d, func(m *ir.Message) { - m.Payload = ir.Payload{Contents: []ir.Content{multipartContent(enc)}} - }) + {"Message.Payload", "msg/a", false, func(d *ir.Document, p ir.Payload) { + putMessage(d, func(m *ir.Message) { m.Payload = p }) }}, } } @@ -61,19 +63,50 @@ func TestValidate_EncodingKeyAddressesNoProperty(t *testing.T) { t.Run(tc.field, func(t *testing.T) { t.Parallel() doc := validDoc() - tc.plant(doc, map[ir.PropID]ir.PartEncoding{"p/m/ghost": {Multi: true}}) + tc.plant(doc, multipartPayload(map[ir.PropID]ir.PartEncoding{"p/m/ghost": {Multi: true}})) found := withCode(pass.Validate(doc), "ir/encoding-key-unknown-property") require.Len(t, found, 1, "exactly the planted key must address nothing") assert.Equal(t, ir.SeverityError, found[0].Severity) - assert.Equal(t, tc.at+"/encoding/p/m/ghost", found[0].Provenance.Pointer) - assert.Equal(t, `encoding key "p/m/ghost" at `+tc.at+ - `/encoding/p/m/ghost addresses no property of the content's type "t/m"`, found[0].Message, + at := tc.at + "/contents/0/encoding/p/m/ghost" + assert.Equal(t, at, found[0].Provenance.Pointer) + assert.Equal(t, `encoding key "p/m/ghost" at `+at+ + ` addresses no property of the content's type "t/m"`, found[0].Message, "the message names the key, where it sits, and what it failed to address") }) } } +// TestValidate_PayloadRequiredOutsideARequest sets Payload.Required in each +// field that carries a Payload. Only the request may state it: ir.Payload +// defines the field for that one position, and an emitter rendering "required" +// off the boolean would print it on a response (GitHub #421). No compiler +// produces the shape, so the fixture is built by hand. +func TestValidate_PayloadRequiredOutsideARequest(t *testing.T) { + t.Parallel() + for _, tc := range encodingCarriers() { + t.Run(tc.field, func(t *testing.T) { + t.Parallel() + doc := validDoc() + payload := multipartPayload(nil) + required := false + payload.Required = &required + tc.plant(doc, payload) + + found := withCode(pass.Validate(doc), "ir/payload-required-outside-request") + if tc.request { + assert.Empty(t, found, "a request body is the position the field is defined for") + return + } + require.Len(t, found, 1, "the one planted field must be reported once") + assert.Equal(t, ir.SeverityError, found[0].Severity) + assert.Equal(t, tc.at, found[0].Provenance.Pointer) + assert.Equal(t, "payload at "+tc.at+" sets required, which only a request body can state", + found[0].Message) + }) + } +} + // TestValidate_EncodingKeysNamingRealPropertiesAreClean is the silent half of the // proof: with every carrier keyed by a property the content's type really // declares, Validate reports nothing at all. A check that cannot stay silent is @@ -82,7 +115,7 @@ func TestValidate_EncodingKeysNamingRealPropertiesAreClean(t *testing.T) { t.Parallel() doc := validDoc() for _, tc := range encodingCarriers() { - tc.plant(doc, map[ir.PropID]ir.PartEncoding{"p/m/a": {Multi: true}}) + tc.plant(doc, multipartPayload(map[ir.PropID]ir.PartEncoding{"p/m/a": {Multi: true}})) } assert.Empty(t, pass.Validate(doc)) } From 13f0e69be5f631b834b535bb32bb1651ed56e427 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:34:56 +0300 Subject: [PATCH 6/9] test(compilers/openapi): hold the promotion carriers to the IR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit promotionCarriers is the sweep over every node the compiler promotes a deprecation into, and it was hand-written with nothing holding it to the IR: the IR declares eight structs carrying a Deprecation, the map named five of them, and a carrier added to the compiler without a PromoteDeprecation call reddened nothing — the shape of defect Parameter had before #423. A reflective walk over the IR, seeded from Document and every TypeDef kind the ir sources declare, now requires each such struct to be mapped to the rows that witness it or exempted with a reason held to the IR: Variant and EnumMember until they gain a Provenance, Message because the OpenAPI compiler builds none. A second test holds the mapping and the corpus rows to each other. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- compilers/openapi/promotion_carriers_test.go | 253 +++++++++++++++++++ 1 file changed, 253 insertions(+) create mode 100644 compilers/openapi/promotion_carriers_test.go diff --git a/compilers/openapi/promotion_carriers_test.go b/compilers/openapi/promotion_carriers_test.go new file mode 100644 index 00000000..05225472 --- /dev/null +++ b/compilers/openapi/promotion_carriers_test.go @@ -0,0 +1,253 @@ +// This file holds promotionCarriers (promotion_test.go) to the IR. That map is +// the sweep over every node the compiler promotes a deprecation into, and it is +// hand-written: a carrier the IR gains, and the compiler starts building without +// a PromoteDeprecation call, would be covered by no row and reddens nothing — +// the exact shape of the defect Parameter had before GitHub #423. +package openapi_test // external test package — exercises only the public API + +import ( + "go/ast" + "go/parser" + "go/token" + "maps" + "path/filepath" + "reflect" + "runtime" + "slices" + "strconv" + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/dexpace/morphic/ir" +) + +// deprecationPtrType is the field type that makes an IR struct a carrier. +var deprecationPtrType = reflect.TypeFor[*ir.Deprecation]() + +// maxIRTypeGraphDepth bounds the walk over the IR's static type graph (the +// bounded-recursion rule). Each distinct reflect.Type is visited once, so the +// seen set already terminates it; reaching the cap means the walk stopped being +// a walk over the IR, so it fails rather than truncating. +const maxIRTypeGraphDepth = 512 + +// promotedCarrierRows maps each IR struct the compiler promotes into to the +// promotionCarriers rows that witness it. Property appears twice because the +// compiler builds one at two positions — a response header and a model +// property — and the sweep has to reach both construction sites. +var promotedCarrierRows = map[string][]string{ + "AuthScheme": {"auth scheme"}, + "Operation": {"operation"}, + "Parameter": {"parameter"}, + "Property": {"header", "property"}, + "TypeCommon": {"type"}, +} + +// unpromotedCarriers names every IR struct carrying a Deprecation that no row +// witnesses, against the reason. An entry is a claim a reviewer has to agree +// with, which is why it is spelled here rather than dropped from the walk; each +// reason is held to the IR below, so an entry cannot outlive it. +var unpromotedCarriers = map[string]string{ + "EnumMember": "carries no Provenance, so rule 4 of ir-design §12 keeps promotion out until it gains one", + "Message": "the OpenAPI compiler builds none: a Message is an AsyncAPI node", + "Variant": "carries no Provenance, so rule 4 of ir-design §12 keeps promotion out until it gains one", +} + +// TestPromotionCarriers_NameEveryDeprecationCarrierInTheIR fails when the IR +// declares a struct carrying a *Deprecation that promotedCarrierRows neither maps +// to a promotionCarriers row nor unpromotedCarriers exempts with a reason that +// still holds — and when either list names a struct the IR no longer declares. +func TestPromotionCarriers_NameEveryDeprecationCarrierInTheIR(t *testing.T) { + t.Parallel() + found := deprecationCarrierTypes(t) + require.NotEmpty(t, found, "the walk found no *ir.Deprecation field at all, so it reached "+ + "nothing and proves nothing about the carriers the lists name") + + listed := slices.Sorted(slices.Values(slices.Concat( + slices.Collect(maps.Keys(promotedCarrierRows)), slices.Collect(maps.Keys(unpromotedCarriers))))) + names := make([]string, 0, len(found)) + for _, rt := range found { + names = append(names, rt.Name()) + } + assert.Empty(t, cmp.Diff(names, listed), + "every ir struct carrying a *Deprecation must be mapped to its promotion rows or "+ + "exempted with a reason, once each (-declared +listed)") + + for _, rt := range found { + _, hasProvenance := rt.FieldByName("Provenance") + if _, promoted := promotedCarrierRows[rt.Name()]; promoted { + assert.True(t, hasProvenance, "%s is promoted into, so rule 4 needs it to carry a Provenance", rt.Name()) + continue + } + reason := unpromotedCarriers[rt.Name()] + if strings.HasPrefix(reason, "carries no Provenance") { + assert.False(t, hasProvenance, "%s now carries a Provenance, so its exemption no longer holds: "+ + "promote into it and move it to promotedCarrierRows", rt.Name()) + } + } +} + +// TestPromotionCarriers_RowsAreTheOnesTheCorpusBuilds holds promotedCarrierRows +// and promotionCarriers to each other: every row a struct claims is one the +// corpus builds, and every row the corpus builds is claimed by exactly one +// struct. Together with the test above, the chain runs IR → mapping → corpus row +// → construction site, and no link can be dropped alone. +func TestPromotionCarriers_RowsAreTheOnesTheCorpusBuilds(t *testing.T) { + t.Parallel() + doc, diags := parseCorpus(t, "extension-promotion") + assertNoErrorDiags(t, diags) + assert.Empty(t, doc.Messages, "the reason unpromotedCarriers gives for Message is that the "+ + "OpenAPI compiler builds none") + + var claimed []string + for _, rows := range promotedCarrierRows { + claimed = append(claimed, rows...) + } + slices.Sort(claimed) + built := slices.Sorted(maps.Keys(promotionCarriers(t, doc))) + assert.Empty(t, cmp.Diff(built, claimed), + "promotedCarrierRows must claim every promotionCarriers row, once each (-built +claimed)") +} + +// deprecationCarrierTypes returns every struct type the IR declares with a field +// of type *ir.Deprecation, sorted by name. +// +// The walk starts at ir.Document and visits each distinct reflect.Type once, so +// recursive shapes terminate. The sealed TypeDef sum is reached only through an +// interface, which a walk over the static type graph cannot descend into, so each +// concrete kind is walked from its own root as well — seeded from the kinds the +// ir sources declare rather than a list here, so a kind is covered the day it is +// added (pass/validate_carriers_test.go walks the same two halves for the same +// reason). +func deprecationCarrierTypes(t *testing.T) []reflect.Type { + t.Helper() + var found []reflect.Type + seen := map[reflect.Type]bool{} + + var walk func(rt reflect.Type, depth int) + walk = func(rt reflect.Type, depth int) { + require.Less(t, depth, maxIRTypeGraphDepth, "the IR type graph nests past %d", maxIRTypeGraphDepth) + if seen[rt] { + return + } + seen[rt] = true + switch rt.Kind() { + case reflect.Pointer, reflect.Slice, reflect.Array: + walk(rt.Elem(), depth+1) + case reflect.Map: + walk(rt.Key(), depth+1) + walk(rt.Elem(), depth+1) + case reflect.Struct: + for f := range rt.Fields() { + if f.Type == deprecationPtrType { + found = append(found, rt) + } + walk(f.Type, depth+1) + } + default: + // A leaf: no other kind has a component type to descend into. + } + } + + walk(reflect.TypeFor[ir.Document](), 0) + for _, kind := range irTypeKinds(t) { + td, ok := ir.NewTypeDef(kind) + require.True(t, ok, "no concrete type is registered for kind %q", kind) + rt := reflect.TypeOf(td) + require.Equal(t, reflect.Pointer, rt.Kind(), "NewTypeDef must return a pointer for %q", kind) + walk(rt.Elem(), 0) + } + slices.SortFunc(found, func(a, b reflect.Type) int { return strings.Compare(a.Name(), b.Name()) }) + return found +} + +// irTypeKinds returns every TypeKind constant the ir sources declare, read from +// the sources so that the list cannot go stale. The count is held to the +// typeDef() marker methods that seal the sum, which reads the same sources by a +// different route: finding constants proves the parse ran, not that it saw them +// all, and a walk seeded from a subset skips whole concrete kinds in silence. +func irTypeKinds(t *testing.T) []ir.TypeKind { + t.Helper() + var kinds []ir.TypeKind + impls := 0 + for _, path := range irSourcePaths(t) { + f, err := parser.ParseFile(token.NewFileSet(), path, nil, parser.SkipObjectResolution) + require.NoError(t, err, "parsing %s", path) + for _, decl := range f.Decls { + switch d := decl.(type) { + case *ast.GenDecl: + if d.Tok == token.CONST { + kinds = append(kinds, typeKindsIn(t, d)...) + } + case *ast.FuncDecl: + if d.Recv != nil && d.Name.Name == "typeDef" { + impls++ + } + default: + // Neither a constant block nor a method: nothing to read. + } + } + } + require.NotZero(t, impls, "the ir sources must seal concrete types into the TypeDef sum") + require.Len(t, kinds, impls, "the sum holds one concrete type per kind, so a count that "+ + "disagrees means this parse stopped seeing every constant rather than that the IR changed") + return kinds +} + +// typeKindsIn returns the TypeKind constants one const group declares. A spec +// naming neither type nor value repeats the previous one, so the group's last +// explicit type carries forward; a spec with a value of its own declares its own +// type. +func typeKindsIn(t *testing.T, gd *ast.GenDecl) []ir.TypeKind { + t.Helper() + var kinds []ir.TypeKind + isKind := false + for _, spec := range gd.Specs { + vs, isValue := spec.(*ast.ValueSpec) + require.True(t, isValue, "const spec is not a ValueSpec: %#v", spec) + switch { + case vs.Type != nil: + id, isIdent := vs.Type.(*ast.Ident) + isKind = isIdent && id.Name == "TypeKind" + case len(vs.Values) > 0: + isKind = false + } + if !isKind { + continue + } + for i, name := range vs.Names { + require.Less(t, i, len(vs.Values), "TypeKind constant %s must declare its own value", name.Name) + lit, isLit := vs.Values[i].(*ast.BasicLit) + require.True(t, isLit, "constant %s must be declared as a string literal", name.Name) + require.Equal(t, token.STRING, lit.Kind, "constant %s must be declared as a string literal", name.Name) + unquoted, err := strconv.Unquote(lit.Value) + require.NoError(t, err, "unquoting the value of %s", name.Name) + kinds = append(kinds, ir.TypeKind(unquoted)) + } + } + return kinds +} + +// irSourcePaths lists the ir package's non-test Go files, resolved against this +// file's own directory so the result does not depend on the working directory the +// suite runs from. +func irSourcePaths(t *testing.T) []string { + t.Helper() + _, self, _, ok := runtime.Caller(0) + require.True(t, ok, "runtime.Caller must report this test's path") + matches, err := filepath.Glob(filepath.Join(filepath.Dir(self), "..", "..", "ir", "*.go")) + require.NoError(t, err) + + paths := make([]string, 0, len(matches)) + for _, path := range matches { + if strings.HasSuffix(path, "_test.go") { + continue + } + paths = append(paths, path) + } + require.NotEmpty(t, paths, "the ir package must hold production Go sources") + return paths +} From 6d71aeccfb74c22724273c79622b22639fa68633 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:34:56 +0300 Subject: [PATCH 7/9] refactor(compilers/openapi): unexport schema.Preserve Its last caller outside the package went with the openapi:required write; every remaining one is in-package. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- compilers/openapi/internal/schema/accumulate.go | 4 ++-- compilers/openapi/internal/schema/compose.go | 2 +- compilers/openapi/internal/schema/schema.go | 4 ++-- compilers/openapi/internal/schema/schema_internal_test.go | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/compilers/openapi/internal/schema/accumulate.go b/compilers/openapi/internal/schema/accumulate.go index ea056307..0fd08598 100644 --- a/compilers/openapi/internal/schema/accumulate.go +++ b/compilers/openapi/internal/schema/accumulate.go @@ -46,10 +46,10 @@ func AppendExample(c lowering.Ctx, out []ir.Example, proto ir.Example, node *yam return append(out, proto), nil } -// Preserve records raw under key in *p with why it was kept and where it was +// preserve records raw under key in *p with why it was kept and where it was // written, allocating the map on first write. An absent or unconvertible // payload records nothing, so no caller needs a nil guard of its own. -func Preserve(c lowering.Ctx, p *ir.Unmodeled, key string, raw ir.RawValue, +func preserve(c lowering.Ctx, p *ir.Unmodeled, key string, raw ir.RawValue, reason ir.UnmodeledReason, pointer string, ) { annotation.PreserveInto(p, key, raw, reason, pointer, c.SrcIndex) diff --git a/compilers/openapi/internal/schema/compose.go b/compilers/openapi/internal/schema/compose.go index 4d2c5b2d..6e844244 100644 --- a/compilers/openapi/internal/schema/compose.go +++ b/compilers/openapi/internal/schema/compose.go @@ -206,7 +206,7 @@ func applyFalseBranches(c lowering.Ctx, m *ir.Model, s *oas3.Schema, pointer str } bptr := pointer + ids.Ptr("allOf", strconv.Itoa(i)) m.Additional = ir.AdditionalClosed - Preserve(c, &m.Unmodeled, "openapi:allOf/"+strconv.Itoa(i), + preserve(c, &m.Unmodeled, "openapi:allOf/"+strconv.Itoa(i), ir.RawValue("false"), ir.ReasonDegradedLowering, bptr) diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.FalseSchema, bptr, diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index 3a177b5d..57b2180a 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -462,7 +462,7 @@ func preserveUnionSiblings(c lowering.Ctx, ts *compile.Types, id ir.TypeID, s *o pointer, pointer+ids.Ptr(kw), kw)...) continue } - Preserve(c, &common.Unmodeled, "openapi:"+kw, raw, reason, pointer+ids.Ptr(kw)) + preserve(c, &common.Unmodeled, "openapi:"+kw, raw, reason, pointer+ids.Ptr(kw)) kept = kept || len(raw) > 0 } if reason == ir.ReasonValidationOnly || !kept { @@ -494,7 +494,7 @@ func falseSchema(c lowering.Ctx, ts *compile.Types, pointer, hint string) (ir.Ty // The key names the position rather than a keyword, because a boolean // schema writes none. Nothing can collide with it: a schema that is a // boolean has no other keywords to preserve. - Preserve(c, &common.Unmodeled, "openapi:schema", + preserve(c, &common.Unmodeled, "openapi:schema", ir.RawValue("false"), ir.ReasonDegradedLowering, pointer) diags = append(diags, c.DiagAt(ir.SeverityInfo, diag.FalseSchema, pointer, diff --git a/compilers/openapi/internal/schema/schema_internal_test.go b/compilers/openapi/internal/schema/schema_internal_test.go index f312ef87..dbd6a75e 100644 --- a/compilers/openapi/internal/schema/schema_internal_test.go +++ b/compilers/openapi/internal/schema/schema_internal_test.go @@ -188,7 +188,7 @@ func TestPreserve_EmptyRawIsRejectedLikeNil(t *testing.T) { t.Parallel() l := &lowerer{} var p ir.Unmodeled - Preserve(l.ctx, &p, "openapi:k", raw, ir.ReasonVendorExtension, "/p/k") + preserve(l.ctx, &p, "openapi:k", raw, ir.ReasonVendorExtension, "/p/k") assert.Nil(t, p, "a payload with no bytes preserves no construct") var q ir.Unmodeled From b404af05400b98d1b443bdf5aa6545573e3c912d Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:34:56 +0300 Subject: [PATCH 8/9] test(compilers/openapi): assert a shared body's required at both uses The fixture's required: false contributed to nothing the test observed. Both operations now assert the flag, as two values rather than one aliased pointer. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- .../openapi/internal/operation/operations_test.go | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/compilers/openapi/internal/operation/operations_test.go b/compilers/openapi/internal/operation/operations_test.go index 04211af1..d4c552b6 100644 --- a/compilers/openapi/internal/operation/operations_test.go +++ b/compilers/openapi/internal/operation/operations_test.go @@ -1263,7 +1263,7 @@ components: // component shared by twenty operations would repeat each line twenty times. func TestDiag_SharedDeclarationReportsEachDefectOnce(t *testing.T) { t.Parallel() - _, diags := parseFull(t, sharedDefectiveBodySpec) + doc, diags := parseFull(t, sharedDefectiveBodySpec) seen := map[string]int{} for _, d := range diags { @@ -1277,6 +1277,19 @@ func TestDiag_SharedDeclarationReportsEachDefectOnce(t *testing.T) { assert.Equal(t, 3, openapitest.CountDiagsAt(diags, diag.DegradedConstruct, ir.SeverityInfo), "the body schema's homeless required, the homeless error headers and the homeless "+ "error media type are three distinct defects") + + // The shared component's own `required: false` reaches both use sites, as + // two values rather than one aliased pointer: lowering it at its declaration + // de-duplicates the diagnostics, not the field. + for _, name := range []string{"postA", "postB"} { + op := openapitest.FindOp(t, doc, name) + require.NotNil(t, op.Request, "%s has a body", name) + require.NotNil(t, op.Request.Required, "%s: OpenAPI states body optionality", name) + assert.False(t, *op.Request.Required, "%s: the component declares required: false", name) + } + assert.NotSame(t, openapitest.FindOp(t, doc, "postA").Request.Required, + openapitest.FindOp(t, doc, "postB").Request.Required, + "each use site owns its flag, so an emitter mutating one cannot reach the other") } // TestDiag_DistinctDefectsAtOnePointerBothSurvive is the control for the rule From a72c1056e59d0ef2924face644a1054081700352 Mon Sep 17 00:00:00 2001 From: Fuad Daoud Date: Sat, 12 Sep 2026 13:34:56 +0300 Subject: [PATCH 9/9] docs(ir): note the 0.4.0 shape changes in flight on IRVersion The stack squash-merges bottom-up and the bump lands with its top, so a main between the first of those merges and the last carries a 0.4.0 shape stamped 0.3.0. Say so where a reader of main will look. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_011T5no6iADeMGgYjsYcV5in --- ir/document.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/ir/document.go b/ir/document.go index d1de046c..2f3d361f 100644 --- a/ir/document.go +++ b/ir/document.go @@ -19,6 +19,13 @@ package ir // 0.3.0 renames that field to Unmodeled on every carrier, so the JSON key // "preserved" is now "unmodeled". A consumer pinned to 0.2.0 finds no key it // recognizes and drops every unmodeled construct in silence. +// +// In flight: the stack/* branches (dexpace/morphic #436–#442) change the shape +// under this version — Payload gains "required", Parameter a non-omitempty +// "provenance", and more above them — and the bump to 0.4.0 lands with the +// top of that stack (#442). The stack squash-merges bottom-up, so a main +// between the first of those and the last carries a 0.4.0 shape stamped +// 0.3.0; this paragraph goes with the bump. const IRVersion = "0.3.0" // CompatibleVersion reports whether a document stamped version can be read by