Repository navigation
fix(differ): report a list envelope field added, removed or retyped - #459
gjtorikian wants to merge 1 commit into
Conversation
[Medium impact] The PR appears safe to merge. SummaryThe PR compares list envelope fields through
|
MichaelDeBoey
left a comment
There was a problem hiding this comment.
Thanks, this fixes #439!
The issue's script now prints the field-added entry for total_count under envelopeFieldChanges. oagen diff exits 1 for the addition and 2 for the removal and for the retype to string, and identical documents still exit 0. I checked OAS 3.1 and 3.0 (identical results), with the envelope inline, behind a components/responses $ref and built with allOf. All of these follow a response-only model's field:
- a required field added: additive,
required on a response-only model - requiredness flipped either way
- a format-only change: additive
- a field made nullable: breaking
- an
objectconst retyped fromlisttopage: breaking - an inline union that gains a variant: additive
readOnlygained: breakingdeprecatedandx-*onhas_more: additive, and mirrored as anenvelope-fieldnode inbehaviorChanges
A page_info companion object added, removed or turned into a string is reported here beside model-added/model-removed. So is a $ref companion pointed at another component. A field added inside page_info, inside a nested page_info.cursors, or inside a $ref'd PageInfo component is still reported only under model-modified. An envelope kept as a $ref component model still has no envelopeFields, and its fields are reported under model-modified as the new List envelopes paragraph says. The docs/cli.md, ir-types.md and JSDoc changes match what the head does.
96c21c3 to
a96b666
Compare
`PaginationMeta.envelopeFields` is every property of an unwrapped list
envelope besides the data array, what an emitter types the envelope
from. `diffOperations` compared those fields only for their
`extensions` (`diffEnvelopeFieldExtensions`), skipping a field one side
lacked, so `total_count` added to `{ data, has_more }`, removed from it
or retyped to `string` was no change and `oagen diff` exited 0.
The envelope's fields are now compared as a model's fields are, by
`diffFields` (src/differ/models.ts, now exported and reading anything
that carries `fields`), judged as a response-only model since the SDK
only ever reads the envelope: a field added is additive, required or
not; one removed or retyped is breaking; format, requiredness, access
flags, `deprecated` and extensions are classified as on any model's
field, each an entry in `envelopeFieldChanges`. The operation is
breaking when an entry is. A deprecation flip on an envelope field is
mirrored into `behaviorChanges` as an `envelope-field` node, as its
extension change already was. `docs/cli.md` gains a **List envelopes**
paragraph; the Deprecation and Extensions paragraphs, the Deprecated
section of `docs/architecture/ir-types.md` and the
`envelopeFieldChanges` JSDoc say the same.
Closes #439
ffb3326 to
918d9b9
Compare
Closes #439
What
oagen diffcompared a list envelope'senvelopeFieldsonly for theirextensions. In the issue's documents,total_countadded to{ data, has_more }was no change, and so were its removal and its retype tostring(exit 0 each time). The envelope's fields are now compared as a model's fields are. The added field is an additivefield-added(exit 1). The removed or retyped one is a breakingfield-removed/field-type-changed(exit 2). Each sits under theoperation-modifiedchange'senvelopeFieldChanges.How
diffOperations(src/differ/operations.ts) read the envelope throughdiffEnvelopeFieldExtensions. That function skipped a field only one side declared and compared nothing butextensions. It is replaced by a call todiffFields(src/differ/models.ts, now exported and typed to read anything that carriesfields) on the twoenvelopeFieldslists, with directionresponse, since the SDK only ever reads the envelope. An absent list reads as no field, and the comparison still runs only while both specs paginate. So the entries and classifications are exactly a response-only model's:details: 'required on a response-only model')deprecatedand extensions: as on any model's fieldhasBreakingnow also checksenvelopeFieldChangesfor a breaking entry.Since
diffFieldsalso reports adeprecatedflip,collectBehaviorChanges(src/differ/diff.ts) mirrors an envelopefield-deprecated-changedentry as adeprecated-changedentry with a newenvelope-fieldnode (DeprecatedNode,src/differ/types.ts), the node the extension mirror already uses.Docs:
docs/cli.mdgains a List envelopes paragraph. Its Deprecation and Extensions paragraphs and itsoperation-modifiedtable row, the Deprecated section ofdocs/architecture/ir-types.md, and theenvelopeFieldChangesJSDoc now state the same rule. The sentence "No other property of an envelope field is compared" is gone.Tests
test/differ/pagination-changes.test.ts,diffOperationson an IRlistPets:required on a response-only model)readOnlyanddeprecated, as on a model's fieldparseSpec+diffSpecs. Added gives additive (modified: 1, breaking: 0, so exit 1). Removed and retyped give breaking (breaking: 1, so exit 2). Identical documents give no change.test/differ/deprecated.test.ts: ahas_moredeprecation flip is afield-deprecated-changedentry inenvelopeFieldChangesand adeprecated-changedbehavior change on anenvelope-fieldnode, both ways.Stack
Based on
issue/428-inline-union-variant-added(#458). Merge that first; GitHub retargets this PR when the base branch is deleted.Decision / Assumptions
diffFieldsrather than a narrower added/removed/retyped check. The envelope is only ever read, so a required field added is additive, as on any response-only model, and itsdetailsreuse that wording.field-required-changedstays direction-blind, asdiffFieldskeeps it for every model (optional → required breaking, required → optional additive). I didn't give envelope fields a reader-side requiredness rule of their own like response headers have.diffFieldsbringsfield-deprecated-changedentries, and the docs say every flip is mirrored intobehaviorChanges. SoDeprecatedNodegainsenvelope-field, following theExtensionsNodeprecedent. The alternatives were dropping deprecation entries for envelope fields (unlike a model's field) or reporting them unmirrored (against the documented invariant).envelopeFieldsreads as none. A list response that moves between an unwrapped envelope and a kept wrapper model ($ref) also lists its companions as removed or added. That operation is already breaking throughresponseChanged/paginationDataPathChanged, so no classification moves.!. No change that was reported before is reclassified: the issue's pairs went from no change (exit 0) to additive or breaking, and the IR is untouched.field-type-changedhere besidemodel-removed/model-added. A removed or retyped stop field is an entry here besidepaginationEndOfIterationChanged.