[object Object]

← back to Cli Printing Press

refactor(cli): extract body-map block to a shared helper (#450) (#457)

2c59d1c60de40cc75af5f5b38106afa9fa7ca6ab · 2026-04-30 23:51:32 -0700 · Trevin Chow

* refactor(generator): extract body-map block to a shared helper (#450)

The same 22-line "build map[string]any from --field flags" block was
duplicated four times — three in command_endpoint.go.tmpl (POST/PUT/PATCH)
and one in command_promoted.go.tmpl. A fifth body-shape category would
have meant lockstep updates across all four sites.

Lift the block into a bodyMap(body, indent) Go helper registered in the
generator's FuncMap. Each template now invokes {{bodyMap .Endpoint.Body
"<indent>"}}, parameterizing the indent so the same logic produces
correctly indented Go code at the 4-tab endpoint sites and the 3-tab
promoted site without resorting to text/template named partials (which
can't preserve per-line indentation across a multi-line block).

Goldens pass byte-identical — the helper output matches the previous
template output for every API in the golden harness. Added body_map_test
to cover the object/array and JSON-string branches, which the goldens
don't exercise.

Closes #450

* refactor(generator): merge bodyMap's twin JSON branches + cover IdentName

Simplify pass on bodyMap. Two findings from the post-PR review:

- The object/array and jsonStringParam branches were themselves nearly
  identical — the exact duplication this PR exists to remove from
  templates, just reproduced 7 lines lower. Merge into one block with
  isComplex deciding whether body[k] gets the parsed value or the raw
  string. Cuts ~7 lines and removes the lockstep-edit hazard.
- body_map_test never set IdentName, the field paramIdent's whole
  contract is built around (the dedup pass's output when two params
  collide on the same Go identifier). Add a focused test asserting the
  variable uses IdentName while body["wire-name"] keeps Name — without
  this invariant the generated CLI either won't compile or will send
  the wrong field to the server.

Goldens still byte-identical; the merge preserves output exactly.

Files touched

Diff

commit 2c59d1c60de40cc75af5f5b38106afa9fa7ca6ab
Author: Trevin Chow <trevin@trevinchow.com>
Date:   Thu Apr 30 23:51:32 2026 -0700

    refactor(cli): extract body-map block to a shared helper (#450) (#457)
    
    * refactor(generator): extract body-map block to a shared helper (#450)
    
    The same 22-line "build map[string]any from --field flags" block was
    duplicated four times — three in command_endpoint.go.tmpl (POST/PUT/PATCH)
    and one in command_promoted.go.tmpl. A fifth body-shape category would
    have meant lockstep updates across all four sites.
    
    Lift the block into a bodyMap(body, indent) Go helper registered in the
    generator's FuncMap. Each template now invokes {{bodyMap .Endpoint.Body
    "<indent>"}}, parameterizing the indent so the same logic produces
    correctly indented Go code at the 4-tab endpoint sites and the 3-tab
    promoted site without resorting to text/template named partials (which
    can't preserve per-line indentation across a multi-line block).
    
    Goldens pass byte-identical — the helper output matches the previous
    template output for every API in the golden harness. Added body_map_test
    to cover the object/array and JSON-string branches, which the goldens
    don't exercise.
    
    Closes #450
    
    * refactor(generator): merge bodyMap's twin JSON branches + cover IdentName
    
    Simplify pass on bodyMap. Two findings from the post-PR review:
    
    - The object/array and jsonStringParam branches were themselves nearly
      identical — the exact duplication this PR exists to remove from
      templates, just reproduced 7 lines lower. Merge into one block with
      isComplex deciding whether body[k] gets the parsed value or the raw
      string. Cuts ~7 lines and removes the lockstep-edit hazard.
    - body_map_test never set IdentName, the field paramIdent's whole
      contract is built around (the dedup pass's output when two params
      collide on the same Go identifier). Add a focused test asserting the
      variable uses IdentName while body["wire-name"] keeps Name — without
      this invariant the generated CLI either won't compile or will send
      the wrong field to the server.
    
    Goldens still byte-identical; the merge preserves output exactly.
---
 internal/generator/body_map_test.go                | 145 +++++++++++++++++++++
 internal/generator/generator.go                    |  39 ++++++
 .../generator/templates/command_endpoint.go.tmpl   |  75 +----------
 .../generator/templates/command_promoted.go.tmpl   |  25 +---
 4 files changed, 188 insertions(+), 96 deletions(-)

diff --git a/internal/generator/body_map_test.go b/internal/generator/body_map_test.go
new file mode 100644
index 00000000..f5eb408a
--- /dev/null
+++ b/internal/generator/body_map_test.go
@@ -0,0 +1,145 @@
+package generator
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/mvanhorn/cli-printing-press/v3/internal/spec"
+)
+
+// TestBodyMap pins the rendered Go code for each of the three body-param
+// shapes (object/array, JSON-string, scalar). The generator's golden
+// harness only exercises the scalar branch, so this test guards the
+// other two against silent drift after the bash → helper extraction.
+func TestBodyMap(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name   string
+		body   []spec.Param
+		indent string
+		want   string
+	}{
+		{
+			name:   "scalar string",
+			body:   []spec.Param{{Name: "name", Type: "string"}},
+			indent: "\t\t\t\t",
+			want: "\t\t\t\tif bodyName != \"\" {\n" +
+				"\t\t\t\t\tbody[\"name\"] = bodyName\n" +
+				"\t\t\t\t}\n",
+		},
+		{
+			name:   "scalar int",
+			body:   []spec.Param{{Name: "count", Type: "int"}},
+			indent: "\t\t\t",
+			want: "\t\t\tif bodyCount != 0 {\n" +
+				"\t\t\t\tbody[\"count\"] = bodyCount\n" +
+				"\t\t\t}\n",
+		},
+		{
+			name:   "object branch parses JSON and stores parsed value",
+			body:   []spec.Param{{Name: "metadata", Type: "object"}},
+			indent: "\t\t\t",
+			want: "\t\t\tif bodyMetadata != \"\" {\n" +
+				"\t\t\t\tvar parsedMetadata any\n" +
+				"\t\t\t\tif err := json.Unmarshal([]byte(bodyMetadata), &parsedMetadata); err != nil {\n" +
+				"\t\t\t\t\treturn fmt.Errorf(\"parsing --metadata JSON: %w\", err)\n" +
+				"\t\t\t\t}\n" +
+				"\t\t\t\tbody[\"metadata\"] = parsedMetadata\n" +
+				"\t\t\t}\n",
+		},
+		{
+			name:   "array branch matches object branch shape",
+			body:   []spec.Param{{Name: "tags", Type: "array"}},
+			indent: "\t\t\t",
+			want: "\t\t\tif bodyTags != \"\" {\n" +
+				"\t\t\t\tvar parsedTags any\n" +
+				"\t\t\t\tif err := json.Unmarshal([]byte(bodyTags), &parsedTags); err != nil {\n" +
+				"\t\t\t\t\treturn fmt.Errorf(\"parsing --tags JSON: %w\", err)\n" +
+				"\t\t\t\t}\n" +
+				"\t\t\t\tbody[\"tags\"] = parsedTags\n" +
+				"\t\t\t}\n",
+		},
+		{
+			// JSON-string params: type is "string" but the format/description
+			// signal JSON content. The branch validates JSON before sending
+			// but stores the raw string (not the parsed value) so the API
+			// receives the user's exact bytes.
+			name:   "jsonString branch validates but stores raw",
+			body:   []spec.Param{{Name: "config", Type: "string", Format: "json"}},
+			indent: "\t\t\t",
+			want: "\t\t\tif bodyConfig != \"\" {\n" +
+				"\t\t\t\tvar parsedConfig any\n" +
+				"\t\t\t\tif err := json.Unmarshal([]byte(bodyConfig), &parsedConfig); err != nil {\n" +
+				"\t\t\t\t\treturn fmt.Errorf(\"parsing --config JSON: %w\", err)\n" +
+				"\t\t\t\t}\n" +
+				"\t\t\t\tbody[\"config\"] = bodyConfig\n" +
+				"\t\t\t}\n",
+		},
+		{
+			name: "multiple params concatenate in order",
+			body: []spec.Param{
+				{Name: "name", Type: "string"},
+				{Name: "tags", Type: "array"},
+			},
+			indent: "\t",
+			want: "\tif bodyName != \"\" {\n" +
+				"\t\tbody[\"name\"] = bodyName\n" +
+				"\t}\n" +
+				"\tif bodyTags != \"\" {\n" +
+				"\t\tvar parsedTags any\n" +
+				"\t\tif err := json.Unmarshal([]byte(bodyTags), &parsedTags); err != nil {\n" +
+				"\t\t\treturn fmt.Errorf(\"parsing --tags JSON: %w\", err)\n" +
+				"\t\t}\n" +
+				"\t\tbody[\"tags\"] = parsedTags\n" +
+				"\t}\n",
+		},
+		{
+			name:   "empty body produces empty string",
+			body:   nil,
+			indent: "\t\t\t",
+			want:   "",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			got := bodyMap(tc.body, tc.indent)
+			if got != tc.want {
+				t.Errorf("bodyMap mismatch.\n got:\n%s\nwant:\n%s\nraw got: %q\nraw want: %q",
+					got, tc.want, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestBodyMap_DashIdentifier verifies hyphenated param names route through
+// paramIdent + camelCase the same way the templates do — `user-id` becomes
+// `bodyUserId` (for the variable) but stays `user-id` in the JSON key.
+func TestBodyMap_DashIdentifier(t *testing.T) {
+	t.Parallel()
+	got := bodyMap([]spec.Param{{Name: "user-id", Type: "string"}}, "\t")
+	if !strings.Contains(got, "bodyUserId") {
+		t.Errorf("expected camelCased identifier, got: %s", got)
+	}
+	if !strings.Contains(got, `body["user-id"]`) {
+		t.Errorf("expected JSON key with dash preserved, got: %s", got)
+	}
+}
+
+// TestBodyMap_IdentName verifies the dedup pass's output: when IdentName
+// is set (because two params would otherwise collide on the same Go
+// identifier), the variable name uses IdentName but body[key] keeps the
+// wire Name. Without this, the generated CLI would either fail to compile
+// or send the wrong field name to the server.
+func TestBodyMap_IdentName(t *testing.T) {
+	t.Parallel()
+	got := bodyMap([]spec.Param{{Name: "start", IdentName: "StartGT", Type: "string"}}, "\t")
+	if !strings.Contains(got, "bodyStartGT") {
+		t.Errorf("expected variable to use IdentName, got: %s", got)
+	}
+	if !strings.Contains(got, `body["start"]`) {
+		t.Errorf("expected wire key to use Name (not IdentName), got: %s", got)
+	}
+}
diff --git a/internal/generator/generator.go b/internal/generator/generator.go
index a900435e..e28e89b3 100644
--- a/internal/generator/generator.go
+++ b/internal/generator/generator.go
@@ -275,6 +275,7 @@ func New(s *spec.APISpec, outputDir string) *Generator {
 		},
 		"jsonStringParam":    isJSONStringParam,
 		"jsonEnumSuggestion": jsonEnumSuggestion,
+		"bodyMap":            bodyMap,
 		// endpointNeedsClientLimit reports whether a list endpoint needs
 		// client-side truncation. True when the endpoint has a `limit`-named
 		// param AND no Pagination block — the spec author asked for a
@@ -2469,6 +2470,44 @@ func endpointNeedsClientLimit(endpoint spec.Endpoint) bool {
 	return false
 }
 
+// bodyMap renders the per-flag body-building block shared by the
+// POST/PUT/PATCH branches in command_endpoint.go.tmpl and the body
+// branch in command_promoted.go.tmpl. The four sites generated the
+// same Go code at different indentation levels; this consolidates them
+// and parameterizes the indent. The output is the body of the
+// `body := map[string]any{}` block — callers emit the surrounding
+// declaration and the closing brace themselves.
+func bodyMap(body []spec.Param, indent string) string {
+	var b strings.Builder
+	for _, p := range body {
+		id := paramIdent(p)
+		ident := toCamel(id)
+		flag := flagName(id)
+		isComplex := p.Type == "object" || p.Type == "array"
+		if isComplex || isJSONStringParam(p) {
+			// object/array: store the parsed value (so the API receives
+			// real JSON). jsonStringParam: validate then store the raw
+			// string (the API expects a JSON-encoded string field).
+			rhs := "body" + ident
+			if isComplex {
+				rhs = "parsed" + ident
+			}
+			fmt.Fprintf(&b, "%sif body%s != \"\" {\n", indent, ident)
+			fmt.Fprintf(&b, "%s\tvar parsed%s any\n", indent, ident)
+			fmt.Fprintf(&b, "%s\tif err := json.Unmarshal([]byte(body%s), &parsed%s); err != nil {\n", indent, ident, ident)
+			fmt.Fprintf(&b, "%s\t\treturn fmt.Errorf(\"parsing --%s JSON: %%w\", err)\n", indent, flag)
+			fmt.Fprintf(&b, "%s\t}\n", indent)
+			fmt.Fprintf(&b, "%s\tbody[%q] = %s\n", indent, p.Name, rhs)
+			fmt.Fprintf(&b, "%s}\n", indent)
+			continue
+		}
+		fmt.Fprintf(&b, "%sif body%s != %s {\n", indent, ident, zeroVal(p.Type))
+		fmt.Fprintf(&b, "%s\tbody[%q] = body%s\n", indent, p.Name, ident)
+		fmt.Fprintf(&b, "%s}\n", indent)
+	}
+	return b.String()
+}
+
 func isJSONStringParam(p spec.Param) bool {
 	if p.Type != "string" {
 		return false
diff --git a/internal/generator/templates/command_endpoint.go.tmpl b/internal/generator/templates/command_endpoint.go.tmpl
index 7ba753f5..cab0d4b8 100644
--- a/internal/generator/templates/command_endpoint.go.tmpl
+++ b/internal/generator/templates/command_endpoint.go.tmpl
@@ -215,30 +215,7 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 				body = jsonBody
 			} else {
 				body = map[string]any{}
-{{- range .Endpoint.Body}}
-{{- if or (eq .Type "object") (eq .Type "array")}}
-				if body{{camel (paramIdent .)}} != "" {
-					var parsed{{camel (paramIdent .)}} any
-					if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-						return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-					}
-					body["{{.Name}}"] = parsed{{camel (paramIdent .)}}
-				}
-{{- else if jsonStringParam .}}
-				if body{{camel (paramIdent .)}} != "" {
-					var parsed{{camel (paramIdent .)}} any
-					if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-						return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-					}
-					body["{{.Name}}"] = body{{camel (paramIdent .)}}
-				}
-{{- else}}
-				if body{{camel (paramIdent .)}} != {{zeroVal .Type}} {
-					body["{{.Name}}"] = body{{camel (paramIdent .)}}
-				}
-{{- end}}
-{{- end}}
-			}
+{{bodyMap .Endpoint.Body "\t\t\t\t"}}			}
 {{- if .Endpoint.HeaderOverrides}}
 			data, statusCode, err := c.PostWithHeaders(path, body, headerOverrides)
 {{- else}}
@@ -264,30 +241,7 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 				body = jsonBody
 			} else {
 				body = map[string]any{}
-{{- range .Endpoint.Body}}
-{{- if or (eq .Type "object") (eq .Type "array")}}
-				if body{{camel (paramIdent .)}} != "" {
-					var parsed{{camel (paramIdent .)}} any
-					if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-						return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-					}
-					body["{{.Name}}"] = parsed{{camel (paramIdent .)}}
-				}
-{{- else if jsonStringParam .}}
-				if body{{camel (paramIdent .)}} != "" {
-					var parsed{{camel (paramIdent .)}} any
-					if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-						return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-					}
-					body["{{.Name}}"] = body{{camel (paramIdent .)}}
-				}
-{{- else}}
-				if body{{camel (paramIdent .)}} != {{zeroVal .Type}} {
-					body["{{.Name}}"] = body{{camel (paramIdent .)}}
-				}
-{{- end}}
-{{- end}}
-			}
+{{bodyMap .Endpoint.Body "\t\t\t\t"}}			}
 {{- if .Endpoint.HeaderOverrides}}
 			data, statusCode, err := c.PutWithHeaders(path, body, headerOverrides)
 {{- else}}
@@ -307,30 +261,7 @@ func new{{camel .FuncPrefix}}{{camel .EndpointName}}Cmd(flags *rootFlags) *cobra
 				body = jsonBody
 			} else {
 				body = map[string]any{}
-{{- range .Endpoint.Body}}
-{{- if or (eq .Type "object") (eq .Type "array")}}
-				if body{{camel (paramIdent .)}} != "" {
-					var parsed{{camel (paramIdent .)}} any
-					if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-						return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-					}
-					body["{{.Name}}"] = parsed{{camel (paramIdent .)}}
-				}
-{{- else if jsonStringParam .}}
-				if body{{camel (paramIdent .)}} != "" {
-					var parsed{{camel (paramIdent .)}} any
-					if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-						return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-					}
-					body["{{.Name}}"] = body{{camel (paramIdent .)}}
-				}
-{{- else}}
-				if body{{camel (paramIdent .)}} != {{zeroVal .Type}} {
-					body["{{.Name}}"] = body{{camel (paramIdent .)}}
-				}
-{{- end}}
-{{- end}}
-			}
+{{bodyMap .Endpoint.Body "\t\t\t\t"}}			}
 {{- if .Endpoint.HeaderOverrides}}
 			data, statusCode, err := c.PatchWithHeaders(path, body, headerOverrides)
 {{- else}}
diff --git a/internal/generator/templates/command_promoted.go.tmpl b/internal/generator/templates/command_promoted.go.tmpl
index dd756cb8..e333edd7 100644
--- a/internal/generator/templates/command_promoted.go.tmpl
+++ b/internal/generator/templates/command_promoted.go.tmpl
@@ -165,30 +165,7 @@ func new{{camel .PromotedName}}PromotedCmd(flags *rootFlags) *cobra.Command {
 			// query params with body for non-GET verbs. Promoted commands
 			// cover the common case: body-only POST/PUT/PATCH.
 			body := map[string]any{}
-{{- range .Endpoint.Body}}
-{{- if or (eq .Type "object") (eq .Type "array")}}
-			if body{{camel (paramIdent .)}} != "" {
-				var parsed{{camel (paramIdent .)}} any
-				if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-					return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-				}
-				body["{{.Name}}"] = parsed{{camel (paramIdent .)}}
-			}
-{{- else if jsonStringParam .}}
-			if body{{camel (paramIdent .)}} != "" {
-				var parsed{{camel (paramIdent .)}} any
-				if err := json.Unmarshal([]byte(body{{camel (paramIdent .)}}), &parsed{{camel (paramIdent .)}}); err != nil {
-					return fmt.Errorf("parsing --{{flagName (paramIdent .)}} JSON: %w", err)
-				}
-				body["{{.Name}}"] = body{{camel (paramIdent .)}}
-			}
-{{- else}}
-			if body{{camel (paramIdent .)}} != {{zeroVal .Type}} {
-				body["{{.Name}}"] = body{{camel (paramIdent .)}}
-			}
-{{- end}}
-{{- end}}
-			data, _, err := c.{{pascal (lower .Endpoint.Method)}}(path, body)
+{{bodyMap .Endpoint.Body "\t\t\t"}}			data, _, err := c.{{pascal (lower .Endpoint.Method)}}(path, body)
 {{- else}}
 			// HEAD/OPTIONS and unknown verbs fall back to GET so generation
 			// stays compileable. Spec authors who genuinely need these verbs

← 8a548d09 feat(cli): add validate-narrative subcommand to replace bash  ·  back to Cli Printing Press  ·  feat(cli): cost-based throttling primitives for GraphQL CLIs de3a4e7c →