Skip to content

openapi: name every node from one function of the document and the coordinate, and remove the provisional-name machinery #981

Closed

Description

@OmarAlJarrah

Problem

A node's name is derived by two mechanisms that must agree, and a third reconciles them when a reference arrives before the declaration. It is the largest piece of order-dependence machinery in the lowering, it is incomplete, and on the golden spec it changes no name.

  1. Threaded forward. Lowerers pass hint down (40 signatures in schema and operation declare hint string) and extend it with compile.SubHint (13 call sites), ids.DeclarationHint and requestBodyHint.
  2. Replayed from the pointer. positionHint, positionStep, ownHint, subSchemaHint and the tables structuralRoles, keyedSchemaMaps and compositionKeywords (111 lines in schema/resolve.go) recompute the same names for a $ref that reaches a coordinate before its declaration. structuralRoles has to list the suffixes the SubHint call sites pass, and TestInlinePosition_HintIsTheSameInBothOrders fails when they drift.
  3. Reconciled by building twice. compile.Types keeps three maps of arrival history (provisional, declared, byReference) and three methods over them: InternProvisional, InternDeclared, which runs build a second time for the same coordinate, and NameFromDeclaration (95 lines). lowering.Ctx carries NamingByReference, NamesByReference and NamingByReferenceAt (37 lines). Sixteen tests pin the mechanism: eleven in compilers/compile/compile_test.go, two in compilers/openapi/internal/lowering/lowering_test.go and three in compilers/openapi/internal/schema/schema_test.go.

The reconciliation is incomplete. Two open issues are the same defect, and six closed ones fixed one site each (#372, #433, #518, #519, #521, #529):

What it costs and where it fires

Add three lines to compile.Types, one at each point where the mechanism records or settles an arrival, and count them with the CLI:

// InternProvisional, before `t.provisional[pointer] = true`
println("naming: provisional", pointer)
// InternDeclared, after `delete(t.byReference, pointer)`
println("naming: rebuild", pointer)
// NameFromDeclaration, after `delete(t.provisional, pointer)`
println("naming: rename", pointer)

Reference-first against declaration-first, on a position inside another declaration:

openapi: 3.1.0
info: {title: t, version: "1"}
paths: {}
components:
  schemas:
    B: {$ref: '#/components/schemas/A/properties/p/items'}   # declared before A
    A:
      type: object
      properties:
        p: {type: array, items: {type: object, properties: {x: {type: string}}}}

go run ./cmd/morphic compile spec.yaml 2>&1 >/dev/null | grep '^naming:' prints provisional, rename and rebuild for /components/schemas/A/properties/p/items. With A declared first it prints nothing.

Over the same 1,952 specs as in the comparison below (morphic @3a706a7b):

  • 262 specs mark a node provisional. Three reach the rebuild: one of the 1,837 from the uncommitted corpus, and two of the 115 under testdata/.
  • On cloudflare/api-schemas @843eae2f the mechanism marks 60 nodes, all under /components/requestBodies/<name>/content/…, and runs no rebuild and no rename. The 60 marks are never settled, and each node's hint already equals the one a pointer replay gives from the entry name (60 of 60). A single derivation changes none of them.

Cause

A node's hint is part of the node when it is built, so it depends on which path reaches the coordinate first. A reference arriving first can only guess, and the declaration corrects the guess by rebuilding the node and everything interned beneath it. A position that no declaration lowers (#747) is never corrected. Names under /paths are not a function of the pointer alone, which is why the replay stops at /components/schemas.

Direction

A draft, to be improved by whoever builds it. It follows the comparison in #980, which decides whether the function exists.

  1. Name every node from nameAt(doc, pointer), the function openapi: derive a node's name from its coordinate in one function, and diff it against the names the lowering gives #980 validates, instead of from a threaded hint. Whether it runs when the node is built or once over the finished registry is decided by what reads Naming.Hint during lowering. A pass after interning removes arrival order outright.
  2. Remove what has no caller afterwards: the hint parameter, the replay and the tables only it reads, InternProvisional, InternDeclared, NameFromDeclaration and their three maps, NamingByReference, NamesByReference, NamingByReferenceAt and the field behind them, and the sixteen tests that pin them. compile.NamingHint and its neutralization stay.
  3. Record the rule in docs/micro-compiler-design.md: a node's name is a function of the document and its coordinate.

The change is justified if the function is smaller than the 243 production lines it replaces (111 + 95 + 37, doc comments included), or if the #747 and #729 reproductions pass with it. #980 reports which.

Out of scope

A coordinate's role and content depending on arrival order (#917). Library resolution (#920). The *AnchorIndex parameter, which crosses the same signatures; #982 covers it and says why it should land with this.

Acceptance

Blocked by #980. Would resolve #747 and, once checked against its reproduction, #729.

Related: #917, #747, #729, #372, #433, #518, #519, #521, #529

Priority: P3, rule 3 (off the golden spec: the mechanism marks 60 nodes on cloudflare/api-schemas and changes none of their names; the change fixes valid-spec shapes it does not use). With the three lines above, go run ./cmd/morphic compile openapi.json 2>&1 >/dev/null | grep -c '^naming: provisional' → 60 and … | grep -cE '^naming: (rebuild|rename)' → 0 (spec @843eae2f, morphic @3a706a7b).

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority:P3Later emitters, M4 and up, 3.2-only, streaming, webhooks, rare constructstype:choreRefactor or cleanup, no behavior change

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions