Skip to content

fix(compilers/openapi): name a $ref'd body or header from source - #452

Open
fuad-daoud wants to merge 1 commit into
mainfrom
fix/ref-declaration-naming-order
Open

fix(compilers/openapi): name a $ref'd body or header from source#452
fuad-daoud wants to merge 1 commit into
mainfrom
fix/ref-declaration-naming-order

Conversation

@fuad-daoud

Copy link
Copy Markdown
Collaborator

Summary

A request body or response header reached through a $ref lowers its schema under a hint derived from the use site — the referencing operation's ID, the headers-map key — while the node interns at the declaration's pointer. Intern is first-write-wins, so two use sites reaching one declaration handed the shared node to whichever lowered first: the same document compiled to two different IR documents depending on declaration order, which is the property invariant 3 and the order-invariance oracle exist to deny.

ids.DeclarationHint already guarded the case where the declaration is a top-level component entry, which is named alike from every use site. It matches on the /components/<kind>/<name> shape, so a $ref naming a construct declared inline elsewhere — just as shared — fell through it.

Ctx.NamingByReferenceAt marks a lowering as naming by reference when the declaration is not at the position that reached it. That is the condition NamingByReference already describes, and the owning declaration then replaces the placeholder through NameFromDeclaration in whichever order the two run. The marking had no counterpart at these positions because $ref is resolved before the lowering starts, leaving nothing downstream able to tell that a reference was what reached it — hoistSubSchema marks only schema-level $refs.

Not a regression: both cases reproduce on main as it stands.

Scope

This is half of #433. The issue reports the defect at a response mounted at two status classes; that half touches code #438 rewrites, so it lands separately once that PR merges rather than forcing a conflict onto an already-green branch. The two halves are independent — this one touches lines that branch never edits.

Parameters were checked and are safe by construction: they take their hint from p.GetName(), the declaration's own name, not a use-site key.

Test plan

  • Two corpus fixtures under testdata/openapi/, one per site. Both reproduce the order dependence before the change and pass after; go run ./cmd/morphic-harness testdata is clean.
  • Each site's fix was reverted individually to confirm its fixture reddens for its own reason and no other — neither fixture passes for the wrong reason, neither is redundant.
  • TestCtx_NamingByReferenceAtMarksOnlyAForeignDeclaration covers both branches of the new predicate.
  • No golden churn: no committed spec reached either input, so the order-invariance oracle had been running green over a corpus that never produced the case it exists to catch.
  • Every package at 100% statement coverage.

make gate could not be run end-to-end locally: under Go 1.27 the pinned golangci-lint v2.12.2 crashes inside staticcheck's IR builder on the stdlib poll package, and compilers/openapi/internal/annotation's TestRawFromNode_DiffersFromTheOldDecodeOnlyWhereRecorded fails on a escaping difference. Both reproduce on unmodified main and are toolchain skew, not this branch. Every other gate step passes locally; CI pins its own Go.

Refs #433

🤖 Generated with Claude Code

https://claude.ai/code/session_01Uvz7tyMXhnkKroffBP7Ptu

A request body or response header reached through a $ref lowers its schema
under a hint derived from the use site — the referencing operation's ID, the
headers-map key — while the node interns at the declaration's pointer. Intern
is first-write-wins, so two use sites reaching one declaration handed the
shared node to whichever lowered first, and the same document compiled to two
different IR documents depending on declaration order.

ids.DeclarationHint already guarded the case where the declaration is a
top-level component entry, named alike from every use site. It matched on the
/components/<kind>/<name> shape, so a $ref naming a construct declared inline
elsewhere — just as shared — fell through it.

Ctx.NamingByReferenceAt marks a lowering as naming by reference when the
declaration is not at the position that reached it, which is the condition
NamingByReference already describes; the owning declaration then replaces the
placeholder through NameFromDeclaration in whichever order the two run. The
marking had no counterpart at these positions because $ref is resolved before
the lowering starts, leaving nothing downstream able to tell that a reference
was what reached it.

Two corpus fixtures cover the pair. Neither case was reachable by any committed
spec, so the order-invariance oracle ran green over a corpus that never
produced the input it exists to catch.

Refs #433

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uvz7tyMXhnkKroffBP7Ptu
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