Repository navigation
fix(compilers/openapi): keep the declarations that were dropped - #741
Open
fuad-daoud wants to merge 10 commits into
Open
fuad-daoud wants to merge 10 commits into
fuad-daoud wants to merge 10 commits into
Conversation
Add three optional IR members the OpenAPI 3.2 silent-drop batch needs: - Payload.Docs *Docs (json:"docs,omitzero") — a Request Body Object's summary/description describe the body rather than any media type inside it, and reached the IR in no form (GitHub #609). - TagDef.Parent and TagDef.Kind (both omitempty strings) — OpenAPI 3.2's tag parent hierarchy and tag kind, dropped at the declared-tag registry (GitHub #613). Kind is recorded verbatim; reading a role into it is grouping policy, not a property of the document. Each is a new optional member, so ir.IRVersion stays 0.6.0 (ir-design §2.1: no rename, removal, encoding change, or change to an existing key's meaning). Payload.Docs is a pointer for the same three-state reason Payload.Required is, and no pass rule forbids it outside a request: ir.Payload is the body of a request, a response or a message, so an "outside-request" rule would forbid a legitimate use. ir-design §2 and §7.2 record the shapes. TestPayload_DocsIsTriState and TestTagDef_JSONContract pin each tag; unwitnessed.golden.txt grows by exactly Payload.Docs, TagDef.Kind and TagDef.Parent.
A Request Body Object's description reaches the IR in no form: the parser models the field, so the unknown-key census never sees it, and lowerRequestBody read only `required` and `content`. A body description therefore vanished with no field, no Unmodeled entry and no diagnostic (#609). lowerRequestBody now writes Payload.Docs when the body declares a description, after the payload-nil guard so a body with no content still lowers to nothing. A response or message payload leaves Docs nil — the same three-state reading Payload.Required takes — so no existing payload golden moves. No diagnostic: the description has a typed home now, which is the whole fix. The corpus case covers both a body $ref'd from components (two operations mounting one declaration, so it interns once) and one written inline. TestContent_RequestBodyDocsAreOrderIndependent is the two-order diff: Payload.Docs is neither the type registry nor a diagnostic, so the in-package oracle does not reach it.
OpenAPI lets the entry holding a `$ref` carry summary and description beside it, describing that use of the referenced object, and says they override the referenced object's. The parser presents them on the Reference wrapper, but the compiler reads the resolved object and discards the wrapper — so a use-site override vanished with no evidence it existed, at 3.0, 3.1 and 3.2 alike (#610). One helper, resolve.RefDocs, folds the pair over the declaration's docs field by field, and all six sites that carry a docs field and can hold a Reference Object apply it: parameters, responses and error cases (through the one shared lowerResponseParts, so both status classes fold by construction), headers, examples, request bodies, and security schemes. The sites deliberately left out — callbacks, links, path items, component schemas — are unchanged. A departure, recorded in ir-design §12.2 and intended: OpenAPI says such a sibling has "no effect" where the referenced object type does not allow summary/description. The IR has a docs field at each of these positions, so keeping it is lossless, and honoring the clause would restore exactly the silent drop this fixes. No version gate: the wrapper is version-independent and a 3.0 document lost its siblings the same way. TestConformance/ref-site-docs covers all six sites in one document, one component mounted with different overrides so each mount keeps its own, and TestRefSiteDocs_MountsKeepTheirOwnInEitherOrder is the two-order diff the in-package oracle cannot see (a use-site description is neither a registry entry nor a diagnostic).
electTypeSpelling elects a parameter's or header's `content` spelling and returns only the elected media type's schema. Everything else the Media Type Object declares is read by nobody: its example and examples, its x-* and undeclared keys, and the fields only a body media type has an IR home for (itemSchema, itemEncoding, prefixEncoding, encoding). None of them reached a field, an Unmodeled entry or a diagnostic, and the parser-modelled fields produced no unknown-key warning either — so they vanished twice over (#611). contentEntryFields now folds the whole elected object: its examples reach Parameter.Examples / Property.Examples, its x-* and undefined keys are kept under the content entry's own scope (`openapi:content/<mt>/…`, so two media types cannot collide on one key), and the four content-only fields are kept verbatim with ReasonNoIRHome plus one info each naming the position, since the IR can close that gap by growing the field. Widened past the issue's examples-and-extensions framing: itemSchema/itemEncoding/prefixEncoding/encoding are the same one-field read at the same call, and are shipped here. Unchanged: which spelling wins, the passed-over-schema warning, the "exactly one media type" warning, Property.Encoding.MediaType from the elected media type, and body media types (lowerContent untouched). No ir-design edit: this lands no IR shape. The key form is §12's existing scope rule and the reason is §12's no_ir_home, not §4.8's degradations.
appendValuelessExample dropped anything without `value` or `externalValue` and claimed in its comment that a 3.2 dataValue/serializedValue "carries no example at all". Both fields exist on the parser's Example and neither was read, so a 3.2 document's examples were dropped with a warning that misdescribed them (#612). appendExampleValue now chooses the spelling the entry wrote: - value lowers as before; - dataValue lowers exactly as value does — the parser's values.Value is a yaml node — onto Example.Value, diagnosed at its own node; - externalValue is unchanged; - serializedValue keeps the entry with the raw node under Unmodeled["openapi:serializedValue"] (ReasonNoIRHome) plus one info. It is kept verbatim rather than typed: it is a single-format serialization spelling of the data form Example.Value already holds, and a typed field would put a format-specific representation on a neutral node with no other consumer. The promotion path stays open. The "declares neither value nor externalValue" warning now fires only for a genuinely empty stub, which is what the existing fixture carries; TestContent_ExampleWithoutValueSkipped passes unchanged. The $ref-entry diagnostic pointer rule is unchanged. examples-32 covers the content, parameter, header and components/examples sites the issue names.
OpenAPI 3.2 gives a tag a `kind` and a `parent`. The declared-tag registry kept only name/docs/summary/description/externalDocs, so both reached the IR in no form; and grouping took the operation's first tag unconditionally, so an operation whose first tag was a badge was filed under a section that badge does not describe, and a declared hierarchy was flattened (#613). TagDef now records Parent and Kind verbatim — reading a role out of Kind is grouping policy, not a property of the document. Grouping: the group comes from the operation's first *navigational* tag, and a tag's declared parent nests its group under the parent's, one level per declared ancestor, so Service.Groups reads as the tree the document describes. navigational = kind absent or `nav`; any other declared kind is skipped, registered or not, because reading a section out of an unregistered string is an inference (invariant 6). An operation whose every tag is non-navigational falls to the default group, and no diagnostic is reported when a tag is skipped: TagDef keeps the kind and Operation.Tags keeps every membership, so nothing is lost. A parent that is not itself a declared navigational tag ends the walk and the group stays top-level; the parser already reports a missing parent and a circular one. The walk is bounded by the declared tag count. Order is preserved: groups and siblings stay first-seen from operations, and a missing ancestor is created immediately before its first descendant, so every 3.0/3.1 document and every flat golden is byte-identical. internal/archtest's value-selector pin moves from serviceGroups.group to serviceGroups.place on lowerPathItem, which is the method it now calls — same shape (a method on the *serviceGroups parameter), and the other caller still pins group. unwitnessed.golden.txt loses exactly OperationGroup.Groups, TagDef.Kind and TagDef.Parent; tags-grouping-32 is their witness.
Four OpenAPI 3.2 fields reach the IR in no form, each because the bundled parser's model names no member for it — so the unknown-key census, which grades a key by that model, reports a field the document defines as undefined, or says nothing at all while the key is dropped (#615): - Response.summary is now read off the raw node into Docs.Summary, in the one shared lowerResponseParts, so success and error cases both get it. - components/mediaTypes entries are resolved where a content entry references one: the raw entry is unmarshalled (marshaller.UnmarshalNode) and lowered through the ordinary media-type lowering, so a shared media type arrives with its schema, examples and extensions. A target that does not resolve — another document, another section, an undeclared entry, a non-object entry, or an entry that is itself a $ref — lowers as written with one openapi/unresolved-ref and the reference kept verbatim. - xml.nodeType now fills XMLHints.NodeType, which already existed and whose GoDoc already named the version. annotation.Read takes the version fact so the reader and the census agree; the three call sites pass it. - Nested Encoding encoding/prefixEncoding/itemEncoding are kept verbatim under the part's own scope with one info each, at exactly the key and pointer the census uses. The census mechanism is new and version-gated: annotation.UnknownKeysDecided takes the keys a reader has already read raw. Every suppression is passed only for a 3.2 document, so the same key on a 3.1 document keeps the unknown-object-key warning it has always drawn — probe-verified for Response.summary, xml.nodeType and the nested encoding fields. The components/mediaTypes whole-map Unmodeled entry is no longer written for 3.2, replaced by per-entry handling. Widened as approved: the sweep's other three keys ship here too, since they are the identical mechanism (OD-5). Not yet covered, and next: a components/mediaTypes entry nothing references is kept by no rule until the unreferenced-components retention lands. ir-design §12 records the raw-read-plus-census rule.
A compiler lowers a component only where a reference finds it, and only components/schemas and components/securitySchemes are lowered unconditionally. Every other section's unreferenced entry therefore reached no node, no Unmodeled entry and no diagnostic: a declaration the document makes disappeared in silence (#616). Each entry of a section with no registry — responses, parameters, examples, requestBodies, headers, links, callbacks, pathItems, and 3.2's mediaTypes — that nothing references is now kept verbatim on the document under openapi:components/<section>/<name> with ReasonNoIRHome, at the entry's own pointer, with no diagnostic (the links precedent generalized). Referenced is transitive, not syntactic: internal/componentreach walks the raw tree for `$ref` strings rooted outside those sections and follows the references a reached entry writes in turn. A `$ref` inside an unreferenced entry is not a root, so the component it names is kept too — keeping the entry and dropping its target would restore the loss one hop later. The walk is bounded by a node budget and by expanding each entry once, so a reference cycle between two entries terminates. The new package has its own archtest rules entry: it reads yaml nodes and nothing else, because a package that could reach the lowering could decide reachability by what the lowering happened to resolve. components/mediaTypes joins the retained set only from 3.2, the version that defines it; below that the key is one the dialect does not define and the components census keeps the whole map, which is also what closes the gap the previous commit left open. preserveResponseExtras' links decision is revised in place rather than left contradictory, and ir-design §12 and §14 record the rule.
retainUnreferencedComponents passes over a component entry keyed "" and the only place that said why was a test comment. The guard now carries the why in the code: the name is empty, OpenAPI's component-name rule makes such a key invalid, so ids.ComponentEntry refuses it, and an Unmodeled key with an empty name segment names no entry of that document. ir-design §12's paragraph claimed every entry of those sections is kept, which is false for exactly that one; the paragraph now names the exception in its own voice. The test comment defers the why to the code (part of #616).
Resolve the conflicts with main: the default response lowers in the scope Within gives it while still passing its entry for the use-site docs fold, and the doc comments this branch adds come under the 100-word cap main now enforces. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GCzyGPq9DQ7RLfagZm2JB5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Seven constructs a document could declare reached the IR in no form — or were reported as undefined keys — because the parser's model, the unknown-key census, or the lowering never read them. Each now has a home, or is kept verbatim with the reason recorded:
descriptionreachesPayload.Docs(new optional member), with the request-body contract stated in the field's GoDoc.summary/descriptionwritten beside a$refoverride the component's docs at every position with a docs field: parameters, responses and error cases (through the one shared lowering), headers, examples, request bodies and security schemes. Use-site precedence, uniformly at 3.0/3.1/3.2. One recorded departure: OpenAPI says such a sibling has "no effect" where the referenced object type disallows one, but the IR has a docs field at each of these positions and honouring the clause would restore the silent drop (ir-design.md§12.2).contentfolds the media type'sexample/examplesintoParameter.Examples/Property.Examplesand keeps itsx-*, undeclared keys, and the 3.2itemSchema/itemEncoding/prefixEncoding/encodingfields under scopedUnmodeled.dataValuelowers toExample.Value;serializedValueis kept verbatim underopenapi:serializedValue(no_ir_home) instead of both forms drawing a misleading "declares neither value nor externalValue" warning. A genuinely empty stub keeps that warning.TagDefgainsParentandKind; grouping takes an operation's first navigational tag and nests groups along declared parents, so a badge no longer becomes a section and a hierarchy is not flattened.Response.summary→Docs.Summary(success and error alike),components/mediaTypesresolved where referenced,xml.nodeType→XMLHints.NodeType, and nestedencoding/prefixEncoding/itemEncodingkept verbatim. The census no longer calls these undefined on 3.2 documents; below 3.2 they still warn.$refnames is kept verbatim on the document (openapi:components/<section>/<name>,no_ir_home). Referenced is transitive: a$refinside an unreferenced entry is not a root, so the component it names is kept too, rather than orphaning it one hop later. The links decision is revised in place rather than left contradictory. One exception, stated in the code and §12: an entry keyed with the empty string (invalid under OpenAPI's component-name rule) has no name to key under and is passed over.IRVersionstays0.6.0: every shape change here is a new optional member, which §2.1's list (renamed, removed, encoding changed, meaning changed) does not move — the same reading PR #732 took. The OpenAPI lowering row and §12/§14 ofir-design.mdrecord the new rules. New internal packagecomponentreach(reads yaml nodes only, with its own archtest entry).#614 (
$self) is not in this change: it is PR #732.Closes #609, #610, #611, #612, #613, #615, #616.
Test plan
make gate— exit 0;golangci-lint0 issues;Coverage gate passed: all 7918 statements covered.testdata/conformance/openapi/, each with aconformanceCasesrow and assert function;unwitnessed.golden.txtloses exactlyPayload.Docs,TagDef.Parent,TagDef.KindandOperationGroup.Groups; the corpus sweep (go run ./cmd/morphic-harness testdata/conformance/openapi) reports every spec ok.Document.Unmodeled, per-mount docs, mediaTypes$reforder:TestGrouping_TreeIsIndependentOfDeclarationOrder,TestUnreferencedComponents_RetentionIsOrderIndependent,TestRefSiteDocs_MountsKeepTheirOwnInEitherOrder,TestContent_MediaTypesRefIsOrderIndependent,TestContent_RequestBodyDocsAreOrderIndependent.go test ./... -count=1green;-updateregenerates nothing.