fmt nits

[grandizzy]

Motivation

Solution

PR Checklist

• Added Tests
• Added Documentation
• Breaking changes

[onbjerg]

testing this it looks good from my end

[DaniPopes]

this comment is outdated, can this be fixed / what's missing? @0xrusowsky
the until is correct here, but should be to as indicated by the comment

[0xrusowsky]

why is it outdated? @DaniPopes

    /// An `if` statement with an optional `else` block: `if (expr) { ... } else { ... }`.
    If(Box<'ast, Expr<'ast>>, Box<'ast, Stmt<'ast>>, Option<Box<'ast, Stmt<'ast>>>),

with this info i cannot print the comments as in the original code for these 2 scenarios:

    if /* comment10 */ (anotherLongCondition) // comment11
       /* comment12 */ {
            execute() ;
    } // comment13

    if (
        step.opcode == 0x52
            && /*MSTORE*/ step.stack[0] == testContract.memPtr() // MSTORE offset
            && step.stack[1] == testContract.expectedValueInMemory() // MSTORE val
    ) {
        mstoreCalled = true;
    }

cause the span of the if expr in the 1st scenario is only the anotherLongCondition ident, and in the 2nd one the binary operator expr

so i either need to:

• self.print_if_cond("if", cond, then.span.lo())
• self.print_if_cond("if", cond, cond.span.hi())

the former is the one we do now, cause imo is more consistent with the old behavior (puts everything inside the condition's parenthesis)

the later leaves all comments out of the parenthesis, so the first scenario would work, but the other one would output something like:

    if (
        step.opcode == 0x52
            && /*MSTORE*/ step.stack[0] == testContract.memPtr() // MSTORE offset
            && step.stack[1] == testContract.expectedValueInMemory()
    ) // MSTORE val
    {
        mstoreCalled = true;
    }

anyways, i think we can simply remove the comment. i only added it initially because i didn't know we were willing to not have 100% exact compatibility with the old formatter.
imo the if statement comment handling looks good, so the comment is not really necessary (unlike the DeclMulti comment, which could be a little bit more annoying for users, although they can simply use inline disable)

[0xrusowsky]

fyi, i deleted the cmnt.

the only implication is that this code:

    if /* comment10 */ (anotherLongCondition) // comment11
       /* comment12 */ {
            execute() ;
    } // comment13

was previously formatted as:

    if /* comment10 */ (
       anotherLongCondition // comment11
    ) {
       /* comment12 */
       execute();
    } // comment13

and now is:

    if /* comment10 */ (
       anotherLongCondition // comment11
       /* comment12 */
    ) {
       execute();
    } // comment13

which is totally acceptable.
and if users want cmnt12 to go below, they simply need to put it inside the block, so that its parsed as part of the stmt

[DaniPopes]

that's fine

the comment was outdated wrt the inline code .span etc inside of the comment itself

[0xrusowsky]

LGTM, pending Dani's validation