docs(web/guides): document reserved CFML scope names in the controllers guide#2834
Conversation
Documents that client, url, form, session, cgi, request, application, cookie, server, arguments, and variables must not be used as local variable names in controllers or models. Calls out the client collision specifically — Lucee 7 throws "client scope is not enabled" when clientManagement is off, making the cause hard to spot. Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR adds a well-needed "Reserved scope names" section to the Controllers guide and a matching CHANGELOG entry, directly addressing the Lucee 7 client scope footgun in #2833. The docs content is accurate and the code examples are correct. I have three minor nits — one CHANGELOG categorisation mistake, one scope-list omission worth calling out, and one forward-looking concern about an immediate doc inconsistency the PR itself flags. No correctness, cross-engine, or security issues. Verdict: comment (minor nits only, no blocking issues).
Docs
1. CHANGELOG entry is in ### Changed, not ### Added
File: CHANGELOG.md, line 25 (the new bullet).
The entry reads:
### Changed
- Added a "Reserved scope names" section …
Keep a Changelog format reserves ### Changed for modifications to existing behaviour; a brand-new guide section belongs under ### Added. The word "Added" even appears in the bullet itself, which makes the categorisation look inconsistent.
Suggested fix: move the bullet to ### Added (create the heading if this is the first Added entry in [Unreleased]).
2. CHANGELOG bullet says "in controllers or models" — the guide only covers controllers
Same line, CHANGELOG.md:25:
…that must not be used as local variable names in controllers or models…
The changed file is controllers-and-actions.mdx and the new section lives under the "Controllers and Actions" page. Nothing in the diff touches a models guide or explains how the rule applies to model CFCs specifically. "or models" implies coverage that doesn't exist yet.
Suggested fix: drop "or models" or broaden to "in CFML components" if the intent is to cover the general case:
…that must not be used as local variable names in Wheels controllers (and CFML components generally)…
3. local and this are missing from the reserved-scope identifier list
File: web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx, line 257 (new content):
`client`, `url`, `form`, `session`, `cgi`, `request`, `application`, `cookie`, `server`, `arguments`, `variables`
Two additional CFML scopes cause real footguns:
local— the implicit function-local scope. Writingvar local = gateway; local.charge(amount)storesgatewayatlocal.local(becausevarwrites into thelocalscope), solocal.chargeresolves against the scope, not the object. This is a subtle trap for anyone naming a service variablelocal.this— the component public scope.var this = someStructinside a function shadows component state in ways that differ across engines.
CLAUDE.md § Anti-Pattern #11 lists the same eleven identifiers and also omits these two — so this is a pre-existing gap in both places — but since this PR's purpose is precisely to document the hazard list, it's worth making the list more complete here, even if a separate pass is needed to update CLAUDE.md.
Suggested addition at the end of the list:
`client`, `url`, `form`, `session`, `cgi`, `request`, `application`, `cookie`, `server`, `arguments`, `variables`, `local`, `this`Consistency (forward-looking, not blocking)
The new Aside correctly flags var client = $testClient() as the most common occurrence, and the PR body honestly notes ~18 remaining uses across four testing guide files. That's a real inconsistency a reader will hit immediately: the controllers page now says "don't use client as a variable name" and the testing page right beside it shows var client = $testClient() as the canonical pattern throughout. The follow-up PR mentioned in the body should ideally be opened at the same time this one merges (or as a direct follow-on) rather than left open-ended.
No commits to flag — the single docs(web/guides): commit is well-formed, subject is under 100 chars, and the scope is appropriate.
Wheels Bot — Reviewer B (round 1)A's review is accurate, well-supported by the diff, and free of sycophancy. All three nits check out against the actual changed lines. I agree with A's findings and the actionable suggestions. Emitting SycophancyNone detected. A gave a specific verdict ("comment"), cited exact lines for each nit, and did not approve unconditionally. False positivesNone detected.
Missed issuesOne minor gap: CLAUDE.md Anti-Pattern 11 carries the identical eleven-identifier list and has the same omission. A explicitly acknowledged this as a "pre-existing gap" and deferred it to a separate pass, which is a reasonable scoping call for a docs PR. However, since Verdict alignmentA's "comment" verdict is consistent with the findings: the guide content itself is correct, and all three nits are in the CHANGELOG text or the scope list — none warrant blocking the merge outright. ConvergenceAligned. A has three concrete, actionable findings (CHANGELOG category, CHANGELOG text scope, missing |
- Move the new "Reserved scope names" CHANGELOG entry from `### Changed` to `### Added`, matching Keep a Changelog semantics for brand-new documentation content (CHANGELOG.md). - Drop "or models" from the CHANGELOG bullet, since the diff only touches `controllers-and-actions.mdx`; rephrase to "Wheels controllers (and CFML components generally)" so the wording matches the scope of the actual change (CHANGELOG.md). - Append `local` and `this` to the reserved-scope identifier list in the Controllers and Actions guide. `local` is the implicit function-local scope and `var local = x` writes to `local.local`; `this` is the component public scope. Both produce real footguns in the same class as the listed identifiers (web/sites/guides/src/content/docs/v4-0-0/basics/controllers-and-actions.mdx). - Apply the same `local`/`this` addition to CLAUDE.md Anti-Pattern 11, which carries the identical hazard list and had the same omission (CLAUDE.md). B suggested this as a low-cost bundle since address-review was touching the PR anyway. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Wheels Bot — Address Review (round 1)Applied consensus findings from Reviewer A and Reviewer B's convergence (round 1):
Skipped:
No test run was required — The new commit will trigger a fresh Reviewer A run on the updated SHA. Convergence loop continues until reviewers align on |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: Round-1 address-review commit correctly resolves all three findings from the previous review: the CHANGELOG entry is now under ### Added, the "or models" wording is dropped in favour of "(and CFML components generally)", and local and this are appended to the identifier list in both the controllers guide and CLAUDE.md Anti-Pattern 11. No correctness, cross-engine, or security issues in the new material. Verdict: approve (submitting as comment because the bot cannot approve its own PR).
Docs
All three round-1 items are clean:
- CHANGELOG under
### Added— confirmed atCHANGELOG.md:23. ✓ - Wording — bullet now reads "Wheels controllers (and CFML components generally)". ✓
- Identifier list — both
localandthisappear atcontrollers-and-actions.mdx:257and inCLAUDE.mdAnti-Pattern 11. ✓
The description for local ("the engine may resolve the identifier to the scope struct instead of the value you declared") is technically accurate: var local = x writes local.local = x (the var keyword targets the local scope), so a subsequent local.charge(amount) resolves through the scope rather than the declared value. Adequate for a warning section.
Commits
Two commits, both well-formed:
4844f3b1a—docs(web/guides): add reserved scope names section to controllers guide— subject under 100 chars, correct type/scope, DCO sign-off present and matching (claude[bot]). ✓a0ffc93c4—docs(web/guides): address Reviewer A/B consensus findings (round 1)— same; sign-off matches author. ✓
The first commit also carries a wheels-bot[bot] sign-off trailer from the write-docs run. That extra trailer is harmless — the DCO app keys on the author-matching trailer, which is present.
No new correctness, cross-engine, test-coverage, or security issues introduced.
Wheels Bot — Reviewer B (round 1)A's review of the round-1 address commit is accurate and well-supported by the diff. All three prior findings are confirmed resolved. No sycophancy, no false positives, no missed issues on a pure documentation change. Emitting SycophancyNone detected. A verified each of the three prior findings against specific diff locations before declaring approve, and noted the bot-cannot-approve-its-own-PR constraint rather than silently upgrading to a formal approval. False positivesNone detected.
A's note that the second commit's Missed issuesNone detected. This is a docs-only change across Verdict alignmentA's "approve" (submitted as comment due to bot-self-review constraint) is consistent with all prior findings being resolved and no new issues introduced. ConvergenceAligned. A and I agree the PR is review-clean on this SHA. No pending findings on either side. Joint recommendation: approve and merge. |
This PR adds a "Reserved scope names" section to the Controllers and Actions guide, directly addressing the user report in #2833.
The reporter discovered that naming a local variable
clientin a Wheels controller causes Lucee 7 to throw"client scope is not enabled"— an error that looks like an application configuration problem rather than a naming problem. The root cause is CFML scope precedence: Lucee resolvesclientto the (disabled) built-inclientscope instead of the localvardeclaration.The new section:
client,url,form,session,cgi,request,application,cookie,server,arguments,variablesclientcase in detail, including the exact Lucee 7 error messagevar client = $testClient()in test code as the most common occurrenceFixes #2833
Screenshots needed
None — this PR is text-only documentation.
Follow-up not included in this PR
The testing guide examples in
testing/controller-tests.mdx,testing/functional-tests.mdx,testing/view-and-form-tests.mdx, andtesting/integration-tests.mdxall usevar client = $testClient()as the canonical pattern. Renaming those (~22 occurrences across 4 files, plus all subsequentclient.get(...)/client.post(...)/client.assertXxx(...)uses) would add significant churn and widen the scope of this PR. A separate PR should renameclienttohttpClientortestClientthroughout those examples for consistency with the guidance added here.