refactor(diag): unify warning prefixes onto the warn() severity ladder #1199

Status: fixed on fix/1199-unify-warning-prefixes (315efcbb),
PR pending. All nine plan steps done. Two corrections to the analysis
below, recorded here so the body is not read as current:

• The library has five capitalised sites, not three. The sweep
  also found src/output/code_climate.rs:94 (empty repo-relative
  path) and src/concurrent_files.rs:227 (not a regular file).
• Step 3 is answered by a library-side src/diag.rs carrying a
  single pub(crate) fn warn, the counterpart to the CLI ladder,
  in addition to bringing CSV onto warn_non_utf8_path. The helper
  alone covers only the non-UTF-8-path warnings.

Steps 1-2 are byte-for-byte a no-op as predicted. The one deliberate
output change is the IncludeCycle trailing newline, dropped per
step 5. See the closing comment for the evidence.

Problem

Diagnostic severity prefixes are produced in three different places
with two different conventions, and the library has no access to the
CLI helper that defines the house style.

big-code-analysis-cli/src/diag.rs:24 is the intended shape — the
three-severity ladder added in #609:

pub(crate) fn warn(msg: impl Display) {
    eprintln!("warning: {msg}");
}

Two library surfaces bypass it and bake a capitalised prefix into the
message instead:

• src/output/csv.rs:194 and :229 — "Warning: skipping non-UTF-8 source path in CSV output: {}"
• src/output/offenders.rs:35 — "Warning: skipping non-UTF-8 path in {format} output: {}"

A third, PreprocDiagnostic's Display impl (src/preproc.rs:76-108),
also self-prefixes. Its five variants were internally inconsistent —
three capitalised, two not — which #1198 fixed by lowercasing all five.
That was chosen deliberately as a stepping stone to this issue rather
than as the end state (see below).

bca preproc surfaces the diagnostics with a bare eprintln!, not
through warn():

// big-code-analysis-cli/src/commands/preproc.rs:82-84
for diagnostic in fix_includes(&mut data.files, &all_files) {
    eprintln!("{diagnostic}");
}

Proposed change

Move the severity prefix out of the message producers and onto the
presentation layer, so warn() is the single place warning: is
written.

1. Drop the warning: prefix from all five PreprocDiagnostic
   variants, leaving Display as the bare message.
2. Route commands/preproc.rs:83 through diag::warn.
3. Decide the same question for output/csv.rs and
   output/offenders.rs. These live in the library and cannot call the
   CLI's pub(crate) helper, so they need either their own convention
   or a library-side equivalent — worth settling explicitly rather than
   leaving two capitalised stragglers.

Why step 1 and 2 are a no-op on output

warn(msg) expands to eprintln!("warning: {msg}"). Because #1198
lowercased the prefix first, eprintln!("{diagnostic}") on a
Display that yields warning: possible self inclusion foo.h and
warn("possible self inclusion foo.h") emit the same bytes. So steps
1-2 should be verifiable as a pure refactor with no user-visible
diff — which is the whole reason the flip went downward.

preproc_diagnostic_display_renders_each_single_line_variant and
preproc_diagnostic_display_lists_every_cycle_member
(src/preproc_tests.rs) pin the current text and are the check that it
stayed a no-op; both will need their expectations moved to the CLI
layer as part of the change.

Wrinkles

• IncludeCycle is multi-line. warn() prefixes only the first
  line, which happens to be the shape wanted here (header prefixed,
  member lines indented under it) — but confirm rather than assume.
• IncludeCycle ends with a trailing newline and eprintln! adds
  another, so each cycle block currently prints followed by a blank
  line. Pre-existing and now pinned by the test above. This is the
  natural place to decide whether that blank line is wanted.

Not a breaking change

STABILITY.md:158 — "The Display impls are stable; the exact wording
of Display output is not." PreprocDiagnostic is a public export
(src/lib.rs:340), so this clause is what clears the change.

Resolution Plan

Confirmed against main (39674df6). All five PreprocDiagnostic
variants are lowercase (src/preproc.rs:80, :83, :95, :100,
:104) per #1198; the two capitalised stragglers are live at
src/output/csv.rs:194, :229 and src/output/offenders.rs:35; and
big-code-analysis-cli/src/commands/preproc.rs:83 still prints with a
bare eprintln!.

Step 3 already has an answer in the tree

The issue leaves the library-side convention open. It is largely settled
already: offenders.rs:30 defines warn_non_utf8_path(format, path),
and five output formats route through it — SARIF (sarif.rs:230),
Code Climate (code_climate.rs:89), Checkstyle (checkstyle.rs:52) and
both warning-line flavours (warning_line.rs:58, :91). CSV is the only
format that does not; it open-codes the same warning twice with a
different wording. So the library does not need a new convention, it
needs CSV brought onto the existing one.

That also settles the more principled alternative — propagating the
diagnostic to the caller so the CLI can route it through warn().
write_csv and write_csv_aggregate (src/output/csv.rs:185, :217)
are pub, so changing their signatures is a SemVer break and cannot
land in the 2.x line (STABILITY.md, AGENTS.md). Not worth deferring the
consistency fix to a major bump for; note it as the 3.0 option and move on.

1. Drop the warning: prefix from all five PreprocDiagnostic
   variants, leaving Display as the bare message.
2. Route commands/preproc.rs:83 through diag::warn.
3. Move the two PreprocDiagnostic Display tests
   (preproc_diagnostic_display_renders_each_single_line_variant,
   preproc_diagnostic_display_lists_every_cycle_member, in
   src/preproc_tests.rs) to assert the bare message, and add a CLI-side
   test asserting the rendered stderr still reads warning: …. Both
   halves are needed: the library test alone stops guarding the prefix,
   and the CLI test alone stops guarding the message.
4. Prove steps 1-3 are a no-op on output. warn(msg) expands to
   eprintln!("warning: {msg}"), so post-fix: batch of fifteen issues (#1180-#1182, #1184-#1192, #1194-#1196) #1198 the bytes should be
   identical. Capture bca preproc stderr on a fixture producing each of
   the five variants before and after and diff them. This is the whole
   justification for fix: batch of fifteen issues (#1180-#1182, #1184-#1192, #1194-#1196) #1198 having flipped downward first; if the diff is
   non-empty, something in the wrinkles below is wrong.
5. Settle the two IncludeCycle wrinkles the issue flags, rather than
   inheriting them. warn() prefixes only the first line, which is the
   shape wanted (header prefixed, members indented under it) — confirm on
   real output. The trailing newline plus eprintln!'s own produces a
   blank line after each cycle block; that is pre-existing and now pinned
   by the test above, so this is the moment to decide whether it stays.
   Recommend dropping it: it is an artifact of writeln! in a Display
   impl, not a deliberate separator, and Display impls should not end in
   a newline. If it stays, say why in a comment so the next reader does
   not "fix" it.
6. Bring CSV onto warn_non_utf8_path. Replace both csv.rs sites
   with warn_non_utf8_path("CSV", source_path), which lowercases the
   prefix and removes the duplicated wording as a side effect. Check the
   wording change is acceptable: the shared helper says "skipping
   non-UTF-8 path in CSV output", the current CSV text says "non-UTF-8
   source path". If that distinction matters, widen the helper rather
   than keeping a second copy.
7. Lowercase the remaining prefix in warn_non_utf8_path itself
   (offenders.rs:35) — after step 6 this is the library's single site,
   and it fixes all six formats at once.
8. Add a guard so a seventh site cannot reappear. The failure mode
   here is that a new eprintln!("Warning: …") reads as correct in
   review. A rg-based check in utils/ wired into make lint
   (asserting no Warning: / Error: capitalised literal outside test
   modules) is a few lines and follows the existing gate conventions,
   including a *-test.py self-test. Without it this issue is a cleanup
   that will need doing again.
9. Changelog. ## [Unreleased] → Changed, noting the CSV
   non-UTF-8 warning's wording and casing changed. Not breaking:
   STABILITY.md:158 — "The Display impls are stable; the exact
   wording of Display output is not" — covers PreprocDiagnostic,
   which is a public export (src/lib.rs:340).

Assessment

Difficulty — Low. Five string edits, one call-site swap, two CSV
sites redirected to an existing helper. The optional lint gate in step 8
is the only piece with any substance, and it is a few lines of rg.

Complexity — Low. Two crates, but the seam between them is one
function call. No public-API change, no metric computation, no snapshots.
The one thing to get right is that the test expectations move across the
crate boundary rather than being deleted on one side.

Priority — Low. Purely presentational consistency in diagnostic
output. #1198 already removed the user-visible inconsistency within
PreprocDiagnostic; what remains is two capitalised stragglers and a
structural tidy-up so the prefix is written in one place per crate. No
correctness impact and no workaround needed.

Labelled low-priority.