Skip to content

fix(compilers/openapi): preserve the bound a co-declaration loses - #346

Open
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-preserve-codeclared-bounds
Open

fix(compilers/openapi): preserve the bound a co-declaration loses#346
OmarAlJarrah wants to merge 1 commit into
mainfrom
fix/openapi-preserve-codeclared-bounds

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

Under the 2020-12 dialect a side may declare both of its bound keywords — minimum alongside exclusiveMinimum, maximum alongside exclusiveMaximum. They are conjunctive, and ir.Constraints holds one bound plus one exclusivity flag per side, so only one of them can reach a field. Which one that is was settled in #274: the tighter, named in a diagnostic together with the keyword it displaced.

The displaced keyword went nowhere else. {minimum: 10, exclusiveMinimum: 0} and {minimum: 10} compiled to byte-identical documents, and the only record that the source spelled the bound twice lived in a diagnostic message — a channel a consumer may filter by severity, and not part of what a downstream stage reads as data. A consumer reconstructing the source, or diffing two revisions of it, could not see the difference. That is the lossless-by-default invariant.

The keyword that reaches no field is now kept verbatim under Unmodeled with ReasonDegradedLowering and provenance at the keyword's own pointer, on whichever carrier the reading position owns. All three carriers are covered: a model property, a parameter, and the alias node a component's body reduces to.

Mechanically, annotation.Constraints gains the pointer and source index it needs to locate an entry — the same two arguments annotation.Read already takes — and returns the entry beside the constraints, which is what its three callers route onto their own carriers. The keyword is written back from the literal already read rather than by re-reading its raw node: both render the same JSON, since a numeric scalar converts through the same value.NumericLiteral the bound came from, but only the first cannot fail, and a preservation that can fail needs a diagnostic saying which happened (#144). The diagnostic now says the keyword is kept verbatim, which it can state unconditionally for that reason.

One nearby gap is deliberately left alone and filed as #343: the alias hoisted beside an unmodeled oneOf/anyOf is the one alias hoist that reads no constraints at all, so a position that co-declares a union with structural keywords loses its bounds outright. That predates this change, is a different mechanism, and is noted at the call site.

Test plan

  • TestReconcileBound_KeepsTheTighterOfTwoCoDeclaredBounds now asserts the entry the reconciliation leaves over — key, reason, exact literal, and pointer — over both sides and both directions of "which keyword wins", plus the ties and the magnitudes no rational holds.
  • New TestCoDeclaredBound_KeptOnTheCarrierThatReadIt (schema) and TestParams_CoDeclaredBoundKeptOnTheParameter (operation) drive whole specs through the compiler and assert the entry on each of the three carriers, in both directions; their negative twins assert that a side writing one keyword keeps nothing, so a bound never gets two homes.
  • testdata/conformance/openapi/constraints.yaml gains a component whose body reduces to a shared primitive, so the corpus witnesses the alias-node carrier as well as the property one; assertCoDeclaredBoundKept checks all three entries. The regenerated golden adds only the preserved entries, the new component, and the reworded diagnostics.
  • Verified by planting the defect: with the preservation call removed, every one of those assertions goes red across all three carriers, and the conformance row with it. Verified the corpus bites: deleting exclusiveMinimum from the new fixture reddens TestConformance/constraints.
  • Full gate green — gofmt, go vet, golangci-lint, go build, and the coverage gate at exactly 100%.

Closes #286

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

openapi: a co-declared bound keyword is reported but not preserved

1 participant