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 1bff31b9..a4172f64 100644 --- a/docs/examples/shaping/aliases/aliases.go +++ b/docs/examples/shaping/aliases/aliases.go @@ -4,11 +4,9 @@ // 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 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 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,