From 83108cb512d31e5bb4293e8b9e4e99e101856aed Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 22 Jun 2026 16:08:34 +0200 Subject: [PATCH 1/3] fix(provenance): re-point definition anchors through name reduction Definition provenance was emitted inline during the schema build under the type's short leaf name, but reduceDefinitionNames renames collision groups afterwards (cross-package same-named models: Item -> XItem/YItem) without touching provenance. Every /definitions/... pointer for a renamed definition was left dangling, so the OnProvenance consumer (the genspec TUI) could not resolve it to a spec node. This affected any scan with a cross-package name collision, independent of model pruning. OnProvenance is a push stream (RecordOrigin fires immediately and accumulates nothing), so inline anchors cannot be retracted after the rename. Buffer definition-scoped anchors instead: BeginDefOrigins/EndDefOrigins open a window around each definition build keyed by its fully-qualified DefKey; RecordOrigin buffers inside the window and fires inline outside it (path, response, info and parameter anchors are never renamed by reduction). reduceDefinitionNames now returns its old->new rename map, and FlushDefOrigins re-points each buffered anchor to the definition's final name and emits it once names are settled. Witness: name-identity-mixed and name-identity-3way join the geometry test, plus a focused TestCoverage_ProvenanceCollisionRename. Before the fix the mixed fixture emits 8 dangling anchors; after, every emitted anchor resolves against the rendered spec. Zero golden drift: provenance is a side channel and never alters the spec. Signed-off-by: Frederic BIDON Co-authored-by: Claude Opus 4.8 (1M context) --- internal/builders/spec/reduce.go | 11 ++- internal/builders/spec/spec.go | 29 +++++-- .../integration/coverage_provenance_test.go | 64 ++++++++++++++ internal/scanner/scan_context.go | 87 ++++++++++++++++++- 4 files changed, 181 insertions(+), 10 deletions(-) diff --git a/internal/builders/spec/reduce.go b/internal/builders/spec/reduce.go index 73599a1..f0f524b 100644 --- a/internal/builders/spec/reduce.go +++ b/internal/builders/spec/reduce.go @@ -41,19 +41,24 @@ import ( // output is deterministic regardless of discovery / map-iteration order // (G4): the previous silent merge was the only source of non-determinism // and it is gone now that distinct types get distinct keys. -func (s *Builder) reduceDefinitionNames() { +// reduceDefinitionNames returns the old->new rename map it applied, so the +// caller can re-point buffered provenance anchors to the final names +// (FlushDefOrigins). A nil/empty map means no definition was renamed. +func (s *Builder) reduceDefinitionNames() map[string]string { if len(s.input.Definitions) == 0 { - return + return nil } renames := s.computeNameReductions() if len(renames) == 0 { - return + return nil } rewriteAllRefs(s.input, repointer(renames)) s.rekeyDefinitions(renames) s.placeHierarchical(renames) + + return renames } // computeNameReductions groups definition keys by their leaf name (the diff --git a/internal/builders/spec/spec.go b/internal/builders/spec/spec.go index a47636e..fab114c 100644 --- a/internal/builders/spec/spec.go +++ b/internal/builders/spec/spec.go @@ -115,7 +115,19 @@ func (s *Builder) Build() (*oaispec.Swagger, error) { // names and re-point every $ref. Runs last because buildRoutes / // buildOperations also emit definition refs. See // .claude/plans/name-identity-cyclic-ref.md §9/§12. - s.reduceDefinitionNames() + renames := s.reduceDefinitionNames() + + // Definition provenance was buffered under each definition's fully-qualified + // key while building (BeginDefOrigins); now that names are final, re-point + // every buffered anchor to its final name and emit it. Anchors for pruned + // definitions were already dropped, so none dangle. See + // .claude/plans/prune-unused-models.md. + s.ctx.FlushDefOrigins(func(defKey string) string { + if final, ok := renames[defKey]; ok { + return final + } + return defKey + }) if s.input.Swagger == "" { s.input.Swagger = "2.0" @@ -260,13 +272,20 @@ func (s *Builder) buildDiscoveredSchema(decl *scanner.EntityDecl) error { // Cross-ref linkage: initiate the base pointer for this definition so the // schema builder path-joins its members (properties, …) under it, and anchor // the definition node itself to its type declaration. + // + // The base uses the fully-qualified DefKey, not the user-facing name: the + // definition is keyed by DefKey throughout discovery and only renamed to its + // final name at the end of the build (reduceDefinitionNames), after a + // possible prune. Anchors are buffered under DefKey (BeginDefOrigins) and + // re-pointed to the final name by FlushDefOrigins once names are settled, so + // every emitted pointer resolves against the final document. var defPtr string opts := []schema.Option{schema.WithDefinitions(s.definitions)} if s.ctx.OriginEnabled() { - if name, _ := decl.Names(); name != "" { - defPtr = scanner.JSONPointer("definitions", name) - opts = append(opts, schema.WithPath(defPtr)) - } + defPtr = scanner.JSONPointer("definitions", decl.DefKey()) + opts = append(opts, schema.WithPath(defPtr)) + s.ctx.BeginDefOrigins(decl.DefKey()) + defer s.ctx.EndDefOrigins() } if err := sb.Build(opts...); err != nil { diff --git a/internal/integration/coverage_provenance_test.go b/internal/integration/coverage_provenance_test.go index c854e7d..ffd5e11 100644 --- a/internal/integration/coverage_provenance_test.go +++ b/internal/integration/coverage_provenance_test.go @@ -62,6 +62,63 @@ func TestCoverage_ProvenanceDefinitions(t *testing.T) { } } +// TestCoverage_ProvenanceCollisionRename is the regression witness for the +// definition-provenance / name-reduction ordering bug. Two packages declare the +// same short-named model (x.Item + y.Item); reduceDefinitionNames renames them +// (Item -> XItem / YItem). Provenance is buffered under the fully-qualified key +// while building and re-pointed to the final name on flush, so every anchor — +// the definition node AND its fields — resolves against the renamed definition. +// Before the fix, anchors fired inline under the un-renamed leaf +// (/definitions/Item/...) and dangled after the rename. +func TestCoverage_ProvenanceCollisionRename(t *testing.T) { + byPointer := map[string]scanner.Provenance{} + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{"./enhancements/name-identity-mixed/..."}, + WorkDir: scantest.FixturesDir(), + ScanModels: true, + OnProvenance: func(p scanner.Provenance) { + byPointer[p.Pointer] = p + }, + }) + require.NoError(t, err) + require.NotNil(t, doc) + + // The colliding models were renamed; the bare leaf is gone from the spec. + require.Contains(t, doc.Definitions, "XItem") + require.Contains(t, doc.Definitions, "YItem") + require.NotContains(t, doc.Definitions, "Item") + + // Anchors land on the renamed definition nodes, each carrying a source line. + for _, ptr := range []string{"/definitions/XItem", "/definitions/YItem"} { + prov, ok := byPointer[ptr] + require.Truef(t, ok, "expected an anchor for renamed definition %q; got %v", ptr, keysOf(byPointer)) + assert.Positivef(t, prov.Pos.Line, "%q should carry a source line", ptr) + } + + // No anchor may reference the pre-rename leaf — that's the dangling bug. + for ptr := range byPointer { + assert.Falsef(t, ptr == "/definitions/Item" || strings.HasPrefix(ptr, "/definitions/Item/"), + "anchor %q references the un-renamed leaf — provenance was not re-pointed", ptr) + } + + // Every emitted anchor resolves in the rendered spec, including at least one + // field-level anchor under a renamed definition. + raw, err := json.Marshal(doc) + require.NoError(t, err) + var root any + require.NoError(t, json.Unmarshal(raw, &root)) + var fieldAnchors int + for ptr := range byPointer { + assert.Truef(t, resolveJSONPointer(root, ptr), + "anchor %q does not resolve in the rendered spec", ptr) + if strings.HasPrefix(ptr, "/definitions/XItem/properties/") || + strings.HasPrefix(ptr, "/definitions/YItem/properties/") { + fieldAnchors++ + } + } + assert.Positive(t, fieldAnchors, "expected at least one field-level anchor under a renamed definition") +} + // TestCoverage_ProvenanceOffByDefault confirms the callback is opt-in: a scan // without OnProvenance set produces the same spec and records nothing. func TestCoverage_ProvenanceOffByDefault(t *testing.T) { @@ -111,6 +168,13 @@ func TestCoverage_ProvenanceGeometry(t *testing.T) { "./enhancements/pattern-properties-typed", "./enhancements/swagger-type-array", "./enhancements/provenance-params-responses", // param + response header/body anchors + // Cross-package name collisions: definitions are renamed by + // reduceDefinitionNames (Item -> XItem/YItem, Widget -> …). Provenance is + // buffered under the fully-qualified key and re-pointed to the final name + // on flush; before that fix these anchored under the un-renamed leaf and + // dangled. See TestCoverage_ProvenanceCollisionRename. + "./enhancements/name-identity-mixed/...", + "./enhancements/name-identity-3way/...", // Full-surface fixture: meta, routes/operations, parameters, // top-level responses and enum definitions — exercises every anchor // kind against the resolves-in-spec invariant. diff --git a/internal/scanner/scan_context.go b/internal/scanner/scan_context.go index 94a2407..3bde36e 100644 --- a/internal/scanner/scan_context.go +++ b/internal/scanner/scan_context.go @@ -51,6 +51,20 @@ type ScanCtx struct { // (see the spec builder) once paths are built. Cross-ref linkage only. paramOrigins map[string]map[string]token.Position + // defOrigins buffers definition-scoped provenance anchors (the definition + // node and every field/enum sub-anchor under it), keyed by the + // fully-qualified definition key (EntityDecl.DefKey). They cannot fire + // inline: the definition is keyed by its fqn during discovery but renamed to + // its final user-facing name only at the end of the build + // (reduceDefinitionNames), and an unreferenced definition may be pruned + // before that. Buffered here, then re-pointed to the final name and emitted + // by FlushDefOrigins after prune + name reduction, so every pointer handed + // to OnProvenance resolves against the final document. curDefKey marks the + // definition currently being built (empty outside a definition build); + // non-reentrant, since each definition is built in its own pass. + defOrigins map[string][]Provenance + curDefKey string + // seenDiags suppresses exact-duplicate diagnostics on the OnDiagnostic // stream over one scan (see EmitDiagnostic). seenDiags map[diagKey]struct{} @@ -318,10 +332,79 @@ func (s *ScanCtx) OriginEnabled() bool { // RecordOrigin fires the consumer's [Options.OnProvenance] callback for one // anchor node, when wired. Unlike diagnostics it accumulates nothing — the // cross-ref index is owned by the consumer (see the genspec-tui linkage design). +// +// Exception: while a definition build is in progress (between [BeginDefOrigins] +// and [EndDefOrigins]) the anchor is buffered instead of fired, so it can be +// re-pointed to the definition's final name — or dropped if the definition is +// pruned — by [FlushDefOrigins] at the end of the build. Anchors outside a +// definition build (paths, responses, info, parameters) fire inline as before; +// name reduction never renames those. func (s *ScanCtx) RecordOrigin(pointer string, pos token.Position) { - if cb := s.opts.OnProvenance; cb != nil { - cb(Provenance{Pointer: pointer, Pos: pos}) + cb := s.opts.OnProvenance + if cb == nil { + return + } + if s.curDefKey != "" { + s.defOrigins[s.curDefKey] = append(s.defOrigins[s.curDefKey], Provenance{Pointer: pointer, Pos: pos}) + return + } + cb(Provenance{Pointer: pointer, Pos: pos}) +} + +// BeginDefOrigins opens a buffering window for the definition keyed by defKey +// (its fully-qualified [EntityDecl.DefKey]). Until [EndDefOrigins], every +// [RecordOrigin] call is buffered under defKey instead of fired. No-op when no +// provenance sink is wired. Non-reentrant: each definition is built in its own +// pass, so windows never nest. +func (s *ScanCtx) BeginDefOrigins(defKey string) { + if s.opts.OnProvenance == nil { + return + } + if s.defOrigins == nil { + s.defOrigins = make(map[string][]Provenance) + } + s.curDefKey = defKey +} + +// EndDefOrigins closes the current definition buffering window. +func (s *ScanCtx) EndDefOrigins() { + s.curDefKey = "" +} + +// DropDefOrigins discards the buffered anchors for a definition that has been +// pruned, so its provenance is never emitted (no orphan pointer into a +// definition absent from the final document). +func (s *ScanCtx) DropDefOrigins(defKey string) { + delete(s.defOrigins, defKey) +} + +// FlushDefOrigins fires every buffered definition anchor, re-pointing each from +// its build-time fully-qualified base (#/definitions/) to the +// definition's final name. finalName maps a definition key to the name the spec +// emits for it (identity when unchanged). Pointers are emitted in a +// deterministic (sorted) order. After the flush the buffer is cleared. +func (s *ScanCtx) FlushDefOrigins(finalName func(defKey string) string) { + cb := s.opts.OnProvenance + if cb == nil || len(s.defOrigins) == 0 { + return } + + keys := make([]string, 0, len(s.defOrigins)) + for k := range s.defOrigins { + keys = append(keys, k) + } + slices.Sort(keys) + + for _, defKey := range keys { + oldBase := JSONPointer("definitions", defKey) + newBase := JSONPointer("definitions", finalName(defKey)) + for _, rec := range s.defOrigins[defKey] { + cb(Provenance{Pointer: newBase + strings.TrimPrefix(rec.Pointer, oldBase), Pos: rec.Pos}) + } + } + + s.defOrigins = nil + s.curDefKey = "" } // RecordParamOrigin stashes the source position of one parameter field, keyed From 9b4fc1f0c7f26a009752a6b3d9ce4a8ad76e4aad Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 22 Jun 2026 17:55:17 +0200 Subject: [PATCH 2/3] feat(scanner): prune unreferenced models under -m (PruneUnusedModels) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ScanModels (`-m`) deliberately emits every swagger:model type, reachable or not. On a large shared library that buries the wanted models under noise; users worked around it with go-swagger's `flatten`, partly to dodge cross-package name collisions that an unused model should never have caused (go-swagger/go-swagger#2639). Add Options.PruneUnusedModels: with ScanModels, run discovery as before, then drop every discovered definition not transitively referenced from a reachability root. Roots are the paths (operation body parameters + response schemas), the shared responses and parameters, and every definition supplied via InputSpec — overlay definitions are pinned (never pruned, seeded as roots). A model referenced only by another unreferenced model is pruned too; recursive/cyclic models terminate on a visited set. Without ScanModels the flag is a no-op (the set is already reachable-only) and says so with one Hint. The prune runs BEFORE reduceDefinitionNames, in the fully-qualified definition-key namespace. That ordering is the point of the feature, not a detail: name reduction deconflicts colliding leaves (a.Thing / b.Thing -> AThing / BThing). Pruning the unused twin first means the collision never materialises, so the surviving model keeps its bare leaf name with no concat churn. Each prune raises a located scan.pruned-unused Hint; the collision renames the reduce stage still performs are surfaced as located scan.renamed-definition Hints. Buffered provenance for a pruned node is dropped, so no anchor dangles. Fixture enhancements/prune-unused exercises the reachable closure (root -> chain -> recursive node), the dead chain, and the collision pair; tests lock both regression witnesses (collision evaporation to a bare name; InputSpec pinning) plus the no-op, diagnostics and no-orphan-provenance contracts. Default false: flag-off output is byte-identical. Signed-off-by: Frederic BIDON Co-authored-by: Claude Opus 4.8 (1M context) --- .claude/CLAUDE.md | 6 + fixtures/enhancements/prune-unused/a/a.go | 13 ++ fixtures/enhancements/prune-unused/api.go | 93 ++++++++ fixtures/enhancements/prune-unused/b/b.go | 14 ++ .../golden/enhancements_prune_unused.json | 84 ++++++++ .../golden/enhancements_prune_unused_all.json | 116 ++++++++++ internal/builders/spec/prune.go | 199 ++++++++++++++++++ internal/builders/spec/reduce.go | 35 +++ internal/builders/spec/spec.go | 16 ++ .../integration/coverage_prune_unused_test.go | 178 ++++++++++++++++ internal/parsers/grammar/diagnostic.go | 16 ++ internal/scanner/README.md | 47 +++++ internal/scanner/options.go | 28 +++ internal/scanner/scan_context.go | 7 + 14 files changed, 852 insertions(+) create mode 100644 fixtures/enhancements/prune-unused/a/a.go create mode 100644 fixtures/enhancements/prune-unused/api.go create mode 100644 fixtures/enhancements/prune-unused/b/b.go create mode 100644 fixtures/integration/golden/enhancements_prune_unused.json create mode 100644 fixtures/integration/golden/enhancements_prune_unused_all.json create mode 100644 internal/builders/spec/prune.go create mode 100644 internal/integration/coverage_prune_unused_test.go diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 55edb53..5286503 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -111,6 +111,12 @@ malformed input, the petstore, aliased schemas, go123-specific forms, and cross- - `codescan.Run(*Options) (*spec.Swagger, error)` — the main entry point. - `codescan.Options` — configuration. Notable fields beyond `Packages`/`WorkDir`: - `ScanModels` — also emit definitions for `swagger:model` types. + - `PruneUnusedModels` — with `ScanModels`, prune discovered definitions not + transitively referenced from any path/response/parameter/overlay root. + Runs before name reduction (so an unused model can't force a spurious + collision rename on a used one); `InputSpec` definitions are pinned. Each + drop raises a `scan.pruned-unused` Hint; collision renames raise + `scan.renamed-definition`. See `internal/scanner/README.md#prune`. - `InputSpec` — overlay: merge discoveries on top of an existing spec. - `BuildTags`, `Include`/`Exclude`, `IncludeTags`/`ExcludeTags`, `ExcludeDeps` — scope control. - `RefAliases`, `TransparentAliases`, `DescWithRef` — alias handling knobs diff --git a/fixtures/enhancements/prune-unused/a/a.go b/fixtures/enhancements/prune-unused/a/a.go new file mode 100644 index 0000000..49f8ed3 --- /dev/null +++ b/fixtures/enhancements/prune-unused/a/a.go @@ -0,0 +1,13 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package a holds the referenced half of the a.Thing / b.Thing collision pair. +package a + +// Thing is referenced from Used.Thing, so it survives a prune. With b.Thing +// pruned away first, this keeps the bare name "Thing". +// +// swagger:model Thing +type Thing struct { + A string `json:"a"` +} diff --git a/fixtures/enhancements/prune-unused/api.go b/fixtures/enhancements/prune-unused/api.go new file mode 100644 index 0000000..16ec7d5 --- /dev/null +++ b/fixtures/enhancements/prune-unused/api.go @@ -0,0 +1,93 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package pruneunused exercises Options.PruneUnusedModels. +// +// A single route references Used, which transitively reaches a chain (B -> C), +// a self-recursive model (Node), and a cross-package model (a.Thing). These are +// the reachable set and must survive a prune. +// +// Unused (a swagger:model nothing references) and OnlyByUnused (referenced only +// by Unused) must be pruned. b.Thing is a swagger:model in a sibling package +// that nothing references and must also be pruned — and because it is pruned +// BEFORE name reduction, its collision with a.Thing never materialises, so +// a.Thing keeps the bare name "Thing" instead of being deconflicted to AThing / +// BThing. That is the headline property of pruning before reduction. +package pruneunused + +import ( + "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused/a" + "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused/b" +) + +// Used is referenced from the route response — the reachability root. +// +// swagger:model Used +type Used struct { + B B `json:"b"` + Thing a.Thing `json:"thing"` + Node *Node `json:"node"` +} + +// B is reached via Used -> B -> C. +// +// swagger:model B +type B struct { + C C `json:"c"` +} + +// C is the chain tail. +// +// swagger:model C +type C struct { + Name string `json:"name"` +} + +// Node is self-recursive and reached from Used; the reachability walk must +// terminate on the cycle, not loop. +// +// swagger:model Node +type Node struct { + Next *Node `json:"next"` +} + +// Unused is a swagger:model that nothing references — pruned. +// +// swagger:model Unused +type Unused struct { + Only OnlyByUnused `json:"only"` +} + +// OnlyByUnused is referenced only by Unused (itself unreferenced), so it is +// reachable only through dead nodes — pruned too. +// +// swagger:model OnlyByUnused +type OnlyByUnused struct { + V string `json:"v"` +} + +// usedResp carries Used in its body so the route reaches it. +// +// swagger:response usedResp +type usedResp struct { + // in: body + Body Used `json:"body"` +} + +// handler is the only route. +// +// swagger:route GET /used usedOp +// +// Used. +// +// responses: +// +// 200: usedResp +func handler() {} + +// keep the imported packages referenced so the tree type-checks even though +// b is only reached by the scanner, not by Go code. +var ( + _ a.Thing + _ b.Thing +) diff --git a/fixtures/enhancements/prune-unused/b/b.go b/fixtures/enhancements/prune-unused/b/b.go new file mode 100644 index 0000000..b79e2d9 --- /dev/null +++ b/fixtures/enhancements/prune-unused/b/b.go @@ -0,0 +1,14 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +// Package b holds the unreferenced half of the a.Thing / b.Thing collision +// pair. Nothing references b.Thing, so PruneUnusedModels drops it before name +// reduction — and the a.Thing / b.Thing collision never surfaces. +package b + +// Thing is a swagger:model that nothing references — pruned. +// +// swagger:model Thing +type Thing struct { + B string `json:"b"` +} diff --git a/fixtures/integration/golden/enhancements_prune_unused.json b/fixtures/integration/golden/enhancements_prune_unused.json new file mode 100644 index 0000000..16ea52a --- /dev/null +++ b/fixtures/integration/golden/enhancements_prune_unused.json @@ -0,0 +1,84 @@ +{ + "swagger": "2.0", + "paths": { + "/used": { + "get": { + "summary": "Used.", + "operationId": "usedOp", + "responses": { + "200": { + "$ref": "#/responses/usedResp" + } + } + } + } + }, + "definitions": { + "B": { + "type": "object", + "title": "B is reached via Used -\u003e B -\u003e C.", + "properties": { + "c": { + "$ref": "#/definitions/C" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "C": { + "type": "object", + "title": "C is the chain tail.", + "properties": { + "name": { + "type": "string", + "x-go-name": "Name" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "Node": { + "description": "Node is self-recursive and reached from Used; the reachability walk must\nterminate on the cycle, not loop.", + "type": "object", + "properties": { + "next": { + "$ref": "#/definitions/Node" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "Thing": { + "description": "Thing is referenced from Used.Thing, so it survives a prune. With b.Thing\npruned away first, this keeps the bare name \"Thing\".", + "type": "object", + "properties": { + "a": { + "type": "string", + "x-go-name": "A" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused/a" + }, + "Used": { + "type": "object", + "title": "Used is referenced from the route response — the reachability root.", + "properties": { + "b": { + "$ref": "#/definitions/B" + }, + "node": { + "$ref": "#/definitions/Node" + }, + "thing": { + "$ref": "#/definitions/Thing" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + } + }, + "responses": { + "usedResp": { + "description": "usedResp carries Used in its body so the route reaches it.", + "schema": { + "$ref": "#/definitions/Used" + } + } + } +} \ No newline at end of file diff --git a/fixtures/integration/golden/enhancements_prune_unused_all.json b/fixtures/integration/golden/enhancements_prune_unused_all.json new file mode 100644 index 0000000..213760d --- /dev/null +++ b/fixtures/integration/golden/enhancements_prune_unused_all.json @@ -0,0 +1,116 @@ +{ + "swagger": "2.0", + "paths": { + "/used": { + "get": { + "summary": "Used.", + "operationId": "usedOp", + "responses": { + "200": { + "$ref": "#/responses/usedResp" + } + } + } + } + }, + "definitions": { + "AThing": { + "description": "Thing is referenced from Used.Thing, so it survives a prune. With b.Thing\npruned away first, this keeps the bare name \"Thing\".", + "type": "object", + "properties": { + "a": { + "type": "string", + "x-go-name": "A" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused/a" + }, + "B": { + "type": "object", + "title": "B is reached via Used -\u003e B -\u003e C.", + "properties": { + "c": { + "$ref": "#/definitions/C" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "BThing": { + "type": "object", + "title": "Thing is a swagger:model that nothing references — pruned.", + "properties": { + "b": { + "type": "string", + "x-go-name": "B" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused/b" + }, + "C": { + "type": "object", + "title": "C is the chain tail.", + "properties": { + "name": { + "type": "string", + "x-go-name": "Name" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "Node": { + "description": "Node is self-recursive and reached from Used; the reachability walk must\nterminate on the cycle, not loop.", + "type": "object", + "properties": { + "next": { + "$ref": "#/definitions/Node" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "OnlyByUnused": { + "description": "OnlyByUnused is referenced only by Unused (itself unreferenced), so it is\nreachable only through dead nodes — pruned too.", + "type": "object", + "properties": { + "v": { + "type": "string", + "x-go-name": "V" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "Unused": { + "type": "object", + "title": "Unused is a swagger:model that nothing references — pruned.", + "properties": { + "only": { + "$ref": "#/definitions/OnlyByUnused" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + }, + "Used": { + "type": "object", + "title": "Used is referenced from the route response — the reachability root.", + "properties": { + "b": { + "$ref": "#/definitions/B" + }, + "node": { + "$ref": "#/definitions/Node" + }, + "thing": { + "$ref": "#/definitions/AThing" + } + }, + "x-go-package": "github.com/go-openapi/codescan/fixtures/enhancements/prune-unused" + } + }, + "responses": { + "usedResp": { + "description": "usedResp carries Used in its body so the route reaches it.", + "schema": { + "$ref": "#/definitions/Used" + } + } + } +} \ No newline at end of file diff --git a/internal/builders/spec/prune.go b/internal/builders/spec/prune.go new file mode 100644 index 0000000..cf8616d --- /dev/null +++ b/internal/builders/spec/prune.go @@ -0,0 +1,199 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package spec + +import ( + "go/token" + "sort" + "strings" + + "github.com/go-openapi/codescan/internal/parsers/grammar" + oaispec "github.com/go-openapi/spec" +) + +const defRefPrefix = "#/definitions/" + +// pruneUnusedModels drops every discovered definition not transitively +// referenced from a reachability root, when the caller set PruneUnusedModels. +// It is the middle ground between the default route-reachable-only mode and the +// emit-everything ScanModels mode: run ScanModels discovery, then prune the +// unreachable tail (go-swagger/go-swagger#2639). +// +// It runs BEFORE reduceDefinitionNames, in the fully-qualified definition-key +// namespace, so a pruned (unused) model can no longer force a spurious +// cross-package name collision on a model that IS used — the survivor keeps its +// clean short name. Buffered provenance for each pruned definition is dropped so +// no anchor dangles, and each prune raises a scan.pruned-unused Hint. +// +// Reachability roots are the paths (operation parameters + responses), the +// shared responses and parameters, and every definition supplied via InputSpec +// (pinned: never pruned, and seeded as roots so their $ref targets survive). +// Definitions themselves are not roots — a model referenced only by another +// unreferenced model is pruned too. +func (s *Builder) pruneUnusedModels() { + if !s.ctx.PruneUnusedModels() { + return + } + + onDiag := s.ctx.OnDiagnostic() + + if !s.scanModels { + // Without ScanModels the emitted set is already reference-reachable, so + // there is nothing to prune. Surface one Hint so a caller who set the + // flag alone learns it did nothing. + if onDiag != nil { + onDiag(grammar.Hintf(token.Position{}, grammar.CodePrunedUnused, + "PruneUnusedModels has no effect without ScanModels: "+ + "the emitted definition set is already reference-reachable")) + } + return + } + + if len(s.input.Definitions) == 0 { + return + } + + reachable := make(map[string]struct{}, len(s.input.Definitions)) + var queue []string + mark := func(key string) { + if key == "" { + return + } + if _, seen := reachable[key]; seen { + return + } + if _, ok := s.input.Definitions[key]; !ok { + return // a $ref to something that is not a definition we hold + } + reachable[key] = struct{}{} + queue = append(queue, key) + } + + // Seed roots: refs from paths / shared responses / shared parameters. + for _, key := range s.rootRefs() { + mark(key) + } + // Pin overlay-supplied definitions (those built from no source declaration) + // as roots: the caller put them there deliberately. + for key := range s.input.Definitions { + if _, built := s.declPos[key]; !built { + mark(key) + } + } + + // Transitive closure over the definition graph. + for len(queue) > 0 { + cur := queue[0] + queue = queue[1:] + sch := s.input.Definitions[cur] + collectDefRefs(&sch, mark) + } + + // Prune the unreachable, deterministically. + pruned := make([]string, 0, len(s.input.Definitions)) + for key := range s.input.Definitions { + if _, keep := reachable[key]; !keep { + pruned = append(pruned, key) + } + } + sort.Strings(pruned) + + for _, key := range pruned { + delete(s.input.Definitions, key) + s.ctx.DropDefOrigins(key) // no orphan provenance for a pruned node + if onDiag != nil { + onDiag(grammar.Hintf(s.declPos[key], grammar.CodePrunedUnused, + "definition %q pruned: not referenced from any path, response, "+ + "parameter or overlay definition", key)) + } + } +} + +// rootRefs returns every definition key referenced from a reachability root: +// path operations (body parameters + response schemas), shared responses and +// shared parameters. Definitions are deliberately excluded — they stay only if +// reached transitively from here. +func (s *Builder) rootRefs() []string { + var out []string + collect := func(sch *oaispec.Schema) { + collectDefRefs(sch, func(key string) { out = append(out, key) }) + } + + for _, p := range s.input.Parameters { + collect(p.Schema) + } + for _, r := range s.input.Responses { + collect(r.Schema) + } + if s.input.Paths != nil { + for _, pi := range s.input.Paths.Paths { + for _, op := range operationsOf(pi) { + if op == nil { + continue + } + for i := range op.Parameters { + collect(op.Parameters[i].Schema) + } + if op.Responses == nil { + continue + } + if op.Responses.Default != nil { + collect(op.Responses.Default.Schema) + } + for _, resp := range op.Responses.StatusCodeResponses { + collect(resp.Schema) + } + } + } + } + + return out +} + +// collectDefRefs walks sch and every sub-schema, invoking mark for each +// "#/definitions/" reference it carries. It is the read-only mirror of +// rewriteSchemaRefs and must cover the same container set; a missed container +// would make a referenced model look unreachable and wrongly prune it. +func collectDefRefs(sch *oaispec.Schema, mark func(key string)) { + if sch == nil { + return + } + if r := sch.Ref.String(); strings.HasPrefix(r, defRefPrefix) { + mark(r[len(defRefPrefix):]) + } + for k := range sch.Properties { + v := sch.Properties[k] + collectDefRefs(&v, mark) + } + for k := range sch.PatternProperties { + v := sch.PatternProperties[k] + collectDefRefs(&v, mark) + } + for i := range sch.AllOf { + collectDefRefs(&sch.AllOf[i], mark) + } + for i := range sch.AnyOf { + collectDefRefs(&sch.AnyOf[i], mark) + } + for i := range sch.OneOf { + collectDefRefs(&sch.OneOf[i], mark) + } + collectDefRefs(sch.Not, mark) + if sch.Items != nil { + collectDefRefs(sch.Items.Schema, mark) + for i := range sch.Items.Schemas { + collectDefRefs(&sch.Items.Schemas[i], mark) + } + } + if sch.AdditionalProperties != nil { + collectDefRefs(sch.AdditionalProperties.Schema, mark) + } + if sch.AdditionalItems != nil { + collectDefRefs(sch.AdditionalItems.Schema, mark) + } + for k := range sch.Definitions { + v := sch.Definitions[k] + collectDefRefs(&v, mark) + } +} diff --git a/internal/builders/spec/reduce.go b/internal/builders/spec/reduce.go index f0f524b..be12ca1 100644 --- a/internal/builders/spec/reduce.go +++ b/internal/builders/spec/reduce.go @@ -57,10 +57,45 @@ func (s *Builder) reduceDefinitionNames() map[string]string { rewriteAllRefs(s.input, repointer(renames)) s.rekeyDefinitions(renames) s.placeHierarchical(renames) + s.diagnoseRenames(renames) return renames } +// diagnoseRenames emits a scan.renamed-definition Hint for every definition the +// reduce stage renamed to deconflict a cross-package collision. The trivial +// lift of a globally-unique leaf to its bare name is not a rename and is +// skipped (final == leaf). Unlike the positionless group Warning +// (diagnoseCollision), each Hint carries the source position of the originating +// Go type — captured during the build in declPos — so a source<->spec consumer +// (the genspec TUI) can follow a renamed type back to its declaration. No +// provenance is emitted here: the renamed node's anchor is the normal flushed +// one under its final name (see FlushDefOrigins). +func (s *Builder) diagnoseRenames(renames map[string]string) { + onDiag := s.ctx.OnDiagnostic() + if onDiag == nil { + return + } + + keys := make([]string, 0, len(renames)) + for key, final := range renames { + if final != leafName(key) { // skip unique-leaf lifts, keep true renames + keys = append(keys, key) + } + } + sort.Strings(keys) + + for _, key := range keys { + pos, ok := s.declPos[key] + if !ok { + continue // no source declaration (e.g. an overlay-supplied definition) + } + onDiag(grammar.Hintf(pos, grammar.CodeRenamedDefinition, + "definition %q renamed to %q to deconflict a cross-package name collision", + key, renames[key])) + } +} + // computeNameReductions groups definition keys by their leaf name (the // segment after the last '/') and returns the rename map old->new: // diff --git a/internal/builders/spec/spec.go b/internal/builders/spec/spec.go index fab114c..3a6f9a8 100644 --- a/internal/builders/spec/spec.go +++ b/internal/builders/spec/spec.go @@ -36,6 +36,11 @@ type Builder struct { definitions map[string]oaispec.Schema responses map[string]oaispec.Response operations map[string]*oaispec.Operation + // declPos records the source position of each discovered definition, keyed + // by its fully-qualified DefKey, so the prune (scan.pruned-unused) and + // rename (scan.renamed-definition) Hints can be located at the originating + // Go type even though the spec node may by then be gone or renamed. + declPos map[string]token.Position } func NewBuilder(input *oaispec.Swagger, sc *scanner.ScanCtx, scanModels bool) *Builder { @@ -64,6 +69,7 @@ func NewBuilder(input *oaispec.Swagger, sc *scanner.ScanCtx, scanModels bool) *B operations: collectOperationsFromInput(input), definitions: input.Definitions, responses: input.Responses, + declPos: make(map[string]token.Position), } } @@ -110,6 +116,12 @@ func (s *Builder) Build() (*oaispec.Swagger, error) { // definition-name reduction below. s.emitParameterAnchors() + // Prune discovered definitions not reachable from a root, when the caller + // opted in (PruneUnusedModels). Runs before name reduction so an unused + // model cannot force a spurious collision rename on a used one. See + // .claude/plans/prune-unused-models.md. + s.pruneUnusedModels() + // Final stage: shorten the fully-qualified, collision-proof // definition keys produced during discovery back to user-facing // names and re-point every $ref. Runs last because buildRoutes / @@ -269,6 +281,10 @@ func (s *Builder) buildDiscoveredSchema(decl *scanner.EntityDecl) error { sb := schema.NewBuilder(s.ctx, decl) sb.SetDiscovered(s.discovered) + // Stash the definition's source position for the prune / rename Hints, which + // fire after the spec node may have been dropped or renamed. + s.declPos[decl.DefKey()] = s.ctx.PosOf(decl.Ident.Pos()) + // Cross-ref linkage: initiate the base pointer for this definition so the // schema builder path-joins its members (properties, …) under it, and anchor // the definition node itself to its type declaration. diff --git a/internal/integration/coverage_prune_unused_test.go b/internal/integration/coverage_prune_unused_test.go new file mode 100644 index 0000000..523747a --- /dev/null +++ b/internal/integration/coverage_prune_unused_test.go @@ -0,0 +1,178 @@ +// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers +// SPDX-License-Identifier: Apache-2.0 + +package integration_test + +import ( + "encoding/json" + "testing" + + "github.com/go-openapi/codescan" + "github.com/go-openapi/codescan/internal/parsers/grammar" + "github.com/go-openapi/codescan/internal/scanner" + "github.com/go-openapi/codescan/internal/scantest" + oaispec "github.com/go-openapi/spec" + "github.com/go-openapi/testify/v2/assert" + "github.com/go-openapi/testify/v2/require" +) + +const pruneUnusedPkg = "./enhancements/prune-unused/..." + +// runPrune scans the prune-unused fixture, capturing diagnostics by code. +func runPrune(t *testing.T, scanModels, prune bool, input *oaispec.Swagger) (*oaispec.Swagger, map[grammar.Code][]grammar.Diagnostic) { + t.Helper() + byCode := map[grammar.Code][]grammar.Diagnostic{} + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{pruneUnusedPkg}, + WorkDir: scantest.FixturesDir(), + InputSpec: input, + ScanModels: scanModels, + PruneUnusedModels: prune, + OnDiagnostic: func(d grammar.Diagnostic) { + byCode[d.Code] = append(byCode[d.Code], d) + }, + }) + require.NoError(t, err) + require.NotNil(t, doc) + return doc, byCode +} + +// TestPruneUnused_Off is the control: with ScanModels and no prune, every +// swagger:model is emitted — including the unreferenced Unused / OnlyByUnused — +// and the a.Thing / b.Thing collision is deconflicted to AThing / BThing (two +// rename Hints). This is the shape the prune pass reduces. +func TestPruneUnused_Off(t *testing.T) { + doc, byCode := runPrune(t, true, false, nil) + + for _, name := range []string{"Used", "B", "C", "Node", "Unused", "OnlyByUnused", "AThing", "BThing"} { + assert.Contains(t, doc.Definitions, name) + } + assert.Len(t, doc.Definitions, 8) + assert.Empty(t, byCode[grammar.CodePrunedUnused], "nothing is pruned without the flag") + assert.Len(t, byCode[grammar.CodeRenamedDefinition], 2, + "the a.Thing / b.Thing collision is deconflicted into two renames") + + scantest.CompareOrDumpJSON(t, doc, "enhancements_prune_unused_all.json") +} + +// TestPruneUnused_On is the core case. ScanModels + PruneUnusedModels keeps only +// the route-reachable closure (Used -> B -> C, the recursive Node, and a.Thing), +// drops the dead set (Unused, OnlyByUnused, b.Thing), and — because the prune +// runs BEFORE name reduction — the a.Thing / b.Thing collision never surfaces, +// so a.Thing keeps the bare name "Thing" with no AThing / BThing churn. +func TestPruneUnused_On(t *testing.T) { + doc, byCode := runPrune(t, true, true, nil) + + // Reachable closure survives. + for _, name := range []string{"Used", "B", "C", "Node", "Thing"} { + assert.Contains(t, doc.Definitions, name) + } + assert.Len(t, doc.Definitions, 5) + + // Dead set is gone. + for _, name := range []string{"Unused", "OnlyByUnused"} { + assert.NotContains(t, doc.Definitions, name) + } + + // The collision evaporated: bare "Thing", no deconflicted concat, no rename. + assert.NotContains(t, doc.Definitions, "AThing") + assert.NotContains(t, doc.Definitions, "BThing") + assert.Empty(t, byCode[grammar.CodeRenamedDefinition], + "pruning the unused twin removes the collision, so nothing is renamed") + + // The kept reference re-points to the bare name; the cycle is intact. + thingRef := doc.Definitions["Used"].Properties["thing"].Ref + nodeRef := doc.Definitions["Node"].Properties["next"].Ref + assert.Equal(t, "#/definitions/Thing", thingRef.String()) + assert.Equal(t, "#/definitions/Node", nodeRef.String(), + "the self-recursive cycle survives and back-refs to its own definition") + + scantest.CompareOrDumpJSON(t, doc, "enhancements_prune_unused.json") +} + +// TestPruneUnused_Diagnostics asserts the prune is loud: one located Hint per +// pruned definition (Unused, OnlyByUnused, b.Thing), each a Hint severity with a +// source line, and no rename Hints (the collision was pruned away). +func TestPruneUnused_Diagnostics(t *testing.T) { + _, byCode := runPrune(t, true, true, nil) + + hints := byCode[grammar.CodePrunedUnused] + require.Len(t, hints, 3, "one Hint per pruned definition") + for _, d := range hints { + assert.Equal(t, grammar.SeverityHint, d.Severity, "a prune is informational") + assert.Positive(t, d.Pos.Line, "the Hint is located at the pruned type's declaration") + } + assert.Empty(t, byCode[grammar.CodeRenamedDefinition]) +} + +// TestPruneUnused_NoOpWithoutScanModels confirms the flag is inert without +// ScanModels (the emitted set is already reachable-only): the output matches the +// plain reachable scan, and exactly one positionless Hint explains the no-op. +func TestPruneUnused_NoOpWithoutScanModels(t *testing.T) { + doc, byCode := runPrune(t, false, true, nil) + + // Same reachable closure as a plain no-ScanModels scan — nothing extra to prune. + for _, name := range []string{"Used", "B", "C", "Node", "Thing"} { + assert.Contains(t, doc.Definitions, name) + } + assert.NotContains(t, doc.Definitions, "Unused") + + hints := byCode[grammar.CodePrunedUnused] + require.Len(t, hints, 1, "a single no-op notice") + assert.Equal(t, grammar.SeverityHint, hints[0].Severity) + assert.Zero(t, hints[0].Pos.Line, "the no-op notice is positionless") +} + +// TestPruneUnused_InputSpecPinned confirms decision 2: a definition supplied via +// InputSpec is pinned — never pruned even when no path references it — and seeds +// the reachability roots. +func TestPruneUnused_InputSpecPinned(t *testing.T) { + input := &oaispec.Swagger{ + SwaggerProps: oaispec.SwaggerProps{ + Definitions: oaispec.Definitions{ + "Pinned": {SchemaProps: oaispec.SchemaProps{Type: oaispec.StringOrArray{"object"}}}, + }, + }, + } + doc, _ := runPrune(t, true, true, input) + + assert.Contains(t, doc.Definitions, "Pinned", "an overlay-supplied definition is never pruned") + // The discovered dead set is still pruned around the pinned definition. + assert.NotContains(t, doc.Definitions, "Unused") +} + +// TestPruneUnused_Provenance confirms a pruned definition leaves no orphan +// provenance: no anchor references a pruned node, and every emitted anchor +// resolves in the rendered spec. +func TestPruneUnused_Provenance(t *testing.T) { + var recorded []scanner.Provenance + doc, err := codescan.Run(&codescan.Options{ + Packages: []string{pruneUnusedPkg}, + WorkDir: scantest.FixturesDir(), + ScanModels: true, + PruneUnusedModels: true, + OnProvenance: func(p scanner.Provenance) { + recorded = append(recorded, p) + }, + }) + require.NoError(t, err) + require.NotNil(t, doc) + require.NotEmpty(t, recorded) + + raw, err := json.Marshal(doc) + require.NoError(t, err) + var root any + require.NoError(t, json.Unmarshal(raw, &root)) + + var sawKept bool + for _, p := range recorded { + assert.Truef(t, resolveJSONPointer(root, p.Pointer), + "anchor %q (from %s:%d) does not resolve — orphaned by a prune?", + p.Pointer, p.Pos.Filename, p.Pos.Line) + switch p.Pointer { + case "/definitions/Used", "/definitions/Thing": + sawKept = true + } + } + assert.True(t, sawKept, "kept definitions still carry provenance") +} diff --git a/internal/parsers/grammar/diagnostic.go b/internal/parsers/grammar/diagnostic.go index 1206e74..3b5e687 100644 --- a/internal/parsers/grammar/diagnostic.go +++ b/internal/parsers/grammar/diagnostic.go @@ -177,6 +177,22 @@ const ( // keyword so the loss is never silent. See scanner.Options // SkipAllOfCompounding. CodeDroppedRefSibling Code = "validate.dropped-ref-sibling" + + // CodePrunedUnused fires when PruneUnusedModels is set and a discovered + // definition is dropped because it is not transitively referenced from any + // path, shared response, shared parameter or overlay definition. Carries the + // originating Go type's source position so the loss is never silent. + // Informational (Hint): the prune is the caller's own opt-in, surfaced to aid + // "why is my model missing" triage. See scanner.Options PruneUnusedModels. + CodePrunedUnused Code = "scan.pruned-unused" + + // CodeRenamedDefinition fires when the reduce stage renames a definition to + // deconflict a cross-package name collision (e.g. b.Test / c.Test -> BTest / + // CTest), so a consumer that tracks source <-> spec links (the genspec TUI) + // learns the final name a Go type landed under. Informational (Hint); carries + // the Go type's source position. The bare-leaf zero-churn case (a globally + // unique name lifted to its leaf) is NOT reported — only true renames. + CodeRenamedDefinition Code = "scan.renamed-definition" ) // Diagnostic is one observation about a comment block. diff --git a/internal/scanner/README.md b/internal/scanner/README.md index 2958111..ca50a2c 100644 --- a/internal/scanner/README.md +++ b/internal/scanner/README.md @@ -18,6 +18,8 @@ parameters, responses) consumed by the builder layer. $ref shape and why it has a flag - [§diagnostics](#diagnostics) — `OnDiagnostic` contract and experimental-API caveat +- [§prune](#prune) — `PruneUnusedModels` reachability and why it + runs before name reduction - [§model-lookup](#model-lookup) — `GetModel` vs `FindModel` — pure read vs implicit registration - [§classifier](#classifier) — `detectNodes` bitmask semantics and @@ -42,6 +44,8 @@ warrant the inline godoc and the deeper notes below: See [§descwithref](#descwithref). - `OnDiagnostic` — diagnostic callback hook. See [§diagnostics](#diagnostics). +- `PruneUnusedModels` — drop discovered definitions unreachable from + any root, on top of `ScanModels`. See [§prune](#prune). ## §descwithref — description-only-decoration $ref shape @@ -94,6 +98,49 @@ breaking change in a future minor release. `ScanCtx.OnDiagnostic` returns the user-supplied callback verbatim; builders pipe diagnostics through it via `common.Builder.RecordDiagnostic`. +## §prune — `PruneUnusedModels` reachability + +`Options.PruneUnusedModels` is a modifier on `ScanModels` (`-m`). The +three emission modes: + +1. **no `ScanModels`** — only models transitively reachable from + routes/responses/parameters are emitted (discovery-driven). +2. **`ScanModels`** — every `swagger:model` type is emitted, reachable + or not. +3. **`ScanModels` + `PruneUnusedModels`** — discovery runs as in (2), + then unreachable definitions are pruned again. The middle ground a + shared-library scan wants: keep only the `$ref`'d subset + (go-swagger/go-swagger#2639). + +Without `ScanModels` the flag is a no-op (the set is already +reachable-only) and raises one positionless `scan.pruned-unused` Hint. + +**Reachability.** Roots are the paths (operation body parameters + +response schemas), the shared `responses` and `parameters`, and every +definition supplied via `InputSpec`. Overlay definitions are **pinned**: +never pruned and seeded as roots so their `$ref` targets survive. The +walk (`spec/prune.go`, `collectDefRefs`) is the read-only mirror of the +ref-rewriter (`reduce.go`, `rewriteSchemaRefs`) and must cover the same +container set; a `visited` set handles recursive / cyclic models. A +model referenced only by another unreferenced model is itself pruned. + +**Ordering — before name reduction.** The prune runs *before* +`reduceDefinitionNames`, in the fully-qualified `#/definitions// +` key namespace. This is the point of the feature, not an +implementation detail: name reduction deconflicts cross-package leaf +collisions (`a.Thing` / `b.Thing` → `AThing` / `BThing`). Pruning an +*unused* twin first means the collision never materialises, so the +surviving model keeps its bare leaf name — no spurious concat churn. +Each prune raises a located `scan.pruned-unused` Hint; the buffered +provenance for a pruned node is dropped so no anchor dangles. The +collision renames the reduce stage *does* perform are surfaced as +`scan.renamed-definition` Hints (located at the Go type). + +**Known limitation.** A discriminator base references its subtypes by +mapping string, not by `$ref`, so a subtype reachable only through a +discriminator could be pruned. codescan does not auto-wire discriminator +subtypes today; revisit if it ever does (forthcoming-features §15). + ## §model-lookup — `GetModel` vs `FindModel` `ScanCtx` exposes two lookup helpers with similar signatures but diff --git a/internal/scanner/options.go b/internal/scanner/options.go index 4cdb30b..4a308cd 100644 --- a/internal/scanner/options.go +++ b/internal/scanner/options.go @@ -178,6 +178,34 @@ type Options struct { // See go-swagger/go-swagger#2626. SingleLineCommentAsDescription bool + // PruneUnusedModels, when set together with ScanModels, drops every + // discovered definition that is not transitively referenced from a path, a + // shared response, a shared parameter, or a definition supplied via + // InputSpec. It is the middle ground between the two default modes: + // + // - without ScanModels: only route-reachable models are emitted; + // - with ScanModels (`-m`): every swagger:model type is emitted, reachable + // or not; + // - with ScanModels + PruneUnusedModels: swagger:model discovery runs, then + // the unreachable definitions are pruned again — useful when scanning a + // large shared library where only the $ref'd subset is wanted. + // + // Pruning runs BEFORE definition-name reduction, so an unused model can no + // longer force a spurious cross-package name collision on a model that IS + // used (the survivor keeps its clean short name). Definitions supplied via + // InputSpec are pinned: they are never pruned and seed the reachability + // roots. Each pruned definition raises a scan.pruned-unused Hint through + // OnDiagnostic — the loss is never silent. + // + // Without ScanModels this flag is a no-op (the emitted set is already + // reachable-only); setting it alone raises one Hint. Default false. + // + // Note: a discriminator base references its subtypes by mapping string, not + // by $ref, so a subtype reachable only through a discriminator could be + // pruned. codescan does not auto-wire discriminator subtypes today; revisit + // if it ever does. See go-swagger/go-swagger#2639. + PruneUnusedModels bool + // Debug is deprecated and has no effect. // // It formerly enabled verbose debug logging to stderr during scanning. diff --git a/internal/scanner/scan_context.go b/internal/scanner/scan_context.go index 3bde36e..df153f9 100644 --- a/internal/scanner/scan_context.go +++ b/internal/scanner/scan_context.go @@ -323,6 +323,13 @@ func (s *ScanCtx) EmitHierarchicalNames() bool { return s.opts.EmitHierarchicalNames } +// PruneUnusedModels reports whether the caller opted into pruning discovered +// definitions that are not transitively referenced from a root (paths, shared +// responses/parameters, overlay definitions). See [Options.PruneUnusedModels]. +func (s *ScanCtx) PruneUnusedModels() bool { + return s.opts.PruneUnusedModels +} + // OriginEnabled reports whether a provenance sink is wired, so callers can skip // JSON-pointer construction entirely when no consumer is listening. func (s *ScanCtx) OriginEnabled() bool { From cc3187576e275fbac86d96fad69363c0973a7841 Mon Sep 17 00:00:00 2001 From: Frederic BIDON Date: Mon, 22 Jun 2026 19:13:33 +0200 Subject: [PATCH 3/3] docs(doc-site): add "Pruning unused models" shaping-the-output guide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Document Options.PruneUnusedModels as a new shaping-the-output page (weight 16, between type-discovery and resolving-name-conflicts). Prose-only: presents the option, what it is useful for (scanning a shared model library with -m and keeping only the reachable subset), and contrasts the three emission modes — reachable-only (default), every model (ScanModels), and models-then-pruned (ScanModels + PruneUnusedModels). Covers what counts as reachable (path/response/parameter roots, InputSpec defs pinned), the before-name-reduction ordering that makes pruning avoid spurious cross-package collision renames, and the scan.pruned-unused / scan.renamed- definition diagnostics. Cross-links the type-discovery page (forward, from its "Standalone" note) and the resolving-name-conflicts "What's next". Markdown-lint and link-check clean. Signed-off-by: Frederic BIDON Co-authored-by: Claude Opus 4.8 (1M context) --- .../pruning-unused-models.md | 105 ++++++++++++++++++ .../resolving-name-conflicts.md | 3 + .../shaping-the-output/type-discovery.md | 4 +- 3 files changed, 111 insertions(+), 1 deletion(-) create mode 100644 docs/doc-site/shaping-the-output/pruning-unused-models.md diff --git a/docs/doc-site/shaping-the-output/pruning-unused-models.md b/docs/doc-site/shaping-the-output/pruning-unused-models.md new file mode 100644 index 0000000..2c0cecc --- /dev/null +++ b/docs/doc-site/shaping-the-output/pruning-unused-models.md @@ -0,0 +1,105 @@ +--- +title: Pruning unused models +weight: 16 +description: | + Scan a shared library with swagger:model discovery, then keep only the + definitions actually reachable from your API — the middle ground between + "only what routes use" and "every model, used or not". +--- + +`Options.ScanModels` (the `-m` flag) publishes **every** `swagger:model` type it +finds, whether or not anything references it — see +[When the scanner emits a type]({{% relref "/shaping-the-output/type-discovery" %}}). +That is exactly what you want when the annotated package *is* the contract. It is +the wrong default when you point codescan at a large **shared model library** and +only care about the slice your API actually exposes: the spec fills up with +definitions no operation, parameter or response ever references. + +`Options.PruneUnusedModels` is the middle ground. It runs `swagger:model` +discovery as usual, then drops every discovered definition that is not +reachable from your API surface. + +## Three emission modes + +The same source renders three ways, depending on two options: + +| Mode | Options | What is emitted | +|---|---|---| +| **Reachable only** | *(default)* | Only models reachable from an operation, parameter or response — discovery-driven. A `swagger:model` that nothing references is **not** emitted. | +| **Every model** | `ScanModels` | Every `swagger:model` type, reachable or not. The library's whole annotated surface lands in `definitions`. | +| **Models, then pruned** | `ScanModels` + `PruneUnusedModels` | Discovery runs as in *Every model*, then the unreachable definitions are pruned away — you keep the reachable subset, including models discovered only because `swagger:model` published them. | + +`PruneUnusedModels` is a **modifier on `ScanModels`**. Without `ScanModels` the +emitted set is already reachable-only, so the flag has nothing to do: it is a +no-op and says so with a single informational diagnostic. + +## What counts as reachable + +A definition survives the prune when it is reachable — directly or transitively +through any `$ref` — from one of these **roots**: + +- an operation's body parameters and response schemas; +- a top-level shared `response` or `parameter`; +- a definition supplied via [`InputSpec`]({{% relref "/shaping-the-output/overlaying-a-spec" %}}). + +The walk follows references through every schema shape — properties, `allOf` / +`anyOf` / `oneOf`, array items, `additionalProperties`, and so on — and +terminates cleanly on recursive or cyclic models. A model referenced **only by +another unreferenced model** is itself unreachable, so the whole dead subtree is +removed, not just its entry point. + +{{% notice style="info" %}} +Definitions you supply through `InputSpec` are **pinned**: they are never pruned, +and they seed the reachability roots, so anything they `$ref` survives too. The +prune only ever removes definitions codescan *discovered*, never ones you handed +it. +{{% /notice %}} + +## Pruning happens before name resolution + +This is the part that makes pruning more than a convenience. codescan keys every +definition by a compiler-unique identity while it builds, then a final stage +projects each one back to the shortest unique name — deconflicting cross-package +collisions along the way (`billing.Account` / `identity.Account` → +`BillingAccount` / `IdentityAccount`; see +[Resolving $ref name conflicts]({{% relref "/shaping-the-output/resolving-name-conflicts" %}})). + +`PruneUnusedModels` runs **before** that name-resolution stage. So when one half +of a colliding pair is unused, it is pruned *first* — and the collision never +happens. The surviving model keeps its clean, unqualified name instead of being +pushed to a package-qualified one to avoid a twin that is not even in your spec. +Pruning a shared library this way removes a whole class of surprising +`#/definitions/` renames that only existed because of models you were +not using. + +## Diagnostics + +The prune is never silent. Through the +[`OnDiagnostic`](https://pkg.go.dev/github.com/go-openapi/codescan#Options) sink +codescan reports: + +- `scan.pruned-unused` — one informational diagnostic per pruned definition, + located at the originating Go type, so you can see exactly what was dropped and + why; and the single no-op notice when the flag is set without `ScanModels`. +- `scan.renamed-definition` — one per collision the name stage *did* resolve, + located at the Go type, recording the final name it landed under. With pruning + on, collisions that vanish produce no such diagnostic at all. + +## When to use it + +- **Reach for `PruneUnusedModels`** when you scan a shared or third-party model + package with `-m` and want only the definitions your API actually exposes — + the reachable subset, with the noise dropped and the collision churn gone. +- **Stay on plain `ScanModels`** when the annotated package is itself the + published contract and every `swagger:model` is meant to appear. +- **Stay on the default** (neither flag) when you only ever want what your routes + reference; there is nothing extra to discover or prune. + +## What's next + +- [When the scanner emits a type]({{% relref "/shaping-the-output/type-discovery" %}}) — + reachability and `swagger:model`, the rules pruning builds on. +- [Resolving $ref name conflicts]({{% relref "/shaping-the-output/resolving-name-conflicts" %}}) — + the name-resolution stage pruning runs ahead of. +- [Overlaying a spec]({{% relref "/shaping-the-output/overlaying-a-spec" %}}) — + `InputSpec`, whose definitions are pinned against the prune. diff --git a/docs/doc-site/shaping-the-output/resolving-name-conflicts.md b/docs/doc-site/shaping-the-output/resolving-name-conflicts.md index 3b2328e..afbf25a 100644 --- a/docs/doc-site/shaping-the-output/resolving-name-conflicts.md +++ b/docs/doc-site/shaping-the-output/resolving-name-conflicts.md @@ -139,6 +139,9 @@ the nested shape only when you prefer it for the over-budget tail. - [Type discovery]({{% relref "/shaping-the-output/type-discovery" %}}) — which types become definitions in the first place. +- [Pruning unused models]({{% relref "/shaping-the-output/pruning-unused-models" %}}) — + drops unreferenced models *before* this name stage, so collisions caused only + by models you do not use never arise. - [Maps & free-form objects]({{% relref "/tutorials/maps-and-free-form-objects" %}}) — the typed `additionalProperties` / `patternProperties` value forms. - [Model definitions]({{% relref "/tutorials/model-definitions" %}}) — diff --git a/docs/doc-site/shaping-the-output/type-discovery.md b/docs/doc-site/shaping-the-output/type-discovery.md index 989f9c5..9e5d7d8 100644 --- a/docs/doc-site/shaping-the-output/type-discovery.md +++ b/docs/doc-site/shaping-the-output/type-discovery.md @@ -35,7 +35,9 @@ Scanned with `ScanModels: true`, the definitions are: {{% notice style="info" %}} If a model is missing from your spec, it is almost always **unreachable**: no operation/parameter/response/model leads to it. Either reference it, or annotate -it `swagger:model` and scan with `ScanModels`. +it `swagger:model` and scan with `ScanModels`. For the opposite problem — a +`ScanModels` scan that pulls in models you do *not* want, like `Standalone` — +see [Pruning unused models]({{% relref "/shaping-the-output/pruning-unused-models" %}}). {{% /notice %}} ## Generic and embedded types