Skip to content

Commit a391bf4

Browse files
bpamiriclaudegithub-actions[bot]
authored
fix(cli): exit non-zero when wheels validate finds errors (#2907)
* fix(cli): exit non-zero when wheels validate finds errors wheels validate printed its report and returned "" on every path, so the process exited 0 even when validation found errors and CI could not gate on it (framework review H5, same family as #2890 / CLI audit H6). - errors found: record the failure inside the try, throw Wheels.ValidationFailed after the report is flushed (runTests pattern, out of reach of the catch-all) - analyzer crash: the catch now prints then rethrows instead of swallowing, matching migrate() - no app/ directory: throw Wheels.InvalidArguments after the red hint, matching the other user-error paths - warnings-only stays exit 0: results.valid is true when no severity=="error" issues exist, so validate remains usable as a soft linter; output text and ordering are unchanged Adds ValidateCommandSpec covering all four paths. Verified locally on the Lucee 7 docker harness: 4/4 new specs pass, InfoCommandSpec's existing validate case stays green. Intentional behavior change: scripts that relied on exit 0 despite reported errors will now fail; the MCP wheels_validate tool surfaces a proper tool error instead of a silent empty result. Out of scope: U1 (wheels upgrade check exit code) and the runTests mid-run HTTP catch-swallow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Peter Amiri <peter@alurium.com> * fix(cli): address Reviewer A/B consensus findings (round 1) - Replace stale "Both commands always exit 0" claim in web/sites/guides/.../code-quality.mdx with accurate split: validate exits non-zero on errors, analyze always exits 0. - Condense the two multi-line comment blocks in Module.cfc::validate() (3-line and 5-line) to single-line per CLAUDE.md "one short line max" convention — invariants preserved, just shorter prose. - Condense the 11-line component docstring and 4-line $makeProject() docstring in ValidateCommandSpec.cfc to single-line comments for the same reason. All changes are pure comment/text edits; no runtime behaviour change. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> * fix(cli): address Reviewer A consensus findings (round 2) - cli/lucli/tests/specs/commands/ValidateCommandSpec.cfc lines 7-8: condense 2-line comment block to single-line per CLAUDE.md ("Never write multi-paragraph docstrings or multi-line comment blocks — one short line max"). - cli/lucli/tests/specs/commands/ValidateCommandSpec.cfc lines 17-19: condense 3-line comment block to single-line, same rule. Comment-only changes; no runtime behaviour impact. Test bodies and the four-case coverage matrix are untouched. Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> --------- Signed-off-by: Peter Amiri <peter@alurium.com> Signed-off-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
1 parent a2c4a9f commit a391bf4

3 files changed

Lines changed: 92 additions & 2 deletions

File tree

cli/lucli/Module.cfc

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1728,12 +1728,16 @@ component extends="modules.BaseModule" {
17281728
public string function validate() {
17291729
if (!directoryExists(variables.projectRoot & "/app")) {
17301730
out("No app/ directory found. Are you in a Wheels project?", "red");
1731-
return "";
1731+
// throw maps to non-zero exit; return "" would silently succeed.
1732+
throw(type = "Wheels.InvalidArguments", message = "No app/ directory found — run wheels validate from a Wheels project root.");
17321733
}
17331734

17341735
out("Validating...", "cyan");
17351736
out("");
17361737

1738+
var validationFailed = false;
1739+
var issueCount = 0;
1740+
17371741
try {
17381742
var analysis = getService("analysis");
17391743
var results = analysis.validate();
@@ -1749,8 +1753,19 @@ component extends="modules.BaseModule" {
17491753
var severity = issue.severity == "error" ? "red" : "yellow";
17501754
out(" [#uCase(issue.severity)#] #fileName# — #issue.message#", severity);
17511755
}
1756+
1757+
// Capture before try ends; throwing inside would be swallowed by the catch.
1758+
validationFailed = !results.valid;
1759+
issueCount = results.totalIssues;
17521760
} catch (any e) {
17531761
out("Validation failed: #e.message#", "red");
1762+
// rethrow maps to non-zero exit; an analyzer crash must not exit 0.
1763+
rethrow;
1764+
}
1765+
1766+
// Throw after the full report flushes — errors exit non-zero, warnings stay green.
1767+
if (validationFailed) {
1768+
throw(type = "Wheels.ValidationFailed", message = "Validation found #issueCount# issue(s) — see the report above.");
17541769
}
17551770

17561771
return "";
Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
// Covers all four validate() exit paths; each case gets its own temp project.
2+
component extends="wheels.wheelstest.system.BaseSpec" {
3+
4+
function beforeAll() {
5+
variables.roots = [];
6+
7+
// Missing extends="Model" → Analysis.validateModel error → results.valid = false.
8+
variables.errorRoot = $makeProject();
9+
fileWrite(variables.errorRoot & "/app/models/Bad.cfc", "component { function config() {} }");
10+
variables.errorMod = new cli.lucli.Module(cwd = variables.errorRoot);
11+
12+
// Project with no offending files at all.
13+
variables.cleanRoot = $makeProject();
14+
variables.cleanMod = new cli.lucli.Module(cwd = variables.cleanRoot);
15+
16+
// Hash without cfparam → validateView warning; results.valid stays true.
17+
variables.warningRoot = $makeProject();
18+
directoryCreate(variables.warningRoot & "/app/views/things", true, true);
19+
fileWrite(variables.warningRoot & "/app/views/things/index.cfm", "<p>##foo##</p>");
20+
variables.warningMod = new cli.lucli.Module(cwd = variables.warningRoot);
21+
22+
// Project root with no app/ directory — user-error path.
23+
variables.noAppRoot = $makeProject(includeApp = false);
24+
variables.noAppMod = new cli.lucli.Module(cwd = variables.noAppRoot);
25+
}
26+
27+
function afterAll() {
28+
for (var root in variables.roots) {
29+
if (len(root) > 10 && directoryExists(root)) {
30+
directoryDelete(root, true);
31+
}
32+
}
33+
}
34+
35+
// vendor/wheels stub anchors resolveProjectRoot to the temp dir.
36+
private string function $makeProject(boolean includeApp = true) {
37+
var root = getTempDirectory() & "wheels-cli-validate-" & createUUID();
38+
directoryCreate(root & "/vendor/wheels", true, true);
39+
if (arguments.includeApp) {
40+
directoryCreate(root & "/app/models", true, true);
41+
directoryCreate(root & "/app/controllers", true, true);
42+
directoryCreate(root & "/app/views", true, true);
43+
directoryCreate(root & "/config", true, true);
44+
}
45+
arrayAppend(variables.roots, root);
46+
return root;
47+
}
48+
49+
function run() {
50+
51+
describe("wheels validate exit codes", () => {
52+
53+
it("throws Wheels.ValidationFailed when validation finds errors", () => {
54+
expect(() => variables.errorMod.validate()).toThrow(type = "Wheels.ValidationFailed");
55+
});
56+
57+
it("returns normally on a clean project", () => {
58+
variables.cleanMod.validate();
59+
expect(true).toBeTrue();
60+
});
61+
62+
it("stays green when only warnings exist", () => {
63+
variables.warningMod.validate();
64+
expect(true).toBeTrue();
65+
});
66+
67+
it("throws Wheels.InvalidArguments when no app directory exists", () => {
68+
expect(() => variables.noAppMod.validate()).toThrow(type = "Wheels.InvalidArguments");
69+
});
70+
71+
});
72+
73+
}
74+
75+
}

web/sites/guides/src/content/docs/v4-0-0/command-line-tools/wheels-commands/code-quality.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ wheels validate
140140
wheels analyze
141141
```
142142

143-
Run `validate` first and read its output for any ✗ errors or ⚠ warnings. Then run `analyze` for a broader look at health scores and anti-patterns. Both commands always exit 0 regardless of findings, so read each report before moving on — the results are in the output, not the exit code.
143+
Run `validate` first and read its output for any ✗ errors or ⚠ warnings. Then run `analyze` for a broader look at health scores and anti-patterns. `wheels validate` exits non-zero when it finds error-severity issues, so you can gate CI on it. `wheels analyze` always exits 0 — its findings live in the output only.
144144

145145
### Scope the analysis
146146

0 commit comments

Comments
 (0)