Skip to content

fix(compilers/openapi): reconcile allOf branches that redeclare a field - #8

Merged
OmarAlJarrah merged 3 commits into
mainfrom
fix/openapi-allof-property-reconcile
Jul 20, 2026
Merged

fix(compilers/openapi): reconcile allOf branches that redeclare a field#8
OmarAlJarrah merged 3 commits into
mainfrom
fix/openapi-allof-property-reconcile

Conversation

@OmarAlJarrah

Copy link
Copy Markdown
Member

Summary

Fixes hundreds of spurious pass/duplicate-wire-name errors when compiling
specs whose allOf schemas have inline branches that redeclare the same field.

allOf lowering appended every inline branch's properties into one model. When
two branches declared a property with the same name — a common real-world shape
where a documented object is followed by a doc-stripped duplicate that marks
some fields required — the model ended up with several properties sharing one
wire name. The validate pass then correctly flagged that corrupted model.

On the GitHub REST API spec this produced 304 errors across forkee,
issue, workflow_job, project_card, author, and committer schemas
(e.g. webhook-fork/properties/forkee alone had 168 properties, 78 duplicated).

allOf is an intersection: a redeclared field is one logical property, present
if the instance satisfies every branch and required if any branch requires it.
The fix reconciles same-wire-name properties into a single property (OR-ing
Required, first declaration defining the shape) instead of appending
duplicates. Ref/mixin classification (ir-design §4.3) is unchanged.

Test plan

  • New TestAllOf_OverlappingInlineBranchesReconcile — two inline branches
    redeclaring a field reconcile to one property; Required reflects the
    branch that requires it; the documented declaration defines the shape.
  • go test ./... — all packages pass (golden snapshots unchanged).
  • End-to-end on the GitHub REST spec: 304 errors -> 0; only the pre-existing
    240 benign openapi/degraded-construct info diagnostics remain; exit code 0.

An allOf whose inline branches redeclare the same property was lowered by
appending every branch's properties into one model, producing multiple
properties that share a wire name. The validate pass then correctly flagged
the corrupted model, raising hundreds of spurious duplicate-wire-name errors
(304 on the GitHub REST API spec, where webhook `forkee`, `issue`, and
`workflow_job` schemas use an allOf of a documented object plus a doc-stripped
duplicate that marks some fields required).

allOf is an intersection: a redeclared field is one logical property, required
when any branch requires it. Reconcile same-wire-name properties into a single
property — OR-ing Required, with the first (richest) declaration defining the
shape — instead of appending duplicates.
…roperty

fillModelProperties always sets a non-empty WireName, so the Name.Source
fallback branches were dead code and left the package below the 100%
coverage gate. Compare WireName directly.
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/openapi-allof-property-reconcile branch from 94bf8ad to 6971ef5 Compare July 20, 2026 17:44
…ly OR-ing required

Reconciling an allOf field that is redeclared across branches previously
combined only Required and kept the first declaration's shape wholesale, so
any type detail on a later branch — a default, constraints, deprecation, XML
hints, examples — was silently dropped, and the surviving shape depended on
branch order.

Make reconciliation a proper intersection: OR Required and Secret, and adopt
each optional detail the surviving property lacks from the redeclaration, so a
documented declaration and a bare one reconcile to the richer property whatever
their order. A genuinely conflicting redeclared description (both present and
different) is reported as an info diagnostic rather than dropped without trace.

Index properties by wire name so a redeclaration reconciles in one lookup
rather than rescanning, keeping property fill linear for wide models.
@OmarAlJarrah
OmarAlJarrah merged commit b05d652 into main Jul 20, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/openapi-allof-property-reconcile branch July 20, 2026 21:13
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.

1 participant