diff --git a/compilers/compile/naming.go b/compilers/compile/naming.go index aa4f447..53cb2f8 100644 --- a/compilers/compile/naming.go +++ b/compilers/compile/naming.go @@ -43,14 +43,21 @@ func NamingFor(source string) ir.Naming { // NamingHint builds the Naming of an entity nothing declared a name for, from // the context-derived hint an emitter should synthesize one from. // -// It is the second half of the same invariant NamingFor holds: a compiler that -// derives a hint from a position — the property a schema was inlined at, the -// method and path of an operation with no operationId — derives an empty one -// wherever that position is itself unnamed, and passing it through leaves the -// node with no name in any channel. Minting one here means no caller has to -// remember the case. +// It is the second half of the same invariant NamingFor holds, and holds it the +// same way. A hint is the only name an anonymous entity carries, so it is what +// an emitter renders that entity's identifier from — the job Canonical does for +// a declared name — and it is therefore neutral words here too (invariant 4). +// That matters because a hint is nearly always derived from something a source +// *did* spell: a component key, an operationId, a header name, a $ref target. +// Passing those through carried their casing and their punctuation into the one +// channel no rule was holding (GitHub #54). +// +// A position that carries no name of its own derives an empty hint, and a +// spelling with no word rune in it ("***") derives no words; both leave the node +// with no name in any channel, so both are minted one here rather than at every +// caller. func NamingHint(hint string) ir.Naming { - return ir.Naming{Hint: hintOr(hint)} + return ir.Naming{Hint: neutralHint(hint)} } // SubHint composes the hint of a node named after its position inside another — @@ -59,22 +66,24 @@ func NamingHint(hint string) ir.Naming { // // Composing by hand is what NamingHint cannot protect: "" + "_item" is "_item", // which is non-empty, so the presence rule passes it, and which is a leading -// separator no grammar produces, so nothing else reports it either — Naming.Hint -// is held to none of the content rules (GitHub #54). Minting the enclosing hint -// first makes the child agree with the node it hangs off, "empty_item" under -// "empty", rather than leaking the emptiness one level down. +// separator no grammar produces. Neutralizing each half first makes the child +// agree with the node it hangs off, "empty_item" under "empty", rather than +// leaking the emptiness one level down. // -// suffix is the caller's own role or index and is never empty; a caller with -// neither has no child to distinguish and no reason to be here. +// Both halves go through the same minting because either can arrive from a +// source spelling: the enclosing position's name, and the $ref target a +// composition branch takes its role from. Two neutral words joined by a single +// "_" are a neutral word sequence again, which is what lets a composed hint be +// fed back in as the parent of the next one. func SubHint(parent, suffix string) string { - return hintOr(parent) + "_" + suffix + return neutralHint(parent) + "_" + neutralHint(suffix) } -// hintOr returns hint, or the minted name when the position it was derived from -// carries none. -func hintOr(hint string) string { - if hint == "" { - return emptyNameHint +// neutralHint returns the neutral word sequence of hint, or the minted name when +// the position it was derived from names nothing a word can be read out of. +func neutralHint(hint string) string { + if words := ir.CanonicalWords(hint); words != "" { + return words } - return hint + return emptyNameHint } diff --git a/compilers/compile/naming_test.go b/compilers/compile/naming_test.go index 9883a1c..0bf6143 100644 --- a/compilers/compile/naming_test.go +++ b/compilers/compile/naming_test.go @@ -39,6 +39,26 @@ func TestNamingHint_KeepsADerivedHint(t *testing.T) { assert.Equal(t, ir.Naming{Hint: "connection_domain"}, compile.NamingHint("connection_domain")) } +// TestNamingHint_NeutralizesTheContextItWasDerivedFrom is what makes the hint +// channel a name rather than a transcription. A hint is derived from a position +// the source named — a component key, an operationId, a header name, a $ref +// target — so it arrives carrying whatever casing and punctuation that source +// used, and it is the only name an anonymous type has for an emitter to render +// (GitHub #54). +func TestNamingHint_NeutralizesTheContextItWasDerivedFrom(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ hint, want string }{ + {"connectionDomain", "connection_domain"}, + {"OrderBody", "order_body"}, + {"X-Report-List", "x_report_list"}, + {"rollout.state", "rollout_state"}, + {"get /pets/{petId}", "get_pets_pet_id"}, + {"***", "empty"}, // no words to render, so the same minting an empty hint gets + } { + assert.Equal(t, ir.Naming{Hint: tc.want}, compile.NamingHint(tc.hint), "hint %q", tc.hint) + } +} + // TestNamingHint_EmptyHintIsMintedAName is the same defect reached through the // other channel: a hint derived from a position the source left unnamed comes // out empty, and passing it through leaves the node nameless just as an empty @@ -69,3 +89,32 @@ func TestSubHint_MintsTheEnclosingHint(t *testing.T) { assert.Equal(t, "widget_item", compile.SubHint("widget", "item"), "an enclosing hint that is really there is untouched") } + +// TestSubHint_NeutralizesBothHalves pins that a composed hint is neutral however +// its two halves were spelled. Either can arrive from a source name — the +// enclosing position's, and the $ref target a union branch takes its role from — +// so neutralizing only the whole would still be a word sequence whichever half +// carried the casing, and neutralizing only the parent would not. +func TestSubHint_NeutralizesBothHalves(t *testing.T) { + t.Parallel() + for _, tc := range []struct{ parent, suffix, want string }{ + {"Combo", "Alt", "combo_alt"}, + {"X-Report", "item", "x_report_item"}, + {"widget", "0", "widget_0"}, + {"widget", "***", "widget_empty"}, + } { + assert.Equal(t, tc.want, compile.SubHint(tc.parent, tc.suffix), "%q + %q", tc.parent, tc.suffix) + } +} + +// TestSubHint_IsItselfANeutralHint is the composition property the callers rely +// on: a composed hint is fed back in as the parent of the next one, so joining +// two neutral halves has to produce something the grammar leaves alone. A join +// that introduced a boundary — a doubled or trailing separator, a letter run +// against a digit — would compound one level down. +func TestSubHint_IsItselfANeutralHint(t *testing.T) { + t.Parallel() + nested := compile.SubHint(compile.SubHint("Combo_A", "2"), "item") + assert.Equal(t, "combo_a_2_item", nested) + assert.Equal(t, nested, ir.CanonicalWords(nested), "the grammar leaves a composed hint alone") +} diff --git a/compilers/openapi/conformance_test.go b/compilers/openapi/conformance_test.go index 17ae7d9..2d1a3e7 100644 --- a/compilers/openapi/conformance_test.go +++ b/compilers/openapi/conformance_test.go @@ -325,9 +325,11 @@ func assertNamedTypes(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { // things in plain identifiers, so the compiler and the goldens shared one blind // spot and the segmentation could not be wrong in a way any of them saw. // -// Naming.Hint is deliberately not covered: it is built from context strings -// rather than through the grammar, and the golden shows one ("rollout.state") -// still carrying the source punctuation. That is GitHub #54, left open. +// Naming.Hint is covered by the same spec and for the same reason: a hint is +// built from a context string the source spelled, so this is the fixture whose +// context strings carry the punctuation. The enum property's hoisted node used +// to be hinted "rollout.state" verbatim, which is a name no emitter can render +// (GitHub #54). func assertNeutralNaming(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { require.Len(t, doc.Services, 1) svc := doc.Services[0] @@ -371,6 +373,8 @@ func assertNeutralNamingLeaves(t *testing.T, doc *ir.Document, widget *ir.Model, require.True(t, ok, "the enum property hoists an Enum node") require.NotEmpty(t, rollout.Members) assert.Equal(t, "in_progress", rollout.Members[0].Name.Canonical, "enum member") + assert.Equal(t, "rollout_state", rollout.Name.Hint, + "the hoisted node's hint is words too, not the property key verbatim") require.Len(t, op.Responses, 1) require.Len(t, op.Responses[0].Headers, 1) @@ -469,7 +473,7 @@ var composedHints = []struct { {"t/anon/components/schemas/Tuple/properties//prefixItems/0", "empty_0"}, {"t/anon/components/schemas/Mixed/properties//enum/0", "empty_0"}, {"t/anon/components/schemas/Mixed/properties//enum/1", "empty_1"}, - {"t/composed/components/schemas/Host/properties//oneOf/0", "empty_Alt"}, + {"t/composed/components/schemas/Host/properties//oneOf/0", "empty_alt"}, } // assertEmptyDerivedHints is the case minting at the node alone does not reach. @@ -562,7 +566,7 @@ func assertComponentReuse(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { bodyID := ir.TypeID("t/anon/components/requestBodies/OrderBody/content/application~1json/schema") require.NotNil(t, order.Request) assert.Equal(t, bodyID, order.Request.Contents[0].Type.Target) - assert.Equal(t, "OrderBody", doc.Types[bodyID].Common().Name.Hint, + assert.Equal(t, "order_body", doc.Types[bodyID].Common().Name.Hint, "a shared body is named after its component, not the operation that reached it first") require.Len(t, widgets.Responses[0].Headers, 1) @@ -929,6 +933,41 @@ func assertNullable31Ref(t *testing.T, doc *ir.Document, _ []ir.Diagnostic) { u, ok := doc.Types[namedID("UnionTarget")].(*ir.Union) require.True(t, ok) assert.Len(t, u.Variants, 2, "the null branch lifts to the ref rather than becoming a variant") + + assertCollapsedBranchHint(t, doc) +} + +// assertCollapsedBranchHint covers the {X, null} collapse's naming of the branch +// it keeps. The branch pointer is nameable from outside — BranchRef names it — +// and only the first lowering to reach it interns the node, so the collapse and +// an outside $ref must derive the same hint or the document depends on which +// component is declared first. The collapse used to hand the branch the +// *enclosing* schema's hint, which is neither what its composition would give it +// nor what the pointer walk derives (GitHub #281). +// +// The spec declares Collapsed before BranchRef on purpose: that is the order in +// which the collapse reaches the pointer first, and so the order that carried +// the enclosing name. The permutation half is the corpus-wide two-order oracle's +// (internal/harness), which compares hints with nothing excluded. +func assertCollapsedBranchHint(t *testing.T, doc *ir.Document) { + t.Helper() + const branchID = ir.TypeID("t/anon/components/schemas/Collapsed/oneOf/0") + + collapsed, ok := doc.Types[namedID("Collapsed")].(*ir.Scalar) + require.True(t, ok, "a {X, null} set resolves to its one branch rather than to a union node") + require.NotNil(t, collapsed.Base) + assert.True(t, collapsed.Base.Nullable, "the null branch lifts onto the reference") + assert.Equal(t, branchID, collapsed.Base.Target) + + branch, ok := doc.Types[branchID] + require.True(t, ok, "the branch declares a description, so it owns a node of its own") + assert.Equal(t, "variant_0", branch.Common().Name.Hint, + "the branch is named by its position in the composition, which is what a $ref to it derives too") + + ref, ok := doc.Types[namedID("BranchRef")].(*ir.Scalar) + require.True(t, ok) + require.NotNil(t, ref.Base) + assert.Equal(t, branchID, ref.Base.Target, "the outside reference reaches that same node") } // assertNullableEnum31 covers 3.1's spelling of a nullable enum: `null` listed diff --git a/compilers/openapi/internal/operation/content_test.go b/compilers/openapi/internal/operation/content_test.go index 4273d5d..e095cd9 100644 --- a/compilers/openapi/internal/operation/content_test.go +++ b/compilers/openapi/internal/operation/content_test.go @@ -829,7 +829,8 @@ func TestContent_HeaderMapEntriesSharingComponentGetDistinctIDs(t *testing.T) { // a body, the map key for a header — would name the one shared node after // whichever reference happened to lower first. Naming.Hint is what emitters // render from, so "postA_request" on a body two operations share is a wrong -// name, not a cosmetic one. +// name, not a cosmetic one. The hint is the component name in neutral words, +// which is what every name channel in the IR carries (invariant 4). func TestContent_SharedComponentSchemaTakesItsDeclarationHint(t *testing.T) { t.Parallel() doc, diags := parseFull(t, componentBodyRefSpec) @@ -838,14 +839,14 @@ func TestContent_SharedComponentSchemaTakesItsDeclarationHint(t *testing.T) { body, ok := doc.Types[bodyID] require.True(t, ok) assert.True(t, body.Common().Anonymous, "a requestBody component is not a named type") - assert.Equal(t, "Body", body.Common().Name.Hint, + assert.Equal(t, "body", body.Common().Name.Hint, "the shared body schema is hinted from its component, not from postA or postB") hdrDoc, hdrDiags := parseFull(t, headerIdentitySpec) requireNoErrorDiags(t, hdrDiags) hdr, ok := hdrDoc.Types[ir.TypeID("t/anon/components/headers/Rate/schema")] require.True(t, ok) - assert.Equal(t, "Rate", hdr.Common().Name.Hint, + assert.Equal(t, "rate", hdr.Common().Name.Hint, "the shared header schema is hinted from its component, not from X-Rate or X-Limit") } @@ -868,7 +869,7 @@ func TestContent_InlineSchemaKeepsItsUseSiteHint(t *testing.T) { requireNoErrorDiags(t, diags) td, ok := doc.Types[ir.TypeID("t/anon/paths/~1a/post/requestBody/content/application~1json/schema")] require.True(t, ok) - assert.Equal(t, "postA_request", td.Common().Name.Hint) + assert.Equal(t, "post_a_request", td.Common().Name.Hint) } const refdEncodingHeaderSpec = `openapi: 3.1.0 diff --git a/compilers/openapi/internal/operation/operations.go b/compilers/openapi/internal/operation/operations.go index 351fb05..55ec3e6 100644 --- a/compilers/openapi/internal/operation/operations.go +++ b/compilers/openapi/internal/operation/operations.go @@ -363,7 +363,7 @@ func operationName(src *soa.Operation, method, uriTemplate string) ir.Naming { if id := src.GetOperationID(); id != "" { return compile.NamingFor(id) } - return compile.NamingHint(ir.CanonicalWords(method + " " + uriTemplate)) + return compile.NamingHint(method + " " + uriTemplate) } // fillOperationDocs maps an operation's summary, description, and externalDocs @@ -485,7 +485,7 @@ func lowerResponse(c lowering.Ctx, ts *compile.Types, anchors *schema.AnchorInde // Naming at all and so has no counterpart here, and would be held by the // presence rule at once if it gained one, since irverify does not exempt it. func responseName(code string) ir.Naming { - return compile.NamingHint(ir.CanonicalWords(code)) + return compile.NamingHint(code) } // lowerErrorCase lowers one error response into an ErrorCase, classifying its diff --git a/compilers/openapi/internal/schema/compose.go b/compilers/openapi/internal/schema/compose.go index e4d93ed..342fedc 100644 --- a/compilers/openapi/internal/schema/compose.go +++ b/compilers/openapi/internal/schema/compose.go @@ -443,7 +443,7 @@ func baseBranchDiscriminator(branches []*oas3.JSONSchema[oas3.Referenceable]) *o // with one Variant per branch (oneOf exclusive, anyOf not), never collapsing a // union into optional fields. func lowerOneOfAnyOf(c lowering.Ctx, ts *compile.Types, anchors *AnchorIndex, depth int, s *oas3.Schema, pointer, hint string) (ir.TypeRef, []ir.Diagnostic) { - if inner, ip, ih, ok := nullUnionCollapse(s, pointer, hint); ok { + if inner, ip, ih, ok := nullUnionCollapse(s, pointer); ok { ref, diags := Ref(c, ts, anchors, depth, inner, ip, ih) ref.Nullable = true return ref, diags diff --git a/compilers/openapi/internal/schema/compose_test.go b/compilers/openapi/internal/schema/compose_test.go index a44e5d0..72a06e4 100644 --- a/compilers/openapi/internal/schema/compose_test.go +++ b/compilers/openapi/internal/schema/compose_test.go @@ -1357,7 +1357,7 @@ func TestUnion_VariantHints(t *testing.T) { u := typeByName(doc, "U").(*ir.Union) require.Len(t, u.Variants, 2) hints := []string{u.Variants[0].Name.Hint, u.Variants[1].Name.Hint} - assert.Contains(t, hints, "Named", "ref-with-siblings hint from target name") + assert.Contains(t, hints, "named", "ref-with-siblings hint from target name") assert.Contains(t, hints, "variant_1", "inline branch positional hint") } @@ -1565,7 +1565,8 @@ func TestOneOf_CoDeclaredCompositionDistributes(t *testing.T) { assert.Equal(t, componentID("Base"), v.Base.Target) require.Len(t, v.Mixins, 1, "the branch joins as a mixin, the composition already having a base") assert.Equal(t, componentID(branch), v.Mixins[0].Target) - assert.Equal(t, branch, u.Variants[i].Name.Hint) + assert.Equal(t, ir.CanonicalWords(branch), u.Variants[i].Name.Hint, + "the variant is named after its branch, in the neutral words every name channel carries") } assert.Equal(t, 1, countDiagsAt(diags, diag.CompositionLowering, ir.SeverityInfo), "the reshaping is reported once; got %+v", diags) @@ -1780,6 +1781,50 @@ func TestComposition_BranchAliasIsOrderIndependent(t *testing.T) { } } +// nullCollapseSpec writes the ordinary single-combinator `{X, null}` collapse +// with an outside component naming its surviving branch's pointer. The branch +// declares a description on purpose: a bare `{type: string}` branch resolves to +// the shared primitive and owns no node, so nothing would compete for the +// pointer and no hint would ever have to agree with another. hostFirst permutes +// which of the two components is declared first, and components lower in source +// order, so the two spellings are the two orders the pointer can be reached in. +func nullCollapseSpec(hostFirst bool) string { + host := " S:\n oneOf:\n - {type: string, description: the branch}\n" + + " - {type: \"null\"}\n" + outside := " Outsider: {$ref: '#/components/schemas/S/oneOf/0'}\n" + if hostFirst { + return componentSpec(host + outside) + } + return componentSpec(outside + host) +} + +// TestNullCollapse_BranchHintIsOrderIndependent is TestComposition_Branch- +// AliasIsOrderIndependent's rule at the site the collapse takes instead. The +// collapse lowers the surviving branch at the branch pointer but used to hand it +// the *enclosing* schema's hint, while an outside $ref naming that same pointer +// derives variant_ through subSchemaHint — so whichever lowering arrived +// first decided the name and the two declaration orders produced two different +// documents, with no diagnostic on either side (GitHub #281). +// +// The single-order assertion is written against the order that was wrong: with S +// declared first the collapse reaches the pointer first, which is where the +// enclosing schema's hint used to land. +func TestNullCollapse_BranchHintIsOrderIndependent(t *testing.T) { + t.Parallel() + first, diags := parseFull(t, nullCollapseSpec(true)) + requireNoErrorDiags(t, diags) + last, diags := parseFull(t, nullCollapseSpec(false)) + requireNoErrorDiags(t, diags) + + branch := ir.TypeID("t/anon/components/schemas/S/oneOf/0") + require.Contains(t, first.Types, branch, "the surviving branch owns a node of its own") + assert.Equal(t, "variant_0", first.Types[branch].Common().Name.Hint, + "the branch takes the hint its composition gives it, not the enclosing schema's") + assert.Empty(t, cmp.Diff(first.Types, last.Types), + "declaring the collapse before or after the outside $ref must not change the registry") + assert.Empty(t, cmp.Diff(first, last, orderInvariantIR()...), "nor the rest of the document") +} + // TestOneOf_CoDeclaredVariantCarriesDiscriminatorValue pins the tag the variants // inherit. The enclosing schema is an allOf subtype of a discriminated base, so // every variant is written on the wire with that subtype's tag — and the tag @@ -1822,7 +1867,7 @@ func TestOneOf_CoDeclaredAdditionalPropsHintNamesTheBody(t *testing.T) { v, ok := doc.Types[u.Variants[0].Type.Target].(*ir.Model) require.True(t, ok) require.NotNil(t, v.AdditionalProps) - assert.Equal(t, "Combo_value", doc.Types[v.AdditionalProps.Value.Target].Common().Name.Hint) + assert.Equal(t, "combo_value", doc.Types[v.AdditionalProps.Value.Target].Common().Name.Hint) } // TestOneOf_CoDeclaredNonModelBranchIsCarriedAsWritten pins what §4.3 says diff --git a/compilers/openapi/internal/schema/schema.go b/compilers/openapi/internal/schema/schema.go index d06d1eb..7dc5bd6 100644 --- a/compilers/openapi/internal/schema/schema.go +++ b/compilers/openapi/internal/schema/schema.go @@ -1708,14 +1708,14 @@ func schemaAdmitsNull(s *oas3.Schema) bool { // (ir-design §3.3). A set with two or more non-null branches falls through to a // Union (with its null branches stripped and lifted onto the enclosing ref). // -// The hint it returns for the surviving branch is the *enclosing* schema's, -// while an outside $ref to that same branch pointer derives variant_ -// through subSchemaHint — so which of the two lowerings reaches the pointer -// first decides the name, and the two declaration orders produce different -// documents. That predates this function's co-declaration rule and is #181's -// mechanism at a site #181 did not sweep; GitHub #281 holds it. It is narrowed -// but not settled here: declining the collapse below removes the one order in -// which a co-declared anyOf could reach it. +// The hint is the branch's own (branchHint), not the enclosing schema's. The +// pointer returned is the branch's, so an outside $ref can name it too and +// derives its hint through subSchemaHint; only the first lowering to arrive +// interns the node, so a hint derived differently here made the document depend +// on declaration order — silently, since either spelling is a valid hint. That +// was #181's mechanism at a site #181 did not sweep (GitHub #281), and the +// repair is #181's: both paths ask branchHint's question, so they agree whichever +// arrives first. // // A schema declaring both combinators collapses neither. The collapse says the // position *is* nullable X, and a co-declared anyOf conjoins with it, so it is @@ -1724,7 +1724,7 @@ func schemaAdmitsNull(s *oas3.Schema) bool { // (preserveUnusedCombinator). Collapsing here would resolve the position // straight to X's own node — a shared primitive for `{type: string}` — leaving // the loser nowhere to sit that is not shared with every other declaration of X. -func nullUnionCollapse(s *oas3.Schema, pointer, hint string) (*oas3.JSONSchema[oas3.Referenceable], string, string, bool) { +func nullUnionCollapse(s *oas3.Schema, pointer string) (*oas3.JSONSchema[oas3.Referenceable], string, string, bool) { if len(s.GetOneOf()) > 0 && len(s.GetAnyOf()) > 0 { return nil, "", "", false } @@ -1742,7 +1742,7 @@ func nullUnionCollapse(s *oas3.Schema, pointer, hint string) (*oas3.JSONSchema[o if nullCount == 0 || nonNullCount != 1 { return nil, "", "", false } - return nonNull, pointer + ids.Ptr(key, strconv.Itoa(nonNullIdx)), hint, true + return nonNull, pointer + ids.Ptr(key, strconv.Itoa(nonNullIdx)), branchHint(nonNull, nonNullIdx), true } // isNullSchema reports whether a variant schema is the bare null-typed schema. diff --git a/compilers/openapi/internal/schema/schema_test.go b/compilers/openapi/internal/schema/schema_test.go index f4b1c91..1deccc0 100644 --- a/compilers/openapi/internal/schema/schema_test.go +++ b/compilers/openapi/internal/schema/schema_test.go @@ -2135,13 +2135,23 @@ func TestInlinePosition_OutsideRefDoesNotMoveTheHome(t *testing.T) { // differ by construction when the same components are declared in two orders. // // SourceInfo.Hash digests the source bytes, which are the thing being permuted. -// Naming.Hint is a live gap: the hint a node hoisted at a pointer carries is -// minted by whichever of the two namers reaches the pointer first — the -// declaration's context ("A_item") or the reference's last pointer segment -// ("items") — because intern keeps the first name it is given. That divergence -// is naming only, and predates the annotation work: a $ref to an object-bodied -// `items` shows it with no annotations involved at all. Everything else is -// compared. +// +// Naming.Hint is a live gap, and a narrower one than it was: the hint a node +// hoisted at a pointer carries is minted by whichever of the two namers reaches +// the pointer first, because intern keeps the first name it is given. The +// composition-branch family no longer diverges — both namers ask branchHint's +// question there (GitHub #181, #281) — so what is left is the four inline +// structural positions, where the declaration's context ("a_item") and the +// reference's last pointer segment ("items") genuinely hold different +// information: GitHub #353, which permutes exactly the positions +// TestInlinePosition_OutsideRefDoesNotMoveTheHome does. That divergence is +// naming only, and predates the annotation work: a $ref to an object-bodied +// `items` shows it with no annotations involved at all. +// +// Everything else is compared, and a caller whose shapes settle their hints +// identically both ways should compare the registry a second time with nothing +// excluded rather than rely on this — see TestNullCollapse_BranchHintIsOrder- +// Independent. func orderInvariantIR() []cmp.Option { return []cmp.Option{ cmpopts.IgnoreFields(ir.Naming{}, "Hint"), diff --git a/docs/ir-design.md b/docs/ir-design.md index a72ceea..6997228 100644 --- a/docs/ir-design.md +++ b/docs/ir-design.md @@ -143,6 +143,15 @@ type Naming struct { policy, and reserved-word escaping. Anonymous (hoisted) types have empty `Source` and a `Hint`; whether a emitter inlines them or names them is its choice. +**`Hint` is the same word sequence in the same grammar.** It is not a presentation affordance +sitting outside the neutrality rule: a hoisted type has no other name, so `Hint` is exactly what an +emitter renders that type's identifier from, which is the job `Canonical` does for a declared one. +That matters because a hint is nearly always derived from something a source *did* spell — a +component key, an `operationId`, a header name, a `$ref` target — so a compiler that passes the +context through puts precisely the casing and punctuation this section rules out into the one +channel that carries a name and no spelling (GitHub #54). What `Hint` is not held to is the +recomputation below: it has no `Source` beside it to be derived from, which is why it exists. + The segmentation is part of the contract, not each compiler's dialect: a word is a run of letters and digits (with the combining marks that belong to them), a camel-case or letter/digit boundary starts a new one, and **every other character separates** — `.`, `/`, `[`, `-`, a space alike. So @@ -171,8 +180,9 @@ to a run of capitals whether or not lowercasing would change it, and `DžBc` is ` titlecase letter belongs to one too. A transition test there would lose the first; the uppercase category alone would lose the second. -`irverify` holds every `Naming` to this: it recomputes the canonical from the `Source` beside it, so -a boundary in the wrong place is a compiler bug rather than a variant reading. What no check can say +`irverify` holds every `Naming` to this: the shape rules run over `Canonical` and `Hint` alike, and +it recomputes the canonical from the `Source` beside it, so a boundary in the wrong place is a +compiler bug rather than a variant reading. What no check can say is that the grammar itself is right — a check that recomputes moves with what it recomputes through — so the answers are pinned by a conformance table and the properties every answer must satisfy by a fuzz target beside it (GitHub #186). diff --git a/docs/micro-compiler-design.md b/docs/micro-compiler-design.md index 0400569..c92c671 100644 --- a/docs/micro-compiler-design.md +++ b/docs/micro-compiler-design.md @@ -139,7 +139,7 @@ golden update, argued there. The OpenAPI goldens did not move, since #161 had al the grammar that ships. Deleting two copies is a state rather than a rule, so the architecture test now asserts that only -the framework and `ir` may fill `Naming.Canonical`. What it still cannot see is a `Hint` (#54). +the framework and `ir` may fill a name channel — `Naming.Canonical`, and `Naming.Hint` since #54. The divergence was filed separately so it would not be lost if this work were deferred; that is what it is now closed by. @@ -721,7 +721,7 @@ Each row is one PR unless noted. "Done when" is the acceptance test, not a summa | ~~0.3~~ | ~~ID-collision oracle~~ | **Landed**, split by where each half is decidable (§8.2): `irverify.checkIDs` holds every ID to its shape and to `path == Provenance.Pointer`, and `compile.Types` refuses a derivation that collapses two coordinates. Both proven by planting one | — | | ~~1.1~~ | ~~Promote the ID grammar into `compilers/compile`~~ | **Landed.** `compilers/openapi` derives no ID except through the framework, goldens byte-identical, and the minted-namespace rule is refused by `compile.Types` rather than asserted in a comment | — | | ~~1.2~~ | ~~Promote the canonical naming grammar (the segmentation is decided — §3.1)~~ | **Landed.** `compilers/compile` holds the one implementation, an architecture test keeps a second from being written, and each rebasing draft carries its own golden update | — | -| ~~1.3~~ | ~~Extend `irverify` to check segmentation, not only casing (#73, #54)~~ | **Landed with #161**, ahead of 1.2: `ir/naming-not-words` rejects a lowercase but unsegmented canonical, proven by planting the old grammar and watching the corpus sweep redden. #54 (`Hint`) stays open | — | +| ~~1.3~~ | ~~Extend `irverify` to check segmentation, not only casing (#73, #54)~~ | **Landed with #161**, ahead of 1.2: `ir/naming-not-words` rejects a lowercase but unsegmented canonical, proven by planting the old grammar and watching the corpus sweep redden. #54 extended the same rules to `Hint` | — | | ~~2.1~~ | ~~Tier-0 extraction: `diag`~~ | **Landed.** Goldens byte-identical, its own rules entry admits `ir` alone, and the package carries table-driven tests needing no document | 0.1 | | ~~2.2–2.7~~ | ~~The remaining Tier-0 extractions: `load`, `scan`, `ids`, `value`, `annotation` (+ site), `merge`~~ | **Landed.** Goldens byte-identical, each package carries its own rules entry proven by planting a forbidden import, and each carries unit tests needing no document. Two departures from the plan above: the scan took `nodeview` with it as a package of its own, because `schema` and `compose` read the source through it too, and `annotation` took the readers `schema.go` declared as well as the four in `resolve.go` — the two files were mutually dependent | 0.1 | | 3.1 | Introduce `Ctx` with accessors; derive indexes at entry | No exported `Ctx` field is a map; goldens byte-identical | 2.x | @@ -742,7 +742,7 @@ landing them first would only encode the current one. |---|---| | #57 archtest cannot enforce compiler isolation | **Closed.** Landed with #161/#143; it was a prerequisite for every package boundary here | | #73 naming grammar and primitive IDs are cross-compiler ABI in one compiler | **Closed, and its own proposal was right about both halves.** The naming grammar lives in `ir` with `irverify` validating against it. The ID grammar stayed in `compilers/compile` — a compiler's path is its own and nothing in `ir` can compute one — but `t/prim/` is the path there is none of, so `ir.PrimTypeID` went to `ir` with it, and `irverify` holds every producer to it: §3.4 | -| #54 cased `Naming.Hint` passes the neutrality check | **Still open.** 1.3's segmentation work did not reach `Hint`: closing it means changing how hints are derived and regenerating every golden, which is a different change from tightening the checker. The exclusion is now stated in `checkNaming` rather than left to be inferred | +| #54 cased `Naming.Hint` passes the neutrality check | **Closed**, as its own entry said it would have to be: by changing how the compilers derive hints — `compile.NamingHint` and `compile.SubHint` run the grammar — and regenerating every golden, not by tightening the checker alone | | #83 enforce size and complexity caps in lint | **Closed by 4.2**, deliberately last | | #66 extract a shared JSON-Schema→IR lowering core before the next compilers land | **Superseded.** Its premise expired — the next compilers landed without it (#20, #21). §3 replaces it with evidence-based promotion. To be closed with that reasoning, not silently | | #142 the annotation matrix cannot reach a carrier position | **Closed**, independently of this work as §8.4 said it could be: the grid gained a kind per carrier, and the two are separate kinds because their carriers hold different sets | diff --git a/docs/micro-compiler-plan.md b/docs/micro-compiler-plan.md index 73a4ba4..c2ced3c 100644 --- a/docs/micro-compiler-plan.md +++ b/docs/micro-compiler-plan.md @@ -62,7 +62,7 @@ landed with it, in that order, in one pull request. |---|---|---| | ~~#162~~ | **Landed.** Identifier grammar into `compilers/compile`: `compile.TypeID` and friends over a `compile.Space`, with the minted-namespace rule refused by `compile.Types` | — | | ~~#163~~ | **Landed.** Canonical naming grammar into `compilers/compile`, with `compile.NamingFor` beside it and a conformance suite pinning the boundaries `irverify` cannot see | — | -| ~~#164~~ | **Landed** in three parts: #161 brought `ir/naming-not-words`; `ir/naming-unsegmented` followed for the letter/digit boundary, which a neutral name still carries evidence of; and `ir/naming-not-derived` closed the rest by moving the grammar to `ir` so the verifier can recompute a canonical from its source, which is the only way to see a camel-case boundary. `Hint` (#54) stays out | — | +| ~~#164~~ | **Landed** in three parts: #161 brought `ir/naming-not-words`; `ir/naming-unsegmented` followed for the letter/digit boundary, which a neutral name still carries evidence of; and `ir/naming-not-derived` closed the rest by moving the grammar to `ir` so the verifier can recompute a canonical from its source, which is the only way to see a camel-case boundary. `Hint` followed later, with #54 | — | | ~~#73~~ | **Landed.** Its text proposed `ir` for both halves and was right about both. The naming grammar went there, and `irverify` validates against it. The ID *grammar* stayed in `compilers/compile`, but `t/prim/` followed the naming half to `ir` as `ir.PrimTypeID` — a primitive has no path for a compiler to own — with `ir/prim-id-not-derived` and `ir/prim-space-reserved` holding every producer to it | — | #163 changed no output here: #161 had already fixed the segmentation in `compilers/openapi` and @@ -142,7 +142,7 @@ Work already filed that lands inside this restructuring rather than alongside it | #179 | Source index — blocked on `$ref` handling being correct first (#40; #141 and #143 are closed) | | ~~#161~~ | **Landed**, deliberately unblocked from #163: promotion would have fixed it, but it was a contract violation shipping today and did not wait on an architecture programme | | ~~#142~~ | **Landed**, independently as its entry said it could be: the matrix gained a kind per carrier, and property and parameter are separate kinds because their carriers hold different sets | -| #54 | Cased `Naming.Hint` passes the neutrality check. #164 landed without reaching `Hint`, so this stays open — `neutral-naming.golden.json` shows one | +| ~~#54~~ | **Landed** after #164, which had not reached `Hint`: the shape rules now run over both name channels, and the compilers derive hints through the grammar so the corpus can satisfy them | | #66 | Closed as superseded — its premise expired when the next compilers landed without it | | #20, #21 | GraphQL and Protobuf drafts are read-only evidence here, not work items | diff --git a/internal/archtest/grammar_test.go b/internal/archtest/grammar_test.go index 46bafe2..f865ee9 100644 --- a/internal/archtest/grammar_test.go +++ b/internal/archtest/grammar_test.go @@ -8,6 +8,7 @@ import ( "go/token" "io/fs" "path/filepath" + "slices" "strings" "testing" @@ -21,54 +22,69 @@ import ( // derived from it. var grammarOwners = []string{"compilers/compile", "ir"} -// TestNamingGrammar_CanonicalIsFilledByTheFrameworkOnly asserts that no -// production package outside grammarOwners fills ir.Naming.Canonical itself. +// nameChannels are the ir.Naming fields that carry a name for an emitter to +// render, and so the fields the grammar has to have produced. Both are held for +// the same reason and by the same rule: Canonical is the words of a declared +// name, Hint the words of a generated one, and an emitter reading either cannot +// tell which compiler wrote it (GitHub #163, #54). // -// Canonical is ABI: an emitter reading it cannot tell which compiler produced the -// name, so a compiler holding its own segmentation opinion makes the field mean -// two things at once. That is not hypothetical — three copies of the grammar -// disagreed about "." and about every other non-word character (GitHub #163) — -// and deleting two copies does not stop a fourth being written. Only a rule -// outside the compilers does. +// Source and Aliases are not here. They are source spellings kept as written, +// which is the opposite requirement. +var nameChannels = []string{"Canonical", "Hint"} + +// TestNamingGrammar_NameChannelsAreFilledByTheFrameworkOnly asserts that no +// production package outside grammarOwners fills a nameChannels field itself. +// +// Those fields are ABI: an emitter reading one cannot tell which compiler +// produced the name, so a compiler holding its own segmentation opinion makes +// the field mean two things at once. That is not hypothetical — three copies of +// the grammar disagreed about "." and about every other non-word character +// (GitHub #163) — and deleting two copies does not stop a fourth being written. +// Only a rule outside the compilers does. // -// Deliberately not checked: a Canonical filled from a local variable reads as a +// Deliberately not checked: a name filled from a local variable reads as a // violation even when the variable came from the framework, so the call belongs -// at the site; Naming.Hint is out of scope and is cased today (GitHub #54); and a -// literal inside package ir is spelled Naming rather than ir.Naming, which is one -// reason ir is an owner rather than a swept package. -func TestNamingGrammar_CanonicalIsFilledByTheFrameworkOnly(t *testing.T) { +// at the site; and a literal inside package ir is spelled Naming rather than +// ir.Naming, which is one reason ir is an owner rather than a swept package. +func TestNamingGrammar_NameChannelsAreFilledByTheFrameworkOnly(t *testing.T) { t.Parallel() - offenders := sweepProduction(t, repoRoot(t), "", grammarOwners, canonicalViolations) + offenders := sweepProduction(t, repoRoot(t), "", grammarOwners, nameChannelViolations) assert.Empty(t, offenders, - "only %v may derive a canonical name; everything else goes through compile.NamingFor or ir.CanonicalWords", + "only %v may derive a name; everything else goes through compile.NamingFor, compile.NamingHint or ir.CanonicalWords", grammarOwners) } -// TestCanonicalViolations_LocalGrammarIsCaught plants what the sweep exists to -// find — a compiler deriving its own canonical, in both the shapes it can be -// written — and pins that the framework call beside it stays clean. Without the -// planted half, a matcher recognizing nothing at all would pass the sweep above -// and read as proof. -func TestCanonicalViolations_LocalGrammarIsCaught(t *testing.T) { +// TestNameChannelViolations_LocalGrammarIsCaught plants what the sweep exists to +// find — a compiler deriving its own name, in every shape it can be written, in +// both channels — and pins that the framework calls beside it stay clean. +// Without the planted half, a matcher recognizing nothing at all would pass the +// sweep above and read as proof. +func TestNameChannelViolations_LocalGrammarIsCaught(t *testing.T) { t.Parallel() const src = `package graphql func lower(name string) []ir.Naming { declared := ir.Naming{Source: name, Canonical: canonicalWords(name)} framework := ir.Naming{Source: name, Canonical: ir.CanonicalWords(name)} - hint := ir.Naming{Hint: localWords(name)} + hinted := ir.Naming{Hint: localWords(name)} + fromFramework := ir.Naming{Hint: compile.SubHint(name, "item")} + kept := ir.Naming{Source: name, Aliases: []string{name}} var late ir.Naming late.Canonical = strings.ToLower(name) - return []ir.Naming{declared, framework, hint, late, {Canonical: lower(name)}} + late.Hint = strings.ToLower(name) + return []ir.Naming{declared, framework, hinted, fromFramework, kept, late, {Canonical: lower(name)}} } ` - offenders, err := canonicalViolations("planted.go", "compilers/graphql/naming.go", src) + offenders, err := nameChannelViolations("planted.go", "compilers/graphql/naming.go", src) require.NoError(t, err) - require.Len(t, offenders, 3, - "the literal, the assignment and the elided literal — not the framework call or the hint: %v", offenders) - assert.Contains(t, offenders[0], "canonicalWords(name)") - assert.Contains(t, offenders[1], "strings.ToLower(name)") - assert.Contains(t, offenders[2], "lower(name)", "an element of a []ir.Naming elides the type") + require.Len(t, offenders, 5, + "the two literals, the two assignments and the elided literal — not the framework calls, and not Aliases: %v", + offenders) + assert.Contains(t, offenders[0], "Canonical is filled by canonicalWords(name)") + assert.Contains(t, offenders[1], "Hint is filled by localWords(name)") + assert.Contains(t, offenders[2], "Canonical is filled by strings.ToLower(name)") + assert.Contains(t, offenders[3], "Hint is filled by strings.ToLower(name)") + assert.Contains(t, offenders[4], "lower(name)", "an element of a []ir.Naming elides the type") } // idOwners is the package permitted to construct an ir ID type from a string. @@ -188,13 +204,14 @@ func holdsStringLiteral(exprs []ast.Expr) bool { return false } -// canonicalViolations reports every place in one file that fills -// ir.Naming.Canonical from something other than a call into the framework -// package, whether as a composite-literal field or an assignment afterwards. +// nameChannelViolations reports every place in one file that fills a +// nameChannels field of ir.Naming from something other than a call into the +// framework package, whether as a composite-literal field or an assignment +// afterwards. // // src is nil to read the file at path, or the source itself for a planted test; // rel names the file in the messages. -func canonicalViolations(path, rel string, src any) ([]string, error) { +func nameChannelViolations(path, rel string, src any) ([]string, error) { fset := token.NewFileSet() file, err := parser.ParseFile(fset, path, src, 0) if err != nil { @@ -202,16 +219,16 @@ func canonicalViolations(path, rel string, src any) ([]string, error) { } var found []string - report := func(expr ast.Expr) { - found = append(found, fmt.Sprintf("%s:%d: Canonical is filled by %s rather than by the framework", - rel, fset.Position(expr.Pos()).Line, exprText(fset, expr))) + report := func(field string, expr ast.Expr) { + found = append(found, fmt.Sprintf("%s:%d: %s is filled by %s rather than by the framework", + rel, fset.Position(expr.Pos()).Line, field, exprText(fset, expr))) } ast.Inspect(file, func(n ast.Node) bool { switch node := n.(type) { case *ast.CompositeLit: - reportCanonicalField(node, report) + reportNameChannelField(node, report) case *ast.AssignStmt: - reportCanonicalAssign(node, report) + reportNameChannelAssign(node, report) default: } return true @@ -219,49 +236,52 @@ func canonicalViolations(path, rel string, src any) ([]string, error) { return found, nil } -// reportCanonicalField reports a Canonical field of a composite literal when it -// is not filled by a framework call. +// reportNameChannelField reports a name-channel field of a composite literal +// when it is not filled by a framework call. // // The literal's type is not required to be ir.Naming, and not only because an // element of a []ir.Naming elides it: ir.Naming is the one type in the repository -// with a Canonical field, so the field name identifies it. A second type carrying -// that name would need this narrowed — and would be worth a look on its own. -func reportCanonicalField(lit *ast.CompositeLit, report func(ast.Expr)) { +// with a Canonical field, and the one with a Hint field — the neighbouring hint +// carriers are spelled MediaTypeHint and XMLHints — so the field name identifies +// it. A second type carrying either name would need this narrowed, and would be +// worth a look on its own. +func reportNameChannelField(lit *ast.CompositeLit, report func(string, ast.Expr)) { for _, elt := range lit.Elts { kv, ok := elt.(*ast.KeyValueExpr) if !ok { continue } key, ok := kv.Key.(*ast.Ident) - if ok && key.Name == "Canonical" && !isFrameworkCall(kv.Value) { - report(kv.Value) + if ok && slices.Contains(nameChannels, key.Name) && !isFrameworkCall(kv.Value) { + report(key.Name, kv.Value) } } } -// reportCanonicalAssign reports an assignment to a .Canonical field whose value -// is not a framework call. It closes the way round the literal check: build an -// empty Naming, then fill the field. -func reportCanonicalAssign(stmt *ast.AssignStmt, report func(ast.Expr)) { +// reportNameChannelAssign reports an assignment to a name-channel field whose +// value is not a framework call. It closes the way round the literal check: build +// an empty Naming, then fill the field. +func reportNameChannelAssign(stmt *ast.AssignStmt, report func(string, ast.Expr)) { for i, lhs := range stmt.Lhs { sel, ok := lhs.(*ast.SelectorExpr) - if !ok || sel.Sel.Name != "Canonical" { + if !ok || !slices.Contains(nameChannels, sel.Sel.Name) { continue } // A multi-value right-hand side has no single expression to attribute to - // the field, and no legitimate shape assigns Canonical that way. + // the field, and no legitimate shape assigns a name channel that way. if len(stmt.Rhs) != len(stmt.Lhs) { - report(lhs) + report(sel.Sel.Name, lhs) continue } if !isFrameworkCall(stmt.Rhs[i]) { - report(stmt.Rhs[i]) + report(sel.Sel.Name, stmt.Rhs[i]) } } } // isFrameworkCall reports whether expr is a call on a package that owns the -// grammar — compile.NamingFor(name) or ir.CanonicalWords(name). Any of their +// grammar — compile.NamingFor(name), compile.SubHint(...) or +// ir.CanonicalWords(name). Any of their // functions counts: the rule is where the grammar lives, not which entry point // reaches it, and the two entry points sit in different packages because the // grammar is beside the field it fills while the constructor that pairs it with diff --git a/ir/irverify/naming.go b/ir/irverify/naming.go index 9899a11..264652f 100644 --- a/ir/irverify/naming.go +++ b/ir/irverify/naming.go @@ -41,9 +41,9 @@ var nameOptional = map[reflect.Type]bool{ reflect.TypeFor[ir.Primitive](): true, } -// checkNaming asserts every named entity has a name at all, and that every -// Naming.Canonical is what invariant #4 promises: a neutral lower_snake word -// sequence, carrying no casing an emitter should own and no character that is +// checkNaming asserts every named entity has a name at all, and that the names +// it carries are what invariant #4 promises: neutral lower_snake word +// sequences, carrying no casing an emitter should own and no character that is // not part of a word. It reuses the shared bounded walk to reach every ir.Naming // value in the document, and reports whether that walk was cut short so a name // past the cap cannot go unchecked in silence. @@ -52,12 +52,17 @@ var nameOptional = map[reflect.Type]bool{ // vacuously true of the empty string: an entirely empty Naming satisfied all // three while leaving an emitter nothing to name the entity by (GitHub #251). // -// Only Canonical is checked for content. Naming.Hint — the generated-name -// channel — is held to none of the content rules, so casing -// and punctuation still reach the IR through it. That is GitHub #54, left open -// deliberately: closing it means changing how the compilers derive hints and -// regenerating every golden, which is a different change from tightening this -// checker. +// Canonical and Hint are both held. They differ in where the name came from — +// one from a spelling the source wrote, the other from the position the entity +// occupies — and not in what it has to be: a hint is the only name an anonymous +// type has, so it is exactly what an emitter renders that type's identifier +// from. A hint is still derived from a string the source spelled, though — a +// component key, an operationId, a header name — so while nothing held it, that +// spelling's casing and punctuation reached the IR through it (GitHub #54). +// Only the grammar rule stays canonical-only, because a hint has no source +// spelling beside it to be recomputed from. +// +// Naming.Aliases is still read by no rule here (GitHub #317). func checkNaming(doc *ir.Document, _ declarations) ([]Violation, bool) { var vs []Violation optional := map[string]bool{} @@ -78,7 +83,7 @@ func checkNaming(doc *ir.Document, _ declarations) ([]Violation, bool) { if !optional[path] { vs = appendAbsentViolation(vs, source, canon, hint, path) } - vs = appendNamingViolations(vs, source, canon, path) + vs = appendNamingViolations(vs, source, canon, hint, path) return false // Naming holds no references or nested Naming to descend into }) return vs, truncated @@ -110,31 +115,43 @@ func appendAbsentViolation(vs []Violation, source, canon, hint, path string) []V }) } -// appendNamingViolations reports the ways canon can break neutrality. They are -// checked separately because each can hold without the others — "userID" is -// segmented but cased, "com.example.user" is lowercase but unsegmented, -// "foo2bar" is both lowercase and made of word characters yet runs two words -// together — and each names a different repair. -func appendNamingViolations(vs []Violation, source, canon, path string) []Violation { +// appendNamingViolations reports the ways one Naming can break neutrality: the +// grammar rule over the canonical, and the content rules over each channel that +// carries a name for an emitter to render. +func appendNamingViolations(vs []Violation, source, canon, hint, path string) []Violation { vs = appendGrammarViolation(vs, source, canon, path) - if isCased(canon) { + vs = appendContentViolations(vs, "canonical name", canon, path) + return appendContentViolations(vs, "name hint", hint, path) +} + +// appendContentViolations reports the ways the name in one channel can break +// neutrality. channel is how the message spells which channel was wrong: the +// two share a Path and a defect class, so the message is what tells them +// apart. +// +// The three are checked separately because each can hold without the others — +// "userID" is segmented but cased, "com.example.user" is lowercase but +// unsegmented, "foo2bar" is both lowercase and made of word characters yet runs +// two words together — and each names a different repair. +func appendContentViolations(vs []Violation, channel, name, path string) []Violation { + if isCased(name) { vs = append(vs, Violation{ Code: "ir/naming-cased", - Message: "canonical name " + canon + " carries casing; store neutral words", + Message: channel + " " + name + " carries casing; store neutral words", Path: path, }) } - if !isWordSequence(canon) { + if !isWordSequence(name) { vs = append(vs, Violation{ Code: "ir/naming-not-words", - Message: "canonical name " + canon + " is not a word sequence; split it on every non-word character", + Message: channel + " " + name + " is not a word sequence; split it on every non-word character", Path: path, }) } - if !isSegmented(canon) { + if !isSegmented(name) { vs = append(vs, Violation{ Code: "ir/naming-unsegmented", - Message: "canonical name " + canon + + Message: channel + " " + name + " runs a letter and a digit together in one word; the grammar splits that boundary", Path: path, }) @@ -206,11 +223,11 @@ func straddlesLetterDigit(prev, r rune) bool { (unicode.IsDigit(prev) && unicode.IsLetter(r)) } -// isWordSequence reports whether s is the shape Naming.Canonical promises: words -// joined by single underscores, each word made of letters, digits and the -// combining marks that belong to them. The empty string qualifies — a Naming may -// carry only a Hint, and a source name with no word rune in it has no words to -// report. +// isWordSequence reports whether s is the shape a neutral name channel +// promises: words joined by single underscores, each word made of letters, +// digits and the combining marks that belong to them. The empty string +// qualifies — a Naming fills one channel or the other, and a source name with no +// word rune in it has no words to report. // // It says nothing about where the boundaries fall inside a run of word // characters: "foo2bar" and "foo_2_bar" are both word sequences by this test. diff --git a/ir/irverify/naming_test.go b/ir/irverify/naming_test.go index 604beb3..626c7fa 100644 --- a/ir/irverify/naming_test.go +++ b/ir/irverify/naming_test.go @@ -18,6 +18,13 @@ func canonicalOnly(canon string) *ir.Document { return modelNamed(ir.Naming{Canonical: canon}) } +// hintOnly names a model the way an anonymous type is named: a hint and nothing +// else. It is the shape every content rule below is measured against for the +// hint channel, since a Source beside it would bring the grammar check in. +func hintOnly(hint string) *ir.Document { + return modelNamed(ir.Naming{Hint: hint}) +} + func modelNamed(n ir.Naming) *ir.Document { m := &ir.Model{TypeCommon: ir.TypeCommon{ID: "t/x/M", Name: n}} return &ir.Document{Types: ir.TypeRegistry{m.ID: m}} @@ -147,6 +154,58 @@ func TestVerify_UnsplitCamelCasePassesEveryOtherCheck(t *testing.T) { "carried without a source, the same value is indistinguishable from one genuine word") } +// TestVerify_CasedOrPunctuatedHintIsAViolation holds the hint channel to the +// same content rules as the canonical one. Hint is the *only* name an anonymous +// type carries, so it is what an emitter renders that type's identifier from — +// the same job Canonical does for a declared name, and so the same rules. While +// it was held to none of them, casing and source punctuation reached the IR +// through it: every value below but the first is a hint the compilers really +// emitted into the committed goldens (GitHub #54). +func TestVerify_CasedOrPunctuatedHintIsAViolation(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + hint string + codes []string + }{ + {"camel case", "connectionDomain", []string{"ir/naming-cased"}}, + {"pascal case", "OrderBody", []string{"ir/naming-cased"}}, + {"a dotted property name", "rollout.state", []string{"ir/naming-not-words"}}, + {"a path template", "get_/pets/{pet_id}", []string{"ir/naming-not-words"}}, + {"a header name", "X-Report-List", []string{"ir/naming-cased", "ir/naming-not-words"}}, + {"a letter-digit run", "foo2bar", []string{"ir/naming-unsegmented"}}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.codes, codesOf(irverify.Verify(hintOnly(tc.hint))), "hint %q", tc.hint) + }) + } +} + +// TestVerify_NeutralHintIsClean is the other direction: the shapes a compiler +// that derives hints through the grammar emits are not reported, so the rules +// above cannot pass by rejecting every hint. +func TestVerify_NeutralHintIsClean(t *testing.T) { + t.Parallel() + for _, hint := range []string{ + "connection_domain", "empty", "variant_0", "order_body", + "get_pets_pet_id", "empty_item", "count_ℤ", + } { + assert.Empty(t, irverify.Verify(hintOnly(hint)), "hint %q is a neutral word sequence", hint) + } +} + +// TestVerify_HintIsNotDerivedFromTheSource pins what the hint channel is *not* +// held to. The grammar check recomputes a canonical from the spelling beside it; +// a hint has no such spelling to recompute from — it is derived from the +// position, which is the whole reason it exists — so asking it to agree with a +// Source would be inventing a relation the IR does not claim. +func TestVerify_HintIsNotDerivedFromTheSource(t *testing.T) { + t.Parallel() + assert.Empty(t, irverify.Verify(modelNamed( + ir.Naming{Source: "Widget", Canonical: "widget", Hint: "order_body"}))) +} + // TestVerify_DerivedNamingIsClean is the control: a Naming the grammar produced // reports nothing, and neither does one that carries a hint instead of a source. func TestVerify_DerivedNamingIsClean(t *testing.T) { diff --git a/ir/naming.go b/ir/naming.go index 94a9fea..36c868c 100644 --- a/ir/naming.go +++ b/ir/naming.go @@ -23,7 +23,10 @@ type Naming struct { Canonical string `json:"canonical,omitempty"` // Hint is a context-derived suggestion for an entity with no source name to // render: a hoisted anonymous type (e.g. "connection_domain"), or one the - // source named with the empty string. + // source named with the empty string. It is in the same neutral form as + // Canonical and for the same reason — it is the only name such an entity + // has, so it is what an emitter renders its identifier from — however the + // position it was derived from was spelled. Hint string `json:"hint,omitempty"` // Aliases are alternate names for schema-resolution matching (Avro // aliases). Versionless — rename history tied to version labels lives in diff --git a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json index c43e993..1e943d3 100644 --- a/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json +++ b/testdata/conformance/openapi/allof-oneof-cooccurrence.golden.json @@ -22,7 +22,7 @@ "kind": "scalar", "id": "t/anon/components/schemas/Combo/oneOf/0", "name": { - "hint": "A" + "hint": "a" }, "anonymous": true, "docs": {}, @@ -60,7 +60,7 @@ "kind": "model", "id": "t/composed/components/schemas/Combo/oneOf/0", "name": { - "hint": "Combo_A" + "hint": "combo_a" }, "anonymous": true, "docs": {}, @@ -87,7 +87,7 @@ "kind": "model", "id": "t/composed/components/schemas/Combo/oneOf/1", "name": { - "hint": "Combo_B" + "hint": "combo_b" }, "anonymous": true, "docs": {}, @@ -268,7 +268,7 @@ "variants": [ { "name": { - "hint": "A" + "hint": "a" }, "type": { "target": "t/composed/components/schemas/Combo/oneOf/0", @@ -278,7 +278,7 @@ }, { "name": { - "hint": "B" + "hint": "b" }, "type": { "target": "t/composed/components/schemas/Combo/oneOf/1", diff --git a/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json b/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json index 91c0dc7..7b3440c 100644 --- a/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json +++ b/testdata/conformance/openapi/allof-ref-branch-siblings.golden.json @@ -22,7 +22,7 @@ "kind": "scalar", "id": "t/anon/components/schemas/Annotated/allOf/0", "name": { - "hint": "Base" + "hint": "base" }, "anonymous": true, "docs": { @@ -58,7 +58,7 @@ "kind": "scalar", "id": "t/anon/components/schemas/Mixed/allOf/0", "name": { - "hint": "Base" + "hint": "base" }, "anonymous": true, "docs": { diff --git a/testdata/conformance/openapi/component-reuse.golden.json b/testdata/conformance/openapi/component-reuse.golden.json index f5394bd..504132a 100644 --- a/testdata/conformance/openapi/component-reuse.golden.json +++ b/testdata/conformance/openapi/component-reuse.golden.json @@ -436,7 +436,7 @@ "kind": "enum", "id": "t/anon/components/headers/RateUnit/schema", "name": { - "hint": "RateUnit" + "hint": "rate_unit" }, "anonymous": true, "docs": {}, @@ -530,7 +530,7 @@ "kind": "model", "id": "t/anon/components/requestBodies/OrderBody/content/application~1json/schema", "name": { - "hint": "OrderBody" + "hint": "order_body" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/empty-names.golden.json b/testdata/conformance/openapi/empty-names.golden.json index 718e344..7687808 100644 --- a/testdata/conformance/openapi/empty-names.golden.json +++ b/testdata/conformance/openapi/empty-names.golden.json @@ -108,7 +108,7 @@ "variants": [ { "name": { - "hint": "Alt" + "hint": "alt" }, "type": { "target": "t/composed/components/schemas/Host/properties//oneOf/0", @@ -527,7 +527,7 @@ "kind": "model", "id": "t/composed/components/schemas/Host/properties//oneOf/0", "name": { - "hint": "empty_Alt" + "hint": "empty_alt" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/header-content-schema.golden.json b/testdata/conformance/openapi/header-content-schema.golden.json index 30bbf40..40b0674 100644 --- a/testdata/conformance/openapi/header-content-schema.golden.json +++ b/testdata/conformance/openapi/header-content-schema.golden.json @@ -234,7 +234,7 @@ "kind": "list", "id": "t/anon/paths/~1reports/get/responses/200/headers/X-Report-List/schema", "name": { - "hint": "X-Report-List" + "hint": "x_report_list" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/inline-annotations.golden.json b/testdata/conformance/openapi/inline-annotations.golden.json index ebadbb5..6c00f95 100644 --- a/testdata/conformance/openapi/inline-annotations.golden.json +++ b/testdata/conformance/openapi/inline-annotations.golden.json @@ -176,7 +176,7 @@ "kind": "scalar", "id": "t/anon/components/schemas/CodeIndex/additionalProperties", "name": { - "hint": "CodeIndex_value" + "hint": "code_index_value" }, "anonymous": true, "docs": { @@ -202,7 +202,7 @@ "kind": "scalar", "id": "t/anon/components/schemas/Codes/items", "name": { - "hint": "Codes_item" + "hint": "codes_item" }, "anonymous": true, "docs": { diff --git a/testdata/conformance/openapi/inline-residue.golden.json b/testdata/conformance/openapi/inline-residue.golden.json index 09dd323..1234994 100644 --- a/testdata/conformance/openapi/inline-residue.golden.json +++ b/testdata/conformance/openapi/inline-residue.golden.json @@ -87,7 +87,7 @@ "kind": "scalar", "id": "t/anon/components/schemas/Batch/items", "name": { - "hint": "Batch_item" + "hint": "batch_item" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/multipart-encoding.golden.json b/testdata/conformance/openapi/multipart-encoding.golden.json index fde8c82..41cde6c 100644 --- a/testdata/conformance/openapi/multipart-encoding.golden.json +++ b/testdata/conformance/openapi/multipart-encoding.golden.json @@ -355,7 +355,7 @@ "kind": "model", "id": "t/anon/paths/~1submit/post/requestBody/content/application~1x-www-form-urlencoded/schema", "name": { - "hint": "submitForm_request" + "hint": "submit_form_request" }, "anonymous": true, "docs": {}, @@ -419,7 +419,7 @@ "kind": "model", "id": "t/anon/paths/~1upload-composed/post/requestBody/content/multipart~1form-data/schema", "name": { - "hint": "uploadComposed_request" + "hint": "upload_composed_request" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/neutral-naming.golden.json b/testdata/conformance/openapi/neutral-naming.golden.json index 32ec93d..9eceb45 100644 --- a/testdata/conformance/openapi/neutral-naming.golden.json +++ b/testdata/conformance/openapi/neutral-naming.golden.json @@ -172,7 +172,7 @@ "kind": "enum", "id": "t/anon/components/schemas/com.example.Widget/properties/rollout.state", "name": { - "hint": "rollout.state" + "hint": "rollout_state" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/nullable-31-ref.golden.json b/testdata/conformance/openapi/nullable-31-ref.golden.json index eaea3a1..52d16ec 100644 --- a/testdata/conformance/openapi/nullable-31-ref.golden.json +++ b/testdata/conformance/openapi/nullable-31-ref.golden.json @@ -18,6 +18,64 @@ } ], "types": { + "t/anon/components/schemas/Collapsed/oneOf/0": { + "kind": "scalar", + "id": "t/anon/components/schemas/Collapsed/oneOf/0", + "name": { + "hint": "variant_0" + }, + "anonymous": true, + "docs": { + "description": "the surviving branch" + }, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Collapsed/oneOf/0" + }, + "base": { + "target": "t/prim/string", + "nullable": false + } + }, + "t/openapi/components/schemas/BranchRef": { + "kind": "scalar", + "id": "t/openapi/components/schemas/BranchRef", + "name": { + "source": "BranchRef", + "canonical": "branch_ref" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/BranchRef" + }, + "base": { + "target": "t/anon/components/schemas/Collapsed/oneOf/0", + "nullable": false + } + }, + "t/openapi/components/schemas/Collapsed": { + "kind": "scalar", + "id": "t/openapi/components/schemas/Collapsed", + "name": { + "source": "Collapsed", + "canonical": "collapsed" + }, + "anonymous": false, + "docs": {}, + "sensitive": false, + "provenance": { + "source": 0, + "pointer": "/components/schemas/Collapsed" + }, + "base": { + "target": "t/anon/components/schemas/Collapsed/oneOf/0", + "nullable": true + } + }, "t/openapi/components/schemas/Owner": { "kind": "model", "id": "t/openapi/components/schemas/Owner", @@ -188,7 +246,7 @@ { "format": "openapi@3.1", "path": "nullable-31-ref.yaml", - "hash": "209757a2b4823e315fc5b016d88527efc1cbe592152d55f68020f301992885d5" + "hash": "f281021007a8f1e0030f45abf44f6909a4742e57925c3c2e859bd72bd6c42fe4" } ] } diff --git a/testdata/conformance/openapi/nullable-31-ref.yaml b/testdata/conformance/openapi/nullable-31-ref.yaml index 0a685f8..d73d4dc 100644 --- a/testdata/conformance/openapi/nullable-31-ref.yaml +++ b/testdata/conformance/openapi/nullable-31-ref.yaml @@ -7,6 +7,11 @@ components: type: [object, "null"] UnionTarget: oneOf: [{type: string}, {type: integer}, {type: "null"}] + Collapsed: + oneOf: + - {type: string, description: the surviving branch} + - {type: "null"} + BranchRef: {$ref: '#/components/schemas/Collapsed/oneOf/0'} Owner: type: object properties: diff --git a/testdata/conformance/openapi/oneof-discriminated.golden.json b/testdata/conformance/openapi/oneof-discriminated.golden.json index fd6203b..71452e1 100644 --- a/testdata/conformance/openapi/oneof-discriminated.golden.json +++ b/testdata/conformance/openapi/oneof-discriminated.golden.json @@ -129,7 +129,7 @@ "variants": [ { "name": { - "hint": "Cat" + "hint": "cat" }, "type": { "target": "t/openapi/components/schemas/Cat", @@ -139,7 +139,7 @@ }, { "name": { - "hint": "Dog" + "hint": "dog" }, "type": { "target": "t/openapi/components/schemas/Dog", diff --git a/testdata/conformance/openapi/param-styles.golden.json b/testdata/conformance/openapi/param-styles.golden.json index e5c36fa..acb0ad1 100644 --- a/testdata/conformance/openapi/param-styles.golden.json +++ b/testdata/conformance/openapi/param-styles.golden.json @@ -313,7 +313,7 @@ "kind": "enum", "id": "t/anon/paths/~1search/parameters/0/schema", "name": { - "hint": "requestId" + "hint": "request_id" }, "anonymous": true, "docs": {}, diff --git a/testdata/conformance/openapi/webhooks.golden.json b/testdata/conformance/openapi/webhooks.golden.json index a050faa..5377aa9 100644 --- a/testdata/conformance/openapi/webhooks.golden.json +++ b/testdata/conformance/openapi/webhooks.golden.json @@ -126,7 +126,7 @@ "kind": "model", "id": "t/anon/webhooks/newPet/post/requestBody/content/application~1json/schema", "name": { - "hint": "onNewPet_request" + "hint": "on_new_pet_request" }, "anonymous": true, "docs": {},