Incorrect comment indent inside if/else

[dhardy]

Example:

fn main() {
    let x = if true {
        1
    // comment with wrong indent
    } else {
        0
    };
}

(And no, this definitely should not be a comment on the else — actually, it should not be rustfmt's decision whether this applies to the else or is a free comment after the 1.)

[dtolnay]

It seems this isn't specific to if/else -- the same might apply to any indented comment before a }. I just got a diff like this from rustfmt as of current master (4a88598):

  extern "C" {
      fn first();

-     // TODO: add rest
+ // TODO: add rest
  }

[whizsid]

I like to work on this issue. Probably this issue was caused in here.

rustfmt/src/formatting/visitor.rs

Line 298 in 19e3caa

fn close_block(&mut self, span: Span, unindent_comment: bool) {

[calebcartwright]

There's been a bit of back and forth on this over the years, and unfortunately that complicates things a bit (refs #1575, #1737).

My initial reaction was that the comment resides within the block, and thus it should receive the same indent as other items (including comments) within the block, and that folks that really wanted a comment on part of a control flow could do something like

fn main() {
    let x = if true {
        1
    }
    // comment
    else {

        0
    };
}

but that causes problems due to a separate issue with the handling of if expressions.

The indentation in the extern block is just plain wrong and needs to be fixed. However, in the if/else case, I feel there's valid scenarios for both indentations, especially since rustfmt doesn't really know if the comment is just the last member of the block or intended to be associated to the else

@whizsid - if possible (and possible to do so idempotently) my preference and recommendation for the if/else case would be to indent based on the original placement to attempt to honor the author's intent. If the comment as at or to the right of the block indented level, then align it with the other block items. If it's to the left then the author most likely deliberately wants the comment associated with the below content (else...)

So

fn main() {
    let x = if true {
        1
        // comment with wrong indent
    } else {
        0
    };
    
    let x = if true {
        1
            // comment with wrong indent
    } else {
        0
    };
    
    if cond {
        "if"
    // else comment
    } else {
        "else"
    }
    
    if cond {
        "if"
// else comment
    } else {
        "else"
    }
}

would be formatted as

fn main() {
    let x = if true {
        1
        // comment with wrong indent
    } else {
        0
    };
    
    let x = if true {
        1
        // comment with wrong indent
    } else {
        0
    };
    
    if cond {
        "if"
    // else comment
    } else {
        "else"
    }
    
    if cond {
        "if"
    // else comment
    } else {
        "else"
    }
}

However, that should only be utilized on if/else, so

extern "C" {
    fn first();
    // TODO: add rest
}

extern "C" {
    fn first();
// TODO: add rest
}

extern "C" {
    fn first();
        // TODO: add rest
}

would be formatted as

extern "C" {
    fn first();
    // TODO: add rest
}

extern "C" {
    fn first();
    // TODO: add rest
}

extern "C" {
    fn first();
    // TODO: add rest
}

(edit should really only apply to if/else, not the other types of control flows)

[dhardy]

If I wanted to comment an else block, I'd use one of the following (unfortunately rustfmt does not like the first two):

if cond() {
    ...
} else /* !cond() */ {
    ...
}

if cond() {
    ...
} else { // !cond()
    ...
}

if cond() {
    ...
} else {
    // cond() is false
    ...
}

So IMO it is completely wrong to assume a comment on the last line of the if block refers to the else condition.

[calebcartwright]

So IMO it is completely wrong to assume a comment on the last line of the if block refers to the else condition

I agree, we should not categorically make that assumption. As I stated in my previous comment, we should only do that alignment conditionally, and this is what was subsequently implemented in #4459

This issue was closed as is our practice when changes are merged in source, and the fix will be immediately available for folks using v2.0 from source. We'll also look at backporting this to an upcoming 1.x release

[calebcartwright]

Change will be available in v1.4.26 which should hit nightly within the next couple days.

Note that rustfmt won't modify the existing indentation, but if you manually re-indent a comment within the if block (e.g. the // comment with wrong indent comment from the issue description), then rustfmt will honor that indentation level going forward for trailing comments in if/else.

In the case of trailing comments in an extern block, that's also been fixed in v1.4.26 and those will always be properly indented.