From 9ec25d714a4eff6647a0feb6f800e681c3c4131e Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Thu, 30 Jul 2026 22:20:04 +0200 Subject: [PATCH 1/2] docs(quirks): retire the resolved quirk entries, point at the live register MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An audit of the planning trackers found the quirk records disagreeing with the code in both directions: entries documented as open had been fixed by a different stream, and one entry documented as fixed was not. The cause is structural — three registers with no cross-links (the Q-series, the D-series, and each package's README §quirks-open), so an item logged in one and fixed via another was never updated. The planning side now has a single live register (`.claude/plans/quirks-open.md`, with the historical ones archived); this is the repo side of the same cleanup. schema/README.md — two entries verified fixed, moved to §quirks-resolved: - "Named-strfmt + swagger:model" claimed a field/definition mismatch ({string, format} at the field, a struct walk at the definition). The golden enhancements_named_struct_tags-ref.json now shows PhoneNumber as {type: string, format: phone} with Contact.phone a $ref to it. Fixed by the F-series pass (8e20d2f, F1) — not by the attempt the entry described, which was reverted, hence nobody updated it. - "Cross-package definition-name collisions silently overwrite" was fixed by the name-identity work: definitions are keyed by a compiler-unique DefKey while building and deconflicted by the reduce stage, with a scan.renamed-definition Hint. Goldens show AWidget/BWidget deconfliction. The third entry — a field-level `enum:` override discarding the type's x-go-enum-desc — is real and still present (probed: no diagnostic fires for the override case; only the no-consts case warns). It is condensed here and its design question moved to the feature that owns it, since whether the per-value docs should be filtered through or dropped with a Hint depends on what an override is meant to mean. features/enum-richer-values.md §1.2b now carries it, correcting that plan's own claim that the quirk was closed with "nothing remains to do" — half of it was (the desc is no longer left stale), half was not (the strip is silent and lossy). routebody/README.md — the "no column tracking" caveat now names the feature that owns it (column-precision-unicode, an LSP prerequisite) rather than sitting loose; the two column issues touch the same Line.Pos contract and want doing together. Each §quirks-open section gains a pointer to the project-wide register, so a reader knows the package caveats are not the whole picture. Also: repoint the five references to quirks-F-series-fix.md at its archived path, and drop a stale warning in docs/examples/shaping/aliases/aliases.go which still told readers that swagger:model type aliases "currently hang the scanner — until F9 is fixed". F9 was fixed and is locked by TestQuirk_AliasModelNoHang across all three alias modes; the example keeps using the dissolve case, but no longer implies the other modes are unsafe. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- docs/examples/shaping/aliases/aliases.go | 12 +- internal/builders/schema/README.md | 251 +++--------------- internal/builders/schema/type_override.go | 2 +- .../builders/schema/walker_classifiers.go | 2 +- internal/builders/validations/README.md | 2 +- internal/parsers/grammar/diagnostic.go | 2 +- internal/parsers/grammar/disambiguate.go | 2 +- internal/parsers/routebody/README.md | 9 +- internal/scanner/README.md | 4 + 9 files changed, 64 insertions(+), 222 deletions(-) diff --git a/docs/examples/shaping/aliases/aliases.go b/docs/examples/shaping/aliases/aliases.go index 1bff31b9..21e2ef66 100644 --- a/docs/examples/shaping/aliases/aliases.go +++ b/docs/examples/shaping/aliases/aliases.go @@ -4,11 +4,13 @@ // how-to. aliases_test.go scans it and writes the golden fragment the guide // renders. // -// NOTE: the first-class-alias modes (a swagger:model type alias under the -// default/RefAliases modes) currently hang the scanner — see doc-site-quirks F9. -// This example deliberately uses an unannotated alias (the dissolve case), which -// is the safe, common behavior; the first-class modes are described -// conceptually in the guide until F9 is fixed. +// This example uses an unannotated alias (the dissolve case), the common +// behaviour the guide leads with; the first-class-alias modes are described +// conceptually there. +// +// (Historical note: those modes once hung the scanner — quirk F9 — which is why +// this example avoids them. The hang was fixed and is locked across all three +// alias modes by TestQuirk_AliasModelNoHang.) package aliases // snippet:alias diff --git a/internal/builders/schema/README.md b/internal/builders/schema/README.md index 6984caed..63b1fe02 100644 --- a/internal/builders/schema/README.md +++ b/internal/builders/schema/README.md @@ -1541,70 +1541,36 @@ edges fixture covers a different (strfmt-tagged) shape already. --- +### ✅ Named-strfmt + `swagger:model` combo (was 🟡 deferred) + +A type carrying both `swagger:strfmt phone` and `swagger:model` used to emit an +inconsistent pair: `{type: string, format: phone}` at the field site, but a +struct walk (`{type: object, properties: …}`) for the top-level definition. The +decl-level strfmt now wins and the field `$ref`s it — verified in +`enhancements_named_struct_tags-ref.json`: `PhoneNumber` is +`{type: string, format: phone}` and `Contact.phone` is a `$ref` to it. + +Fixed by the F-series pass (`8e20d2f`, quirk F1), not by the attempt described +in the original entry — which was reverted. History: +`.claude/plans/archive/deferred-quirks.md` D3. + +### ✅ Cross-package definition-name collisions (was 🟡 silently overwrite) + +Two packages declaring the same identifier (`pkg/a.User`, `pkg/b.User`) both +mapped to `definitions["User"]`, and the second build silently overwrote the +first — one `User` in the output, no record of the collision. + +Fixed by the name-identity / cyclic-`$ref` work: every definition is keyed by a +compiler-unique `DefKey` (`/`) while building, and a final reduce +stage projects each back to the shortest unique name, deconflicting collisions +(`AWidget` / `BWidget`) and raising a `scan.renamed-definition` Hint. See +[§discovery](#discovery) and `.claude/plans/name-identity-cyclic-ref.md`. + ## §quirks-open — still open -### 🟡 Named-strfmt + `swagger:model` combo (deferred) - -When the author combines `swagger:strfmt` with `swagger:model` -on the same type, the FIELD reference inlines as `{string, format}` -(via the strfmt classifier) but the TOP-LEVEL definition body is -still emitted from walking the underlying struct. - -**Reproduction.** Fixture `fixtures/enhancements/named-struct-tags-ref/types.go` -declares `PhoneNumber` with both `swagger:strfmt phone` and -`swagger:model`, used by `Contact.Phone`. The golden -`enhancements_named_struct_tags-ref.json` captures the observable -inconsistency: - -- Field site: `{type: "string", format: "phone"}` — strfmt wins. -- Top-level definition: `{type: "object", properties: {CountryCode, Number}}` — - the struct walk wins; the strfmt annotation is ignored at decl time. - -The author asked for "named strfmt" (a reusable `PhoneNumber` -definition rendered as a formatted string) but gets an inconsistent -pair: the field says string, the definition says object. - -**Attempted fix and reasons it reverted.** The first attempt -(referred to as "Option 1") would have: - -1. Detected `swagger:strfmt` on the decl in `buildDeclNamed` and - emitted `{string, fmt}` instead of walking the struct body. -2. In `buildNamedStruct`, when the target also has `swagger:model`, - emitted `$ref` instead of inlining the strfmt. - -This was reverted before merge because: - -- Pre-existing fixtures in - `fixtures/goparsing/classification/transitive/mods/aliases.go` use - the same `swagger:strfmt + swagger:model` combination on - defined-from-`time.Time` types (e.g. `SomeTimeType time.Time`). - The existing tests (`TestAliasedTypes`, `TestAliasedModels`) - assert the *inline* baseline (`scantest.AssertProperty(..., "string", ...)`) - rather than a `$ref`. Option 1 flips these to `$ref`, requiring - coordinated test updates. -- The decl-level `StrfmtName` check also over-fires on slice / array / - map underlyings: `type SomeTimesType []time.Time` with - `swagger:strfmt date-time` should emit - `{array, items: {string, date-time}}`, not flatten to `{string}`. - A correct fix would gate the check on struct-underlying first, - then symmetrically consider whether `buildNamedSlice` / - `buildNamedArray` / `buildNamedMap` should also route through - `$ref` under the `swagger:model` combination. - -The surface area is wider than the Option 1 code change suggested, -and the existing test coverage of the combination is entangled with -the inconsistency itself. - -**Why deferred.** The combination is niche, the footgun is narrow -(you get what you asked for on one side of the indirection, not -both), and v2's annotation redesign can reshape the contract without -carrying this legacy. A focused decision on "named strfmt" semantics -belongs in the v2 design, not a bug-fix pass. - -The `named-struct-tags-ref` fixture and its golden are checked in as -a deliberate marker — the golden captures the observable -inconsistency (inline field + struct-body definition) so future work -on this decision has a failing test to anchor against. +> **Where open quirks live.** This section documents caveats *of this package*. +> The project-wide register of what is actually open — verified, with the stale +> historical registers called out — is `.claude/plans/quirks-open.md`. ### 🟦 `interface{}` literals (documented behaviour) @@ -1627,155 +1593,18 @@ the substituted underlying via the `TypeArgs` short-circuit without a concrete instantiation simply have no representable schema. -### 🟡 Cross-package definition-name collisions silently overwrite - -`buildFromDecl` writes the top-level schema as -`s.definitions[s.Name] = schema`, keyed only by the Go identifier -(`decl.Names()[0]`). When two packages in a single scan declare a type -with the same identifier — `pkg/a.User` and `pkg/b.User` — both map -to `definitions["User"]` and the second build silently overwrites the -first. The output spec carries only one `User`, with no record of the -collision and no signal of which package won. - -The existing `nameByJSON` (`propOwner`) map in field emission is **not** -a defense against this case: it tracks JSON property names within a -single struct's field set plus its embeds (for the ambiguous-embed -diagnostic), not type-level identifier conflicts across packages. - -#### Target shape - -A proper fix needs three pieces: - -1. **Detection** — at write time, recognise the case "definition key - already exists with non-empty schema and originates from a different - package" (use `x-go-package`, or stash origin in the `Builder`). -2. **Diagnostic** — emit `CodeNameConflict` (severity - `SeverityWarning` minimum, possibly `SeverityError` under strict - mode) carrying both `(pkg, name)` pairs. -3. **Policy** — open design call: - - **a. Rename** — prefix loser(s) with a stable short-package - (e.g. `a_User`, `b_User`). Stable but ugly; needs all `$ref`s - to follow the rename — cross-cutting. - - **b. Skip + warn** — keep the first writer, drop subsequent - ones, emit a warning. Predictable but lossy. - - **c. Fail the build** — under strict mode, treat as an error. - Forces the author to rename in source. Cleanest semantics, - most disruptive. - -#### Why deferred - -Each policy choice changes the contract for downstream code generators -(go-swagger, oapi-codegen, …) — they have assumptions about -`definitions` keys matching exported Go names. The "rename" path -additionally requires every `$ref` writer in the builders to consult a -rename map; the surface is wide. - -For multi-package scans where the author controls both packages, the -workaround today is to scope scans to one package per spec, or to -rename one of the colliding types at the source. A future strict-mode -flag (e.g. `Options.StrictNameConflicts`) could enable option (c) -without breaking existing scans. - -### 🟡 Stale `x-go-enum-desc` after a field-level enum override - -When a field uses a type marked `swagger:enum TypeName` **and** carries -its own `enum: …` override, v1 mutates the schema in place: it replaces -`Enum`, strips the inherited `x-go-enum-desc`, and trims the matching -description suffix. This is **lossy** — the per-value docs contributed -by `TypeName` are silently discarded. - -Concretely, given (fixture `fixtures/enhancements/enum-overrides/`, -case E): - -```go -// swagger:enum PriorityE -type PriorityE string - -const ( - PriorityELow PriorityE = "low" // low-priority requests - PriorityEMed PriorityE = "medium" // medium-priority requests - PriorityEHigh PriorityE = "high" // high-priority requests -) - -type NotificationE struct { - // Inline enum provides a narrower set than the const block. - // - // enum: urgent, normal - Priority PriorityE `json:"priority"` -} -``` - -v1 emits: - -```yaml -priority: - type: string - enum: [urgent, normal] # the override wins - description: "Inline enum provides a narrower set than the const block." - # x-go-enum-desc removed by clearStaleEnumDesc - # PriorityE's per-value doc lines silently dropped from description -``` - -The cleanup runs reactively from `schemaValidations.SetEnum` -([typable.go](typable.go#L128)) via `clearStaleEnumDesc` -([extensions.go](extensions.go#L42)). It treats any -`x-go-enum-desc` present at `SetEnum` time as inherited (and therefore -stale once `Enum` is replaced), deletes it, and trims the matching -suffix off `Description`. The `TrimSuffix` dance is fragile — it -relies on the enum-desc pipeline having appended the doc lines as a -literal suffix — but it works under v1's emission discipline. - -#### Target shape (allOf composition) - -OpenAPI 2.0 supports `allOf` for schema composition, so the cleaner -model does not have to wait for OAS 3. The replacement shape is: - -```yaml -# PriorityE promoted to a top-level definition: -definitions: - PriorityE: - type: string - enum: [low, medium, high] - description: | - low: low-priority requests - medium: medium-priority requests - high: high-priority requests - x-go-enum-desc: | - low: low-priority requests - medium: medium-priority requests - high: high-priority requests - - NotificationE: - type: object - properties: - priority: - description: "Inline enum provides a narrower set than the const block." - allOf: - - $ref: '#/definitions/PriorityE' # inherited enum + per-value docs - - enum: [urgent, normal] # the override -``` - -Each branch keeps its own concern: - -- the `$ref` branch carries `PriorityE`'s full schema (values + docs + - `x-go-enum-desc`), untouched and reusable by every field that - references `PriorityE`; -- the inline branch carries the narrowing override only. - -No mutation of the inherited schema, no `TrimSuffix` dance. Validator -semantics for enum-narrowing `allOf` aren't perfectly uniform across -tools, but for the documentation / code-gen use cases codescan feeds -(go-swagger, oapi-codegen, redoc, …) this composition preserves both -layers cleanly. +### 🟡 A field-level enum override discards the type's per-value docs, silently -#### Prerequisites for the migration (both currently missing) +When a field whose type is marked `swagger:enum TypeName` carries its own +`enum:` override, the inherited `x-go-enum-desc` is stripped along with the +replaced values — and **no diagnostic is raised**, so the per-value docs +`TypeName` contributed vanish without a trace. Reproduced by +`fixtures/enhancements/enum-overrides` case E (`NotificationE`). -1. **Promote unannotated `swagger:enum` types to top-level definitions** - so the `$ref` branch has a target. Today they exist only as inlined - fragments on each referring field. -2. **Move override detection from `SetEnum` (validation hook) to the - field-emission path**, so the override is composed alongside the - inherited schema instead of mutating it after the fact. +Whether the docs should be filtered through (when the override *subsets* the +type's values) or dropped with a Hint (when it narrows to *different* ones) is a +design call that belongs with the enum feature, not with this package: see +`.claude/plans/features/enum-richer-values.md` §1.2b. -Until both land, `clearStaleEnumDesc` stays in place. The TODO in -`extensions.go` flags it as the replacement target. +The stripping itself lives in `handlers/dispatch_schema.go:clearStaleEnumDesc`, +which is where either resolution would land. diff --git a/internal/builders/schema/type_override.go b/internal/builders/schema/type_override.go index 140be036..78e5a960 100644 --- a/internal/builders/schema/type_override.go +++ b/internal/builders/schema/type_override.go @@ -20,7 +20,7 @@ import ( // (never a $ref). // // It is the single resolution point for the keyword consumed at every swagger:type site (the F3 -// reconciliation — see .claude/plans/quirks-F-series-fix.md). +// reconciliation — see .claude/plans/archive/quirks-F-series-fix.md). // // - ownType is the annotated field/decl's Go type, consumed by the // `inline` / `array` keywords (which expand that type in place). May be diff --git a/internal/builders/schema/walker_classifiers.go b/internal/builders/schema/walker_classifiers.go index 075892d7..fd55c30c 100644 --- a/internal/builders/schema/walker_classifiers.go +++ b/internal/builders/schema/walker_classifiers.go @@ -81,7 +81,7 @@ func (s *Builder) classifierTextMarshal(tpe types.Type, tgt ifaces.SwaggerTypabl // It is the single resolution point for `swagger:type` on a named type: it routes the argument // through resolveTypeOverride (always inlining — keyword scalars / Go builtins / `[]T` / `inline` // / `array` / type-name refs), and applies a co-present `swagger:strfmt` as a supplementary format -// only when compatible with the resolved type (F3 — see .claude/plans/quirks-F-series-fix.md). +// only when compatible with the resolved type (F3 — see .claude/plans/archive/quirks-F-series-fix.md). // ownType is the named Go type (consumed by the `inline`/`array` keywords); pos drives diagnostics. // // Reports back via the (handled, fallthrough) tuple: diff --git a/internal/builders/validations/README.md b/internal/builders/validations/README.md index 1cf13c05..6cfac9c4 100644 --- a/internal/builders/validations/README.md +++ b/internal/builders/validations/README.md @@ -161,7 +161,7 @@ once a concrete consumer asks for them. the `swagger:type` + `swagger:strfmt` combination, where `swagger:type` wins on the type axis and the strfmt format is applied as a **supplementary hint only when it is consistent with that type** (the F3 reconciliation — see -`.claude/plans/quirks-F-series-fix.md`). It is **not** used for the +`.claude/plans/archive/quirks-F-series-fix.md`). It is **not** used for the strfmt-alone path, where strfmt still forces `{type: string, format: X}` (go-swagger#1512). diff --git a/internal/parsers/grammar/diagnostic.go b/internal/parsers/grammar/diagnostic.go index d14ea706..586956ff 100644 --- a/internal/parsers/grammar/diagnostic.go +++ b/internal/parsers/grammar/diagnostic.go @@ -89,7 +89,7 @@ const ( // is not valid (e.g. `inline`/`array` as an array element). // // The override is dropped and the subject falls through to its Go type. - // See the F3 reconciliation in .claude/plans/quirks-F-series-fix.md. + // See the F3 reconciliation in .claude/plans/archive/quirks-F-series-fix.md. CodeUnsupportedType Code = "validate.unsupported-type" // CodeDeprecated fires when an accepted-but-deprecated annotation or keyword value is used (the diff --git a/internal/parsers/grammar/disambiguate.go b/internal/parsers/grammar/disambiguate.go index 26b7b1ed..30b552bb 100644 --- a/internal/parsers/grammar/disambiguate.go +++ b/internal/parsers/grammar/disambiguate.go @@ -184,7 +184,7 @@ func classifyHTTPMethod(s string) (string, bool) { // It is a LEXICAL check only: the grammar no longer owns the closed type vocabulary. // Semantic validity (is it a known keyword / scanned type? is the format compatible?) is resolved // by the builder, which alone knows the scanned definitions and the annotated Go type (the F3 -// reconciliation — see .claude/plans/quirks-F-series-fix.md). +// reconciliation — see .claude/plans/archive/quirks-F-series-fix.md). // // The lexer only rejects structurally malformed tokens (empty, embedded spaces, a bare `[]`, a // leading digit, illegal characters), which the parser flags. diff --git a/internal/parsers/routebody/README.md b/internal/parsers/routebody/README.md index 364dd97e..251bac10 100644 --- a/internal/parsers/routebody/README.md +++ b/internal/parsers/routebody/README.md @@ -151,7 +151,14 @@ orchestrator level. ## §quirks-open — deferred follow-ups -- **Column tracking.** routebody does not track per-line column +> **Where open quirks live.** This section documents caveats *of this package*. +> The project-wide register of what is actually open — verified, with the stale +> historical registers called out — is `.claude/plans/quirks-open.md`. + +- **Column tracking** (owned by + `.claude/plans/features/column-precision-unicode.md` — an LSP prerequisite, + paired there with the multi-byte column question since both touch the same + `Line.Pos` contract). routebody does not track per-line column information; diagnostics inherit `basePos.Column`. If the LSP integration needs per-token positions on body sub-language diagnostics, the body parser will need to track lex state more diff --git a/internal/scanner/README.md b/internal/scanner/README.md index 048e14f2..be5d34fb 100644 --- a/internal/scanner/README.md +++ b/internal/scanner/README.md @@ -382,6 +382,10 @@ prose** — author-written overrides (harvested separately) are never filtered. ## §quirks-open — deferred follow-ups +> **Where open quirks live.** This section documents caveats *of this package*. +> The project-wide register of what is actually open — verified, with the stale +> historical registers called out — is `.claude/plans/quirks-open.md`. + - **`FindModel` deprecation.** The deprecated alias is still on the `ScanCtx` surface for in-tree callers. Once every builder has been audited and migrated to the `GetModel` + `AddDiscoveredModel` pair, From 0926413be63e19520803119a053b39b0cc260e25 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Thu, 30 Jul 2026 22:49:16 +0200 Subject: [PATCH 2/2] docs(doc-site): show the first-class alias modes instead of describing them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Alias rendering" how-to described the first-class-alias modes in prose and sent the reader to a fixture directory for the actual shapes, because the witness could not exercise them: annotating a Go type alias with swagger:model used to hang the scanner (quirk F9). That was fixed and locked (TestQuirk_AliasModelNoHang, all three alias modes), so the guide can now show real output. New witness docs/examples/shaping/aliases-firstclass: one package (Amount, the annotated alias Fee = Amount, and Receipt referencing the alias) scanned three times, once per mode, each emitting the whole definitions map as a golden. The guide compares default vs RefAliases side by side and shows TransparentAliases below, so what the reader sees is what the scanner produced. Writing it against real output corrected the page. It said TransparentAliases makes "aliases dissolve to their target everywhere, overriding the per-declaration annotation" — only half true. The option governs USE SITES: with ScanModels, a swagger:model-annotated alias is still published as its own definition, and merely stops being referenced by anything. The page now says so, and points at PruneUnusedModels for dropping the orphan. The test locks that behaviour rather than the summary of it. Also adds the guidance the old page lacked: prefer RefAliases over the default when the alias is genuinely a synonym, since the default emits a copy of the target that drifts as soon as the target changes. The dissolve example keeps its own witness (shaping/aliases), whose package doc now points at its new sibling rather than explaining which modes it avoids. Verified with a full hugo build: 106 pages, no warnings, both shortcodes resolve their files. Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Frederic BIDON --- .../names-and-refs/alias-rendering.md | 64 +++++++--- .../shaping/aliases-firstclass/firstclass.go | 39 ++++++ .../aliases-firstclass/firstclass_test.go | 118 ++++++++++++++++++ .../aliases-firstclass/testdata/expand.json | 48 +++++++ .../testdata/refaliases.json | 34 +++++ .../testdata/transparent.json | 48 +++++++ docs/examples/shaping/aliases/aliases.go | 10 +- 7 files changed, 340 insertions(+), 21 deletions(-) create mode 100644 docs/examples/shaping/aliases-firstclass/firstclass.go create mode 100644 docs/examples/shaping/aliases-firstclass/firstclass_test.go create mode 100644 docs/examples/shaping/aliases-firstclass/testdata/expand.json create mode 100644 docs/examples/shaping/aliases-firstclass/testdata/refaliases.json create mode 100644 docs/examples/shaping/aliases-firstclass/testdata/transparent.json diff --git a/docs/doc-site/shaping-the-output/names-and-refs/alias-rendering.md b/docs/doc-site/shaping-the-output/names-and-refs/alias-rendering.md index 0e0ae3f4..14101f57 100644 --- a/docs/doc-site/shaping-the-output/names-and-refs/alias-rendering.md +++ b/docs/doc-site/shaping-the-output/names-and-refs/alias-rendering.md @@ -18,20 +18,56 @@ site the alias **dissolves** to its target, producing no definition of its own. ## Exposing an alias as a first-class entity -This is an **advanced, rarely-needed** case. To keep the alias name in the spec -(its own definition that other schemas `$ref`), annotate the alias with -`swagger:model`. Two top-level options then govern how that first-class alias -*definition* is shaped: - -- **default (expand)** — the alias definition is a structural copy of the - target. -- **`RefAliases: true`** — the alias definition is a `$ref` chain to the target - (`{"$ref": "#/definitions/Money"}`), preserving the alias name at use sites. -- **`TransparentAliases: true`** — aliases dissolve to their target everywhere, - overriding the per-declaration annotation (use sites become `$ref` to the - target, as in the default-dissolve example above). - -The canonical witnesses are the `fixtures/enhancements/alias-calibration-embed` +This is an **advanced, rarely-needed** case. To keep the alias name in the spec — +its own definition that other schemas `$ref` — annotate the alias with +`swagger:model`: + +{{< code file="shaping/aliases-firstclass/firstclass.go" lang="go" region="firstclass" >}} + +Two top-level options then govern how that first-class alias *definition* is +shaped. The panes below are the same package scanned under each. + +### Default — the alias definition is a copy + +`Fee` is emitted as a structural duplicate of `Amount`, and `Receipt.charge` +points at the alias: + +{{< compare + left="shaping/aliases-firstclass/testdata/expand.json" leftlabel="Default (expand)" + right="shaping/aliases-firstclass/testdata/refaliases.json" rightlabel="RefAliases: true" >}} + +### `RefAliases: true` — the alias definition is a `$ref` chain + +The right pane above: `Fee` becomes `{"$ref": "#/definitions/Amount"}`. One +shape, two names — the alias survives at use sites without duplicating the +target's properties. Prefer this over the default whenever the alias is +genuinely a synonym: a copy drifts the moment the target changes. + +### `TransparentAliases: true` — use sites dissolve + +{{< code file="shaping/aliases-firstclass/testdata/transparent.json" lang="json" >}} + +`Receipt.charge` now points straight at `#/definitions/Amount`: the alias is +gone from the reference graph. + +{{% notice style="warning" %}} +Note what did **not** happen: `Fee` is still emitted. `TransparentAliases` +governs how an alias renders at its *use sites*, not whether an annotated +declaration produces a definition — so with `ScanModels` you get a `Fee` +definition that nothing references. Add +[`PruneUnusedModels`]({{% relref "pruning-unused-models" %}}) to drop it, or +simply do not annotate an alias you intend to dissolve. +{{% /notice %}} + +The three modes at a glance: + +| | `Fee` definition | `Receipt.charge` | +|---|---|---| +| default (expand) | copy of `Amount` | `$ref: Fee` | +| `RefAliases: true` | `$ref: Amount` | `$ref: Fee` | +| `TransparentAliases: true` | copy of `Amount`, unreferenced | `$ref: Amount` | + +Wider calibration lives in the `fixtures/enhancements/alias-calibration-embed` golden trio. {{% notice style="note" %}} diff --git a/docs/examples/shaping/aliases-firstclass/firstclass.go b/docs/examples/shaping/aliases-firstclass/firstclass.go new file mode 100644 index 00000000..19463f7d --- /dev/null +++ b/docs/examples/shaping/aliases-firstclass/firstclass.go @@ -0,0 +1,39 @@ +// SPDX-License-Identifier: Apache-2.0 + +// Package firstclass holds the annotated declarations used by the "first-class +// alias" half of the "Alias rendering" how-to — an alias that keeps its own +// identity in the spec, and the two options that shape it. +// +// firstclass_test.go scans this package three times, once per alias mode, and +// writes one golden per mode, so the guide compares real output rather than +// describing it. +package firstclass + +// snippet:firstclass + +// Amount is the underlying model. +// +// swagger:model +type Amount struct { + // Cents is the amount in cents. + Cents int64 `json:"cents"` + + // Currency is the ISO currency code. + Currency string `json:"currency"` +} + +// Fee is a FIRST-CLASS alias: the swagger:model annotation keeps the alias name +// in the spec instead of dissolving it to Amount. +// +// swagger:model +type Fee = Amount + +// Receipt references the alias, not the target. +// +// swagger:model +type Receipt struct { + // Charge is the fee charged. + Charge Fee `json:"charge"` +} + +// endsnippet:firstclass diff --git a/docs/examples/shaping/aliases-firstclass/firstclass_test.go b/docs/examples/shaping/aliases-firstclass/firstclass_test.go new file mode 100644 index 00000000..c7ce5b79 --- /dev/null +++ b/docs/examples/shaping/aliases-firstclass/firstclass_test.go @@ -0,0 +1,118 @@ +// SPDX-License-Identifier: Apache-2.0 + +package firstclass + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/spec" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +func examplesRoot(t *testing.T) string { + t.Helper() + _, thisFile, _, ok := runtime.Caller(0) + require.True(t, ok) + + return filepath.Clean(filepath.Join(filepath.Dir(thisFile), "..", "..")) +} + +// scanMode scans the package under one alias mode. +func scanMode(t *testing.T, opts codescan.Options) spec.Definitions { + t.Helper() + opts.WorkDir = examplesRoot(t) + opts.Packages = []string{"./shaping/aliases-firstclass"} + opts.ScanModels = true + + doc, err := codescan.Run(&opts) + require.NoError(t, err) + require.NotNil(t, doc) + + return doc.Definitions +} + +// goldenDefs emits and verifies the whole definitions map for one mode. +func goldenDefs(t *testing.T, feature string, defs spec.Definitions) { + t.Helper() + got, err := json.MarshalIndent(defs, "", " ") + require.NoError(t, err) + got = append(got, '\n') + + golden := filepath.Join("testdata", feature+".json") + if os.Getenv("UPDATE_GOLDEN") != "" { + require.NoError(t, os.WriteFile(golden, got, 0o600)) + } + want, err := os.ReadFile(golden) + require.NoError(t, err) + assert.JSONEq(t, string(want), string(got)) +} + +// refOf returns the $ref carried by a property, or "" when it carries none. +func refOf(defs spec.Definitions, def, prop string) string { + prd := defs[def].Properties[prop] + + return prd.Ref.String() +} + +// TestFirstClassAlias_Expand locks the default: the alias definition is a +// structural COPY of its target, and use sites point at the alias. +// +// Regenerate with: UPDATE_GOLDEN=1 go test ./... +func TestFirstClassAlias_Expand(t *testing.T) { + defs := scanMode(t, codescan.Options{}) + + require.Contains(t, defs, "Fee") + assert.Contains(t, defs["Fee"].Properties, "cents", + "the default expands the alias into a copy of the target") + feeDefault := defs["Fee"] + assert.Empty(t, feeDefault.Ref.String(), "no $ref chain in the default mode") + assert.Equal(t, "#/definitions/Fee", refOf(defs, "Receipt", "charge"), + "the use site keeps the alias name") + + goldenDefs(t, "expand", defs) +} + +// TestFirstClassAlias_RefAliases locks RefAliases: the alias definition becomes a +// $ref chain to its target rather than a copy — one shape, two names. +func TestFirstClassAlias_RefAliases(t *testing.T) { + defs := scanMode(t, codescan.Options{RefAliases: true}) + + fee := defs["Fee"] + assert.Equal(t, "#/definitions/Amount", fee.Ref.String(), + "the alias definition is a $ref chain to the target") + assert.Empty(t, defs["Fee"].Properties, "no duplicated properties under RefAliases") + assert.Equal(t, "#/definitions/Fee", refOf(defs, "Receipt", "charge"), + "the use site still keeps the alias name") + + goldenDefs(t, "refaliases", defs) +} + +// TestFirstClassAlias_Transparent locks the part that surprises: TransparentAliases +// dissolves the alias at USE SITES, but a swagger:model-annotated alias is still +// published as its own definition under ScanModels — it just stops being +// referenced by anything. +func TestFirstClassAlias_Transparent(t *testing.T) { + defs := scanMode(t, codescan.Options{TransparentAliases: true}) + + assert.Equal(t, "#/definitions/Amount", refOf(defs, "Receipt", "charge"), + "the use site dissolves to the target") + require.Contains(t, defs, "Fee", + "the annotated alias is still published; the option governs use sites, not the annotation") + assert.Contains(t, defs["Fee"].Properties, "cents") + + // Nothing references Fee any more — with PruneUnusedModels it would be dropped. + for _, def := range defs { + for name, prop := range def.Properties { + assert.NotEqualf(t, "#/definitions/Fee", prop.Ref.String(), + "property %q still points at the dissolved alias", name) + } + } + + goldenDefs(t, "transparent", defs) +} diff --git a/docs/examples/shaping/aliases-firstclass/testdata/expand.json b/docs/examples/shaping/aliases-firstclass/testdata/expand.json new file mode 100644 index 00000000..5bccbfa5 --- /dev/null +++ b/docs/examples/shaping/aliases-firstclass/testdata/expand.json @@ -0,0 +1,48 @@ +{ + "Amount": { + "type": "object", + "title": "Amount is the underlying model.", + "properties": { + "cents": { + "description": "Cents is the amount in cents.", + "type": "integer", + "format": "int64", + "x-go-name": "Cents" + }, + "currency": { + "description": "Currency is the ISO currency code.", + "type": "string", + "x-go-name": "Currency" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + }, + "Fee": { + "description": "Fee is a FIRST-CLASS alias: the swagger:model annotation keeps the alias name\nin the spec instead of dissolving it to Amount.", + "type": "object", + "properties": { + "cents": { + "description": "Cents is the amount in cents.", + "type": "integer", + "format": "int64", + "x-go-name": "Cents" + }, + "currency": { + "description": "Currency is the ISO currency code.", + "type": "string", + "x-go-name": "Currency" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + }, + "Receipt": { + "type": "object", + "title": "Receipt references the alias, not the target.", + "properties": { + "charge": { + "$ref": "#/definitions/Fee" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + } +} diff --git a/docs/examples/shaping/aliases-firstclass/testdata/refaliases.json b/docs/examples/shaping/aliases-firstclass/testdata/refaliases.json new file mode 100644 index 00000000..4e925089 --- /dev/null +++ b/docs/examples/shaping/aliases-firstclass/testdata/refaliases.json @@ -0,0 +1,34 @@ +{ + "Amount": { + "type": "object", + "title": "Amount is the underlying model.", + "properties": { + "cents": { + "description": "Cents is the amount in cents.", + "type": "integer", + "format": "int64", + "x-go-name": "Cents" + }, + "currency": { + "description": "Currency is the ISO currency code.", + "type": "string", + "x-go-name": "Currency" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + }, + "Fee": { + "description": "Fee is a FIRST-CLASS alias: the swagger:model annotation keeps the alias name\nin the spec instead of dissolving it to Amount.", + "$ref": "#/definitions/Amount" + }, + "Receipt": { + "type": "object", + "title": "Receipt references the alias, not the target.", + "properties": { + "charge": { + "$ref": "#/definitions/Fee" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + } +} diff --git a/docs/examples/shaping/aliases-firstclass/testdata/transparent.json b/docs/examples/shaping/aliases-firstclass/testdata/transparent.json new file mode 100644 index 00000000..8467a841 --- /dev/null +++ b/docs/examples/shaping/aliases-firstclass/testdata/transparent.json @@ -0,0 +1,48 @@ +{ + "Amount": { + "type": "object", + "title": "Amount is the underlying model.", + "properties": { + "cents": { + "description": "Cents is the amount in cents.", + "type": "integer", + "format": "int64", + "x-go-name": "Cents" + }, + "currency": { + "description": "Currency is the ISO currency code.", + "type": "string", + "x-go-name": "Currency" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + }, + "Fee": { + "description": "Fee is a FIRST-CLASS alias: the swagger:model annotation keeps the alias name\nin the spec instead of dissolving it to Amount.", + "type": "object", + "properties": { + "cents": { + "description": "Cents is the amount in cents.", + "type": "integer", + "format": "int64", + "x-go-name": "Cents" + }, + "currency": { + "description": "Currency is the ISO currency code.", + "type": "string", + "x-go-name": "Currency" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + }, + "Receipt": { + "type": "object", + "title": "Receipt references the alias, not the target.", + "properties": { + "charge": { + "$ref": "#/definitions/Amount" + } + }, + "x-go-package": "github.com/go-openapi/codescan/docs/examples/shaping/aliases-firstclass" + } +} diff --git a/docs/examples/shaping/aliases/aliases.go b/docs/examples/shaping/aliases/aliases.go index 21e2ef66..a4172f64 100644 --- a/docs/examples/shaping/aliases/aliases.go +++ b/docs/examples/shaping/aliases/aliases.go @@ -4,13 +4,9 @@ // how-to. aliases_test.go scans it and writes the golden fragment the guide // renders. // -// This example uses an unannotated alias (the dissolve case), the common -// behaviour the guide leads with; the first-class-alias modes are described -// conceptually there. -// -// (Historical note: those modes once hung the scanner — quirk F9 — which is why -// this example avoids them. The hang was fixed and is locked across all three -// alias modes by TestQuirk_AliasModelNoHang.) +// This example covers the DISSOLVE case (an unannotated alias), which is the +// common behaviour the guide leads with. The first-class-alias modes have their +// own witness next door, in shaping/aliases-firstclass. package aliases // snippet:alias