Skip to content

Commit 6e8456c

Browse files
parthopdasCopilot
andcommitted
T14: CliApplication composition + fail-fast gate (exit 0/1/2)
Faithful 1:1 port of crap4java CliApplication/Main (execute/parseArguments/ filesForMode/explicitFiles/maxCrap/thresholdExceeded), module-loop removed (departure #7 resolve-once). Composes shipped types only; no new behavioral departure beyond the approved set #1-#8. - Fail-fast gate (departure #1), three greppable stderr anchors: 1) Locate -> null => "No coverage report was produced" => exit 1 2) Parse(report).Count == 0 => "contained no coverage data" => exit 1 (checks coverageMap.Count, not metrics) 3) CoverageRunner throw propagates ("Coverage command failed with exit") => realized as exit 1 at T15 - Exit-code table owned by T14: 0 (help / no files / max<=8.0), 1 (parse error / fail-fast / propagated), 2 (MaxCrap>8.0 strict -> "CRAP threshold exceeded: {max:F1} > 8.0", InvariantCulture). - Double-parse locked (option A); Analyze(map) overload deferred (Mr. Das). - Execute does NOT catch runner/parser throw (Java parity); T15 Program.Main bare-catch -> exit 1 realizes it (D-T14d). 17 parity tests (fake ICommandExecutor, no real dotnet test). 112 tests green x2, 0/0 Release. Bhaskar PASS, Anders green. T14 register + tracker updated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9041c2c commit 6e8456c

4 files changed

Lines changed: 710 additions & 3 deletions

File tree

docs/decisions.md

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,57 @@ with no Java counterpart (JaCoCo's report path was a fixed constant, so Java nee
242242
**throws** on command failure (faithful) and **returns `null`** when the report is absent (the new
243243
locate signal); every exit-code decision (departure #1) stays in T14.
244244

245+
## T14 register — `CliApplication` + fail-fast gate (S4 close-out; Anders T14 review, 🟢)
246+
247+
Resolved decisions and load-bearing invariants from the composition/orchestration layer.
248+
`CliApplication` is a faithful 1:1 port of `crap4java`'s `CliApplication`/`Main`
249+
(`execute`/`parseArguments`/`filesForMode`/`explicitFiles`/`maxCrap`/`thresholdExceeded`) with the
250+
`moduleRootFor` + `groupByModuleRoot`/`analyzeByModule` module-loop removed (departure #7). It composes
251+
already-shipped types only — no resolution/coverage/parse internals of its own — so the S5/T17
252+
resolution-model swap touches **only** `ModuleRootResolver` (D-T12b). **No new behavioral departure
253+
beyond the already-approved set #1#8.**
254+
255+
- **D-T14a — the exit-code table is the canonical CLI contract (T14 owns every exit).** `Execute`
256+
returns exactly: **0** (`--help`; or no files → `"No C# files to analyze."` on stdout; or max
257+
CRAP ≤ 8.0), **1** (parse error `ArgumentException``ex.Message` on stderr + usage on stdout; or a
258+
fail-fast trigger; or a propagated exception realized at T15), **2** (`MaxCrap(metrics) > 8.0`, strict
259+
`"CRAP threshold exceeded: {max:F1} > 8.0"` on stderr, `InvariantCulture`). This realizes
260+
departure #1 (exit 1 for fail-fast) and pins the exit-2 threshold path departure #1 did not
261+
enumerate. Do not move an exit decision out of T14.
262+
263+
- **D-T14b — three fail-fast triggers, greppable stderr anchors (departure #1).** All module/run-level;
264+
the per-method `N/A` path (a method absent from a *populated* report) stays in `CrapAnalyzer`,
265+
**unchanged**. (1) `CoverageReportLocator.Locate` returns `null` → anchor **`No coverage report was
266+
produced`** → exit 1. (2) `CoberturaCoverageParser.Parse(report).Count == 0` → anchor **`contained no
267+
coverage data`** → exit 1 — trigger 2 must inspect **`coverageMap.Count`, not the metrics** (a
268+
populated-but-non-matching report also yields all-`N/A` metrics yet must NOT fail-fast). (3)
269+
`CoverageRunner.GenerateCoverage` throws → **propagates** (anchor **`Coverage command failed with
270+
exit`**), realized as exit 1 at T15. Mr. Das ruled the trigger 1/2 wording to these defaults; tests
271+
assert only the bold anchor substring, so wording may re-tune without touching test structure.
272+
273+
- **D-T14c — double-parse LOCKED (option A); the `Analyze(map)` overload is DEFERRED.** T14 calls
274+
`CoberturaCoverageParser.Parse(report)` for the trigger-2 empty check, then
275+
`CrapAnalyzer.Analyze(filesToAnalyze, report)` re-parses the same small XML — negligible cost
276+
(dominated by the `dotnet test` run) and **zero API change** to a discharged T11 type
277+
(fidelity-first). Option B — add `CrapAnalyzer.Analyze(IReadOnlyList<string>,
278+
IReadOnlyDictionary<string, CoverageData>)` to share one parse — modifies a discharged API and is
279+
**Mr. Das's call**; deferred, not required for T14.
280+
281+
- **D-T14d — the T14/T15 propagation boundary (LOAD-BEARING for T15).** `Execute` does **not** catch
282+
the coverage-runner / parser throw (faithful to Java `execute … throws Exception`); it propagates.
283+
Java relied on the JVM exiting 1 on an uncaught exception, but **.NET does not guarantee exit 1 on an
284+
unhandled exception**, so T15's `Program.Main` **must** catch and convert to exit 1. Mr. Das ruled the
285+
mechanism = a **bare `catch`** (not a `CoverageException` wrapper; Java has none), **applied at T15**,
286+
not T14. `Program.cs` stays the current stub until T15.
287+
288+
- **Below the departure bar (no departure number, no ruling needed).** `MaxCrap`/`ThresholdExceeded`/
289+
`Usage` are housed on `CliApplication` (T14), not on `Program` (T15) as Java put `maxCrap`/`usage` on
290+
`Main` — avoids a composition→entry forward dependency and keeps T14 self-contained/testable; pure
291+
organization, behavior/tests unchanged. Java's pre-format `metrics.sort` is omitted because both
292+
`CrapAnalyzer` (T11) and `ReportFormatter` (T4) already apply the identical stable "scored desc, N/A
293+
last" sort and `MaxCrap` is order-free (behavior-preserving). The single-line trigger-1 message
294+
(avoids SA1118) is byte-identical to the fail-fast contract.
295+
245296
## Deliberate departures from crap4java (approved by Mr. Das)
246297

247298
1. **Fail fast** — when a module produces no coverage / runs no tests, exit non-zero (`1`) with a

docs/features/crap4csharp-port.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Feature: crap4csharp — faithful C# port of crap4java
22
**Branch:** vibe/crap4csharp-port
3-
**Status:** In progress — **S4 underway** (T6 `da02028`, T7 `1b90d24`, T8 `c023ef8`, T13 `e4d1329`, B1 ratify `d0784b3`, T9 `400d49d`, T10 `87488ce`, T11 `debf9de`, T12 next commit); **95 tests green, 0/0 Release**. Coverage pipeline (`CrapAnalyzer` + `CoverageRunner`/`CoverageReportLocator`) landed; next = T14 fail-fast gate → T15 `Program`/e2e T16 README finalize. **Pause at the S4 boundary** for Mr. Das.
3+
**Status:** In progress — **S4 underway** (T6 `da02028`, T7 `1b90d24`, T8 `c023ef8`, T13 `e4d1329`, B1 ratify `d0784b3`, T9 `400d49d`, T10 `87488ce`, T11 `debf9de`, T12 `9041c2c`, T14 next commit); **112 tests green, 0/0 Release**. CLI composition + fail-fast gate landed (departure #1 exit 0/1/2, resolve-once #7); next = T15 `Program`/e2e (bare-catch → exit 1) → T16 README finalize. **Pause at the S4 boundary** for Mr. Das — that closes the core port (S1–S4).
44

55
## Requirements
66

@@ -56,9 +56,9 @@ One or more tasks per slice. Full task detail and the fail-fast delta live in `d
5656
| T9 | S3 | `CSharpMethodParser` + `ComplexityWalker` (augmented node set) + CC oracle tests | Done | `400d49d` |
5757
| T10 | S3 | `CoberturaCoverageParser` (+ empty-report case) + tests; pin FQN normalization vs a real coverlet sample | Done | `87488ce` |
5858
| T11 | S4 | `CrapAnalyzer` (exact→nearest-line lookup, per-method `TypeName`) + tests | Done | `debf9de` |
59-
| T12 | S4 | `CoverageRunner` (`dotnet test --collect`) + `CoverageReportLocator` + tests | Done | `(next commit)` |
59+
| T12 | S4 | `CoverageRunner` (`dotnet test --collect`) + `CoverageReportLocator` + tests | Done | `9041c2c` |
6060
| T13 | S4 | `ModuleRootResolver` (nearest `.sln``.csproj` → root) + tests | Done | `e4d1329` |
61-
| T14 | S4 | `CliApplication` + tests; **fail-fast gate** (no-coverage/empty-report → exit 1) | Pending | - |
61+
| T14 | S4 | `CliApplication` + tests; **fail-fast gate** (no-coverage/empty-report → exit 1) | Done | `(next commit)` |
6262
| T15 | S4 | `Program` entry (+ `CoverageException`) + integration tests (spawn built exe) | Pending | - |
6363
| T16 | S4 | README usage section + end-to-end smoke (positive + negative fail-fast) | Pending | - |
6464
| T17 | S5 | Module & test resolution finalization — **resolution model + test-project resolution + multi-report coverage aggregation**: Anders presents options — (a) keep the locked `.sln`-first model / (b) adopt mutate4csharp's nearest-`.csproj` owning project + `<Project>.Tests`/`<Project>.UnitTests` test-project resolution / (c) hybrid — per `../mutate4csharp` README §"Module & Test Resolution"; Mr. Das rules; then implement the chosen model — rework `ModuleRootResolver` (T13) and shape T12/T14. Module *grouping* is **no longer in scope** (removed by the resolve-once departure #7), but multi-report coverage **aggregation** IS now in scope (per single-pick departure #8) — replace T12's ordinal-first single-pick `CoverageReportLocator` with a union across the per-test-project `coverage.cobertura.xml` reports so multi-test-project solutions stop under-reporting. Keep running ALL module tests (crap4java parity); do NOT adopt unit-only `[Trait]` filtering. | Pending (deferred) | - |

src/Crap4CSharp/CliApplication.cs

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
namespace Microsoft.Crap4CSharp;
2+
3+
using System.Globalization;
4+
5+
// Faithful port of crap4java's CliApplication, adapted per docs/decisions.md departures #1 (fail-fast)
6+
// and #7 (resolve-once). Orchestrates: parse args -> pick files -> resolve module root ONCE ->
7+
// run coverage ONCE -> locate ONCE -> fail-fast gate -> parse -> analyze -> format -> print -> threshold.
8+
// Composes shipped types only; contains no resolution/coverage/parse internals of its own (the S5/T17
9+
// model swap must touch only ModuleRootResolver -- the D-T12b seam).
10+
public sealed class CliApplication
11+
{
12+
// Adapts crap4java's Main.usage() to the C# ecosystem: "Java" -> "C#", ".java" -> ".cs", and the
13+
// executable name to the crap4csharp product. Only ParseArguments consumes it (help + parse-error paths).
14+
private const string Usage = """
15+
Usage:
16+
crap4csharp Analyze all C# files under src/
17+
crap4csharp --changed Analyze changed C# files under src/
18+
crap4csharp <path...> Analyze files, or for directory args analyze <dir>/src/**/*.cs
19+
crap4csharp --help Print this help message
20+
""";
21+
22+
private readonly string _projectRoot;
23+
private readonly TextWriter _output;
24+
private readonly TextWriter _error;
25+
private readonly CoverageRunner _coverageRunner;
26+
27+
public CliApplication(
28+
string projectRoot,
29+
TextWriter output,
30+
TextWriter error,
31+
CoverageRunner coverageRunner)
32+
{
33+
ArgumentNullException.ThrowIfNull(projectRoot);
34+
ArgumentNullException.ThrowIfNull(output);
35+
ArgumentNullException.ThrowIfNull(error);
36+
ArgumentNullException.ThrowIfNull(coverageRunner);
37+
_projectRoot = projectRoot;
38+
_output = output;
39+
_error = error;
40+
_coverageRunner = coverageRunner;
41+
}
42+
43+
// Ports execute(String[]). Owns every exit code (§4): 0 (help / no files / max CRAP <= 8.0),
44+
// 1 (parse error / no report / empty report), 2 (threshold exceeded). The coverage-runner throw and
45+
// any parser throw PROPAGATE (faithful to Java's `throws Exception`); T15's Program.Main converts them
46+
// to exit 1.
47+
public int Execute(string[] args)
48+
{
49+
ParseOutcome parse = ParseArguments(args);
50+
if (parse.ExitCode >= 0)
51+
{
52+
return parse.ExitCode;
53+
}
54+
55+
CliArguments parsed = parse.Arguments!;
56+
IReadOnlyList<string> filesToAnalyze = FilesForMode(parsed);
57+
if (filesToAnalyze.Count == 0)
58+
{
59+
_output.WriteLine("No C# files to analyze.");
60+
return 0;
61+
}
62+
63+
// Resolve the module root ONCE and run coverage ONCE at it (departure #7: no module-group loop).
64+
string moduleRoot = ModuleRootResolver.Resolve(_projectRoot);
65+
_coverageRunner.GenerateCoverage(moduleRoot);
66+
67+
// FAIL-FAST TRIGGER 1 (departure #1): no report produced at all.
68+
string? report = CoverageReportLocator.Locate(moduleRoot);
69+
if (report is null)
70+
{
71+
_error.WriteLine($"No coverage report was produced under '{moduleRoot}'. Ensure a test project references coverlet.collector so 'dotnet test --collect' emits coverage.cobertura.xml.");
72+
return 1;
73+
}
74+
75+
// FAIL-FAST TRIGGER 2 (departure #1): a report was produced but carries no coverage data. Inspect
76+
// the parsed map's emptiness -- NOT the metrics: a populated-but-non-matching report ALSO yields
77+
// all-N/A metrics yet must not fail-fast (that per-method N/A path lives in CrapAnalyzer, unchanged).
78+
IReadOnlyDictionary<string, CoverageData> coverageMap = CoberturaCoverageParser.Parse(report);
79+
if (coverageMap.Count == 0)
80+
{
81+
_error.WriteLine($"Coverage report '{report}' contained no coverage data. Failing fast.");
82+
return 1;
83+
}
84+
85+
// CrapAnalyzer re-parses `report` (locked double-parse, option A): negligible cost, zero API change.
86+
IReadOnlyList<MethodMetrics> metrics = CrapAnalyzer.Analyze(filesToAnalyze, report);
87+
88+
// The report already ends with '\n' and is already sorted by both CrapAnalyzer and ReportFormatter,
89+
// so the Java pre-format sort is redundant and omitted (behavior-preserving; MaxCrap is order-free).
90+
_output.Write(ReportFormatter.Format(metrics));
91+
92+
double max = MaxCrap(metrics);
93+
if (ThresholdExceeded(max))
94+
{
95+
_error.WriteLine(string.Format(
96+
CultureInfo.InvariantCulture,
97+
"CRAP threshold exceeded: {0:F1} > 8.0",
98+
max));
99+
return 2;
100+
}
101+
102+
return 0;
103+
}
104+
105+
// Ports thresholdExceeded: strict '>' against 8.0 via CompareTo (Java's Double.compare analog).
106+
public static bool ThresholdExceeded(double max) => max.CompareTo(8.0) > 0;
107+
108+
// Ports Main.maxCrap: the largest non-null CrapScore, or 0.0 when every method is N/A.
109+
public static double MaxCrap(IReadOnlyList<MethodMetrics> metrics)
110+
{
111+
ArgumentNullException.ThrowIfNull(metrics);
112+
113+
double max = 0.0;
114+
foreach (MethodMetrics metric in metrics)
115+
{
116+
if (metric.CrapScore is not null)
117+
{
118+
max = Math.Max(max, metric.CrapScore.Value);
119+
}
120+
}
121+
122+
return max;
123+
}
124+
125+
// Ports parseArguments: parse; on --help print usage to stdout and exit 0; on ArgumentException (W6:
126+
// the broad type, catching the ArgumentNullException null-args case too) print the message to stderr,
127+
// usage to stdout, and exit 1; otherwise proceed (ExitCode -1).
128+
private ParseOutcome ParseArguments(string[] args)
129+
{
130+
try
131+
{
132+
CliArguments parsed = CliArgumentsParser.Parse(args);
133+
if (parsed.Mode == CliMode.Help)
134+
{
135+
_output.WriteLine(Usage);
136+
return ParseOutcome.Exit(0);
137+
}
138+
139+
return ParseOutcome.Ok(parsed);
140+
}
141+
catch (ArgumentException ex)
142+
{
143+
_error.WriteLine(ex.Message);
144+
_output.WriteLine(Usage);
145+
return ParseOutcome.Exit(1);
146+
}
147+
}
148+
149+
// Ports filesForMode: a switch expression over all four CliMode arms plus a default throw (W7). The
150+
// Help arm is dead (ParseArguments short-circuits it), retained for exhaustiveness.
151+
private IReadOnlyList<string> FilesForMode(CliArguments parsed)
152+
{
153+
return parsed.Mode switch
154+
{
155+
CliMode.AllSrc => SourceFileFinder.FindAllCSharpFilesUnderSrc(_projectRoot),
156+
CliMode.ChangedSrc => ChangedFileDetector.ChangedCSharpFilesUnderSrc(_projectRoot),
157+
CliMode.ExplicitFiles => ExplicitFiles(parsed.FileArgs),
158+
CliMode.Help => [],
159+
_ => throw new InvalidOperationException($"Unhandled CLI mode: {parsed.Mode}"),
160+
};
161+
}
162+
163+
// Ports explicitFiles: resolve each arg against projectRoot; a directory arg expands via SourceFileFinder
164+
// (routing through the finder keeps the bin/obj exclusion, W12), a file arg is taken verbatim.
165+
// De-duplicate and sort with StringComparer.Ordinal (departure #3; replaces Java's LinkedHashSet +
166+
// naturalOrder), matching SourceFileFinder/ChangedFileDetector.
167+
private IReadOnlyList<string> ExplicitFiles(IReadOnlyList<string> fileArgs)
168+
{
169+
HashSet<string> files = new(StringComparer.Ordinal);
170+
foreach (string arg in fileArgs)
171+
{
172+
string path = Path.GetFullPath(Path.Combine(_projectRoot, arg));
173+
if (Directory.Exists(path))
174+
{
175+
foreach (string found in SourceFileFinder.FindAllCSharpFilesUnderSrc(path))
176+
{
177+
files.Add(found);
178+
}
179+
}
180+
else
181+
{
182+
files.Add(path);
183+
}
184+
}
185+
186+
return [.. files.OrderBy(path => path, StringComparer.Ordinal)];
187+
}
188+
189+
// Ports Java's ParseOutcome: Ok(arguments) proceeds (ExitCode -1); Exit(code) short-circuits with an
190+
// exit code. Arguments is non-null exactly when ExitCode < 0.
191+
private sealed class ParseOutcome
192+
{
193+
private ParseOutcome(CliArguments? arguments, int exitCode)
194+
{
195+
Arguments = arguments;
196+
ExitCode = exitCode;
197+
}
198+
199+
public CliArguments? Arguments { get; }
200+
201+
public int ExitCode { get; }
202+
203+
public static ParseOutcome Ok(CliArguments arguments) => new(arguments, -1);
204+
205+
public static ParseOutcome Exit(int code) => new(null, code);
206+
}
207+
}

0 commit comments

Comments
 (0)