Repository navigation
Conversation
`wheels generate controller Name a,b` passed the comma-joined token through verbatim, emitting an invalid `function a,b()` method plus an `a,b.cfm` view file. Normalize the action list in CodeGen.generateController by splitting each positional token on commas, trimming, and de-duplicating, and return the normalized list so Module.cfc renders one view file per real action. The comma form now behaves identically to the documented space-separated form. Fixes #3112 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
…ller Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Docs updatedAdded a doc commit to this PR:
|
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR fixes #3112 — wheels generate controller Name a,b previously passed the comma-joined token through verbatim, emitting an invalid function a,b() method and an a,b.cfm view file. The fix normalizes the action list in CodeGen.generateController (split on commas, trim, case-insensitively de-duplicate, preserve first-seen order) and surfaces the normalized list via result.actions so Module.cfc's view loop writes one view per real action. The change is small, well-commented, tested, and documented. Verdict: comment (no blocking findings; the new specs have not yet been executed anywhere — the CLI test job is still pending on this head — so I'm withholding approval until CI confirms them green).
What I verified
- Caller safety on the failure path —
CodeGen.cfc's early return{success: false, error: ...}(controller exists, no--force) does not carry anactionskey, butModule.cfc:3184-3195only reachesactions = result.actions;after theresult.successbranch; theelsereturns early. No undefined-key access. - All
generateControllercall sites —Module.cfc:3184(CLI + MCP dispatch, both route throughgenerate) consumes the newresult.actions;Scaffold.cfc:86andScaffold.cfc:419pass noactions(normalization is a no-op on the fixed CRUD default) and never readresult.actions. Adding a key to the returned struct breaks nothing. - Template-selection interaction — normalization runs before the
hasCustomActionscheck againstcrudActions, so--crudwith comma-joined CRUD actions now correctly selectsCRUDContent.txtexactly as the space-separated form does. - Test assertions are sound —
ControllerContent.txtrenders actions only through the|Actions|placeholder as individual function stubs, soexpect(content).notToInclude("index,show")cannot false-fail on template boilerplate. The three specs cover splitting, trimming, de-duplication, and the returned normalized array. - Legacy CLI parity —
cli/src/commands/wheels/generate/controller.cfcalready splits--actionsas a comma list (listToArray(trim(arguments.actions))), so the two CLI surfaces now agree; no parity gap introduced. - Edge behavior change (acceptable) — a token of only commas/whitespace (e.g.
wheels generate controller Users ",") now normalizes to an empty list and falls through to the default["index"]instead of emitting garbage. Strictly an improvement. - Docs & metadata — changelog fragment
changelog.d/3112-generate-controller-comma-actions.fixed.mdpresent (no directCHANGELOG.mdedit); guides pagecode-generation.mdxupdated to document the comma form. - Commits — both headers are valid conventional commits ≤ 100 chars (
fix(cli): ...,docs(web/guides): ...), and the PR title (the squash-merge gate) is a valid header. DCO sign-off matches the commit author (claude[bot]) on both commits.
Tests
- The PR honestly discloses that
bash tools/test-cli-local.shcould not run in the authoring sandbox; the same constraint applies in this review environment (no LuCLI on PATH). The Lucee 7 + SQLite (LuCLI) check is stillpendingon this head as of this review. The specs read correct, but no environment has executed them yet — please confirm that job goes green before merge. This is the only reason this review is a comment rather than an approval. - Minor nit (no change required): in
cli/lucli/tests/specs/services/CodeGenSpec.cfcthe first new spec ("splits a comma-joined action token...") assignsvar result = codegen.generateController(...)but never readsresult; the other two specs use it. Dropping the unused assignment would match the spec's own style, e.g. the existing no-actions case at line 217.
Scope note: this review covers the full PR diff at head d967a9c (which adds a docs-only commit on top of the checkout SHA below); the marker SHA is the workflow-captured checkout SHA per the #2848 rule.
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: Re-review at head d967a9c (the prior wheels-bot review carried the earlier checkout SHA 2e39dfe as its marker and withheld approval only because the CLI test job was still pending). That job — Lucee 7 + SQLite (LuCLI), which executes the full CLI suite including the new CodeGenSpec cases — now passes on this head, as does the Bot PR TDD Gate. The fix itself is correct and well-scoped: CodeGen.generateController normalizes the positional action list (split comma-joined tokens, trim, case-insensitively de-duplicate, preserve first-seen order) and surfaces it via result.actions so Module.cfc's view loop writes one view per real action instead of an index,show.cfm. Verdict: comment — no blocking findings; one trivial test-style nit below. Note the PR author is wheels-bot itself, so a bot --approve is not possible (GitHub rejects self-approval); a maintainer can merge on the strength of green CI plus this review.
Correctness — verified clean
- Failure-path safety:
CodeGen.cfc:140early-returns{success: false, error: ..., path: ...}without anactionskey, butModule.cfc:3188-3191returns early on!result.successbefore reachingactions = result.actions;atModule.cfc:3196. On the success pathresult.actionsis always assigned (CodeGen.cfc:190) before return. No undefined-key access. - All call sites checked:
Module.cfc:3184(both the CLI and MCP dispatch route through the same privategenerateControllerviaModule.cfc:550) consumes the new key;Scaffold.cfc:86andScaffold.cfc:419pass noactions(fixed CRUD default — normalization is a no-op) and read only.success/.error/.path. Adding the key togenerateFromTemplate's returned struct (Templates.cfc:35, shape{success, error, path}) collides with nothing. - Template-selection interaction: normalization runs at
CodeGen.cfc:150, before thehasCustomActionscomparison atCodeGen.cfc:174, so--crudwith comma-joined CRUD actions selectsCRUDContent.txtexactly like the space-separated form. - Spec assertions are sound:
cli/lucli/templates/app/app/snippets/ControllerContent.txtrenders actions only through the|Actions|placeholder (no literalindex,showanywhere in the template), soexpect(content).notToInclude("index,show")cannot false-fail on boilerplate — and CI has now executed these specs green.
Tests
- All three new specs pass in the Lucee 7 + SQLite (LuCLI) job on this head — the disclosed "could not run locally" gap from the PR body is closed.
- Nit (non-blocking, carried over from the prior review):
CodeGenSpec.cfc:223assignsvar result = codegen.generateController(...)but never readsresult; the sibling spec at line 217 calls without assignment. Fine to leave as-is.
Docs
- Changelog fragment
changelog.d/3112-generate-controller-comma-actions.fixed.mdpresent; no directCHANGELOG.mdedit. - The new docs commit updates
web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/code-generation.mdxto document the comma form — the correct live guides dir (there is nov4-0-0-snapshotdirectory in this tree).
Commits
- Both headers are valid conventional commits well under 100 chars (
fix(cli): split comma-joined action tokens in generate controller,docs(web/guides): note comma-separated action form in generate controller), and the PR title (the squash-merge lint gate) is itself a valid header. DCO sign-off present on both commits and matches the commit author identity (claude[bot]).
No Correctness, Cross-engine, or Security findings. The change is confined to the CLI codegen path (runs on the bundled Lucee only) and uses engine-safe constructs throughout (listToArray, trim, arrayFindNoCase, plain for loops; no reserved-scope parameter names).
…dex view Review finding on #3131: the unconditional `actions = result.actions` in Module.cfc adopted CodeGen's defaulted ["index"] list when the user passed no actions, so `wheels generate controller Users` started writing app/views/users/index.cfm — contradicting the documented "passing no actions creates an empty controller with no view files" behavior retained in the same code-generation.mdx paragraph. Surface the caller-requested (post-normalization, pre-default) action list as result.actions instead: empty input stays empty, so the view loop writes nothing, while comma-joined tokens still expand to one view per real action. The controller body keeps its default index() stub as before. Covered by a new CodeGenSpec case asserting result.actions is empty (and the stub present) for the no-actions path. Signed-off-by: Peter Amiri <peter@alurium.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: Re-review at head 578db70f3, which adds one commit on top of the previously reviewed d967a9c: fix(cli): keep generate controller no-actions path from writing an index view. The commit fixes a regression the PR's own first commit introduced — the unconditional actions = result.actions in Module.cfc adopted CodeGen's post-default ["index"] list, so wheels generate controller Users (no actions) would have started writing app/views/users/index.cfm, contradicting the documented "passing no actions creates an empty controller with no view files" behavior. The fix captures the caller-requested list before the defaulting (requestedActions, CodeGen.cfc:156) and surfaces that as result.actions (CodeGen.cfc:199), with a new CodeGenSpec case covering the no-actions path. Verdict: comment — no blocking findings; the Lucee 7 + SQLite (LuCLI) job is still pending on this head, so the new spec has not yet been executed in CI, and the PR author is wheels-bot, so a bot --approve is rejected by GitHub regardless. A maintainer can merge once that job goes green.
Correctness — verified clean
- The regression fix is exact. Pre-PR baseline (
git show 2e39dfe33~1:cli/lucli/Module.cfc, view loop) iterated the raw user-passedactionsarray — empty input wrote no view files. The intermediate state (2e39dfe–d967a9c) surfaced post-defaultarguments.actions, which is["index"]for empty input. The new code returns the post-normalization, pre-default list, so empty input stays empty and the view loop atModule.cfc:3203-3214writes nothing — exactly the pre-PR contract. Comma-joined tokens still expand correctly (requestedActionsis thenormalizeActions()output when actions were passed). - All call sites re-verified at this head.
Module.cfc:3184(CLI + MCP dispatch viaModule.cfc:550) is the only consumer ofresult.actions.Scaffold.cfc:86andScaffold.cfc:419passcrud=truewith noactionsand read only.success/.error—result.actionsbeing[]for them is unread and harmless (Scaffold writes its own views). - Failure-path safety unchanged: the
{success: false, ...}early return atCodeGen.cfc:140still carries noactionskey, butModule.cfc:3188-3191returns beforeactions = result.actions;on!result.success. - No mutation after capture:
requestedActionsaliases the normalized array, and nothing mutatesarguments.actionsin place after the defaulting reassignment (which rebinds the argument rather than mutating the array), so the surfaced list cannot drift.
Tests
- The new spec ("returns an empty action list when no actions are passed...",
CodeGenSpec.cfc:253-267) asserts both halves of the contract: the controller body keeps its defaultindex()stub ANDresult.actionsstays empty.toBeEmpty()has prior art in the CLI suite (CommandArgParsingSpec.cfc:55,ArgSpecSpec.cfc:195,StatsSpec.cfc:137); theStubscontroller name collides with nothing. - The Lucee 7 + SQLite (LuCLI) job is
pendingon this head as of this review — the same posture as the first-pass review: the spec reads correct but no environment has executed it yet. Please confirm that job goes green before merge; it is the only thing between this review and a clean bill. - Carried-over nit (non-blocking, noted in both prior reviews):
CodeGenSpec.cfc:223assignsvar result = codegen.generateController(...)but never readsresult.
Docs
- Non-blocking, pre-existing nit surfaced by this commit's own contract:
code-generation.mdx:109says "Passing no actions creates an empty controller with no view files," but the generated controller is not empty — it gets a defaultindex()stub (CodeGen.cfc:159-161), which the new spec itself asserts (expect(content).toInclude("function index()")). The "no view files" half is now accurate again thanks to this commit; consider rewording to "creates a controller with a defaultindex()stub and no view files" in a future docs pass. Not introduced by this PR. - Changelog fragment unchanged and still correct — the no-actions regression never shipped (introduced and fixed within this PR), so it needs no fragment of its own.
Commits
- All three headers are valid conventional commits under 100 chars; the PR title (the squash-merge lint gate) remains a valid header, and Validate Commit Messages passes on this head. The new commit is human-authored (Peter Amiri) with a matching DCO sign-off.
No Correctness, Cross-engine, or Security findings. The change remains confined to the CLI codegen path (bundled Lucee only) and uses engine-safe constructs throughout.
wheels generate controller Name a,bpassed the comma-joined positional token through verbatim, so the generator emitted an invalidfunction a,b()method (which fails to compile at first request) plus a view file literally nameda,b.cfm. The comma form is the natural guess for anyone used to Wheels list arguments (validatesPresenceOf("a,b")), yet only the documented space-separated form (a b) worked. This normalizes the action list inCodeGen.generateController— splitting each positional token on commas, trimming, and de-duplicating (case-insensitive, first-seen order preserved) — and returns the normalized list soModule.cfc's view loop writes one view file per real action instead of one named after the raw token.Fixes #3112
Type of change
How tested
New
CodeGenSpeccases assert thatactions = ["index,show"]producesfunction index()andfunction show()(neverindex,show), that the returnedresult.actionsis the normalized array, and that trimming/de-duplication work across tokens.Checklist
CodeGenSpeccases for cli:wheels generate controller Name a,bsilently emits invalid CFML — comma-joined action list becomes one action #3112bot-update-docs.yml)bot-update-docs.yml)bot-update-docs.yml)changelog.d/3112-generate-controller-comma-actions.fixed.md🤖 Generated with Claude Code