Skip to content

Commit e3ff025

Browse files
sfwclaude
andcommitted
Sprint 43: CLI Error Consistency + Import Validation — check() now loads imports (catches missing modules), build() emits JSON errors for lex/module/type failures, 4 new CLI regression tests (missing_import fixture), 18 CLI tests total
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent c1e70e5 commit e3ff025

4 files changed

Lines changed: 350 additions & 1 deletion

File tree

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
# Sprint 43: CLI Error Consistency + Import Validation
2+
3+
**Goal:** Close remaining CLI correctness gaps so `run`, `check`, and `build` have consistent import handling and JSON error behavior.
4+
**Estimated Effort:** 0.5-1 day
5+
6+
---
7+
8+
## Context
9+
10+
Post-Sprint-42 verification still shows two launch-relevant issues:
11+
12+
1. `check --error-format json` reports success for missing imports.
13+
2. `build --error-format json` can fail silently (non-zero exit, empty JSON output) on early errors.
14+
15+
There is also process confusion around workspace/commit state, so this sprint includes explicit hygiene checks in CI/local verification.
16+
17+
---
18+
19+
## Agent Instructions
20+
21+
1. Treat this as a correctness sprint, not a refactor sprint.
22+
2. Keep behavior aligned across `run`, `check`, and `build` for module loading and JSON error output.
23+
3. Add regression tests first-class (must fail before fix, pass after fix).
24+
4. Do not weaken or skip tests to get green.
25+
26+
---
27+
28+
## Task 43.1: Make `check` Validate Imports
29+
30+
**Priority:** P0
31+
32+
**Files (likely):**
33+
- `src/main.rs`
34+
- `src/module/loader.rs` (only if needed)
35+
- `tests/cli_tests.rs`
36+
- fixtures under `tests/fixtures/`
37+
38+
### Requirements
39+
40+
1. In `check(...)`, load imports the same way `run(...)` and `build(...)` do.
41+
2. Missing imports must produce an error (not success) in both human and JSON modes.
42+
3. Preserve existing `--partial` behavior, but include module errors in the structured response.
43+
44+
### Acceptance Criteria
45+
46+
1. `forma check --error-format json <file-with-missing-import>` returns non-zero and JSON with `success: false` and at least one `MODULE` error.
47+
2. `forma check <file-with-missing-import>` prints human-readable module error and exits non-zero.
48+
3. Existing successful-check flows still pass.
49+
50+
---
51+
52+
## Task 43.2: Ensure `build` Always Emits JSON Errors on Failure
53+
54+
**Priority:** P0
55+
56+
**Files (likely):**
57+
- `src/main.rs`
58+
- `tests/cli_tests.rs`
59+
60+
### Requirements
61+
62+
1. For all early `build(...)` failure paths (lex/parse/module/type/codegen/link), if `--error-format json` is set, emit a JSON error payload before returning.
63+
2. Eliminate silent JSON failures (non-zero with empty stdout/stderr JSON body).
64+
3. Keep current human-format output unchanged.
65+
66+
### Acceptance Criteria
67+
68+
1. `forma build --error-format json <file-with-missing-import>` returns non-zero and prints structured JSON error output.
69+
2. At least one regression test asserts that build failure in JSON mode is never silent.
70+
3. No regression in successful build JSON output.
71+
72+
---
73+
74+
## Task 43.3: CLI Parity Regression Tests
75+
76+
**Priority:** P1
77+
78+
**Files (likely):**
79+
- `tests/cli_tests.rs`
80+
- `tests/fixtures/missing_import.forma`
81+
82+
### Requirements
83+
84+
1. Add tests covering missing-import behavior for:
85+
- `run --error-format json`
86+
- `check --error-format json`
87+
- `build --error-format json`
88+
2. Assert parity of key fields (`success`, `code`, non-empty `errors`, exit status).
89+
90+
### Acceptance Criteria
91+
92+
1. All three commands fail consistently on missing import with structured JSON errors.
93+
2. New tests are deterministic and pass in CI.
94+
95+
---
96+
97+
## Task 43.4: Workspace Hygiene Guardrail
98+
99+
**Priority:** P1
100+
101+
**Files (likely):**
102+
- `.github/workflows/ci.yml` (optional)
103+
- docs/PR checklist (optional)
104+
105+
### Requirements
106+
107+
1. Add a lightweight verification step/checklist item in sprint output requiring:
108+
- `git status --short` capture before final summary
109+
- explicit list of intentionally untracked files (if any)
110+
2. Ensure sprint completion summaries clearly distinguish:
111+
- committed changes
112+
- uncommitted workspace edits
113+
114+
### Acceptance Criteria
115+
116+
1. Final verification output includes commit hash and clean/dirty status.
117+
2. No ambiguity about whether fixes are committed.
118+
119+
---
120+
121+
## Verification Checklist
122+
123+
Run and include results in PR summary:
124+
125+
1. `cargo test --all`
126+
2. `cargo clippy --all-targets -- -D warnings`
127+
3. `cargo clippy --all-features --all-targets -- -D warnings`
128+
4. `cargo fmt --all -- --check`
129+
5. `cargo build --release`
130+
6. Targeted CLI repros:
131+
- `forma check --error-format json tests/fixtures/missing_import.forma`
132+
- `forma build --error-format json tests/fixtures/missing_import.forma`
133+
- `forma run --error-format json tests/fixtures/missing_import.forma`
134+
7. `git status --short` and `git log --oneline -n 1`
135+
136+
---
137+
138+
## Out of Scope
139+
140+
1. New module system features.
141+
2. Capability model changes.
142+
3. Broad CLI redesign.
143+
144+
---
145+
146+
## Definition of Done
147+
148+
1. `check` no longer reports false success on missing imports.
149+
2. `build --error-format json` never fails silently.
150+
3. CLI parity tests cover and lock this behavior.
151+
4. Sprint summary includes explicit commit/worktree state.
152+
153+
---
154+
155+
## Coding Agent Prompt
156+
157+
```text
158+
Implement Sprint 43: CLI Error Consistency + Import Validation.
159+
160+
Fix these exact defects:
161+
1) `forma check --error-format json` currently returns success on missing imports.
162+
2) `forma build --error-format json` can fail silently (non-zero exit with no JSON error output) on early failures.
163+
164+
Requirements:
165+
- Make `check` load/validate imports consistently with `run` and `build`.
166+
- Ensure `build` emits JSON-formatted errors for all failure paths when `--error-format json` is set.
167+
- Add regression tests in tests/cli_tests.rs using a missing-import fixture.
168+
- Keep human-readable output behavior intact.
169+
170+
Validation to run:
171+
- cargo test --all
172+
- cargo clippy --all-targets -- -D warnings
173+
- cargo clippy --all-features --all-targets -- -D warnings
174+
- cargo fmt --all -- --check
175+
- cargo build --release
176+
- forma check/build/run --error-format json on missing-import fixture
177+
- git status --short
178+
- git log --oneline -n 1
179+
180+
Deliverables:
181+
- code changes
182+
- regression tests
183+
- concise summary with file list, command results, and commit hash
184+
```

src/main.rs

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -845,6 +845,43 @@ fn check(file: &PathBuf, partial: bool, error_format: ErrorFormat) -> Result<(),
845845
}
846846
};
847847

848+
// Load imports (module system)
849+
let mut module_loader = ModuleLoader::from_source_file(file);
850+
let ast = match module_loader.load_imports(&ast) {
851+
Ok(imported_items) => {
852+
let mut combined_items = imported_items;
853+
combined_items.extend(ast.items);
854+
forma::parser::SourceFile {
855+
items: combined_items,
856+
span: ast.span,
857+
}
858+
}
859+
Err(e) => {
860+
let error_span = e.span.unwrap_or(forma::lexer::Span {
861+
start: 0,
862+
end: 0,
863+
line: 1,
864+
column: 1,
865+
});
866+
match error_format {
867+
ErrorFormat::Human => {
868+
ctx.error(error_span, &format!("module error: {}", e));
869+
}
870+
ErrorFormat::Json => {
871+
json_errors.push(span_to_json_error(
872+
&filename,
873+
error_span,
874+
"MODULE",
875+
&format!("{}", e),
876+
None,
877+
));
878+
output_json_errors(json_errors, None);
879+
}
880+
}
881+
return Err(format!("module error: {}", e));
882+
}
883+
};
884+
848885
let mut error_count = 0;
849886

850887
// Type check
@@ -1256,6 +1293,9 @@ fn build(
12561293
)),
12571294
}
12581295
}
1296+
if matches!(error_format, ErrorFormat::Json) {
1297+
output_json_errors(json_errors, None);
1298+
}
12591299
return Err("Lexer errors".into());
12601300
}
12611301

@@ -1294,7 +1334,30 @@ fn build(
12941334
span: parsed_ast.span,
12951335
}
12961336
}
1297-
Err(e) => return Err(format!("Module error: {}", e)),
1337+
Err(e) => {
1338+
let error_span = e.span.unwrap_or(forma::lexer::Span {
1339+
start: 0,
1340+
end: 0,
1341+
line: 1,
1342+
column: 1,
1343+
});
1344+
match error_format {
1345+
ErrorFormat::Human => {
1346+
ctx.error(error_span, &format!("module error: {}", e));
1347+
}
1348+
ErrorFormat::Json => {
1349+
json_errors.push(span_to_json_error(
1350+
&filename,
1351+
error_span,
1352+
"MODULE",
1353+
&format!("{}", e),
1354+
None,
1355+
));
1356+
output_json_errors(json_errors, None);
1357+
}
1358+
}
1359+
return Err(format!("Module error: {}", e));
1360+
}
12981361
};
12991362

13001363
// Type check
@@ -1312,6 +1375,9 @@ fn build(
13121375
)),
13131376
}
13141377
}
1378+
if matches!(error_format, ErrorFormat::Json) {
1379+
output_json_errors(json_errors, None);
1380+
}
13151381
return Err("Type errors".into());
13161382
}
13171383

tests/cli_tests.rs

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,3 +240,98 @@ fn test_cli_run_contract_violation() {
240240
"forma run contract_fail.forma should exit nonzero (contract violation)"
241241
);
242242
}
243+
244+
#[test]
245+
fn test_cli_check_missing_import() {
246+
let output = Command::new(forma_bin())
247+
.args(["check"])
248+
.arg(fixture("missing_import.forma"))
249+
.output()
250+
.expect("failed to execute forma");
251+
assert!(
252+
!output.status.success(),
253+
"forma check missing_import.forma should exit nonzero"
254+
);
255+
let stderr = String::from_utf8_lossy(&output.stderr);
256+
let stdout = String::from_utf8_lossy(&output.stdout);
257+
let combined = format!("{}{}", stdout, stderr);
258+
assert!(
259+
combined.contains("module") || combined.contains("Module"),
260+
"error should mention module, got stdout: {}, stderr: {}",
261+
stdout,
262+
stderr
263+
);
264+
}
265+
266+
#[test]
267+
fn test_cli_check_missing_import_json() {
268+
let output = Command::new(forma_bin())
269+
.args(["--error-format", "json", "check"])
270+
.arg(fixture("missing_import.forma"))
271+
.output()
272+
.expect("failed to execute forma");
273+
assert!(
274+
!output.status.success(),
275+
"forma --error-format json check missing_import.forma should exit nonzero"
276+
);
277+
let stdout = String::from_utf8_lossy(&output.stdout);
278+
assert!(
279+
stdout.contains("\"MODULE\""),
280+
"JSON output should contain MODULE error category, got: {}",
281+
stdout
282+
);
283+
assert!(
284+
stdout.contains("\"errors\""),
285+
"JSON output should contain errors key, got: {}",
286+
stdout
287+
);
288+
}
289+
290+
#[test]
291+
fn test_cli_build_missing_import_json() {
292+
// Module error happens before LLVM codegen, so this test works regardless of llvm feature
293+
let output = Command::new(forma_bin())
294+
.args(["--error-format", "json", "build"])
295+
.arg(fixture("missing_import.forma"))
296+
.output()
297+
.expect("failed to execute forma");
298+
assert!(
299+
!output.status.success(),
300+
"forma --error-format json build missing_import.forma should exit nonzero"
301+
);
302+
let stdout = String::from_utf8_lossy(&output.stdout);
303+
assert!(
304+
stdout.contains("\"MODULE\""),
305+
"JSON output should contain MODULE error category, got: {}",
306+
stdout
307+
);
308+
assert!(
309+
stdout.contains("\"errors\""),
310+
"JSON output should contain errors key, got: {}",
311+
stdout
312+
);
313+
}
314+
315+
#[test]
316+
fn test_cli_run_missing_import_json() {
317+
let output = Command::new(forma_bin())
318+
.args(["--error-format", "json", "run", "--allow-all"])
319+
.arg(fixture("missing_import.forma"))
320+
.output()
321+
.expect("failed to execute forma");
322+
assert!(
323+
!output.status.success(),
324+
"forma --error-format json run --allow-all missing_import.forma should exit nonzero"
325+
);
326+
let stdout = String::from_utf8_lossy(&output.stdout);
327+
assert!(
328+
stdout.contains("\"MODULE\""),
329+
"JSON output should contain MODULE error category, got: {}",
330+
stdout
331+
);
332+
assert!(
333+
stdout.contains("\"errors\""),
334+
"JSON output should contain errors key, got: {}",
335+
stdout
336+
);
337+
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
us nonexistent_module
2+
3+
f main()
4+
print("hello")

0 commit comments

Comments
 (0)