From 937ed026a483cec49e09ad7897744199f8d8c258 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 03:27:05 +0300 Subject: [PATCH 1/4] fix(compilers/openapi): keep path-item docs and extra operations --- compilers/openapi/conformance_test.go | 101 +++++ .../openapi/internal/operation/operations.go | 193 ++++++-- .../operation/operations_internal_test.go | 36 ++ .../internal/operation/operations_test.go | 187 ++++++++ docs/ir-design.md | 2 +- .../openapi/path-item-docs.golden.json | 352 +++++++++++++++ .../conformance/openapi/path-item-docs.yaml | 43 ++ .../openapi/path-item-operations.golden.json | 422 ++++++++++++++++++ .../openapi/path-item-operations.yaml | 57 +++ 9 files changed, 1351 insertions(+), 42 deletions(-) create mode 100644 testdata/conformance/openapi/path-item-docs.golden.json create mode 100644 testdata/conformance/openapi/path-item-docs.yaml create mode 100644 testdata/conformance/openapi/path-item-operations.golden.json create mode 100644 testdata/conformance/openapi/path-item-operations.yaml diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d9..923b389 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -191,6 +191,8 @@ func conformanceCases() []conformanceCase { {"response-links", assertResponseLinks}, {"webhooks", assertWebhooks}, {"callbacks", assertCallbacks}, + {"path-item-docs", assertPathItemDocs}, + {"path-item-operations", assertPathItemOperations}, {"deprecation", assertDeprecation}, {"examples", assertExamples}, {"docs-summary-desc", assertDocsSummaryDesc}, @@ -1890,6 +1892,105 @@ func assertCallbacks(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { "the callback's own servers stay on the callback, not on the parent") } +// assertPathItemDocs pins that a path item's own documentation survives at every +// route that reaches a path item, and that keeping it costs the operation +// nothing it declared for itself. +// +// It is kept rather than merged: ir.Docs holds the operation's summary and +// description, a path item's pair documents the path, and merging the two would +// need a precedence rule and would attach to an operation text its author never +// wrote. So the assertion is that both subjects survive side by side. +func assertPathItemDocs(t *testing.T, doc *ir.Document, diags []ir.Diagnostic) { + for _, tc := range []struct{ op, summary, description string }{ + {"listPets", "Pet collection", "Everything addressable at /pets."}, + {"createPet", "Pet collection", "Everything addressable at /pets."}, + {"onPetCreated", "Creation callback", "Delivered once the pet exists."}, + {"onPetDeleted", "Pet deleted", "Delivered when a pet is removed."}, + } { + op, ok := opByName(doc, tc.op) + require.True(t, ok, "operation %s", tc.op) + assertPathItemDocsKept(t, op, tc.summary, tc.description) + } + + listPets, ok := opByName(doc, "listPets") + require.True(t, ok) + assert.Equal(t, "List pets", listPets.Docs.Summary, + "the operation's own summary is what Docs holds") + assert.Equal(t, "Returns every pet.", listPets.Docs.Description) + + createPet, ok := opByName(doc, "createPet") + require.True(t, ok) + assert.Empty(t, createPet.Docs.Summary, + "and an operation that documents nothing has nothing invented for it") + + // And the decision is announced at the operation that carries the entry, + // once, rather than being taken silently. + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/degraded-construct", "/paths/~1pets/get")) + assert.Equal(t, []ir.Severity{ir.SeverityInfo}, + diagsAt(diags, "openapi/degraded-construct", "/webhooks/petDeleted/post")) +} + +// assertPathItemDocsKept checks one operation kept the pair its own path item +// declared, under the keys that name which object the text came from. +func assertPathItemDocsKept(t *testing.T, op ir.Operation, summary, description string) { + t.Helper() + kept, ok := op.Unmodeled["openapi:pathItemSummary"] + require.True(t, ok, "the path item's summary is kept on %s", op.ID) + assert.Equal(t, ir.ReasonNoIRHome, kept.Reason) + assert.JSONEq(t, `"`+summary+`"`, string(kept.Value)) + + kept, ok = op.Unmodeled["openapi:pathItemDescription"] + require.True(t, ok, "the path item's description is kept on %s", op.ID) + assert.Equal(t, ir.ReasonNoIRHome, kept.Reason) + assert.JSONEq(t, `"`+description+`"`, string(kept.Value)) +} + +// assertPathItemOperations pins that every operation a 3.2 path item declares +// becomes an ir.Operation, whichever field declared it and whichever of the +// three routes reaches the path item. +// +// The walk read the fixed method fields and nothing else, so an +// additionalOperations entry was dropped entire — and with it every type +// reachable only through it, which is why the request body's schema is asserted +// to be in the registry rather than merely referenced from an operation. +func assertPathItemOperations(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { + for _, tc := range []struct{ op, method string }{ + {"queryIndex", "QUERY"}, + {"purgeIndex", "PURGE"}, + {"mixedCaseIndex", "mIxEdCase"}, + {"purgeCallback", "PURGE"}, + {"onFlush", "FLUSH"}, + } { + op, ok := opByName(doc, tc.op) + require.True(t, ok, "operation %s", tc.op) + require.Len(t, op.Bindings.HTTP, 1) + assert.Equal(t, tc.method, op.Bindings.HTTP[0].Method, + "%s binds the method as the source spelled it", tc.op) + } + + onFlush, ok := opByName(doc, "onFlush") + require.True(t, ok) + assert.True(t, onFlush.Bindings.HTTP[0].IsWebhook, + "a webhook mount marks the binding whichever field declared the operation") + + subscribe, ok := opByName(doc, "subscribeIndex") + require.True(t, ok) + require.Len(t, subscribe.Bindings.HTTP, 1) + require.Len(t, subscribe.Bindings.HTTP[0].Callbacks, 1) + purgeCallback, ok := opByName(doc, "purgeCallback") + require.True(t, ok) + assert.Equal(t, []ir.OpID{purgeCallback.ID}, subscribe.Bindings.HTTP[0].Callbacks[0].Operations, + "an expression declaring only additionalOperations still binds to its parent") + + queryIndex, ok := opByName(doc, "queryIndex") + require.True(t, ok) + require.NotNil(t, queryIndex.Request) + require.Len(t, queryIndex.Request.Contents, 1) + assert.NotNil(t, doc.Types[queryIndex.Request.Contents[0].Type.Target], + "the request body schema is interned, not merely referenced") +} + func assertDeprecation(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { op, ok := opByName(doc, "oldOp") require.True(t, ok) diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 351fb05..c9dd0af 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -25,11 +25,21 @@ const ( // statusKeyLen is the width of every responses-map key that names a status, // whether digits ("200") or a wildcard range ("2XX"). statusKeyLen = 3 + // additionalOperationsField is the Path Item Object key OpenAPI 3.2 gives to + // operations whose method has no fixed field of its own, spelled once so the + // pointer such an operation is mounted under and the field it is read from + // cannot drift apart. + additionalOperationsField = "additionalOperations" ) -// httpMethods is the fixed set of HTTP method accessors on a PathItem, iterated -// in this order so operation lowering is deterministic across runs. The name is -// the wire method in lowercase; pointers and IDs derive from it. +// httpMethods is the set of HTTP method accessors that are fixed fields on a +// PathItem, iterated in this order so operation lowering is deterministic across +// runs. The name is the field name, which is the wire method in lowercase; +// pointers and IDs derive from it. +// +// It is closed, and the fields outside it are not: OpenAPI 3.2 added `query` +// here and put every other method under `additionalOperations`, which +// pathOperations reads beside this table. var httpMethods = []struct { name string get func(*soa.PathItem) *soa.Operation @@ -42,6 +52,53 @@ var httpMethods = []struct { {"head", (*soa.PathItem).Head}, {"patch", (*soa.PathItem).Patch}, {"trace", (*soa.PathItem).Trace}, + {"query", (*soa.PathItem).Query}, +} + +// pathOperation is one operation a path item declares: the method as sent on the +// wire, the pointer segment the operation is written under, and its source node. +type pathOperation struct { + method string + seg string + src *soa.Operation +} + +// pathOperations returns every operation a path item declares — the fixed method +// fields first, in httpMethods order, then any 3.2 additionalOperations in +// source order. +// +// Reading the fixed fields alone dropped an additionalOperations entry whole: +// its operationId, parameters, request body, responses, and every type reachable +// only through them, with no diagnostic (GitHub #293). Yielding one list is what +// keeps that from recurring per route — the three walks that reach a path item +// all read this, so an operation is lowered on all of them or on none. +// +// A key is the method verbatim. ir.HTTPBinding.Method is the method as sent on +// the wire and OpenAPI reads a method name case-sensitively, so the key is +// neither upper-cased nor neutralized. A fixed field's name is a field name +// rather than a method, so that one is upper-cased into its wire spelling. +func pathOperations(pi *soa.PathItem) []pathOperation { + ops := make([]pathOperation, 0, len(httpMethods)) + for _, m := range httpMethods { + if src := m.get(pi); src != nil { + ops = append(ops, pathOperation{method: strings.ToUpper(m.name), seg: ids.Ptr(m.name), src: src}) + } + } + extra := pi.GetAdditionalOperations() + if extra == nil { + return ops + } + for method, src := range extra.All() { + if src == nil { + continue + } + ops = append(ops, pathOperation{ + method: method, + seg: ids.Ptr(additionalOperationsField, method), + src: src, + }) + } + return ops } // LowerService lowers one document into a single Service: its identity and docs, @@ -118,24 +175,20 @@ func lowerPaths(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, func lowerPathItem(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, operationIDs map[string]string, groups *serviceGroups, path string, pi *soa.PathItem, declPtr string) []ir.Diagnostic { var diags []ir.Diagnostic pathPtr := ids.Ptr("paths", path) - for _, m := range httpMethods { - src := m.get(pi) - if src == nil { - continue - } - key, name, docs, inferred := groupFor(c, src, path) - ptrs := opPointers{mount: pathPtr + ids.Ptr(m.name), decl: declPtr + ids.Ptr(m.name)} + for _, po := range pathOperations(pi) { + key, name, docs, inferred := groupFor(c, po.src, path) + ptrs := opPointers{mount: pathPtr + po.seg, decl: declPtr + po.seg} opCtx := opContext{ - method: m.name, + method: po.method, uriTemplate: path, withCallbacks: true, inferred: inferred, ptrs: ptrs, - params: mergeParameters(pi.GetParameters(), src.GetParameters(), declPtr, ptrs.decl), + params: mergeParameters(pi.GetParameters(), po.src.GetParameters(), declPtr, ptrs.decl), } - op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) + op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, po.src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) + diags = append(diags, applyPathItemResidue(c, &op, pi, declPtr)...) grp := groups.group(key, func() ir.OperationGroup { return ir.OperationGroup{Name: name, Docs: docs} }) grp.Operations = append(grp.Operations, op) grp.Operations = append(grp.Operations, extra...) @@ -157,23 +210,19 @@ func lowerWebhooks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde if pi == nil { continue } - for _, m := range httpMethods { - src := m.get(pi) - if src == nil { - continue - } - ptrs := opPointers{mount: hookPtr + ids.Ptr(m.name), decl: declPtr + ids.Ptr(m.name)} + for _, po := range pathOperations(pi) { + ptrs := opPointers{mount: hookPtr + po.seg, decl: declPtr + po.seg} opCtx := opContext{ - method: m.name, + method: po.method, uriTemplate: name, isWebhook: true, withCallbacks: true, ptrs: ptrs, - params: mergeParameters(pi.GetParameters(), src.GetParameters(), declPtr, ptrs.decl), + params: mergeParameters(pi.GetParameters(), po.src.GetParameters(), declPtr, ptrs.decl), } - op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) + op, extra, opDiags := lowerOperation(c, ts, anchors, operationIDs, po.src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, declPtr)...) + diags = append(diags, applyPathItemResidue(c, &op, pi, declPtr)...) grp := groups.group("webhook", func() ir.OperationGroup { // A hint, not a source name: no document declares this group. The // compiler synthesizes it to hold webhook operations, exactly as it @@ -232,6 +281,9 @@ type opPointers struct { // still carrying the pointer of its own declaration site rather than a position // in the merged list. type opContext struct { + // method is the method as sent on the wire — a fixed field's name upper-cased, + // or an additionalOperations key exactly as the source spelled it — so nothing + // downstream has to know which of the two declared the operation. method string uriTemplate string isWebhook bool @@ -274,7 +326,7 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd diags = append(diags, responseDiags...) op.Responses, op.Errors = responses, errs hb := ir.HTTPBinding{ - Method: strings.ToUpper(opCtx.method), + Method: opCtx.method, URITemplate: opCtx.uriTemplate, IsWebhook: opCtx.isWebhook, ParamBindings: bindings, @@ -376,16 +428,78 @@ func fillOperationDocs(d *ir.Docs, src *soa.Operation) { } } +// applyPathItemResidue keeps everything a path item declares that an +// ir.Operation has no field for: its servers, and its own documentation. +// +// One call per route rather than one per construct. The servers half reached +// only the `paths` walk for exactly as long as each route spelled it out for +// itself (GitHub #39), so the two are gathered here and every route calls this — +// a third such construct is then either kept on all three routes or on none. +func applyPathItemResidue(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { + diags := applyPathServers(c, op, pi, declPtr) + return append(diags, applyPathItemDocs(c, op, pi, declPtr)...) +} + +// pathItemDocFields pairs each Path Item Object documentation keyword with the +// Unmodeled key it is kept under. Both keys name the object the text came from: +// an operation's own summary and description are what ir.Docs holds, so a plain +// `openapi:summary` beside them would read as the operation's own. +var pathItemDocFields = []struct{ keyword, key string }{ + {"summary", "openapi:pathItemSummary"}, + {"description", "openapi:pathItemDescription"}, +} + +// applyPathItemDocs keeps a path item's summary and description verbatim under +// Unmodeled on each operation it holds. Nothing read either one, so both reached +// the IR in no form at all — no field, no Unmodeled entry, no diagnostic +// (GitHub #292). +// +// Kept rather than merged into Docs, deliberately. ir.Docs holds one summary and +// one description and they are the operation's own; a path item's pair documents +// the path. Merging would need a precedence rule against the operation's own, +// would attach to an operation documentation its author did not write — an +// inference, which invariant 6 places in injectable policy rather than in a +// lowering — and would leave an emitter unable to tell the two subjects apart +// afterwards. Preserving takes no position on precedence and loses nothing, +// which is what invariant 2 asks of a construct with no typed home. +// +// ReasonNoIRHome rather than a boundary: the IR could grow a home for this, and +// GitHub #285 is where that class of decision is tracked. The cost is that the +// pair is duplicated onto every operation under the path, exactly as the path +// item's servers already are — a path item is distributed across the operations +// it holds, and there is no ir.PathItem to attach it to. +func applyPathItemDocs(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { + if pi.GetSummary() == "" && pi.GetDescription() == "" { + return nil + } + var diags []ir.Diagnostic + var kept bool + for _, f := range pathItemDocFields { + wrote, fieldDiags := schema.PreserveNode(c, &op.Unmodeled, f.key, + annotation.RawChildNode(pi.GetRootNode(), f.keyword), ir.ReasonNoIRHome, + declPtr+ids.Ptr(f.keyword)) + diags = append(diags, fieldDiags...) + kept = kept || wrote + } + if !kept { + return diags + } + return append(diags, diag.Newf(ir.SeverityInfo, diag.DegradedConstruct, op.Provenance, + "path-item documentation kept under Unmodeled; it documents the path rather than this "+ + "operation, and ir.Docs holds the operation's own summary and description")) +} + // applyPathServers preserves path-item-level servers verbatim under Unmodeled // on the operation. §10 models servers as Document.Servers with per-scope index // lists (Service.Servers, Channel.Servers); ir.Operation just has no such list // yet, so the scoping is kept raw with an info diagnostic — a gap the IR can // close by adding one, hence ReasonNoIRHome rather than a boundary. // -// Every route that lowers a path item reaches it: a path, a webhook, and a -// callback expression are the same object under three parents, and a document -// that overrides the server for one of the latter two was losing the override -// outright while the paths route reported it (GitHub #39). +// Every route that lowers a path item reaches it through applyPathItemResidue: +// a path, a webhook, and a callback expression are the same object under three +// parents, and a document that overrides the server for one of the latter two +// was losing the override outright while the paths route reported it +// (GitHub #39). // // This is the path-item half of the pair; applyOperationServers keeps the // operation's own list, which overrides this one, under its own key. @@ -614,25 +728,22 @@ func lowerCallbacks(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd // declaration base (shared when the callback or its path item is $ref'd; // issue #107). func lowerCallbackOps(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorIndex, operationIDs map[string]string, pi *soa.PathItem, cb opPointers, expr, inferred string) ([]ir.OpID, []ir.Operation, []ir.Diagnostic) { - var opIDs []ir.OpID - var ops []ir.Operation + declared := pathOperations(pi) + opIDs := make([]ir.OpID, 0, len(declared)) + ops := make([]ir.Operation, 0, len(declared)) var diags []ir.Diagnostic - for _, m := range httpMethods { - src := m.get(pi) - if src == nil { - continue - } - ptrs := opPointers{mount: cb.mount + ids.Ptr(m.name), decl: cb.decl + ids.Ptr(m.name)} + for _, po := range declared { + ptrs := opPointers{mount: cb.mount + po.seg, decl: cb.decl + po.seg} opCtx := opContext{ - method: m.name, + method: po.method, uriTemplate: expr, inferred: inferred, ptrs: ptrs, - params: mergeParameters(pi.GetParameters(), src.GetParameters(), cb.decl, ptrs.decl), + params: mergeParameters(pi.GetParameters(), po.src.GetParameters(), cb.decl, ptrs.decl), } - op, _, opDiags := lowerOperation(c, ts, anchors, operationIDs, src, opCtx) + op, _, opDiags := lowerOperation(c, ts, anchors, operationIDs, po.src, opCtx) diags = append(diags, opDiags...) - diags = append(diags, applyPathServers(c, &op, pi, cb.decl)...) + diags = append(diags, applyPathItemResidue(c, &op, pi, cb.decl)...) opIDs = append(opIDs, op.ID) ops = append(ops, op) } diff --git a/compilers/openapi/internal/operation/operations_internal_test.go b/compilers/openapi/internal/operation/operations_internal_test.go index 2004025..ce128f5 100644 --- a/compilers/openapi/internal/operation/operations_internal_test.go +++ b/compilers/openapi/internal/operation/operations_internal_test.go @@ -313,3 +313,39 @@ func TestFaultFor_ClassifiesAtTheClassBoundaries(t *testing.T) { }) } } + +// TestApplyPathItemDocs_WithoutRootNode is the documentation counterpart of the +// two tests above: a declared pair whose source node cannot be read keeps +// nothing, and announces nothing it did not keep. +func TestApplyPathItemDocs_WithoutRootNode(t *testing.T) { + t.Parallel() + l := newRawLowerer(&soa.OpenAPI{}) + summary, description := "documented", "at length" + op := &ir.Operation{} + + diags := applyPathItemDocs(l.ctx, op, + &soa.PathItem{Summary: &summary, Description: &description}, "/paths/~1a") + + assert.Nil(t, op.Unmodeled, "documentation with no raw node is not preserved") + assert.Empty(t, diags) +} + +// TestPathOperations_NilAdditionalOperationSkipped pins the guard on the +// additionalOperations walk. The parser never yields a nil entry, but the map is +// a plain pointer map and lowerOperation reads the operation's fields directly, +// so a nil would panic rather than lower. +func TestPathOperations_NilAdditionalOperationSkipped(t *testing.T) { + t.Parallel() + pi := &soa.PathItem{ + AdditionalOperations: sequencedmap.New( + sequencedmap.NewElem("EMPTY", (*soa.Operation)(nil)), + sequencedmap.NewElem("PURGE", &soa.Operation{}), + ), + } + + ops := pathOperations(pi) + + require.Len(t, ops, 1, "the nil entry is skipped and the real one is not") + assert.Equal(t, "PURGE", ops[0].method) + assert.Equal(t, "/additionalOperations/PURGE", ops[0].seg) +} diff --git a/compilers/openapi/internal/operation/operations_test.go b/compilers/openapi/internal/operation/operations_test.go index b621fdb..da7fc26 100644 --- a/compilers/openapi/internal/operation/operations_test.go +++ b/compilers/openapi/internal/operation/operations_test.go @@ -1671,3 +1671,190 @@ func TestOperations_OwnServersSurviveBesideExtensions(t *testing.T) { assert.Contains(t, op.Unmodeled, "openapi:x-vendor", "and the extensions survive beside them, so neither overwrote the other") } + +// pathItemDocsSpec declares summary and description on each of the three path +// items a document can hold — a path, a webhook, and a callback expression — +// with distinct text apiece so a preserved pair can be traced to the path item +// that wrote it. One operation declares its own summary beside them, which is +// what shows that the path item's pair does not displace it. +const pathItemDocsSpec = `openapi: 3.1.0 +info: {title: T, version: "1"} +paths: + /p: + summary: path summary + description: path description + post: + operationId: postP + summary: operation summary + callbacks: + onEvent: + '{$request.body#/url}': + summary: callback summary + description: callback description + post: + operationId: onEvent + responses: {"200": {description: ok}} + responses: {"200": {description: ok}} +webhooks: + hooked: + summary: webhook summary + description: webhook description + post: + operationId: onHook + responses: {"200": {description: ok}} +` + +// TestPathItem_DocsKeptOnEveryRoute pins that a path item's summary and +// description reach the IR at all, on whichever of the three routes reaches the +// path item. +// +// Both were read nowhere: fillOperationDocs reads the Operation Object's own +// pair, so a path item's documentation reached neither Docs, nor Unmodeled, nor +// a diagnostic. Each route is asserted with its own text, so a fix that kept the +// enclosing path item's pair — or the same one three times — fails rather than +// passing on the shape alone. +func TestPathItem_DocsKeptOnEveryRoute(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathItemDocsSpec) + requireNoErrorDiags(t, diags) + + // kept is the path item's own pointer, where the pair is declared; + // reported is the operation, which is what carries the entry. + for _, tc := range []struct{ op, text, kept, reported string }{ + {"postP", "path", "/paths/~1p", "/paths/~1p/post"}, + {"onHook", "webhook", "/webhooks/hooked", "/webhooks/hooked/post"}, + {"onEvent", "callback", + "/paths/~1p/post/callbacks/onEvent/{$request.body#~1url}", + "/paths/~1p/post/callbacks/onEvent/{$request.body#~1url}/post"}, + } { + op := findOp(t, doc, tc.op) + for _, field := range []struct{ key, keyword string }{ + {"openapi:pathItemSummary", "summary"}, + {"openapi:pathItemDescription", "description"}, + } { + entry, ok := op.Unmodeled[field.key] + require.True(t, ok, "%s keeps its path item's %s", tc.op, field.keyword) + assert.Equal(t, ir.ReasonNoIRHome, entry.Reason) + assert.JSONEq(t, `"`+tc.text+` `+field.keyword+`"`, string(entry.Value), + "%s keeps the text its own path item declared", tc.op) + assert.Equal(t, tc.kept+"/"+field.keyword, entry.Provenance.Pointer) + } + assert.True(t, hasDiagCodeAt(diags, diag.DegradedConstruct, tc.reported), + "%s reports the path item's documentation as kept rather than lowered", tc.op) + } + + assert.Equal(t, "operation summary", findOp(t, doc, "postP").Docs.Summary, + "an operation's own summary is what Docs holds; the path item's never displaces it") + assert.Empty(t, findOp(t, doc, "onHook").Docs.Summary, + "and an operation that declares none gets none invented for it") +} + +// TestPathItem_DocsAbsentKeepNothing pins the other half: a path item that +// documents nothing writes no entry and announces nothing, so the preservation +// is not a per-operation constant. +func TestPathItem_DocsAbsentKeepNothing(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathsSpec(` /p: + get: {operationId: getP, responses: {"200": {description: ok}}} +`)) + requireNoErrorDiags(t, diags) + + op := findOp(t, doc, "getP") + assert.NotContains(t, op.Unmodeled, "openapi:pathItemSummary") + assert.NotContains(t, op.Unmodeled, "openapi:pathItemDescription") + assert.False(t, hasDiagCodeAt(diags, diag.DegradedConstruct, "/paths/~1p/get")) +} + +// pathItemOperationsSpec declares a 3.2 operation with no fixed field of its own +// at each of the three path-item routes, plus the 3.2 `query` fixed field, with +// a distinct operationId apiece. +const pathItemOperationsSpec = `openapi: 3.2.0 +info: {title: T, version: "1"} +paths: + /p: + query: + operationId: queryP + responses: {"200": {description: ok}} + post: + operationId: postP + callbacks: + onEvent: + '{$request.body#/url}': + additionalOperations: + PURGE: + operationId: purgeCallback + responses: {"204": {description: purged}} + responses: {"200": {description: ok}} + additionalOperations: + PURGE: + operationId: purgeP + requestBody: + content: + application/json: {schema: {type: object, properties: {n: {type: string}}}} + responses: {"204": {description: purged}} + lowercase-purge: + operationId: purgeVerbatim + responses: {"204": {description: purged}} +webhooks: + hooked: + additionalOperations: + FLUSH: + operationId: flushHook + responses: {"204": {description: flushed}} +` + +// TestPathItem_AdditionalOperationsLowerOnEveryRoute pins that an operation +// declared under 3.2 additionalOperations becomes an ir.Operation like any +// other, on whichever of the three routes reaches the path item. +// +// The loop over the fixed method fields was the whole of the walk, so every such +// operation was dropped entire — operationId, parameters, request body, +// responses — with no diagnostic. Each route is asserted separately because a +// fix scoped to the paths walk leaves the other two exactly as they were. +func TestPathItem_AdditionalOperationsLowerOnEveryRoute(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathItemOperationsSpec) + requireNoErrorDiags(t, diags) + + for _, tc := range []struct{ op, method, id string }{ + {"queryP", "QUERY", "op/openapi/paths/~1p/query"}, + {"purgeP", "PURGE", "op/openapi/paths/~1p/additionalOperations/PURGE"}, + {"purgeVerbatim", "lowercase-purge", "op/openapi/paths/~1p/additionalOperations/lowercase-purge"}, + {"flushHook", "FLUSH", "op/openapi/webhooks/hooked/additionalOperations/FLUSH"}, + {"purgeCallback", "PURGE", + "op/openapi/paths/~1p/post/callbacks/onEvent/{$request.body#~1url}/additionalOperations/PURGE"}, + } { + op := findOp(t, doc, tc.op) + assert.Equal(t, ir.OpID(tc.id), op.ID, "%s is identified by where it is written", tc.op) + require.Len(t, op.Bindings.HTTP, 1) + assert.Equal(t, tc.method, op.Bindings.HTTP[0].Method, + "%s binds the method key as the source spelled it", tc.op) + } + + assert.True(t, findOp(t, doc, "flushHook").Bindings.HTTP[0].IsWebhook, + "a webhook mount marks the binding whichever field declared the operation") + + // Nothing reachable only through a dropped operation reached the registry + // either: the request body's schema was not interned at all. + purge := findOp(t, doc, "purgeP") + require.NotNil(t, purge.Request) + require.Len(t, purge.Request.Contents, 1) + assert.NotNil(t, doc.Types[purge.Request.Contents[0].Type.Target], + "the request body schema is interned, not merely referenced") +} + +// TestPathItem_AdditionalOperationsBindCallbacks pins that the parent operation +// records a callback lowered from additionalOperations alone, so an expression +// whose path item declares no fixed method is still bound rather than left +// holding an empty operation list. +func TestPathItem_AdditionalOperationsBindCallbacks(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, pathItemOperationsSpec) + requireNoErrorDiags(t, diags) + + parent := findOp(t, doc, "postP") + require.Len(t, parent.Bindings.HTTP, 1) + require.Len(t, parent.Bindings.HTTP[0].Callbacks, 1) + assert.Equal(t, []ir.OpID{findOp(t, doc, "purgeCallback").ID}, + parent.Bindings.HTTP[0].Callbacks[0].Operations) +} diff --git a/docs/ir-design.md b/docs/ir-design.md index a72ceea..6cf2e29 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1698,7 +1698,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and its `content` map whatever its arity → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field: it holds one TypeRef and no media type, so one entry loses the key it was written under just as several lose all but the first); response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and its `content` map whatever its arity → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field: it holds one TypeRef and no media type, so one entry loses the key it was written under just as several lose all but the first); response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; 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; securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled | diff --git a/testdata/conformance/openapi/path-item-docs.golden.json b/testdata/conformance/openapi/path-item-docs.golden.json new file mode 100644 index 0000000..a991f58 --- /dev/null +++ b/testdata/conformance/openapi/path-item-docs.golden.json @@ -0,0 +1,352 @@ +{ + "irVersion": "0.3.0", + "name": "PathItemDocs", + "version": "1.0.0", + "docs": {}, + "services": [ + { + "id": "s/openapi/0", + "name": { + "source": "PathItemDocs", + "canonical": "path_item_docs" + }, + "docs": {}, + "groups": [ + { + "name": { + "hint": "default" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/paths/~1pets/get", + "name": { + "source": "listPets", + "canonical": "list_pets" + }, + "docs": { + "summary": "List pets", + "description": "Returns every pet." + }, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "GET", + "uriTemplate": "/pets", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "unmodeled": { + "openapi:pathItemDescription": { + "reason": "no_ir_home", + "value": "Everything addressable at /pets.", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/description" + } + }, + "openapi:pathItemSummary": { + "reason": "no_ir_home", + "value": "Pet collection", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/summary" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/get" + } + }, + { + "id": "op/openapi/paths/~1pets/post", + "name": { + "source": "createPet", + "canonical": "create_pet" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "201" + }, + "conditions": { + "statusCodes": [ + { + "from": 201, + "to": 201 + } + ] + }, + "docs": { + "description": "created" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "/pets", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false, + "callbacks": [ + { + "expression": "{$request.body#/callbackUrl}", + "operations": [ + "op/openapi/paths/~1pets/post/callbacks/onCreated/{$request.body#~1callbackUrl}/post" + ] + } + ] + } + ] + }, + "unmodeled": { + "openapi:pathItemDescription": { + "reason": "no_ir_home", + "value": "Everything addressable at /pets.", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/description" + } + }, + "openapi:pathItemSummary": { + "reason": "no_ir_home", + "value": "Pet collection", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/summary" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post" + } + }, + { + "id": "op/openapi/paths/~1pets/post/callbacks/onCreated/{$request.body#~1callbackUrl}/post", + "name": { + "source": "onPetCreated", + "canonical": "on_pet_created" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "{$request.body#/callbackUrl}", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "unmodeled": { + "openapi:pathItemDescription": { + "reason": "no_ir_home", + "value": "Delivered once the pet exists.", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post/callbacks/onCreated/{$request.body#~1callbackUrl}/description" + } + }, + "openapi:pathItemSummary": { + "reason": "no_ir_home", + "value": "Creation callback", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post/callbacks/onCreated/{$request.body#~1callbackUrl}/summary" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post/callbacks/onCreated/{$request.body#~1callbackUrl}/post" + } + } + ] + }, + { + "name": { + "hint": "webhooks" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/webhooks/petDeleted/post", + "name": { + "source": "onPetDeleted", + "canonical": "on_pet_deleted" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "petDeleted", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": true + } + ] + }, + "unmodeled": { + "openapi:pathItemDescription": { + "reason": "no_ir_home", + "value": "Delivered when a pet is removed.", + "provenance": { + "source": 0, + "pointer": "/webhooks/petDeleted/description" + } + }, + "openapi:pathItemSummary": { + "reason": "no_ir_home", + "value": "Pet deleted", + "provenance": { + "source": 0, + "pointer": "/webhooks/petDeleted/summary" + } + } + }, + "provenance": { + "source": 0, + "pointer": "/webhooks/petDeleted/post" + } + } + ] + } + ], + "auth": null, + "provenance": { + "source": 0 + } + } + ], + "servers": [ + { + "name": { + "hint": "server" + }, + "urlTemplate": "/", + "description": {}, + "auth": null + } + ], + "diagnostics": [ + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "path-item documentation kept under Unmodeled; it documents the path rather than this operation, and ir.Docs holds the operation's own summary and description", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/get" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "path-item documentation kept under Unmodeled; it documents the path rather than this operation, and ir.Docs holds the operation's own summary and description", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post/callbacks/onCreated/{$request.body#~1callbackUrl}/post" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "path-item documentation kept under Unmodeled; it documents the path rather than this operation, and ir.Docs holds the operation's own summary and description", + "provenance": { + "source": 0, + "pointer": "/paths/~1pets/post" + } + }, + { + "severity": "info", + "code": "openapi/degraded-construct", + "message": "path-item documentation kept under Unmodeled; it documents the path rather than this operation, and ir.Docs holds the operation's own summary and description", + "provenance": { + "source": 0, + "pointer": "/webhooks/petDeleted/post" + } + } + ], + "sources": [ + { + "format": "openapi@3.1", + "path": "path-item-docs.yaml", + "hash": "b6a7b4af97bda527e50e35d3d60aa65fc2bd82647d945da557d50360db015550" + } + ] +} diff --git a/testdata/conformance/openapi/path-item-docs.yaml b/testdata/conformance/openapi/path-item-docs.yaml new file mode 100644 index 0000000..096f3fc --- /dev/null +++ b/testdata/conformance/openapi/path-item-docs.yaml @@ -0,0 +1,43 @@ +openapi: 3.1.0 +info: {title: PathItemDocs, version: "1.0.0"} +paths: + /pets: + # A path item's summary and description document the path, not the + # operations mounted under it, and ir.Docs on an operation already holds the + # operation's own pair. Both survive: the operation's in Docs, the path + # item's kept verbatim beside it on every operation the path item holds. + summary: Pet collection + description: Everything addressable at /pets. + get: + operationId: listPets + summary: List pets + description: Returns every pet. + responses: + "200": + description: ok + post: + operationId: createPet + callbacks: + onCreated: + '{$request.body#/callbackUrl}': + # A callback path item documents its path exactly as a path does. + summary: Creation callback + description: Delivered once the pet exists. + post: + operationId: onPetCreated + responses: + "200": + description: ok + responses: + "201": + description: created +webhooks: + petDeleted: + # And so does a webhook path item. + summary: Pet deleted + description: Delivered when a pet is removed. + post: + operationId: onPetDeleted + responses: + "200": + description: ok diff --git a/testdata/conformance/openapi/path-item-operations.golden.json b/testdata/conformance/openapi/path-item-operations.golden.json new file mode 100644 index 0000000..2549123 --- /dev/null +++ b/testdata/conformance/openapi/path-item-operations.golden.json @@ -0,0 +1,422 @@ +{ + "irVersion": "0.3.0", + "name": "PathItemOperations", + "version": "1.0.0", + "docs": {}, + "services": [ + { + "id": "s/openapi/0", + "name": { + "source": "PathItemOperations", + "canonical": "path_item_operations" + }, + "docs": {}, + "groups": [ + { + "name": { + "hint": "default" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/paths/~1index/post", + "name": { + "source": "subscribeIndex", + "canonical": "subscribe_index" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "POST", + "uriTemplate": "/index", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false, + "callbacks": [ + { + "expression": "{$request.body#/callbackUrl}", + "operations": [ + "op/openapi/paths/~1index/post/callbacks/onEvent/{$request.body#~1callbackUrl}/additionalOperations/PURGE" + ] + } + ] + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/post" + } + }, + { + "id": "op/openapi/paths/~1index/post/callbacks/onEvent/{$request.body#~1callbackUrl}/additionalOperations/PURGE", + "name": { + "source": "purgeCallback", + "canonical": "purge_callback" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "204" + }, + "conditions": { + "statusCodes": [ + { + "from": 204, + "to": 204 + } + ] + }, + "docs": { + "description": "purged" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "PURGE", + "uriTemplate": "{$request.body#/callbackUrl}", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/post/callbacks/onEvent/{$request.body#~1callbackUrl}/additionalOperations/PURGE" + } + }, + { + "id": "op/openapi/paths/~1index/query", + "name": { + "source": "queryIndex", + "canonical": "query_index" + }, + "docs": {}, + "request": { + "contents": [ + { + "mediaType": "application/json", + "type": { + "target": "t/anon/paths/~1index/query/requestBody/content/application~1json/schema", + "nullable": false + } + } + ], + "unmodeled": { + "openapi:required": { + "reason": "no_ir_home", + "value": false, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/query/requestBody/required" + } + } + } + }, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "QUERY", + "uriTemplate": "/index", + "sharedRoute": false, + "requestContentTypes": [ + "application/json" + ], + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/query" + } + }, + { + "id": "op/openapi/paths/~1index/additionalOperations/PURGE", + "name": { + "source": "purgeIndex", + "canonical": "purge_index" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "204" + }, + "conditions": { + "statusCodes": [ + { + "from": 204, + "to": 204 + } + ] + }, + "docs": { + "description": "purged" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "PURGE", + "uriTemplate": "/index", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/additionalOperations/PURGE" + } + }, + { + "id": "op/openapi/paths/~1index/additionalOperations/mIxEdCase", + "name": { + "source": "mixedCaseIndex", + "canonical": "mixed_case_index" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "204" + }, + "conditions": { + "statusCodes": [ + { + "from": 204, + "to": 204 + } + ] + }, + "docs": { + "description": "purged" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "mIxEdCase", + "uriTemplate": "/index", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": false + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/additionalOperations/mIxEdCase" + } + } + ] + }, + { + "name": { + "hint": "webhooks" + }, + "docs": {}, + "operations": [ + { + "id": "op/openapi/webhooks/cacheFlushed/additionalOperations/FLUSH", + "name": { + "source": "onFlush", + "canonical": "on_flush" + }, + "docs": {}, + "responses": [ + { + "name": { + "hint": "200" + }, + "conditions": { + "statusCodes": [ + { + "from": 200, + "to": 200 + } + ] + }, + "docs": { + "description": "ok" + } + } + ], + "oneWay": false, + "idempotency": {}, + "auth": null, + "bindings": { + "http": [ + { + "method": "FLUSH", + "uriTemplate": "cacheFlushed", + "sharedRoute": false, + "checksumRequired": false, + "isWebhook": true + } + ] + }, + "provenance": { + "source": 0, + "pointer": "/webhooks/cacheFlushed/additionalOperations/FLUSH" + } + } + ] + } + ], + "auth": null, + "provenance": { + "source": 0 + } + } + ], + "types": { + "t/anon/paths/~1index/query/requestBody/content/application~1json/schema": { + "kind": "model", + "id": "t/anon/paths/~1index/query/requestBody/content/application~1json/schema", + "name": { + "hint": "queryIndex_request" + }, + "anonymous": true, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/query/requestBody/content/application~1json/schema" + }, + "properties": [ + { + "id": "p/openapi/paths/~1index/query/requestBody/content/application~1json/schema/properties/q", + "name": { + "source": "q", + "canonical": "q" + }, + "wireName": "q", + "type": { + "target": "t/prim/string", + "nullable": false + }, + "required": false, + "clientOptional": false, + "defaultAdded": false, + "visibility": { + "none": false + }, + "flatten": false, + "eventHeader": false, + "eventPayload": false, + "secret": false, + "docs": {}, + "provenance": { + "source": 0, + "pointer": "/paths/~1index/query/requestBody/content/application~1json/schema/properties/q" + } + } + ], + "abstract": false, + "positional": false, + "inputOnly": false + }, + "t/prim/string": { + "kind": "primitive", + "id": "t/prim/string", + "name": {}, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0 + }, + "prim": "string" + } + }, + "servers": [ + { + "name": { + "hint": "server" + }, + "urlTemplate": "/", + "description": {}, + "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", + "path": "path-item-operations.yaml", + "hash": "047cabe1682a7c78101093e9f4a502a0289ea7cc648f77d231b2a09b2a15bff2" + } + ] +} diff --git a/testdata/conformance/openapi/path-item-operations.yaml b/testdata/conformance/openapi/path-item-operations.yaml new file mode 100644 index 0000000..75901fa --- /dev/null +++ b/testdata/conformance/openapi/path-item-operations.yaml @@ -0,0 +1,57 @@ +openapi: 3.2.0 +info: {title: PathItemOperations, version: "1.0.0"} +paths: + /index: + # OpenAPI 3.2 gives QUERY a fixed field of its own and puts every other + # method under additionalOperations, keyed by the method name. Both declare + # ordinary Operation Objects, so both lower to ordinary operations — with + # everything reachable through them, such as this request body's schema. + query: + operationId: queryIndex + requestBody: + content: + application/json: + schema: + type: object + properties: + q: {type: string} + responses: + "200": + description: ok + post: + operationId: subscribeIndex + callbacks: + onEvent: + '{$request.body#/callbackUrl}': + # A callback path item carries additionalOperations like any other, + # and this expression declares no fixed method at all. + additionalOperations: + PURGE: + operationId: purgeCallback + responses: + "204": + description: purged + responses: + "200": + description: ok + additionalOperations: + PURGE: + operationId: purgeIndex + responses: + "204": + description: purged + # A method name is case-sensitive, so the key reaches the binding exactly + # as it was written rather than upper-cased into a conventional spelling. + mIxEdCase: + operationId: mixedCaseIndex + responses: + "204": + description: purged +webhooks: + cacheFlushed: + additionalOperations: + FLUSH: + operationId: onFlush + responses: + "200": + description: ok From 792991cf452e06c8251fd70281b1c77eccd46178 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 11:02:42 +0300 Subject: [PATCH 2/4] fix(compilers/openapi): report an empty additionalOperations key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The key becomes ir.HTTPBinding.Method verbatim, which is right for a method name OpenAPI reads case-sensitively, and is what lets an empty key through as an empty method — a binding no request can be sent with. speakeasy rejects a key naming a standard method, since that method has a fixed field of its own, but accepts this one, so nothing upstream reports it. The entry still lowers. Dropping it would trade a reported defect for a silent one, which is the trade this change exists to undo, so the operation and everything it declares survive and a warning names the entry that declared it. The check sits in lowerOperation because all three routes funnel through it, so a fourth would be covered without being remembered. A fixed field's method is a field name upper-cased and can never be empty. --- compilers/openapi/internal/diag/diag.go | 6 +++ compilers/openapi/internal/diag/diag_test.go | 2 +- .../openapi/internal/operation/operations.go | 9 +++++ .../internal/operation/operations_test.go | 37 +++++++++++++++++++ 4 files changed, 53 insertions(+), 1 deletion(-) diff --git a/compilers/openapi/internal/diag/diag.go b/compilers/openapi/internal/diag/diag.go index 5902708..286f561 100644 --- a/compilers/openapi/internal/diag/diag.go +++ b/compilers/openapi/internal/diag/diag.go @@ -82,6 +82,12 @@ const ( // response still lowers, with no status condition rather than the catch-all // range that "default" alone denotes (GitHub #262). InvalidStatusKey = "openapi/invalid-status-key" + // InvalidMethodKey reports an additionalOperations key that names no method: + // the empty string. The operation still lowers, binding the key as written, so + // nothing the entry declares is lost — what is reported is that the binding's + // method is unusable. speakeasy rejects a key naming a *standard* method, which + // belongs in its own field, but accepts this one. + InvalidMethodKey = "openapi/invalid-method-key" // DegradedConstruct reports a construct the compiler could not carry into the // IR as written: preserved raw for want of a structural home, lowered to a // weaker shape (a heterogeneous enum as a union, an unconvertible value as diff --git a/compilers/openapi/internal/diag/diag_test.go b/compilers/openapi/internal/diag/diag_test.go index 26e351a..c65156b 100644 --- a/compilers/openapi/internal/diag/diag_test.go +++ b/compilers/openapi/internal/diag/diag_test.go @@ -132,7 +132,7 @@ func codes() []string { diag.OverlayAction, diag.OverlayOriginIncomplete, diag.ValidationOnlyKeyword, diag.FalseSchema, diag.NumericPrecision, diag.ExclusiveBoundForm, diag.InvalidStatusKey, - diag.DegradedConstruct, + diag.InvalidMethodKey, diag.DegradedConstruct, diag.CompositionLowering, diag.DynamicRefExpanded, diag.ConflictingRedecl, diag.DisjointVisibility, diag.AliasAmplification, diag.UnattachableRequired, diag.InternalInvariant, diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index c9dd0af..91a5578 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -77,6 +77,10 @@ type pathOperation struct { // the wire and OpenAPI reads a method name case-sensitively, so the key is // neither upper-cased nor neutralized. A fixed field's name is a field name // rather than a method, so that one is upper-cased into its wire spelling. +// +// Taking the key verbatim means an empty one reaches the binding as an empty +// method. It still lowers — dropping the entry would lose everything it declares +// — and lowerOperation reports it, which is where every route funnels through. func pathOperations(pi *soa.PathItem) []pathOperation { ops := make([]pathOperation, 0, len(httpMethods)) for _, m := range httpMethods { @@ -325,6 +329,11 @@ func lowerOperation(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInd responses, errs, responseDiags := lowerResponses(c, ts, anchors, src, decl) diags = append(diags, responseDiags...) op.Responses, op.Errors = responses, errs + if opCtx.method == "" { + diags = append(diags, c.DiagAt(ir.SeverityWarning, diag.InvalidMethodKey, decl, + "additionalOperations key names no method; the operation lowers with an empty "+ + "HTTP method, which no request can be sent with")) + } hb := ir.HTTPBinding{ Method: opCtx.method, URITemplate: opCtx.uriTemplate, diff --git a/compilers/openapi/internal/operation/operations_test.go b/compilers/openapi/internal/operation/operations_test.go index da7fc26..9662578 100644 --- a/compilers/openapi/internal/operation/operations_test.go +++ b/compilers/openapi/internal/operation/operations_test.go @@ -1858,3 +1858,40 @@ func TestPathItem_AdditionalOperationsBindCallbacks(t *testing.T) { assert.Equal(t, []ir.OpID{findOp(t, doc, "purgeCallback").ID}, parent.Bindings.HTTP[0].Callbacks[0].Operations) } + +// TestPathItem_EmptyAdditionalOperationsKeyReported pins the one key that names +// no method. Taking the key verbatim is what makes it reachable: an empty one +// binds an empty HTTP method, which no request can be sent with. +// +// The entry still lowers, so nothing it declares is lost — dropping it would +// trade a reported defect for a silent one, which is the trade this whole change +// exists to undo. speakeasy rejects a key naming a standard method and accepts +// this one, so the compiler is the only thing that can report it. +func TestPathItem_EmptyAdditionalOperationsKeyReported(t *testing.T) { + t.Parallel() + doc, diags := parseFull(t, `openapi: 3.2.0 +info: {title: T, version: "1"} +paths: + /p: + additionalOperations: + "": + operationId: nameless + responses: {"204": {description: done}} +`) + requireNoErrorDiags(t, diags) + + op := findOp(t, doc, "nameless") + require.Len(t, op.Bindings.HTTP, 1) + assert.Empty(t, op.Bindings.HTTP[0].Method, "the key binds as written, empty or not") + assert.Equal(t, ir.OpID("op/openapi/paths/~1p/additionalOperations/"), op.ID, + "and the operation is still mounted where the source writes it") + + assert.True(t, hasDiagCodeAt(diags, diag.InvalidMethodKey, "/paths/~1p/additionalOperations/"), + "the unusable method is reported at the entry that declares it") + for _, d := range diags { + if d.Code == diag.InvalidMethodKey { + assert.Equal(t, ir.SeverityWarning, d.Severity, + "a warning: the operation lowers in full, only its method is unusable") + } + } +} From 26c8248aa780a14d53a13c547748a9bf01ac5979 Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 11:03:02 +0300 Subject: [PATCH 3/4] docs(compilers/openapi): record what path-item docs do not reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cases keep nothing and report nothing, and both are shared with the servers half beside them rather than introduced by the documentation one. A path item that mounts no operation has no operation to carry the entry, so its pair is lost whole; #383 records that gap for the entire class and the unmounted-path-item work is where it lands. And RawChildNode scans a mapping's pairs directly, so a pair arriving through a YAML merge key or an alias is read by the model and not by the lookup: GetSummary is non-empty, the raw node is nil, PreserveNode writes nothing and returns no diagnostic. That is #384, filed against the lookup rather than this caller because 31 non-test sites share it. TestApplyPathItemDocs_WithoutRootNode reads as a guard against an input the parser never produces. It is not — a merge-key document reaches that branch carrying real text — so it now says which behaviour it pins and why. --- compilers/openapi/internal/operation/operations.go | 10 ++++++++++ .../internal/operation/operations_internal_test.go | 6 ++++++ 2 files changed, 16 insertions(+) diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 91a5578..2b56996 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -477,6 +477,16 @@ var pathItemDocFields = []struct{ keyword, key string }{ // pair is duplicated onto every operation under the path, exactly as the path // item's servers already are — a path item is distributed across the operations // it holds, and there is no ir.PathItem to attach it to. +// +// Two cases this does not reach, both shared with applyPathServers beside it: +// +// - A path item that mounts no operation keeps nothing, because there is no +// operation to keep it on. GitHub #383 records that gap for the whole class, +// documentation included, and the unmounted-path-item work is where it lands. +// - A pair supplied through a YAML merge key or an alias is read by the model +// but not by RawChildNode, whose lookup is a plain mapping scan. GetSummary +// is non-empty, the raw node is nil, and nothing is kept or reported — +// GitHub #384. func applyPathItemDocs(c lowering.Ctx, op *ir.Operation, pi *soa.PathItem, declPtr string) []ir.Diagnostic { if pi.GetSummary() == "" && pi.GetDescription() == "" { return nil diff --git a/compilers/openapi/internal/operation/operations_internal_test.go b/compilers/openapi/internal/operation/operations_internal_test.go index ce128f5..cadd30a 100644 --- a/compilers/openapi/internal/operation/operations_internal_test.go +++ b/compilers/openapi/internal/operation/operations_internal_test.go @@ -317,6 +317,12 @@ func TestFaultFor_ClassifiesAtTheClassBoundaries(t *testing.T) { // TestApplyPathItemDocs_WithoutRootNode is the documentation counterpart of the // two tests above: a declared pair whose source node cannot be read keeps // nothing, and announces nothing it did not keep. +// +// The input is synthetic, but the branch is not hypothetical — RawChildNode does +// not expand a merge key, so a real document supplying the pair through one +// reaches here with GetSummary non-empty and loses it silently (GitHub #384). +// This pins the current behaviour rather than endorsing it; when #384 makes the +// lookup view-aware, what changes is the node this receives, not this contract. func TestApplyPathItemDocs_WithoutRootNode(t *testing.T) { t.Parallel() l := newRawLowerer(&soa.OpenAPI{}) From cd5ea2876b5c60116fef036d76f91a15c0fcfcad Mon Sep 17 00:00:00 2001 From: OmarAlJarrah Date: Sun, 9 Aug 2026 11:15:23 +0300 Subject: [PATCH 4/4] docs(ir-design): name the invalid-method-key warning where it fires MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The OpenAPI row names a warning beside the lowering that emits it wherever one exists — a reserved header name is "lowered as declared + reserved-header-name warning". The additionalOperations clause added with this change described the lowering and stopped there, so the one key that lowers to an unusable binding read as though it lowered silently. --- docs/ir-design.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/ir-design.md b/docs/ir-design.md index 6cf2e29..101ca2a 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -1698,7 +1698,7 @@ How each format's distinctive concepts land in the IR (full details live with ea | Format | Lowering highlights | |---|---| -| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and its `content` map whatever its arity → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field: it holds one TypeRef and no media type, so one entry loses the key it was written under just as several lose all but the first); response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; 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; securitySchemes/security → Auth OR-of-ANDs, 3.2 device flow + `oauth2MetadataUrl` → Flows/OAuth2MetadataURL; servers+variables (3.2 named) → Servers; tags (3.2 parent/kind) → groups + TagDefs; info contact/license → Document; schema `example(s)` → Examples; `xml` object (incl. 3.2 nodeType) → XMLHints at type and property level; `not`/`if-then-else`/`dependentSchemas`/`dependentRequired`/`contains`/`propertyNames`/`unevaluated*` → verbatim Unmodeled per §4.7; `contentEncoding`/`contentMediaType` → `Encoding` on the scalar the position lowers to and `contentSchema` → Unmodeled (`no_ir_home`) per §4.7; `$id`/`$schema`/`$vocabulary` → Unmodeled (`out_of_scope`, `$id` not honoured for resolution); `$dynamicRef` → the anchored type by compiler expansion, else verbatim Unmodeled with the reason it was irreducible; an inline `allOf` branch declaring more than the merge consumes → verbatim Unmodeled (`degraded_lowering`) per §4.8; a boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | +| **OpenAPI 3.x** | components/schemas → registry (IDs from pointers); inline schemas hoisted with hints; `allOf` → Base/Mixins per §4.3; `oneOf`/`anyOf` → Union (Exclusive bit), null-variant → Nullable ref, co-declared with structural keywords → the composition distributed across the variants per §4.3, or — for the five shapes that cannot be distributed — structural body + verbatim union per §4.8 (branches that declare no shape at all are `validation_only` per §4.7); competing keywords at one position — `const`/`enum`/`allOf`, elected in that order, and `oneOf` beside `anyOf`, where oneOf wins — lower as the elected keyword with every passed-over one verbatim Unmodeled (`degraded_lowering`) at its own pointer per §4.8, and a `{X, null}` oneOf beside an anyOf stays a Union rather than collapsing to a nullable ref; `discriminator` → Discriminator (3.2 `defaultMapping` → Discriminator.Default); `nullable`/type-arrays → Nullable; readOnly/writeOnly → Visibility and schema-level `default` → the referencing Property/Parameter's Default: both bind a *use* of the type rather than the type, so a declaration-site one is pushed down to referencing properties with use-site precedence and the declaration keeps its own copy verbatim (`no_ir_home` Unmodeled + diagnostic) — which is what a component nothing references would otherwise lose silently; `additionalProperties: false` → Additional=closed, `unevaluatedProperties: false` → closed_after_composition, `minProperties`/`maxProperties` → Model.Constraints (the property set's cardinality, as against Additional's openness); parameters → Params + HTTPBinding locations w/ style/explode, `allowEmptyValue` → Parameter.Unmodeled (`no_ir_home`: HTTPParamBinding holds its neighbours but not this one), a header parameter named Accept/Content-Type/Authorization lowered as declared + `reserved-header-name` warning (OpenAPI says such a definition SHALL be ignored; dropping declared content is an emitter's call, not a compiler's), 3.2 `in: querystring` → querystring location; requestBody/responses all content types → Payload.Contents, a non-required requestBody → Payload.Unmodeled (`no_ir_home`, presence is the IR's only body optionality); 3.2 `itemSchema` → Content.Item and `itemEncoding` → Content.ItemEncoding — except beside a positional `prefixEncoding`, where both go to Content.Unmodeled (`no_ir_home`) because a single every-item encoding cannot state ordinals; per-status responses/default → Conditions + ranges, error-response `headers` and its `content` map whatever its arity → ErrorCase.Unmodeled (`no_ir_home`, ErrorCase has neither field: it holds one TypeRef and no media type, so one entry loses the key it was written under just as several lose all but the first); response/encoding header `style` and `explode` → Property.Unmodeled (`no_ir_home`, ir.Property has neither field), and a `Content-Type` entry in either headers map lowered as declared + the same `reserved-header-name` warning the parameter position gets; webhooks → HTTPBinding.IsWebhook; callbacks → Callbacks; links → Response.Unmodeled (`no_ir_home`, promotable later); path-item `servers`, under `paths`, `webhooks` and a callback expression alike → Operation.Unmodeled (`no_ir_home`: §10 scopes servers by index list at service and channel, and an operation has no such list yet), with an operation's own `servers` — which OpenAPI says override the path item's — kept beside them under `openapi:operationServers`, the one key here not named for the keyword it holds, since two declarations at two pointers cannot share one map key without the survivor depending on lowering order; 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 boolean `false` `allOf` branch → the composed Model closed, branch verbatim Unmodeled (`degraded_lowering`) per §4.8, a `true` branch a silent no-op; a shape applicator (`properties`/`patternProperties`/`additionalProperties`/`required`/`items`/`prefixItems`, and `format` where no type is declared) the lowered node has no field for → verbatim Unmodeled (`degraded_lowering`) per §4.8; a parameter schema's `xml` and its `readOnly`/`writeOnly` → Parameter.Unmodeled (`no_ir_home`: Parameter has no field for either); `patternProperties` → AdditionalProps.Patterns; `prefixItems` → Tuple, with any trailing `items` → Tuple.Unmodeled (`degraded_lowering` per §4.8: an open tuple has no IR combinator, so the fixed head is lowered and the tail kept beside it); `x-*` → namespaced Unmodeled (legal on every object — hence Unmodeled on every node); `$ref`-adjacent sibling keywords (3.1) and ref-target annotations merge onto the referencing Property/Parameter with **use-site precedence**, applied uniformly (oagen's ad-hoc per-site patching is the counterexample), and at a position carrying no Property/Parameter — an `allOf`/`oneOf`/`anyOf` branch, `items`, a component — bind an alias hoisted at that position instead, per §4.3; a oneOf/anyOf whose variants are all string consts normalizes to a closed `Enum` in a `pass/` normalization — not in the compiler — so per-variant `Docs` survive until the collapse is chosen; mutually-exclusive parameter groups (`x-mutually-exclusive-parameter-groups`) stay as namespaced Unmodeled entries, and their documented *promotion* (no dedicated node needed) is a pass that synthesizes one logical `Parameter` typed by a `Union` of variant models, bound via `HTTPParamBinding.ParamPath` per field; pagination only via injectable policy, marked Inferred | | **Swagger 2.0** | lifted to OpenAPI 3.x shape first (body/formData → Payload; host/basePath/schemes → Servers; consumes/produces → content types), then the OpenAPI lowering runs | | **TypeSpec** | consumed post-check (monomorphized, `isFinished`); template instances → TypeCommon.Instantiation incl. value args → TemplateArg; models → Model w/ Base + spread provenance → Mixins; scalars → Scalar chains, constructors in values → Value.Ctor; `@encode`/`@format` → Encoding triple; `@encodedName` → WireNameByFormat at property AND type level; unions w/ named variants → Union, `@discriminated` → Discriminator.PropertyName/Envelope/EnvelopeValueName; `| null` → Nullable; visibility classes (incl. custom, `@invisible` → Visibility.None) → Visibility, op overrides → ParameterVisibility/ReturnTypeVisibility; `@patch` implicitOptionality → HTTPBinding.PatchImplicitOptionality; interfaces → OperationGroups (versionable); `@overload` → OverloadOf; `@sharedRoute` → SharedRoute; `@service` → Service; versioning decorators incl. `@typeChangedFrom`/`@madeOptional`/`@madeRequired` and add/remove cycles → Availability timeline (on members/variants/params too); pagination decorators incl. prev/first/last links and header continuation tokens → Pagination PropPaths (In:"header"); Azure.Core `@pollingOperation`/`@finalOperation` → LongRunning; multipart w/ parts → Content.Encoding/PartEncoding, `Http.File` → FileInfo (content-type set, contents chain, filename location); streams/SSE → StreamDetail + Variant.Event (contentType, terminal); `@error` → UsageFlags.Error; `@example`/`@opExample` → Examples (Input/Output pairs); `@pattern` message → Constraints.PatternMessage; `@mediaTypeHint` → TypeCommon.MediaTypeHint; `never` members deleted + diagnostic per §4.8; TCGC client-shaping decorators (`@clientName`, `@access`, `@usage`, `@scope`, `@override`, …) → namespaced Unmodeled (`out_of_scope`) consumed by emitter policy, never IR semantics; values/consts incl. enum-member refs → Values channel | | **Smithy 2.0** | structures → Model, mixins → Mixins (non-structure mixins flattened — spec-sanctioned); `document` → Any; unions → WireTagged Union, member `@jsonName` → Variant.WireName; enum/intEnum → Enum (open by default); `@sparse` → element Nullable; traits: constraints → Constraints, `@paginated` → Pagination (declared), `@retryable` → ErrorCase.Retryable + Throttling, `@error` fault → ErrorCase.Fault, `@readonly` → Idempotency safe, `@idempotent`/`@idempotencyToken` → Idempotency, `@sensitive` → Sensitive/Secret, `@tags` → Tags, `@clientOptional`/`@input` → Property.ClientOptional (+InputOnly), `@addedDefault` → DefaultAdded, root-shape `@default` pushed down to properties w/ provenance; `@streaming` blob → StreamDetail (+`@requiresLength` → RequiresLength); event streams → StreamDetail.Events union + Property.EventHeader/EventPayload + Initial messages; service-level errors → Service.CommonErrors; protocol traits → Service.Protocols; service `rename`/`version` → Service.Renames/Version; resources → OperationGroup + ResourceInfo (identifiers, properties, lifecycle incl. put/@noReplace, instance vs collection ops); http traits → HTTPBinding incl. `@endpoint`/`@hostLabel` → HostPrefix/host location (additive binding), `@httpPrefixHeaders`/`@httpQueryParams` → Prefix bindings, `@httpResponseCode` → Response.StatusCodeProp, `@requestCompression` → Compression, `@httpChecksumRequired` → ChecksumRequired; `@auth` order → priority-ordered Auth, `@optionalAuth` → empty option; `@jsonName` → WireName; `@mediaType` → Encoding.MediaType; xml traits → XMLHints at type and property level; `@examples` → Examples (Input/Output/Error); waiters + rules-engine traits → verbatim Unmodeled (`out_of_scope`, §15); `smithy.api#Unit` → nil payload / shared empty Model for tag-only variants; other traits → namespaced Unmodeled |