mvdan / gofumpt

A stricter gofmt
https://pkg.go.dev/mvdan.cc/gofumpt
BSD 3-Clause "New" or "Revised" License
3.33k stars 112 forks source link

Incorrect formatting of var block #119

Closed moorereason closed 3 years ago

moorereason commented 3 years ago

Given:

var (
    // foo
    b = 2
)

Becomes:

var (// foo
var b = 2

Tested with gofumpt v0.1.1 and 0952458e3d6b38a311cd744b440dbcd63ffb0c42

mvdan commented 3 years ago

Thanks, this seems like a bug. The easiest solution is likely to not change the declaration to be single-line if there's a comment in between the tokens.

mvdan commented 3 years ago

Hm, I actually can't reproduce:

$ cat /tmp/f.go
package p

var (
        // foo
        b = 2
)

var (
        //_ io.ReadCloser = &Decoder{}

        _ io.WriteCloser = &Encoder{}
)
$ gofumpt -version
v0.1.1-0.20210429074948-20d3d7f4c8ad
$ gofumpt /tmp/f.go
package p

// foo
var b = 2

//_ io.ReadCloser = &Decoder{}

var _ io.WriteCloser = &Encoder{}

Sometimes this is only reproducible as part of a much bigger file. If you could provide such a file, that would help.

cc @mpldr

mvdan commented 3 years ago

Also worth noting that I'm on go version go1.16.4 linux/amd64.

mpldr commented 3 years ago

my file is literally:

package yenc

import "io"

var (
    _ io.ReadCloser = &Decoder{}

    _ io.WriteCloser = &Encoder{}
)

and commenting out the first constraint.

mvdan commented 3 years ago

@mpldr I still can't reproduce, the output isn't broken for me, it's just:

//_ io.ReadCloser = &Decoder{}

var _ io.WriteCloser = &Encoder{}

Can you share the exact versions of Go and gofumpt where this breaks, and how you ran gofumpt?

mpldr commented 3 years ago
❯ go version
go version go1.16.4 linux/amd64
❯ gofumpt --version
v0.1.1
mvdan commented 3 years ago

And how are you running gofumpt? gofumpt your_file.go works fine for me with those same exact versions.

mpldr commented 3 years ago

inside vim-go, let me check

mpldr commented 3 years ago

It seems (if i read the code correctly) that it writes the file the file to a temppath, and then calls gopls with gofumpt set as the formatter.

mvdan commented 3 years ago

Right - that adds its own complexity. You'd have to find out what version of gopls, and thus gofumpt, that ends up running. Because it's not the same version you see on your command line.

mvdan commented 3 years ago

Also, if this is a bug that you can reproduce with vim-go but not with gofumpt directly, I'd say file a bug on vim-go's issue tracker. I honestly cannot fix a bug I cannot reproduce :)

mpldr commented 3 years ago

You are absolutely correct on that one. I just checked and there is only one binary called gofumpt on my entire computer so this is at least interesting.

mvdan commented 3 years ago

The way gopls uses gofumpt is by statically building with it as a library, so it pins a version on its own. Depending on what gopls version vim-go is using, that gofumpt version might be a bit old, or very old.

Even if the version matches v0.1.1, there's still the question of exactly how gofumpt is run, because it might not match gofumpt file.go.

moorereason commented 3 years ago

My bad. I assumed gopls was using the gofumpt binary. I'm using vim-go (HEAD) with gopls v0.6.11 which has pinned gofumpt v0.1.1.

mpldr commented 3 years ago

@moorereason you can just set let g:go_fmt_command = "gofumpt" instead of what the documentation said, and you're good to go

moorereason commented 3 years ago

Fixed upstream in vim-go.