[object Object]

← back to Cli Printing Press

fix(cli): dedupe nested body-field flatten collisions with sibling scalars (#1190)

c5a5441d3771568ee2d6d07db998af6e0c6f236e · 2026-05-12 03:50:13 -0700 · Trevin Chow

When a body schema declares both a top-level convenience scalar (e.g.
`leadAccountId`) and a nested object whose dot-flattened path camelizes to
the same Go identifier (e.g. `lead.accountId`), `renderBodyVarDecls` emits
duplicate `var bodyLeadAccountId` declarations and duplicate
`--lead-account-id` Cobra registrations, refusing to compile.

The existing dedup walked only top-level body params and never saw the
nested-leaf identifiers that the JSON-body emission produces by recursion.
This change makes the body dedup mirror the same tree shape so post-flatten
collisions across levels are caught; the second leaf's `IdentName` is
suffixed via the existing `_2`, `_3`, ... convention. Multipart and
form-encoded bodies keep the flat top-level walk because their emission
paths never recurse into nested Fields.

Closes #1043

Files touched

Diff

commit c5a5441d3771568ee2d6d07db998af6e0c6f236e
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Tue May 12 03:50:13 2026 -0700

    fix(cli): dedupe nested body-field flatten collisions with sibling scalars (#1190)
    
    When a body schema declares both a top-level convenience scalar (e.g.
    `leadAccountId`) and a nested object whose dot-flattened path camelizes to
    the same Go identifier (e.g. `lead.accountId`), `renderBodyVarDecls` emits
    duplicate `var bodyLeadAccountId` declarations and duplicate
    `--lead-account-id` Cobra registrations, refusing to compile.
    
    The existing dedup walked only top-level body params and never saw the
    nested-leaf identifiers that the JSON-body emission produces by recursion.
    This change makes the body dedup mirror the same tree shape so post-flatten
    collisions across levels are caught; the second leaf's `IdentName` is
    suffixed via the existing `_2`, `_3`, ... convention. Multipart and
    form-encoded bodies keep the flat top-level walk because their emission
    paths never recurse into nested Fields.
    
    Closes #1043
---
 internal/generator/body_collision_test.go | 56 ++++++++++++++++++++++++++++
 internal/generator/flag_collision.go      | 61 ++++++++++++++++++++++++++++++-
 2 files changed, 116 insertions(+), 1 deletion(-)

diff --git a/internal/generator/body_collision_test.go b/internal/generator/body_collision_test.go
index 3f37013e..573fa745 100644
--- a/internal/generator/body_collision_test.go
+++ b/internal/generator/body_collision_test.go
@@ -112,6 +112,62 @@ func TestGenerateRenamesBodyFieldCollidingWithQueryParam(t *testing.T) {
 	assert.Contains(t, mcpSource, `PublicName: "tags-2", WireName: "tags", Location: "body"`)
 }
 
+// TestGenerateDeduplicatesNestedBodyFieldCollidingWithSiblingScalar guards
+// the dot-flatten/sibling-scalar collision class. When a body schema declares
+// both a top-level convenience scalar (e.g., `leadAccountId`) and a nested
+// object whose dot-flattened path camelizes to the same Go identifier (e.g.,
+// `lead.accountId`), both produce `bodyLeadAccountId` after camelization. The
+// dedup pass must walk Body recursively so the post-flatten collision is
+// detected; otherwise the generated handler emits duplicate `var
+// bodyLeadAccountId` declarations and refuses to compile.
+func TestGenerateDeduplicatesNestedBodyFieldCollidingWithSiblingScalar(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("collide-nested")
+	apiSpec.Resources["components"] = spec.Resource{
+		Description: "Components",
+		Endpoints: map[string]spec.Endpoint{
+			"create": {
+				Method:      "POST",
+				Path:        "/components",
+				Description: "Create a component with deprecated and canonical lead fields",
+				Body: []spec.Param{
+					{Name: "leadAccountId", Type: "string", Description: "Deprecated convenience scalar"},
+					{Name: "lead", Type: "object", Description: "Canonical nested object", Fields: []spec.Param{
+						{Name: "accountId", Type: "string", Description: "Account id of the lead"},
+					}},
+				},
+			},
+			"get": {
+				Method:      "GET",
+				Path:        "/components/{id}",
+				Description: "Get one component",
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "collide-nested-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	bodyVars, flagBindings := parseBodyDeclarations(t,
+		filepath.Join(outputDir, "internal", "cli", "components_create.go"))
+
+	assertNoDuplicates(t, bodyVars,
+		"a nested-object leaf must produce a Go identifier distinct from a sibling scalar that camelizes to the same name")
+	assertNoDuplicates(t, flagBindings,
+		"a nested-object leaf must register a cobra flag name distinct from a sibling scalar")
+	require.Len(t, bodyVars, 2,
+		"both the convenience scalar and the nested field must survive dedup")
+	assert.Contains(t, bodyVars, "bodyLeadAccountId",
+		"one of the colliding fields keeps the canonical Go identifier")
+	assert.Contains(t, bodyVars, "bodyLeadAccountId2",
+		"the deduped field uses the _2 suffix convention")
+	assert.Contains(t, flagBindings, "lead-account-id",
+		"one of the colliding fields keeps the canonical cobra flag name")
+	assert.Contains(t, flagBindings, "lead-account-id-2",
+		"the deduped field's cobra flag carries the -2 suffix")
+}
+
 // TestGenerateRenamesBodyFieldCollidingWithStdin guards against a body field
 // literally named `stdin` colliding with the `--stdin` flag the template
 // emits for POST/PUT/PATCH endpoints (command_endpoint.go.tmpl:525).
diff --git a/internal/generator/flag_collision.go b/internal/generator/flag_collision.go
index 00bad245..b13abee2 100644
--- a/internal/generator/flag_collision.go
+++ b/internal/generator/flag_collision.go
@@ -81,11 +81,70 @@ func dedupeEndpointIdentifiers(resKey, epName string, ep spec.Endpoint, asyncJob
 			bodyFlagNames[publicFlagName(p)] = struct{}{}
 		}
 	}
-	ep.Body = uniquifyIdentifiers(ep.Body, "body", nil, bodyFlagNames)
+	// renderBodyVarDecls and renderBodyFlagRegs recurse into nested object
+	// Fields on the JSON-body path, so the dedup pass must walk the same
+	// tree to see post-flatten identifiers like body<Parent><Child>; a
+	// flat top-level walk misses parent-prefixed leaves that collide with
+	// sibling scalars. Multipart/form bodies skip the recursion because
+	// their emission keeps one var/flag per top-level param.
+	if bodyUsesFlatEmission(ep) {
+		ep.Body = uniquifyIdentifiers(ep.Body, "body", nil, bodyFlagNames)
+	} else {
+		usedIdents := map[string]struct{}{}
+		usedFlags := map[string]struct{}{}
+		for k := range bodyFlagNames {
+			usedFlags[k] = struct{}{}
+		}
+		ep.Body = uniquifyBodyTree(ep.Body, "", "", usedIdents, usedFlags)
+	}
 
 	return ep, nil
 }
 
+// uniquifyBodyTree walks Body recursively in the same shape that
+// renderBodyVarDecls and renderBodyFlagRegs emit on the JSON-body path.
+// usedIdents and usedFlags carry the accumulated reservations across all
+// levels so a nested leaf whose post-flatten Go identifier collides with a
+// sibling scalar at any level gets its IdentName suffixed via the existing
+// _2, _3, ... convention.
+func uniquifyBodyTree(body []spec.Param, identPrefix, flagPrefix string, usedIdents, usedFlags map[string]struct{}) []spec.Param {
+	out := make([]spec.Param, len(body))
+	for i, p := range body {
+		if p.Type == "object" && len(p.Fields) > 0 {
+			childIdent := identPrefix + toCamel(paramIdent(p))
+			childFlag := joinFlag(flagPrefix, publicFlagName(p))
+			p.Fields = uniquifyBodyTree(p.Fields, childIdent, childFlag, usedIdents, usedFlags)
+			out[i] = p
+			continue
+		}
+		ident := "body" + identPrefix + toCamel(paramIdent(p))
+		flag := joinFlag(flagPrefix, publicFlagName(p))
+		_, identTaken := usedIdents[ident]
+		_, flagTaken := usedFlags[flag]
+		if !identTaken && !flagTaken {
+			usedIdents[ident] = struct{}{}
+			usedFlags[flag] = struct{}{}
+			out[i] = p
+			continue
+		}
+		for n := 2; ; n++ {
+			candidate := fmt.Sprintf("%s_%d", p.Name, n)
+			candIdent := "body" + identPrefix + toCamel(candidate)
+			candFlag := joinFlag(flagPrefix, flagName(candidate))
+			_, identTaken := usedIdents[candIdent]
+			_, flagTaken := usedFlags[candFlag]
+			if !identTaken && !flagTaken {
+				p.IdentName = candidate
+				usedIdents[candIdent] = struct{}{}
+				usedFlags[candFlag] = struct{}{}
+				out[i] = p
+				break
+			}
+		}
+	}
+	return out
+}
+
 // reservedFlagNamesForEndpoint returns identifiers and cobra flag names that
 // the command templates emit themselves and that user params or body fields
 // therefore must not shadow. The returned `idents` set is in the flag<Camel>

← 660829ed fix(cli): preserve x-mcp config when merging combo specs (#1  ·  back to Cli Printing Press  ·  fix(cli): refuse to ship CLIs with placeholder base URL (#11 c9a35b5e →