Skip to content

Feat/format conditions multi line #12636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Feat/format conditions multi line #12636

wants to merge 3 commits into from
+65 −4

Conversation

@Syzygy106
Copy link
Contributor

@Syzygy106 Syzygy106 commented Nov 22, 2025

Motivation

Closes #12508

When conditions in if statements are formatted inline (all on one line), coverage analysis tools like lcov cannot properly distinguish which individual conditions within the chain are actually covered by tests. This happens because the tool sees only one line with the condition, making it difficult to determine branch coverage for each condition.

For example:

if (newNumber % 2 == 0 || newNumber % 2 == 1 || newNumber != 0 || newNumber != 1 || newNumber != 2) {
    number = newNumber;
}

With inline formatting, if a test only covers the first condition (e.g., newNumber % 2 == 0), lcov may show the entire line as covered, but it cannot indicate which specific conditions were actually tested. This makes it difficult to identify untested conditions and achieve accurate branch coverage analysis.

Solution

Added a new format_conditions configuration option in foundry.toml that allows placing each condition on a separate line:

[fmt]
format_conditions = "multi"  # or "inline" (default)

When format_conditions = "multi", logical operators (||, &&) in if conditions are formatted with each condition on a separate line:

if (
    newNumber % 2 == 0
        ||
    newNumber % 2 == 1
        ||
    newNumber != 0
        ||
    newNumber != 1
        ||
    newNumber != 2
) {
    number = newNumber;
}

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@Syzygy106
Copy link
Contributor Author

Syzygy106 commented Nov 22, 2025 

======================================================================

1. With format_conditions = 'inline' (default):

// 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++;
    }
}


======================================================================

2. With format_conditions = 'multi':

// 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++;
    }
}

grandizzy
grandizzy requested changes Nov 25, 2025
View reviewed changes
Copy link
Collaborator

@grandizzy grandizzy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you, makes sense IMO (pending @0xrusowsky ). Please update the readme too to reflect the new config https://github.com/foundry-rs/foundry/tree/master/crates/fmt#configuration and add an ui_test as in https://github.com/foundry-rs/foundry/tree/master/crates/fmt#testing

@0xrusowsky
Copy link
Contributor

0xrusowsky commented Nov 25, 2025

the impl LGTM, but imo it would be nice to make the output prettier.

if possible, let's aim for:

if (
    newNumber % 2 == 0
        || newNumber % 2 == 1
        || newNumber != 0
        || newNumber != 1
        || newNumber != 2
) {

i think you should be able to achieve if with an extra box when the multiline config is enabled

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@grandizzy grandizzy grandizzy requested changes

@DaniPopes DaniPopes Awaiting requested review from DaniPopes DaniPopes is a code owner

@mattsse mattsse Awaiting requested review from mattsse mattsse is a code owner

@zerosnacks zerosnacks Awaiting requested review from zerosnacks zerosnacks is a code owner

@onbjerg onbjerg Awaiting requested review from onbjerg onbjerg is a code owner

@0xrusowsky 0xrusowsky Awaiting requested review from 0xrusowsky 0xrusowsky is a code owner

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

Foundry
Status: No status

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(forge fmt): should give an option to place each if condition in a separate line

3 participants

@Syzygy106 @0xrusowsky @grandizzy