Skip to content
This repository was archived by the owner on Apr 14, 2026. It is now read-only.

Commit 247ab34

Browse files
mromaszewiczclaude
andcommitted
Refactor name collision resolution to strategy-based loop
Replace the iteration-number-dependent branching in resolveCollisions with a clean, uniform loop over an ordered list of named resolution strategies. The old code used iteration 0 for context suffix, iterations 1-8 for disambiguateName (which itself tried 4 strategies sequentially), and iteration 9 for numeric fallback. The new design introduces two function types (collisionGroupStrategy and disambiguationStrategy) and an ordered strategy list. Each iteration applies one strategy to all conflicting buckets, then re-buckets. If a strategy makes no progress, the next one is tried. Strategies extracted into standalone functions: - strategyContextSuffix (from resolveWithContextSuffix) - strategyPerSchemaDisambiguate (wraps 4 sub-strategies from disambiguateName) - strategyNumericFallback (last-resort safety net) - tryContentTypeSuffix, tryStatusCodeSuffix, tryParamIndexSuffix, tryCompositionTypeSuffix (from disambiguateName) Also moves all collision resolution code into its own file (resolve_collisions.go) for better separation of concerns. Behavioral equivalence verified: go generate produces identical output, all tests pass. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ba48d61 commit 247ab34

2 files changed

Lines changed: 288 additions & 205 deletions

File tree

Lines changed: 288 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
package codegen
2+
3+
import (
4+
"fmt"
5+
"strings"
6+
)
7+
8+
// schemaContextSuffix maps a SchemaContext to a disambiguation suffix.
9+
func schemaContextSuffix(ctx SchemaContext) string {
10+
switch ctx {
11+
case ContextComponentSchema:
12+
return "Schema"
13+
case ContextParameter:
14+
return "Parameter"
15+
case ContextRequestBody:
16+
return "Request"
17+
case ContextResponse:
18+
return "Response"
19+
case ContextHeader:
20+
return "Header"
21+
case ContextCallback:
22+
return "Callback"
23+
case ContextWebhook:
24+
return "Webhook"
25+
default:
26+
return ""
27+
}
28+
}
29+
30+
// collisionGroupStrategy attempts to resolve a naming collision within a group
31+
// of schemas that share the same candidate name. It returns true if any name
32+
// was changed.
33+
type collisionGroupStrategy func(
34+
group []*SchemaDescriptor,
35+
candidates map[*SchemaDescriptor]string,
36+
converter *NameConverter,
37+
) bool
38+
39+
// disambiguationStrategy attempts to produce a new, more specific name for a
40+
// single schema. It returns the new name and true if it found a disambiguation,
41+
// or the original name and false otherwise.
42+
type disambiguationStrategy func(
43+
s *SchemaDescriptor,
44+
currentName string,
45+
converter *NameConverter,
46+
) (string, bool)
47+
48+
// disambiguationStrategies is the ordered list of per-schema sub-strategies
49+
// tried by strategyPerSchemaDisambiguate.
50+
var disambiguationStrategies = []disambiguationStrategy{
51+
tryContentTypeSuffix,
52+
tryStatusCodeSuffix,
53+
tryParamIndexSuffix,
54+
tryCompositionTypeSuffix,
55+
}
56+
57+
// collisionStrategies is the ordered list of group-level strategies tried by
58+
// resolveCollisions for each conflicting bucket.
59+
var collisionStrategies = []collisionGroupStrategy{
60+
strategyContextSuffix,
61+
strategyPerSchemaDisambiguate,
62+
strategyNumericFallback,
63+
}
64+
65+
// resolveCollisions detects name collisions and makes them unique.
66+
// Reference schemas are excluded from collision detection because they don't
67+
// generate types — their names are only used for type resolution lookups.
68+
//
69+
// Resolution proceeds by trying one strategy at a time across all conflicting
70+
// buckets, then re-bucketing. When a strategy makes no progress (no name
71+
// changes across any bucket), the next strategy in the list is tried. The
72+
// strategy list (collisionStrategies) is:
73+
// 1. Context suffix — append a suffix derived from the schema's location.
74+
// 2. Per-schema disambiguation — content type, status code, param index,
75+
// composition type, with numeric fallback per schema.
76+
// 3. Numeric fallback — unconditionally append i+1 to every member.
77+
func resolveCollisions(schemas []*SchemaDescriptor, candidates map[*SchemaDescriptor]string, converter *NameConverter) {
78+
// Filter out reference schemas — they don't generate types so their
79+
// short names can safely shadow non-ref names without causing a collision.
80+
var nonRefSchemas []*SchemaDescriptor
81+
for _, s := range schemas {
82+
if s.Ref == "" {
83+
nonRefSchemas = append(nonRefSchemas, s)
84+
}
85+
}
86+
87+
maxIterations := 10 // Prevent infinite loops
88+
strategyIdx := 0
89+
90+
for range maxIterations {
91+
// Group non-ref schemas by candidate name
92+
byName := make(map[string][]*SchemaDescriptor)
93+
for _, s := range nonRefSchemas {
94+
name := candidates[s]
95+
byName[name] = append(byName[name], s)
96+
}
97+
98+
// Check if there are any collisions
99+
hasCollisions := false
100+
for _, group := range byName {
101+
if len(group) > 1 {
102+
hasCollisions = true
103+
break
104+
}
105+
}
106+
107+
if !hasCollisions {
108+
return // All names are unique
109+
}
110+
111+
if strategyIdx >= len(collisionStrategies) {
112+
return // Exhausted all strategies
113+
}
114+
115+
// Apply the current strategy to all conflicting buckets.
116+
strategy := collisionStrategies[strategyIdx]
117+
anyChanged := false
118+
for _, group := range byName {
119+
if len(group) <= 1 {
120+
continue // No collision
121+
}
122+
if strategy(group, candidates, converter) {
123+
anyChanged = true
124+
}
125+
}
126+
127+
// If the strategy made no progress, advance to the next one.
128+
// Otherwise, re-bucket with the same strategy index (the changes
129+
// may have created new collisions that the same strategy can fix).
130+
if !anyChanged {
131+
strategyIdx++
132+
}
133+
}
134+
}
135+
136+
// strategyContextSuffix attempts to disambiguate colliding schemas by
137+
// appending a suffix derived from their path context (e.g. "Request",
138+
// "Response"). If exactly one member is a component schema, it keeps the
139+
// bare name and only the others are suffixed.
140+
func strategyContextSuffix(group []*SchemaDescriptor, candidates map[*SchemaDescriptor]string, _ *NameConverter) bool {
141+
// Count how many are from components/schemas
142+
var componentSchemaCount int
143+
for _, s := range group {
144+
ctx, _ := parsePathContext(s.Path)
145+
if ctx == ContextComponentSchema {
146+
componentSchemaCount++
147+
}
148+
}
149+
150+
// If exactly one is from components/schemas, it is "privileged" and keeps
151+
// the bare name.
152+
privileged := componentSchemaCount == 1
153+
154+
changed := false
155+
for _, s := range group {
156+
ctx, _ := parsePathContext(s.Path)
157+
158+
// Privileged component schema keeps the bare name
159+
if privileged && ctx == ContextComponentSchema {
160+
continue
161+
}
162+
163+
suffix := schemaContextSuffix(ctx)
164+
if suffix != "" {
165+
name := candidates[s]
166+
if !strings.HasSuffix(name, suffix) {
167+
candidates[s] = name + suffix
168+
changed = true
169+
}
170+
}
171+
// If suffix is empty (unknown context), leave unchanged for later
172+
// strategies to handle.
173+
}
174+
return changed
175+
}
176+
177+
// strategyPerSchemaDisambiguate tries per-schema sub-strategies
178+
// (disambiguationStrategies) in order for each member of the group. If no
179+
// sub-strategy matches a given schema, it falls back to a numeric suffix
180+
// (index+1). Returns true if any name was changed.
181+
func strategyPerSchemaDisambiguate(group []*SchemaDescriptor, candidates map[*SchemaDescriptor]string, converter *NameConverter) bool {
182+
changed := false
183+
for i, s := range group {
184+
currentName := candidates[s]
185+
resolved := false
186+
for _, sub := range disambiguationStrategies {
187+
if newName, ok := sub(s, currentName, converter); ok {
188+
candidates[s] = newName
189+
changed = true
190+
resolved = true
191+
break
192+
}
193+
}
194+
if !resolved {
195+
// Numeric fallback per schema
196+
candidates[s] = fmt.Sprintf("%s%d", currentName, i+1)
197+
changed = true
198+
}
199+
}
200+
return changed
201+
}
202+
203+
// strategyNumericFallback unconditionally appends i+1 to every schema in the
204+
// group. This is the last-resort strategy that always succeeds.
205+
func strategyNumericFallback(group []*SchemaDescriptor, candidates map[*SchemaDescriptor]string, _ *NameConverter) bool {
206+
for i, s := range group {
207+
candidates[s] = fmt.Sprintf("%s%d", candidates[s], i+1)
208+
}
209+
return true
210+
}
211+
212+
// tryContentTypeSuffix checks for "content/{type}" in the schema path and
213+
// appends a content-type suffix (JSON, XML, Form, Text, Binary, or the
214+
// normalized content type).
215+
func tryContentTypeSuffix(s *SchemaDescriptor, currentName string, converter *NameConverter) (string, bool) {
216+
for i, part := range s.Path {
217+
if part == "content" && i+1 < len(s.Path) {
218+
contentType := s.Path[i+1]
219+
var suffix string
220+
switch {
221+
case strings.Contains(contentType, "json"):
222+
suffix = "JSON"
223+
case strings.Contains(contentType, "xml"):
224+
suffix = "XML"
225+
case strings.Contains(contentType, "form"):
226+
suffix = "Form"
227+
case strings.Contains(contentType, "text"):
228+
suffix = "Text"
229+
case strings.Contains(contentType, "binary"):
230+
suffix = "Binary"
231+
default:
232+
suffix = converter.ToTypeName(strings.ReplaceAll(contentType, "/", "_"))
233+
}
234+
// Use Contains since the suffix might be embedded before "Response" or "Request"
235+
if !strings.Contains(currentName, suffix) {
236+
return currentName + suffix, true
237+
}
238+
}
239+
}
240+
return currentName, false
241+
}
242+
243+
// tryStatusCodeSuffix checks for "responses/{code}" in the schema path and
244+
// appends the status code.
245+
func tryStatusCodeSuffix(s *SchemaDescriptor, currentName string, _ *NameConverter) (string, bool) {
246+
for i, part := range s.Path {
247+
if part == "responses" && i+1 < len(s.Path) {
248+
code := s.Path[i+1]
249+
if !strings.Contains(currentName, code) {
250+
return currentName + code, true
251+
}
252+
}
253+
}
254+
return currentName, false
255+
}
256+
257+
// tryParamIndexSuffix checks for "parameters/{idx}" in the schema path and
258+
// appends the parameter index.
259+
func tryParamIndexSuffix(s *SchemaDescriptor, currentName string, _ *NameConverter) (string, bool) {
260+
for i, part := range s.Path {
261+
if part == "parameters" && i+1 < len(s.Path) {
262+
idx := s.Path[i+1]
263+
if !strings.HasSuffix(currentName, idx) {
264+
return currentName + idx, true
265+
}
266+
}
267+
}
268+
return currentName, false
269+
}
270+
271+
// tryCompositionTypeSuffix checks for allOf/anyOf/oneOf in the schema path
272+
// (searching from the end) and appends the composition type and index.
273+
func tryCompositionTypeSuffix(s *SchemaDescriptor, currentName string, converter *NameConverter) (string, bool) {
274+
for i := len(s.Path) - 1; i >= 0; i-- {
275+
part := s.Path[i]
276+
switch part {
277+
case "allOf", "anyOf", "oneOf":
278+
suffix := converter.ToTypeName(part)
279+
if i+1 < len(s.Path) {
280+
suffix += s.Path[i+1] // Add index
281+
}
282+
if !strings.Contains(currentName, suffix) {
283+
return currentName + suffix, true
284+
}
285+
}
286+
}
287+
return currentName, false
288+
}

0 commit comments

Comments
 (0)