Repository navigation
fix(compilers/openapi)!: scope and build path item entries once - #1327
Merged
Merged
Conversation
A path item lowers to no node of its own, so what it declares that its operations have no home for (its summary and description, its x- extensions and its undeclared keys) is kept under Unmodeled on every operation it mounts. mountPathItem computed it inside the operation loop: every layer of the mount was censused, rendered from YAML to JSON and reported again for each operation, though nothing in it depends on the operation. Eight operations over two undeclared keys built sixteen findings, of which compile.Diags kept two. applyLayers now runs once a mount, into one map, and keepOn writes a copy of each entry onto each operation, cloning its value so no two operations share bytes. The note that an operation holds its path's documentation stays per operation, since it is sited there. The IR is unchanged. A path item's own findings are now reported before its operations' rather than after the first one's, beside the servers and precedence findings that were already made once a mount, which moves them in two conformance goldens. Closes #414.
A path item's summary and description were kept on each operation under the fixed keys openapi:pathItemSummary and openapi:pathItemDescription. An operation's census spells the same keys for an undeclared key of that name, and the path item's text, written after the census, replaced it: the operation's value reached the IR in no form while its warning said it was kept. The pair is now keyed beneath the carrier's scope, as the item's extensions and undeclared keys already are: openapi:pathItem/summary and openapi:pathItem/description on an operation. The census escapes a "/" in the key it keeps, so no key of the operation's own can spell one. pathItemDocFields becomes pathItemDocKeywords, with the key derived by annotation.EntryKey, and the "pathItem" scope is named once, as pathItemScope. This renames two Unmodeled keys. The path-item-docs rule and three conformance goldens follow, and nothing else in the goldens moves. Closes #1323.
Building a path item's entries once a mount moved the census's check for an occupied entry off the operation's own map. The check now runs on the map the mount's entries are built in, and keepOn copied them onto each operation without looking. No key can clash today, since every one sits beneath the pathItem scope, but nothing would say so if one did. annotation.CopyEntriesInto copies a set of entries onto a carrier without replacing one it holds. It reports the refused copy under openapi/unknown-key-entry-taken, in key order, as the census does, and clones each value it writes. keepOn uses it, and announces the path's documentation only when that entry is the one the operation holds.
TestUnknownKeys_PathItemDeclaredFieldsAreNotUndeclared read every key beneath openapi:pathItem/ as an entry the census made for a key the path item does not declare. The summary and description are now kept there by their own reader, so the check admits exactly those two keywords. A census entry for query or servers still fails it, and TestPathItemFields_MatchTheLibraryModel holds the two documentation keywords among the declared fields.
The note on which keys beneath openapi:pathItem/ the check admits moves from the test's doc comment, which it took past the 100-word cap, to the assertion it explains.
TestPathItem_DocsKeepApartFromAnOperationsOwnKeys says no finding reports a key as unrepresented, but asserted that only at the operation's own key, where no refusal is sited: annotation.CopyEntriesInto reports one at the path item's entry. It now refuses an unknown-key-entry-taken anywhere, so a second copy onto the operation fails it. TestKeepOn_AnnouncesOnlyDocumentationThatLanded counted one diagnostic where documentation lands without checking it is the note, so a note under the wrong code passed. It now checks the code. Two comments still said a path item's entries are built once per operation; they are built once per mount.
read-once-per-run said #414 was still open. This branch closes it, so the rule's Why lists it with the other repeats that were found and removed.
This was referenced Nov 2, 2026
TestCopyEntriesInto_ReportsInKeyOrder said its eight keys left one chance in 40,320 that an unsorted report passes. Go iterates a map of eight or fewer entries as a rotation of the order they were written in, and the keys were written sorted, so one rotation in eight is the sorted order: with the sort removed, the test passed 46 of 400 runs. It now refuses 64 keys written in reverse, which leaves the order to the map's hash seed. With the sort removed it failed all 400 runs.
TestPathItem_KeepsWhatItDeclaresOncePerMount requires one documentation note per operation, but each mount in its fixture wrote one of the two keywords, so a note made for each keyword that landed passed it: an operation under a path item writing both would draw two, which compile.Diags then drops to one. /a now writes both keywords, and /b writes its summary at the use site and its description in the referent, the shape main also reported twice. A note per keyword now fails the test.
annotation.CopyEntriesInto copies one set of entries onto several carriers, and reported an entry a carrier already held with the census's message, which says the key "is represented in the IR in no form at all". Copying onto two operations where only the second held the key, the first kept the entry while the second's finding said the IR had none. The refusal keeps its code and gets a message of its own, saying the node holding the other construct keeps no copy. The comments on the function and on diag.UnknownKeyEntryTaken say the key is missing from that one carrier. No input reaches the refusal today, since every key copied sits beneath the path item's scope.
applyPathItem's comment read "keeps on into what a path item declares", and carrier.keepsDocs said "Only an operation's are", though the carrier for operations is now the map their entries are built in. Both say what they mean now.
Writing both keywords on /a, and a summary at /b's use site, left every mount with a summary and a description, so a note checking only one of the two keywords passed TestPathItem_KeepsWhatItDeclaresOncePerMount. /a now writes only a summary, a new /c only a description, and /b one keyword at each layer. A note checking one keyword, or made for each keyword that landed, now fails the test, as do the plants it already caught.
TestKeepOn_AnnouncesOnlyDocumentationThatLanded refused the one documentation entry it copied, so a keepOn that gave up at the first keyword it was handed, landed or not, passed it: with the summary refused and the description kept, that keepOn makes no note. The test now copies both onto an operation already holding the summary, and requires the refusal and then the note.
Its comment said it returns "every diagnostic after the load's", which reads as the load's diagnostics followed by the rest, as serviceWithGrouping returns them. It returns the later phases' alone.
Keying a path item's summary and description beneath its scope renamed two Unmodeled keys on every operation it mounts: openapi:pathItemSummary and openapi:pathItemDescription became openapi:pathItem/summary and openapi:pathItem/description. The JSON shape is unchanged, so no pin asked for a bump, but a consumer built against 0.27.0 reads the old keys and finds the path's text gone, silently. That is the failure IRVersion exists to report (docs/ir/ir-version.md), and 0.26.0 was bumped for the same reason, when Unmodeled entries a consumer read stopped being written. IRVersion moves to 0.28.0. Its generations row repeats 0.27.0's shape and names and states the meaning that changed, and the version history says what a consumer pinned to 0.27.0 sees: no path text under the old keys, and, where an operation writes an undeclared key of that name, that key's own value, which 0.27.0 let the path's text replace.
main took IRVersion 0.28.0 for the local time formats (#1325), so the renamed path item doc keys move to 0.29.0: a row after 0.28.0's with the same shape and names, and a history entry naming what a consumer pinned to 0.28.0 sees.
5 of 6 tasks
main took IRVersion 0.29.0 for Content.Docs (#1333), so the renamed path item doc keys move to 0.30.0: a row after 0.29.0's with its new shape and the same names, and a history entry naming what a consumer pinned to 0.29.0 sees.
main now keeps an unmounted path item's summary and description beneath its scope (#1322), keyed by annotation.EntryKey, while an operation took the fixed pathItemDocFields keys, told apart by a carrier.operation flag. This branch keys an operation's pair beneath its scope as well, so every carrier takes annotation.EntryKey(into.scope, keyword), and the note an operation draws is keepOn's. The flag has no reader left and goes, as the provenance field and keepsDocs did; itemConstructs reads pathItemDocKeywords. The path-item-docs rule states both carriers' keys and lists both branches' tests.
OmarAlJarrah
added a commit
that referenced
this pull request
Nov 2, 2026
Brings in #1327. - #1327 took IRVersion 0.30.0, with no change of shape, so this branch's bump moves to 0.31.0. Its row keeps the shape both share and this branch's names and meaning, and its version history entry moves with it, pinned against 0.30.0. - #1327 and this branch each added a test at one place in preserve_internal_test.go; both are kept.
5 of 6 tasks
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
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
A path item lowers to no node of its own, so what it declares that its operations have no home for
(its summary and description, its x- extensions and its undeclared keys) is kept under
Unmodeledon every operation it mounts. This changes how those entries are built and how the documentation
pair is keyed.
Built once per mount.
mountPathItemcomputed the entries inside its per-operation loop, onceper operation and per layer, though none of them depends on the operation. Each layer's keys were
censused, sorted, looked up, rendered from YAML to JSON and reported again. With a counter at each
step, a path item writing two undeclared keys over eight operations ran the census 8 times and built
16 findings, of which
compile.Diagskept 2. A$ref'd item with a sibling key of its own (twolayers) ran
applyPathItem16 times and built 24 findings, keeping 3.applyLayersnow runs onceper mount into one map, beside
itemServersand the precedence check, which were already once permount.
keepOnthen copies each entry onto each operation, cloning its value so no two operationsshare bytes. The note that an operation holds its path's documentation stays per operation, since
it is sited there.
applyPathItemDocsno longer makes that note, so the carrier'sprovenancefield had no reader leftand is removed, along with
onNearestNode's context parameter.Documentation keyed beneath the item's scope. The pair was kept under the fixed keys
openapi:pathItemSummaryandopenapi:pathItemDescription. An operation's census spells the samekeys for an undeclared key of that name, and the path item's text, written after the census, replaced
it. The operation's value reached the IR in no form while its warning said it was kept (#1323). The
pair is now keyed beneath the carrier's scope, as the item's extensions and undeclared keys already
are:
openapi:pathItem/summaryandopenapi:pathItem/description. The census escapes a/in thekey it keeps, so no key of the operation's own can spell one. An item mounting no operation already
keeps its pair beneath a scope holding its own pointer (
openapi:pathItem/<item pointer>/summary),so
applyPathItemDocsnow keys every carrier byannotation.EntryKey(into.scope, keyword), and thecarrier's
operationflag, which chose between the two spellings and the note, is removed. Thepath-item-docsrule says so and now lists its tests.No entry replaced. Building once moved the census's check for an occupied entry off the
operation's own map, onto the map the mount's entries are built in.
keepOnnow copies through anew
annotation.CopyEntriesInto, which never replaces an entry the carrier holds. It reports arefused copy under
openapi/unknown-key-entry-taken, in key order as the census does, but with amessage of its own: the operation keeps no copy, where the census's message says the IR holds none,
which would be false while the mount's other operations hold it. It clones each value it writes.
The scoped keys mean no input reaches that branch today. It is there so a later key that clashes
is reported rather than silently replacing the operation's entry. The note about the path's
documentation is made only when that entry is the one the operation holds.
What a consumer sees:
Unmodeledkeys on operations are renamed, as above, soIRVersionmoves to 0.30.0 (seeBreaking).
operation's. That reorders the diagnostics in two conformance goldens.
Closes #414.
Closes #1323.
Breaking
IRVersionmoves from 0.29.0 to 0.30.0. The JSON shape is unchanged, so no pin asks for the bump,but a consumer built against 0.29.0 that reads
openapi:pathItemSummaryoropenapi:pathItemDescriptionon an operation finds the path's text gone, silently: the failuredocs/ir/ir-version.mdsays a version exists to report. 0.26.0 was bumped for the same reason,when
Unmodeledentries a consumer read stopped being written. Where an operation writes anundeclared key of one of those names, the old key now holds that key's own value, which 0.29.0 let
the path's text replace. The
generationsrow repeats 0.29.0's shape and names and states themeaning, and
docs/ir/version-history.mdsays what a consumer pinned to 0.29.0 sees.Test plan
TestPathItem_KeepsWhatItDeclaresOncePerMountreads the walk's diagnostics before the dedupe,through a new
serviceAsReportedhelper that shareswalkServicewithserviceWithGrouping.Over an inline item with three operations writing a summary, a
$ref'd two-layer item with twooperations writing a summary at its use site and a description in its referent, and an item
writing only a description, it requires one
openapi/unknown-object-keyper source key, onedocumentation note per operation, every entry on every operation, an operation's own entry beside
them, and each operation owning its bytes.
main(7f84912) it fails on the once-per-key assertion, on the note, whichmainmakestwice on each operation of the two-layer item, and on the renamed keys.
for each keyword that landed, only
summarychecked, onlydescriptionchecked, entrieswritten onto the first operation alone, the operation's own map replaced, the entries rebuilt
for each operation, and the fixed keys restored.
TestPathItem_DocsKeepApartFromAnOperationsOwnKeysgives an operation undeclared keys spelledpathItemSummaryandpathItem/summaryunder a path with a summary, and requires all threevalues under keys of their own and no
openapi/unknown-key-entry-takenanywhere. Onmainitfails on the operation's
pathItemSummaryholding the path's text. Copying the entries onto anoperation twice fails it, which an assertion at the operation's own key alone would let pass,
since the refusal is sited at the path item's.
TestCopyEntriesInto_KeepsWhatTheCarrierHoldsandTestCopyEntriesInto_ReportsInKeyOrderholdthe copy to keeping the carrier's entry, reporting the refused one at its own pointer, naming the
holder and claiming no loss from the IR, cloning, and sorting 64 refusals by key.
TestKeepOn_AnnouncesOnlyDocumentationThatLandedholds the note to the entry that landed, adescription beside a refused summary included, and to its code. Each of these mutants fails one
of them: the held entry overwritten, the refusal left unreported, the refusal reported with the
census's message, the sort dropped, the clone dropped, the note made whatever landed, the note
given up at a refused summary, and the note made under another code.
of 400. With eight keys written in order, as it first had, it passed 46 of 400: Go iterates a map
of eight or fewer entries as a rotation of the order they were written in.
TestUnknownKeys_PathItemDeclaredFieldsAreNotUndeclaredrefused every key beneathopenapi:pathItem/as a census entry for a declared field. It now admits the documentation pairalone, and still fails when the census is planted to treat
queryorserversas undeclared.Planting the same for
summarypasses it, since the census then finds the docs entry at its ownpointer and stays silent;
TestPathItemFields_MatchTheLibraryModelfails on that plant.TestApplyPathItemDocs_WithoutRootNodenow runs throughkeepOnas well, so it still holds thatnothing is announced when nothing was kept.
morphic compilewith amainbuild and a build of this branch over 2,068 specs: the 227 undertestdata/, 1,840 real-world OpenAPI 3 specs, and the Cloudflare API schema @319dc756. It ran atmainb206dc9, at 7f84912, which bumps the YAML library, at be889c6, which reworks extensionpromotion and adds two of the specs, at 02c972b, which retypes the local time formats, and at
117d29e, which keeps an unmounted item's documentation, each after merging it; the last three
runs also follow the bump. With the two keys renamed in
main'soutput, and its
irVersionstamp read as the branch's where the branch bumps it, exit codes, theIR and the multiset of diagnostics are identical on every one. Ten of the specs hold the renamed
keys, five of them real-world.
Diagnostic order differs on three, each a path item with undeclared keys: the two goldens above
and
testdata/openapi/unknown_keys.yaml, which has no golden.main'sbut for the order of its diagnostics. After the second, renaming the two keys in the first
commit's goldens leaves no difference.
TestIRVersion_PinsTheEncodedShapeandTestIRVersion_PinsTheNamespass with the 0.30.0 row,whose hashes equal 0.29.0's, because it states its meaning.