[Forge FMT]: Should give an option to place each if condition in a separate line

[GalloDaSballo]

Component

Forge

Describe the feature you would like

I'm researching the literature on coverage being a predictor of bugs.

While coverage is not the full story, there's strong correlation between having high coverage and finding more bugs (although the correlation is moderate when adjusting for size).

Either way, we can all agree that professionally we rely on coverage as a indicator of attention and effort put in the testing suite.

Branches, (called Decisions in Condition / Decision Coverage), are a key part of coverage as each branch implies some conditional action.

Branches choice are determined by Conditions (the boolean variables evaluated in the branch).

Forge fmt, currently inlines all conditions in a branch instead of separating them.

forge coverage summary correctly counts statements covered and will address conditions that haven't been covered.

However, generating the lcov report will ignore statement coverage and will lead most developers to miss this nuance.

Ask: Allow option to put each condition in a separate line instead of defaulting to inlining conditions

[fmt]
format_conditions = "inline" // "inline" | "multi"

I would also suggest defaulting to multi, however if this could lead to changes for other projects I wouldn't push for this.

POC

Toy example of conditions that can never be hit, which are ignored by the coverage metric since the if condition is in one line.

Code with realistic 70% coverage

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

contract Counter {
    uint256 public number = 1;

    function setNumber(uint256 newNumber) public {
        if (
            newNumber % 2 == 0 || 
            newNumber % 2 == 1 || 
            newNumber != 0 || 
            newNumber != 1 || 
            newNumber != 2
        ) {
            number = newNumber;
        }
    }

    function increment() public {
        number++;
    }
}

Code with deceptive 100% coverage

Running forge fmt leads to this, which causes lcov to report 100% coverage.

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

contract Counter {
    uint256 public number = 1;

    function setNumber(uint256 newNumber) public {
        if (newNumber % 2 == 0 || newNumber % 2 == 1 || newNumber != 0 || newNumber != 1 || newNumber != 2) {
            number = newNumber;
        }
    }

    function increment() public {
        number++;
    }
}

Lcov for the deceptive snippet

Lcov for multi line snippet

foundry coverage

 forge coverage
Warning: optimizer settings and `viaIR` have been disabled for accurate coverage reports.
If you encounter "stack too deep" errors, consider using `--ir-minimum` which enables `viaIR` with minimum optimization resolving most of the errors
[⠊] Compiling...
[⠔] Compiling 45 files with Solc 0.8.30
[⠒] Solc 0.8.30 finished in 480.34ms
Compiler run successful!
Analysing contracts...
Running tests...

Ran 2 tests for test/recon/CryticToFoundry.sol:CryticToFoundry
[PASS] invariant_number_never_zero() (runs: 0, calls: 0, reverts: 0)
[PASS] test_crytic() (gas: 211)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 4.62ms (1.76ms CPU time)

Ran 2 tests for test/Counter.t.sol:CounterTest
[PASS] testFuzz_SetNumber(uint256) (runs: 256, μ: 15342, ~: 15736)
[PASS] test_Increment() (gas: 14751)
Suite result: ok. 2 passed; 0 failed; 0 skipped; finished in 6.67ms (5.32ms CPU time)

Ran 2 test suites in 100.76ms (11.30ms CPU time): 4 tests passed, 0 failed, 0 skipped (4 total tests)

╭----------------------------------------+----------------+----------------+---------------+---------------╮
| File                                   | % Lines        | % Statements   | % Branches    | % Funcs       |
+==========================================================================================================+
| script/Counter.s.sol                   | 0.00% (0/3)    | 0.00% (0/1)    | 100.00% (0/0) | 0.00% (0/2)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| src/Counter.sol                        | 100.00% (5/5)  | 61.54% (8/13)  | 100.00% (1/1) | 100.00% (2/2) |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/BeforeAfter.sol             | 0.00% (0/7)    | 0.00% (0/4)    | 100.00% (0/0) | 0.00% (0/3)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/CryticTester.sol            | 0.00% (0/2)    | 0.00% (0/1)    | 100.00% (0/0) | 0.00% (0/1)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/Setup.sol                   | 63.64% (7/11)  | 77.78% (7/9)   | 100.00% (0/0) | 33.33% (1/3)  |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/TargetFunctions.sol         | 0.00% (0/14)   | 0.00% (0/12)   | 0.00% (0/4)   | 0.00% (0/3)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/targets/AdminTargets.sol    | 0.00% (0/2)    | 0.00% (0/1)    | 100.00% (0/0) | 0.00% (0/1)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/targets/DoomsdayTargets.sol | 0.00% (0/6)    | 0.00% (0/3)    | 0.00% (0/2)   | 0.00% (0/2)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| test/recon/targets/ManagersTargets.sol | 0.00% (0/11)   | 0.00% (0/7)    | 100.00% (0/0) | 0.00% (0/5)   |
|----------------------------------------+----------------+----------------+---------------+---------------|
| Total                                  | 19.67% (12/61) | 29.41% (15/51) | 14.29% (1/7)  | 13.64% (3/22) |
╰----------------------------------------+----------------+----------------+---------------+---------------╯

Additional context

No response