Skip to content

fix(compilers/openapi): name a node from its declaration - #380

Merged
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-declaration-names-the-node
Aug 13, 2026
Merged

fix(compilers/openapi): name a node from its declaration#380
OmarAlJarrah merged 2 commits into
mainfrom
fix/openapi-declaration-names-the-node

Conversation

@OmarAlJarrah

@OmarAlJarrah OmarAlJarrah commented Aug 9, 2026

Copy link
Copy Markdown
Member

#375 has merged. This was written on top of fix/openapi-inline-position-hint
(#375, for #353); that is now on main, this PR targets main, and the branch is
current with it, so the diff here is this change alone.

Summary

A $ref can spell a pointer inside another declaration's body, and both lowerings
reach that coordinate: the declaration through its own structure, the reference
through the pointer it names. Intern calls build for whichever arrives first, so
a node's Naming.Hint was decided by which one that was.

#353 closed this under /components/schemas by teaching the reference's pointer
walk to replay what the structural lowering composes. That derivation is not total.
A position under /paths takes its enclosing hint from an operationId, a response,
or a media-type key, and the pointer records none of them:

# no reference to it
t/anon/paths/~1x/get/responses/200/content/application~1json/schema/items  hint=response_item

# with an unrelated component pointing at that pointer
t/anon/paths/~1x/get/responses/200/content/application~1json/schema/items  hint=items

Components lower before paths, so the reference always interned first there. The
document stayed deterministic; what moved was the name, decided by whether some
unrelated schema happened to point at the position.

The fix

A name belongs to the declaration that owns the coordinate, never to a reference
to it.
That is the rule, and it needs no agreement between two namers to hold.

  • compile.Types gains InternProvisional, which records that the name a node is
    being given is a placeholder, and NameFromDeclaration, which replaces it. Only
    the name is a question the declaration answers better — the node itself is the
    same one either way — so the reference still builds it.
  • lowering.Ctx gains NamingByReference(), a copy-constructor in the same shape as
    WithAuth. hoistSubSchema lowers under it, so the marking covers the whole
    subtree the reference interns: a reference to an object body interns its children
    too, and their names hang off the enclosing one.
  • The replacement runs in schemaRefHomed, the entry point every schema position
    flows through. It has to be there rather than only at intern, because a position
    whose node already exists resolves to it and returns before interning anything
    — marking the alias alone left the test still green, which is how I found it.

A second declaration at one coordinate is silent here rather than last-write-wins:
that is what claimID refuses, and re-reporting it as a naming problem would name
the symptom instead of the cause.

Test plan

  • TestInlinePosition_UnderPathsIsNamedByItsDeclaration replaces the test that
    pinned this as a known gap. It now asserts response_item in all three cases —
    referenced with the paths block first, referenced with the components block first,
    and not referenced at all.
  • Four tests in compilers/compile for the registry half: a placeholder replaced, a
    placeholder replaced only once, a declared name left alone when the declaration
    interned first, an uninterned coordinate ignored, and a refused intern leaving
    nothing to rename.
  • TestCtx_NamingByReferenceIsScopedToTheCopy holds what makes the flag safe to
    thread — it is set on a copy, so it cannot leak back to a caller still lowering a
    declaration.
  • Watched red, both halves separately: dropping c = c.NamingByReference() fails
    with expected "response_item", actual "items"; dropping the
    NameFromDeclaration call in schemaRefHomed fails the same test.
  • No golden moved.

One thing I checked and deliberately did not do

#372 predicted this would subsume branchPointerHint and structuralPointerHint.
I measured it: the entire suite passes with structuralPointerHint removed, so
nothing in the corpus needs it any more. I have kept it anyway. That measurement
bounds the corpus, not the input space — it shows no committed fixture reaches a
coordinate the declaration never walks, not that no document can — and the pointer
walk is what names such a node if one exists. branchPointerHint is still held by
its own unit test and predates this work (#181, #281), so removing it would settle a
recorded decision as a side effect of an unrelated change.

Full gate green: gofmt, go vet ./..., golangci-lint run (0 issues),
go build ./..., ./scripts/check-coverage.sh (100% of statements; the absolute
count is unstable across runs, see #369).

Closes #372

@OmarAlJarrah
OmarAlJarrah force-pushed the fix/openapi-declaration-names-the-node branch from 3f77944 to 03e1fe5 Compare August 13, 2026 11:01
Base automatically changed from fix/openapi-inline-position-hint to main August 13, 2026 11:11
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/openapi-declaration-names-the-node branch from 03e1fe5 to 59042b4 Compare August 13, 2026 11:11
A $ref can spell a pointer inside another declaration's body, and both
lowerings reach that coordinate: the declaration through its own
structure, the reference through the pointer it names. Intern builds the
node for whichever arrives first, so the node's Naming.Hint was decided
by which one that was.

#353 closed this under /components/schemas by teaching the reference's
pointer walk to replay what the structural lowering composes. That
derivation is not total: a position under /paths takes its enclosing
hint from an operationId, a response or a media-type key, and the
pointer records none of them. Components lower before paths, so the
reference always interned first there and the position was named
"items" rather than "response_item" -- deterministically, but decided by
whether some unrelated schema pointed at it.

A name belongs to the declaration that owns the coordinate, never to a
reference to it. hoistSubSchema now lowers under a context marking its
names as placeholders, and every schema position replaces the
placeholder at its own coordinate as it lowers. The marking covers the
subtree rather than the referenced coordinate alone, since a reference
to an object body interns its children too and their names hang off the
enclosing one.

The replacement sits at the schema entry point rather than only at
intern because a position whose node already exists resolves to it and
returns before interning anything -- which is why marking the alias
alone was not enough.
@OmarAlJarrah
OmarAlJarrah force-pushed the fix/openapi-declaration-names-the-node branch from 59042b4 to ee795bb Compare August 13, 2026 11:54
NameFromDeclaration wrote the caller's hint straight onto the node, where
interning the same hint goes through NamingHint and neutralizes it. A
coordinate a reference reached first therefore ended up holding "A" where
one the declaration reached first held "a" — so the name depended on which
lowering arrived, which is the dependence this path was added to remove.

It is worse than a wrong spelling. Two documents differing only in the
order they declare a component and a reference to one of its inline
positions compiled to different IR, which invariant 7 forbids and the
order-invariance oracle rejects: allof-oneof-cooccurrence in the corpus
failed it, and TestOneOf_CoDeclaredDistributionIsOrderIndependent and
TestComposition_BranchAliasIsOrderIndependent with it.

Write the field the way interning writes it. The regression case asserts
the two paths agree rather than asserting a literal, since what matters is
that they match and not what the grammar produces; restoring the raw
assignment reddens it, both order tests, and the corpus sweep.
@OmarAlJarrah
OmarAlJarrah merged commit 3e48d56 into main Aug 13, 2026
1 check passed
@OmarAlJarrah
OmarAlJarrah deleted the fix/openapi-declaration-names-the-node branch August 13, 2026 12:11
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: an outside $ref weakens the name of an inline position under /paths

1 participant