[object Object]

← back to Cli Printing Press

test(cli): regression tests for body-flag identifier collision shapes (#1376)

d44587e9ed6d6df1eaa76876bd12a5ac6ae3c421 · 2026-05-14 00:53:32 -0700 · Trevin Chow

* test(cli): regression tests for body-flag identifier collision shapes

Locks in `dedupeFlagIdentifiers` / `uniquifyBodyTree` correctness for
three nested body shapes flagged in #1243:

  1. Two convergent nested-object paths whose leaves share a name
     (Project.Customer.name vs Project.PostalAddress.Customer.name).
  2. Two nested-object paths whose joined camelized identifiers
     collapse to the same Go name (Project.Customer.name vs
     ProjectCustomer.name as a top-level sibling).
  3. A Tripletex-shape OpenAPI spec parsed end-to-end with $refs that
     cycle through Customer via ledgerAccount.vatType.customer and
     postalAddress.customer — the parser's pointer-keyed visited[] cut
     terminates re-entry, and the dedupe pass + renderer produce
     unique `body<X>` declarations for every cut leaf.

All three tests pass against current main, which suggests the original
Tripletex retro (#1243) captured pre-#1190 output: #1190
("dedupe nested body-field flatten collisions with sibling scalars")
landed 2026-05-12 03:50, and the Tripletex retro was filed 2026-05-12
19:03 — but Tripletex was generated before #1190 was pulled in. These
tests guard the post-#1190 behavior so future changes to the dedupe
walk can't silently regress the same shapes.

Closes #1243.

* test(cli): tighten body-flag dedup regression coverage

Address review feedback on the three new tests:

- Add `require.Len(t, bodyVars, N)` plus identifier spot-checks so a
  regression that silently elides a colliding field (instead of
  renaming it) gets caught — assertNoDuplicates alone passes when the
  renderer drops a leaf.
- Rewrite the convergent-paths doc-block so it no longer references an
  untested "Wrapper.Inner.value" scenario that confused readers.
- Drop ticket numbers from the doc-blocks per the project convention
  on code-comment hygiene.

Files touched

Diff

commit d44587e9ed6d6df1eaa76876bd12a5ac6ae3c421
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Thu May 14 00:53:32 2026 -0700

    test(cli): regression tests for body-flag identifier collision shapes (#1376)
    
    * test(cli): regression tests for body-flag identifier collision shapes
    
    Locks in `dedupeFlagIdentifiers` / `uniquifyBodyTree` correctness for
    three nested body shapes flagged in #1243:
    
      1. Two convergent nested-object paths whose leaves share a name
         (Project.Customer.name vs Project.PostalAddress.Customer.name).
      2. Two nested-object paths whose joined camelized identifiers
         collapse to the same Go name (Project.Customer.name vs
         ProjectCustomer.name as a top-level sibling).
      3. A Tripletex-shape OpenAPI spec parsed end-to-end with $refs that
         cycle through Customer via ledgerAccount.vatType.customer and
         postalAddress.customer — the parser's pointer-keyed visited[] cut
         terminates re-entry, and the dedupe pass + renderer produce
         unique `body<X>` declarations for every cut leaf.
    
    All three tests pass against current main, which suggests the original
    Tripletex retro (#1243) captured pre-#1190 output: #1190
    ("dedupe nested body-field flatten collisions with sibling scalars")
    landed 2026-05-12 03:50, and the Tripletex retro was filed 2026-05-12
    19:03 — but Tripletex was generated before #1190 was pulled in. These
    tests guard the post-#1190 behavior so future changes to the dedupe
    walk can't silently regress the same shapes.
    
    Closes #1243.
    
    * test(cli): tighten body-flag dedup regression coverage
    
    Address review feedback on the three new tests:
    
    - Add `require.Len(t, bodyVars, N)` plus identifier spot-checks so a
      regression that silently elides a colliding field (instead of
      renaming it) gets caught — assertNoDuplicates alone passes when the
      renderer drops a leaf.
    - Rewrite the convergent-paths doc-block so it no longer references an
      untested "Wrapper.Inner.value" scenario that confused readers.
    - Drop ticket numbers from the doc-blocks per the project convention
      on code-comment hygiene.
---
 internal/generator/body_collision_test.go | 229 ++++++++++++++++++++++++++++++
 1 file changed, 229 insertions(+)

diff --git a/internal/generator/body_collision_test.go b/internal/generator/body_collision_test.go
index 573fa745..59ff0eed 100644
--- a/internal/generator/body_collision_test.go
+++ b/internal/generator/body_collision_test.go
@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/mvanhorn/cli-printing-press/v4/internal/openapi"
 	"github.com/mvanhorn/cli-printing-press/v4/internal/spec"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -204,6 +205,234 @@ func TestGenerateRenamesBodyFieldCollidingWithStdin(t *testing.T) {
 		"the body field named 'stdin' must not collide with the template's --stdin flag")
 }
 
+// TestGenerateDeduplicatesNestedTreesCollapsedToSameIdent guards the
+// case where two distinct nested-object paths whose joined camelized
+// segments collapse to the same Go identifier. Project.Customer.name
+// and ProjectCustomer.name (a sibling object literally named
+// projectCustomer) both produce bodyProjectCustomerName, and the dedup
+// pass must rename one of them.
+func TestGenerateDeduplicatesNestedTreesCollapsedToSameIdent(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("collide-collapsed")
+	apiSpec.Resources["entries"] = spec.Resource{
+		Description: "Entries",
+		Endpoints: map[string]spec.Endpoint{
+			"create": {
+				Method:      "POST",
+				Path:        "/entries",
+				Description: "Two nested objects whose joined paths camelize identically",
+				Body: []spec.Param{
+					{Name: "project", Type: "object", Description: "Project wrapper", Fields: []spec.Param{
+						{Name: "customer", Type: "object", Description: "Customer (nested via project)", Fields: []spec.Param{
+							{Name: "name", Type: "string", Description: "Name via project.customer"},
+						}},
+					}},
+					{Name: "projectCustomer", Type: "object", Description: "Project-customer (sibling at top)", Fields: []spec.Param{
+						{Name: "name", Type: "string", Description: "Name via projectCustomer"},
+					}},
+				},
+			},
+			"get": {Method: "GET", Path: "/entries/{id}", Description: "Get one"},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "collide-collapsed-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	bodyVars, flagBindings := parseBodyDeclarations(t,
+		filepath.Join(outputDir, "internal", "cli", "entries_create.go"))
+
+	assertNoDuplicates(t, bodyVars,
+		"nested paths whose joined camelized identifiers collapse to the same Go name must dedupe")
+	assertNoDuplicates(t, flagBindings,
+		"nested paths whose joined camelized flag names collapse must dedupe")
+	require.Len(t, bodyVars, 2,
+		"both collapsed paths must survive dedup as distinct Go identifiers")
+	assert.Contains(t, flagBindings, "project-customer-name",
+		"the first registrant keeps the canonical cobra flag name")
+	assert.Contains(t, flagBindings, "project-customer-name-2",
+		"the deduped sibling carries the -2 suffix")
+}
+
+// TestGenerateDeduplicatesConvergentNestedBodyPaths guards two distinct
+// nested body paths that converge on the same trailing segments
+// (Project.Customer.name AND Project.PostalAddress.Customer.name). The
+// identPrefix-based walker joins the full path so both leaves produce
+// unique identifiers (bodyProjectCustomerName vs
+// bodyProjectPostalAddressCustomerName) without falling back on the _N
+// suffix.
+func TestGenerateDeduplicatesConvergentNestedBodyPaths(t *testing.T) {
+	t.Parallel()
+
+	apiSpec := minimalSpec("collide-convergent")
+	apiSpec.Resources["assets"] = spec.Resource{
+		Description: "Assets",
+		Endpoints: map[string]spec.Endpoint{
+			"create": {
+				Method:      "POST",
+				Path:        "/assets",
+				Description: "Create an asset whose body has convergent nested paths",
+				Body: []spec.Param{
+					{Name: "project", Type: "object", Description: "Project root", Fields: []spec.Param{
+						{Name: "customer", Type: "object", Description: "Direct customer", Fields: []spec.Param{
+							{Name: "name", Type: "string", Description: "Customer name (direct)"},
+						}},
+						{Name: "postalAddress", Type: "object", Description: "Postal address", Fields: []spec.Param{
+							{Name: "customer", Type: "object", Description: "Postal address customer", Fields: []spec.Param{
+								{Name: "name", Type: "string", Description: "Customer name (via address)"},
+							}},
+						}},
+					}},
+				},
+			},
+			"get": {
+				Method:      "GET",
+				Path:        "/assets/{id}",
+				Description: "Get one asset",
+			},
+		},
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "collide-convergent-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	bodyVars, flagBindings := parseBodyDeclarations(t,
+		filepath.Join(outputDir, "internal", "cli", "assets_create.go"))
+
+	assertNoDuplicates(t, bodyVars,
+		"convergent nested paths must produce distinct Go identifiers")
+	assertNoDuplicates(t, flagBindings,
+		"convergent nested paths must register distinct cobra flag names")
+	require.Len(t, bodyVars, 2,
+		"both convergent name leaves must survive as distinct Go identifiers")
+	assert.Contains(t, bodyVars, "bodyProjectCustomerName",
+		"the direct project.customer.name leaf produces this identifier")
+	assert.Contains(t, bodyVars, "bodyProjectPostalAddressCustomerName",
+		"the project.postalAddress.customer.name leaf produces this identifier via path-prefix joining")
+}
+
+// TestGenerateDeduplicatesCyclicRefBodyShape drives the full OpenAPI
+// parse → dedupe → render path on a body schema where $refs chain
+// through multiple paths that re-enter the same component schema. The
+// parser's cycle detection terminates the recursion when it revisits a
+// schema pointer, producing leaf entries at each cut point. The
+// generator's dedupe pass then walks the resulting tree and uniquifies
+// any collisions so every cycle-cut leaf and every direct leaf emit
+// distinct Go identifiers.
+func TestGenerateDeduplicatesCyclicRefBodyShape(t *testing.T) {
+	t.Parallel()
+
+	yaml := `openapi: 3.0.0
+info:
+  title: cyclic-shape
+  version: 1.0.0
+servers:
+  - url: https://api.example.com
+paths:
+  /asset:
+    post:
+      operationId: createAsset
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/Asset'
+      responses:
+        '200':
+          description: ok
+  /asset/{id}:
+    get:
+      operationId: getAsset
+      parameters:
+        - name: id
+          in: path
+          required: true
+          schema: {type: string}
+      responses:
+        '200':
+          description: ok
+components:
+  schemas:
+    Asset:
+      type: object
+      properties:
+        project: {$ref: '#/components/schemas/Project'}
+    Project:
+      type: object
+      properties:
+        customer: {$ref: '#/components/schemas/Customer'}
+    Customer:
+      type: object
+      properties:
+        name: {type: string}
+        ledgerAccount: {$ref: '#/components/schemas/LedgerAccount'}
+        postalAddress: {$ref: '#/components/schemas/PostalAddress'}
+    LedgerAccount:
+      type: object
+      properties:
+        vatType: {$ref: '#/components/schemas/VatType'}
+    VatType:
+      type: object
+      properties:
+        customer: {$ref: '#/components/schemas/Customer'}
+    PostalAddress:
+      type: object
+      properties:
+        customer: {$ref: '#/components/schemas/Customer'}
+`
+	apiSpec, err := openapi.Parse([]byte(yaml))
+	require.NoError(t, err)
+
+	apiSpec.Name = "cyclic-shape"
+	apiSpec.Owner = "test-owner"
+	apiSpec.OwnerName = "Test Author"
+	if apiSpec.Auth.Type == "" {
+		apiSpec.Auth = spec.AuthConfig{
+			Type:    "api_key",
+			Header:  "Authorization",
+			Format:  "Bearer {token}",
+			EnvVars: []string{"CYCLIC_SHAPE_TOKEN"},
+		}
+	}
+	apiSpec.Config = spec.ConfigSpec{
+		Format: "toml",
+		Path:   "~/.config/cyclic-shape-pp-cli/config.toml",
+	}
+
+	outputDir := filepath.Join(t.TempDir(), "cyclic-shape-pp-cli")
+	require.NoError(t, New(apiSpec, outputDir).Generate())
+
+	// The endpoint file name uses the resource derived by the parser. The
+	// asset resource path is /asset; the POST endpoint becomes <resource>_<op>.go.
+	postFile := filepath.Join(outputDir, "internal", "cli", "asset_create-asset.go")
+	if _, err := os.Stat(postFile); err != nil {
+		postFile = filepath.Join(outputDir, "internal", "cli", "asset_create.go")
+	}
+	require.FileExists(t, postFile,
+		"the createAsset POST command file must exist")
+
+	bodyVars, flagBindings := parseBodyDeclarations(t, postFile)
+
+	assertNoDuplicates(t, bodyVars,
+		"cyclic-ref body shape must produce distinct Go identifiers for every emitted var")
+	assertNoDuplicates(t, flagBindings,
+		"cyclic-ref body shape must register distinct cobra flag names")
+
+	// The cycle-cut shape produces three leaves: the direct
+	// project.customer.name string and two cycle-cut customer objects
+	// at distinct paths.
+	require.Len(t, bodyVars, 3,
+		"every direct and cycle-cut leaf must survive dedup as a distinct Go identifier")
+	assert.Contains(t, bodyVars, "bodyProjectCustomerName",
+		"the direct project.customer.name leaf must emit bodyProjectCustomerName")
+	assert.Contains(t, bodyVars, "bodyProjectCustomerLedgerAccountVatTypeCustomer",
+		"the ledgerAccount.vatType.customer cycle-cut leaf must emit a unique identifier under its full path")
+	assert.Contains(t, bodyVars, "bodyProjectCustomerPostalAddressCustomer",
+		"the postalAddress.customer cycle-cut leaf must emit a unique identifier under its full path")
+}
+
 // parseBodyDeclarations returns the names of all `var bodyXxx` declarations
 // and the literal cobra flag names registered. Cobra registrations may come
 // from either flag<X> or body<X> Go identifiers, so the flag-binding return

← 1f1b0bdb fix(cli): emit WithNumber/WithBoolean for OpenAPI-parsed num  ·  back to Cli Printing Press  ·  fix(cli): emit structured JSON error from parents invoked wi fb9125e8 →