fix(model): tableName() with an argument fails loud in development instead of silently no-opping - #3134
Merged
Merged
Conversation
…stead of silently no-opping
Calling tableName("x") has always been a silent no-op — tableName() is a
zero-arg getter and table() is the canonical setter (unchanged since
v2.5.1) — but 4.0-era docs mistakenly taught it as a setter (issue #3079).
Rather than adding a setter overload (rejected: no code that ever worked
breaks, and the framework keeps one canonical setter), the getter now
throws Wheels.InvalidArgument pointing at table() when called with any
argument while showErrorInformation is on (development/testing — the same
gate exists() in the same file and paginationNav's InvalidArgument check
use). In production the call remains a no-op so upgrades never break a
running app.
Fixes #3079
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: Peter Amiri <peter@alurium.com>
12 tasks
Contributor
There was a problem hiding this comment.
Wheels Bot — Reviewer
TL;DR: This PR makes tableName() throw Wheels.InvalidArgument in development/testing when called with an argument (previously a silent no-op that stranded models on their convention table — issue #3079), while keeping production a no-op and the zero-arg getter byte-for-byte unchanged. The guard, exception type, gate, and test pattern all match established in-repo prior art exactly; the spec covers all three paths; docs and changelog are in order. Verdict: approve.
Verification notes (all claims in the PR body checked against the code)
Correctness
- The guard at
vendor/wheels/model/miscellaneous.cfc:186short-circuits correctly: a zero-arg call hasStructCount(arguments) == 0, so$get("showErrorInformation")is never even consulted on the getter path — the getter is unchanged (and trivially un-mutated, since the function never wrote state to begin with). - Production safety holds: with
showErrorInformationoff, the conjunction is false and the call falls through to the original return logic — same observable behavior as before the PR. - Internal-caller sweep independently re-verified:
git grepfinds no framework/app/CLI call site passing an argument to modeltableName(); the argument-taking hits are the unrelated adapter method$tableName(list, action)(databaseAdapters/Base.cfc:304).
Conventions / prior art
Wheels.InvalidArgumentis the established type for this class of error (JobWorker.cfc:387,model/associations.cfc:252,view/miscellaneous.cfc:564,view/pagination.cfc:603).- The
showErrorInformationgate matchesexists()in the same file (model/miscellaneous.cfc:100), as the doc comment claims. StructCount(arguments)extra-argument detection is an existing framework idiom that already runs across the CI engine matrix (controller/redirection.cfc:75,controller/rendering.cfc:286).- String escaping in the throw is correct: doubled quotes for the embedded example (
table(""my_table"")) and##3079to escape the hash in the interpolated detail string.
Cross-engine
- No anti-pattern from the CLAUDE.md invariant list is touched (no closures-as-constructor-args, no
attributeCollection = arguments, no bare tag statements, noLeft(str, 0)). Worst conceivable engine divergence — an engine not populatingargumentsfor an extra positional arg — would degrade to the pre-PR silent no-op, never a false-positive throw; and the new spec runs in the core suite on every engine × DB, so CI would surface exactly that divergence.
Tests
vendor/wheels/tests/specs/model/miscellaneousSpec.cfc:163-197covers all three paths: dev throw + table untouched, zero-arg getter unchanged, production no-op. BDDwheels.WheelsTestsyntax, and the_origShowErrtry/finally restore matches the established pattern inpaginationHelpersSpec.cfc:441-472byte-for-byte. Thec_o_r_e_authorsexpectation matchestests/_assets/models/Author.cfc:4. Red/green evidence in the PR body is plausible and the spec is non-tautological (RED run shows the expected single failure).
Docs
- Changelog fragment
changelog.d/3079-tablename-getter-guard.fixed.mduses a valid type (fixed) and a complete bullet line; no directCHANGELOG.mdedit. CLAUDE.mdModel Quick Reference one-liner accurately reflects the new dev-throw / prod-no-op split.
Commits
- Single commit; header
fix(model): tableName() with an argument fails loud in development instead of silently no-oppingis 96 chars, valid type/scope, and the body explains the why (including the deliberate rejection of the #3104 setter-overload approach). DCO sign-off present and matches the author.
No blocking findings; nothing to request. A surgical fix with an honest paper trail.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
tableName()on models is a zero-argument getter; the table setter istable(name)— unchanged since v2.5.1. CFML silently accepts extra positional arguments, so the 4.0-docs-eratableName("my_table")form inconfig()has ALWAYS been a silent no-op: the model keeps its convention table and the first finder throwsWheels.TableNotFound(or quietly hits the wrong table).Design decision (maintainer): fail loud instead of adding a setter overload. PR #3104 made
tableName(name)delegate totable(); this PR supersedes that approach because:tableName()has never accepted an argument, so the only call sites passing one are already-broken (silently no-opping) — surfacing them is strictly an improvement.table()stays the single way to map a model to a table; the docs have already been corrected back to it.Behavior
showErrorInformationis on (development/testing),tableName()now throwsWheels.InvalidArgument: "tableName()is a getter and takes no arguments. To set the database table for this model, calltable()inconfig()instead…" (detail references docs+model: guides and CLAUDE.md use non-existenttableName("x")setter — silent no-op, models fall back to the convention table (real setter istable()) #3079).showErrorInformationforced off at boot) the call remains a silent no-op — an upgrade never breaks a running app.The gate matches the in-file prior art exactly:
exists()in the samemodel/miscellaneous.cfcgates itsWheels.IncorrectArgumentsthrow on$get("showErrorInformation"), as doespaginationNav()'sWheels.PaginationNav.InvalidArgumentcheck. All internal callers grep-verified zero-arg (vendor/wheels,app,tests,cli/lucli).Red/Green Evidence (Lucee 7 + SQLite, Docker harness)
The new throw-spec reuses PR #3104's verified non-tautological spec shape, inverted to expect the throw.
RED — specs run against unguarded develop (guard stashed):
GREEN — guard in place, same bundle:
Full core suite:
The 12 failures are all
wheels.tests.specs.internal.testClientSpec— the known tolerated harness artifacts; matches the develop baseline exactly.Changes
vendor/wheels/model/miscellaneous.cfc— argument-presence guard intableName()(StructCount(arguments) && $get("showErrorInformation")→Throw(type="Wheels.InvalidArgument", …)), plus doc comment explaining the getter/setter split and the prod no-op guarantee.vendor/wheels/tests/specs/model/miscellaneousSpec.cfc— new describe "Tests that tableName rejects arguments - issue 3079": (a) dev throw + table untouched, (b) zero-arg getter unchanged, (c) production no-op (flipsapplication.wheels.showErrorInformationwith try/finally restore, the established pattern frompaginationHelpersSpec).CLAUDE.md— Model Quick Reference one-liner updated from "silent no-op" to the dev-throw behavior.changelog.d/3079-tablename-getter-guard.fixed.md—fixedclass: this fixes a silent failure mode (the no-op was the bug being reported), rather than adding new capability.Related Issue
Fixes #3079
Type of Change
🤖 Generated with Claude Code