Skip to content

test(compilers/openapi): hold the loop gates on branch reports - #1514

Open
fuad-daoud wants to merge 15 commits into
mainfrom
test/openapi-loop-gate-reports
Open

fuad-daoud wants to merge 15 commits into
mainfrom
test/openapi-loop-gate-reports

Conversation

@fuad-daoud

@fuad-daoud fuad-daoud commented Nov 5, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

In short: A $ref loop under an allOf or union branch is reported once, as cyclic-ref, and two checks stop the same branch being reported again as unresolved-ref; removing either left the suite green. This adds a test with a case for each check.

An allOf branch or a union branch on a loop of $refs is reported once, as the openapi/cyclic-ref the load step raises. For an allOf branch and a union branch beside structural keywords, two gates keep the lowering from reporting the same branch again as openapi/unresolved-ref: onLoop in fillRefBranch and in diagUnresolvedBranches. Removing either left the compiler's suite green.

This adds one table-driven test through the compiler's public face, with one case per gate, one more for anyOf at the union's gate, and two for a union with no keyword beside it, which refTypeRef lowers, holding the whole diagnostic list rather than the absence of one code. It sits beside the other loop tests in e2e rather than in internal/schema, as #1086 proposed, because the list joins what load reports with what the lowering does. A second test, in internal/schema, holds the report the first gate lets through on a $ref load also reports, which a full compile then drops for load's own. A third test, in e2e, holds both reports where load makes none, on a $ref the binder cannot place, which a full compile keeps. union-beside-structure now says a case-3 branch on a loop of $refs is reported by the loop's one openapi/cyclic-ref rather than openapi/unresolved-ref. The third site, refTypeRef, is held by TestCompile_ReportsAReferenceCycleAnOverlayIntroduces on a component, and by those two cases at a union's branches.

Closes #1086

Before and after

flowchart TB
  subgraph before ["Before"]
    b0["The onLoop gate in fillRefBranch or diagUnresolvedBranches is removed"]
    b1["An unresolved-ref error appears beside the cyclic-ref for one loop"]
    b2["No test notices"]
    b0 --> b1
    b1 --> b2
  end
  subgraph after ["After"]
    a0["The onLoop gate in fillRefBranch or diagUnresolvedBranches is removed"]
    a1["An unresolved-ref error appears beside the cyclic-ref for one loop"]
    a2["The exact-list case for that gate goes red"]
    a0 --> a1
    a1 --> a2
  end
  before ~~~ after
Loading

What it brings: A spec author with an allOf or union branch on a $ref loop gets no unresolved-ref for that branch beside the loop's cyclic-ref, and removing a gate that keeps it away now fails a test.

Test plan

  • CI on this head, main at 2a0996a6 merged in, runs make gate. On f6c1cf19, go build ./..., go test on compilers/openapi, internal/schema, compilers/openapi/e2e and internal/archtest, make vet, make lint and make lint-planted each pass.
  • The new internal/schema case and the allOf case of TestReferenceEnds_ABranchTheBinderCannotPlaceIsReportedAtIt fail with the first gate's report block deleted, with the rest of the suites green, and both pass with the gate forced if true.
  • Each plant that skips a branch by whether the binder read it, the first gate as if _, read := c.Bindings().Entry(b); read && !onLoop(c, b) { and the second as if _, read := c.Bindings().Entry(b); !read || c.RefScope().NamesReferent(b) || onLoop(c, b) {, reddens only that gate's rows of that test: allOf for the first, union beside structural keywords and anyOf beside structural keywords for the second.
  • Planting "oneOf" for key at union.go:537, with _ for key at union.go:530, reddens the anyOf beside structural keywords row alone, and planting branches[0].GetRef() for b.GetRef() at union.go:538 reddens the union beside structural keywords row and both rows of TestOneOf_CoDeclaredBranchAfterASkippedOneIsReported.
  • Planting ir.SeverityWarning for ir.SeverityError at union.go:536 reddens the union beside structural keywords and anyOf beside structural keywords rows, TestOneOf_CoDeclaredUnresolvableBranchIsNotDistributed and TestOneOf_CoDeclaredBranchAfterASkippedOneIsReported, and at allof.go:317 the allOf row and the new internal/schema case.
  • Planting strconv.Itoa(0*i) for strconv.Itoa(i) at union.go:537 reddens the union beside structural keywords row of that test, which holds the reports at oneOf/0 and oneOf/1, and both rows of TestOneOf_CoDeclaredBranchAfterASkippedOneIsReported.
  • Replacing if !onLoop(c, b) with if true in fillRefBranch reddens the allOf branches case alone, dropping || onLoop(c, b) in diagUnresolvedBranches reddens the union branches beside structural keywords and anyOf branches beside structural keywords cases alone, and planting (key == "oneOf" && onLoop(c, b)) for onLoop(c, b) at union.go:533 reddens the anyOf case alone.
  • Planting break for continue at union.go:534 reddens both rows of TestOneOf_CoDeclaredBranchAfterASkippedOneIsReported.
  • Planting onLoop(c, js) && !strings.Contains(string(pointer), "/oneOf/") for onLoop(c, js) in refTypeRef, with strings imported, reddens the union branches with no keyword beside them case alone, "/anyOf/" there the anyOf branches with no keyword beside them case alone, and removing that gate reddens both and TestCompile_ReportsAReferenceCycleAnOverlayIntroduces.
  • Returning unionInlineBranch from classifyUnionSiblings where every branch lies on a loop reddens the two beside structural keywords cases alone, as diagUnresolvedBranches is called for case 3 only.
  • Negating the first gate reddens the allOf branches case, the new internal/schema case and that allOf case, and no other test in internal/schema or e2e, and dropping the degraded-construct row from the expected union list reddens the union case, so the list is held exactly.

Removing the onLoop gate in fillRefBranch, or the one in diagUnresolvedBranches, added an unresolved-ref error beside the cyclic-ref that load already reports for the same loop, and no test noticed. One case per gate compiles the allOf and the union spec and holds the whole diagnostic list.
The struct the test compares now has exported fields, the form assert.Equal is allowed on, and the failure message no longer calls the loop the whole report, since the union case also expects a degraded-construct info.
@fuad-daoud
fuad-daoud marked this pull request as ready for review November 5, 2026 08:39
@fuad-daoud
fuad-daoud marked this pull request as draft November 5, 2026 22:55
@fuad-daoud
fuad-daoud removed the request for review from OmarAlJarrah November 5, 2026 22:55
@fuad-daoud
fuad-daoud marked this pull request as ready for review November 5, 2026 23:37
@fuad-daoud
fuad-daoud marked this pull request as draft November 6, 2026 09:55
@fuad-daoud
fuad-daoud removed the request for review from OmarAlJarrah November 6, 2026 09:55
The union row of the reference-ends test had one branch, so a report at the wrong branch index stayed green. Give it two and hold both reports, and say in the doc comment that a $ref is unread only in the first copy of a key written twice.
The unplaced-branch rows reached diagUnresolvedBranches through oneOf only, so a report filed under the wrong keyword stayed green: it keyed its pointer on the union's keyword and no row used anyOf. Add an anyOf row.

The union row's two branches named one $ref, so a report quoting the wrong branch's target read the same. Give the second branch its own target, and compare each branch's message against its own.
The rows compared the pointer and message of each unresolved-ref report but not its severity, so a report downgraded to a warning stayed green at both the allOf and the union report. Assert that each is an error.
The loop table held the union's gate on oneOf alone, so a gate applied
to oneOf only drew no failure while an anyOf loop was reported twice.
A walk that stopped at the first skipped branch was unheld too, as its
only fixture put the unresolved branch first.
The loop table reached a union only beside structural keywords, so a
gate in refTypeRef scoped to union positions added an unresolved-ref on
a plain oneOf or anyOf loop and no test noticed. Two rows with no
keyword beside the union reach it.

The union rows also did not hold that a loop-only union reaches
diagUnresolvedBranches as case 3 of union-beside-structure: the
degraded-construct info has the same code, severity and pointer in all
five cases. A classifier returning another case left them green with a
false reason and the second gate unheld. The rows now assert the
reason the info gives.
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.

test: no test holds the loop gates on allOf and union branch reports

1 participant