fix(fmt): forge fmt's CLI path bypasses its own idempotency/re-parse guard

[gomesalexandre]

forge fmt's CLI entry point silently bypasses the formatter's own idempotency/re-parse correctness guard - a mid-line comment-corruption bug (see the linked PR) would have been caught immediately if the guard were wired to the path that actually writes to disk.

The two paths

crates/fmt/src/lib.rs has four public entry points. Three of them (format_file, format_source, format) route through format_inner, which formats twice and asserts:

// Check if the two passes produce the same output (idempotency)
match (first_result.ok_ref(), second_result.ok_ref()) {
    (Some(first), Some(second)) if first != second => {
        panic!("formatter is not idempotent:\n{}", diff(first, second));
    }
    _ => {}
}

assert!(
    !(first_result.is_ok() && second_result.is_err() && !DEBUG),
    "failed to format a second time:\n..."
);

The fourth entry point, format_ast, has neither check - single pass, no idempotency comparison, no re-parse validation. That is exactly what the CLI calls:

crates/forge/src/cmd/fmt.rs:247:
    let formatted = forge_fmt::format_ast(gcx, source_unit, source_fmt_config)?;

So the safety net covers the library and test APIs and skips the one path that actually writes formatted output back to a user's files.

Why this matters

The second assertion above is precisely the signature of a real corruption bug: first pass succeeds, second pass fails to re-parse. If forge fmt went through the guarded path, that class of bug would surface loudly the first time anyone hit it, instead of silently rewriting their source with no error. Confirmed this isn't a debug-only guard or recent drift: const DEBUG: bool = false || option_env!("FMT_DEBUG").is_some(), so !DEBUG is true in any normal release build, and the wrapper's logic is materially identical between the current 1.7.1 release and master (an explicit if { panic! } vs. an assert!, same semantics).

It's also why the crate's own testdata harness (crates/fmt/tests/formatter.rs, which asserts idempotency per test case) never caught the concrete for-loop bug: the harness does use the guarded path, but only over the fixed testdata corpus, and no corpus file happened to have a comment in a for header. Guarded path with the wrong inputs; the actual failing input only reaches the unguarded path.

Why this is an issue rather than a PR

The obvious fix - repoint the CLI at format_inner - is wrong as stated, because the guard's failure mode is panic!/assert!. Converting silent corruption into a crash-with-backtrace on a user's own file isn't a fix, it's a different bad outcome (and hits the same "This is a bug, report it at github.com/foundry-rs/foundry" banner problem as other panic-on-input bugs). The right shape reuses the check's logic, not its panicking delivery: re-parse the CLI's own output and refuse to write, with a clean diagnostic, when it doesn't parse.

That still leaves real product decisions we're not positioned to make unilaterally:

• Parseability only, or full idempotency? Idempotency is the stronger guarantee (and the one the existing guard actually asserts), but likelier to trip on inputs that are merely ugly rather than broken.
• Performance. Any version of this formats every file twice. On a repo-wide forge fmt that's a real, user-visible cost - possibly worth gating behind a flag, or only enabling in --check mode.
• Failure semantics. Skip the offending file and continue, or fail the whole run? What should the diagnostic say?

Related

The concrete bug this guard would have caught: #16648 (comment corruption in for-loop headers - forge fmt rewrites a trailing comment in a way that swallows the loop condition/increment into the comment, and the result can't be re-parsed by a second forge fmt pass).

Happy to open a PR once the three questions above have a preferred answer.