gofumpt --fix doesn't always work completely, gives conflicting results with standalone tool

[howardjohn]

• Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• Yes, I've searched similar issues on GitHub and didn't find any.
• Yes, I've included all information below (version, config, etc).
• Yes, I've tried with the standalone linter if available. (https://golangci-lint.run/usage/linters/)

Description of the problem

Running gofumpt standalone fixes more problems than gofumpt in golangci-lint.

Running find | xargs -r -I{} sh -c 'echo {}; gofumpt -w {}' vs golangci-lint run --fix -c ./common/config/.golangci-format.yml -v

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version 1.38.0 built from 507703b4 on 2021-03-03T13:53:01Z

Config file

$ cat .golangci.yml
service:
  golangci-lint-version: 1.38.x
run:
linters:
  disable-all: true
  enable:
  - gofumpt
  fast: false

Go environment

$ go version && go env
# paste output here

Verbose output of running

$ gofumpt -w cni/cmd/istio-cni/main.go
$ golangci-lint run --fix -c ./common/config/.golangci-format.yml -v ./cni/cmd/istio-cni/main.go
INFO [config_reader] Used config file common/config/.golangci-format.yml
INFO [lintersdb] Active 1 linters: [gofumpt]
INFO [loader] Go packages loading at mode 7 (files|name|compiled_files) took 226.271243ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 479.725µs
INFO [linters context/goanalysis] analyzers took 0s with no stages
INFO [runner] Processors filtering stat (out/in): identifier_marker: 1/1, exclude: 1/1, max_per_file_from_linter: 1/1, source_code: 1/1, path_shortener: 1/1, filename_unadjuster: 1/1, skip_files: 1/1, autogenerated_exclude: 1/1, diff: 1/1, max_same_issues: 1/1, max_from_linter: 1/1, path_prettifier: 1/1, skip_dirs: 1/1, nolint: 1/1, uniq_by_line: 1/1, cgo: 1/1, exclude-rules: 1/1, severity-rules: 1/1, path_prefixer: 1/1, sort_results: 1/1
INFO [runner] processing took 641.589µs with stages: nolint: 491.181µs, autogenerated_exclude: 42.911µs, source_code: 33.559µs, path_prettifier: 28.913µs, identifier_marker: 18.312µs, exclude-rules: 9.525µs, skip_dirs: 6.928µs, max_same_issues: 4.602µs, cgo: 1.125µs, filename_unadjuster: 930ns, path_shortener: 909ns, uniq_by_line: 629ns, max_per_file_from_linter: 367ns, diff: 364ns, sort_results: 307ns, skip_files: 283ns, max_from_linter: 262ns, exclude: 211ns, severity-rules: 175ns, path_prefixer: 96ns
INFO [runner] linters took 995.024µs with stages: gofumpt: 278.881µs
INFO Fix issue &result.Issue{FromLinter:"gofumpt", Text:"File is not `gofumpt`-ed", Severity:"", SourceLines:[]string{"\ttypes.NetConf // You may wish to not nest this type"}, Replacement:(*result.Replacement)(0xc000943a40), Pkg:(*packages.Package)(0xc00038a360), LineRange:(*result.Range)(nil), Pos:token.Position{Filename:"cni/cmd/istio-cni/main.go", Offset:0, Line:62, Column:0}, HunkPos:0, ExpectNoLint:false, ExpectedNoLintLinter:""} with range {62 62}
INFO fixer took 481.314µs with stages: all: 481.314µs
INFO File cache stats: 1 entries of total size 9.3KiB
INFO Memory: 4 samples, avg is 71.8MB, max is 71.8MB
INFO Execution took 233.341957ms

Here you can see golangci-lint reverting a change gofumpt made:

 type PluginConf struct {
-       types.NetConf // You may wish to not nest this type
+       types.NetConf           // You may wish to not nest this type
        RuntimeConfig *struct { // SampleConfig map[string]interface{} `json:"sample"`
        } `json:"runtimeConfig"`

$ gofumpt --version
v0.1.0

Code example or link to a public repository

You can run it on https://github.com/istio/istio. There are many files impacted, not just ni/cmd/istio-cni/main.go

[ldez]

gofumpt seems to do the opposite of gofmt:

$ gofumpt -w cni/cmd/istio-cni/main.go
$ git diff
diff --git i/cni/cmd/istio-cni/main.go w/cni/cmd/istio-cni/main.go
index abc3f91f3e..89e2468d2e 100644
--- i/cni/cmd/istio-cni/main.go
+++ w/cni/cmd/istio-cni/main.go
@@ -59,7 +59,7 @@ type Kubernetes struct {
 // is passed in on stdin. Your plugin may wish to expose its functionality via
 // runtime args, see CONVENTIONS.md in the CNI spec.
 type PluginConf struct {
-       types.NetConf           // You may wish to not nest this type
+       types.NetConf // You may wish to not nest this type
        RuntimeConfig *struct { // SampleConfig map[string]interface{} `json:"sample"`
        } `json:"runtimeConfig"`
 
$ gofmt -w cni/cmd/istio-cni/main.go
$ git diff
$ 

@mvdan could you explain the reason?

[mvdan]

@ldez I don't know - I can't diagnose unless you provide the exact versions of gofmt and gofumpt, and a way to reproduce the difference between the two. A recent gofumpt version will only be format-compatible with a recent version of gofmt. Mixing gofumpt v0.1.0 with gofmt from Go 1.8 will likely result in diffs, for example.

[ldez]

@mvdan I use gofmt from the go1.16 tag and the latest stable version of gofumpt:

go install mvdan.cc/gofumpt@v0.1.0

same behavior with gofmt from go1.15, go1.14, go1.13, go1.12, go1.11

[mvdan]

Thanks - that seems like a recent regression, added in mvdan/gofumpt@c49fa44. I mistook that formatting difference as a change in gofmt that happened a couple of years back.

I've fixed the bug here: mvdan/gofumpt@d7cd0a5

[mvdan]

I've now released that as part of a bugfix release: https://github.com/mvdan/gofumpt/releases/tag/v0.1.1

[ldez]

Thank you ❤️