Golang cmd/gofmt: puts else if comments in wrong place

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

play.golang.org

What operating system and processor architecture are you using (go env)?

play.golang.org

What did you do?

Ran this: https://play.golang.org/p/xJ0-Cq8ZYi and clicked format

If possible, provide a recipe for reproducing the error. A complete runnable program is good. A link on play.golang.org is best. here it is: https://play.golang.org/p/xJ0-Cq8ZYi

What did you expect to see?

I expected the B and C branch comments to be aligned the same way the A branch comment is..

What did you see instead?

Instead they were indented as if they were part of the previous block.

I get how this is a tricky issue, but if there is a blank line and the comment is right before the next block, I think it should be indented for that block.

Thanks!

Comment From: dsnet

It's not clear to me that this is wrong. The B and C comments are within the "{" and "}" of the containing if block, so it makes sense to associate the comment at that indentation level.

\cc @griesemer

Comment From: purpleidea

@dsnet If the comments didn't have a space after the previous if statements contents, and if they weren't right against the following line, I'd agree. Where else would you put the comment that should precede the else if?

I've amazingly found an edge case which just wasn't considered previously.

Thanks for looking into it.

Comment From: ALTree

I agree with dsnet.

Where else would you put the comment that should precede the else if?

If a comment is about branch B, I'd definitely put it inside the branch it refers to. Like this:

if A {
    // comment about A goes here, at the top of the A branch scope

    do stuff...

} else if B {
    // comment about B goes here, at the top of the B branch scope

    do stuff...

} else {
   // comment about else goes here, at the top of the 'else' branch scope

    do stuff...

}

IMHO putting a comment about B before its if (so in the scope of the previous, different branch) would be confusing even with the right indentation and I don't think goftm should indent those comments like you suggest.

Comment From: josharian

I think this is effectively a dup of #9910. As Russ says there:

Sometimes you want one or the other. Gofmt preserves what you've done.

Comment From: purpleidea

@josharian Nice find! I'm okay with the fix in #9910 about leaving them untouched. It would be better than them always getting moved incorrectly in my case! Thanks for the comment!

Comment From: josharian

Cool. Closing this as a dup, then.

Comment From: purpleidea

@josharian Sorry if I wasn't clear, but this issue isn't fixed, the other one took the right approach which I think we should too!

Comment From: josharian

OK, reopening, but I'm confused. I clicked on your original playground link, hit format, and nothing moved, which seems like exactly the proposed resolution--when there's ambiguity, don't move things. What am I missing?

Comment From: purpleidea

@josharian Click format on this link:

https://play.golang.org/p/Odchgkj1Tg

I would expect either nothing to move, but unfortunately it does.

On the earlier link, I would expect things to move to look like this link, or if we agree with the solution in #9910 for nothing to move.

Comment From: purpleidea

Found a workaround until this is fixed. You can do comments like this:

https://play.golang.org/p/mcx57OdNvy

It's a bit awkward, but at least it's gofmt legal, and the comments are at the right indentation level.

Comment From: purpleidea

Full code for those who hate link following:

package main

import (
    "fmt"
)

func main() {
    fmt.Println("Hello, sane comment indenting")

    if false {
        fmt.Println("if")
        // bad indentation
    } else
    // do something else...
    if true {
        fmt.Println("elseif")
    } else

    // otherwise...
    {
        fmt.Println("else")
    }
}

Comment From: as

It's a bit awkward, but at least it's gofmt legal

I disagree completely. The bug in gofmt is that it this awkward workaround is legal. I would hate to this type of style in production, especially since gofmt doesn't fix it.

Removing the strategically-placed comments fixes it, but it requires the comments to be removed/replaced. https://play.golang.org/p/OotrW-1T-K

It would be nice if gofmt was less lenient with this.

Comment From: purpleidea

On Sat, Jun 24, 2017 at 4:55 AM, as notifications@github.com wrote:

It would be nice if gofmt was less lenient with this.

The fix to "leave comments as they were" when nested above an else if would be great!

Comment From: igroglaz

amazing that this still hasn't been fixed… so many years have passed..