Conflict gofumpt and wsl for empty lines before if statement

[velp]

Thank you for creating the issue!

• [x] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
• [x] Yes, I've searched similar issues on GitHub and didn't find any.
• [x] Yes, I've included all information below (version, config, etc).

Please include the following information:

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version v1.30.0 built from (unknown, mod sum: "h1:UhdK5WbO0GBd7W+k2lOD7BEJH4Wsa7zKfw8m3/aEJGQ=") on (unknown)

Config file

$ cat .golangci.yml
cat: .golangci.yml: No such file or directory

Go environment

$ go version && go env
go version go1.15 darwin/amd64
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/user/Library/Caches/go-build"
GOENV="/Users/user/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/user/Work/go/pkg/mod"
GONOPROXY="github.com/user"
GONOSUMDB="github.com/user"
GOOS="darwin"
GOPATH="/Users/user/Work/go"
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sb/v5w11ncx5vscty2qslyqvx700000gn/T/go-build126064246=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/velp/Work/go/src/github.com/velp/test2 /Users/velp/Work/go/src/github.com/velp /Users/velp/Work/go/src/github.com /Users/velp/Work/go/src /Users/velp/Work/go /Users/velp/Work /Users/velp /Users /]
INFO [lintersdb] Active 10 linters: [deadcode errcheck gosimple govet ineffassign staticcheck structcheck typecheck unused varcheck]
INFO [loader] Go packages loading at mode 575 (exports_file|types_sizes|compiled_files|deps|files|imports|name) took 242.792256ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 293.194µs
INFO [linters context/goanalysis] analyzers took 18.81894ms with top 10 stages: buildir: 1.103903ms, S1020: 1.063927ms, ineffassign: 940.727µs, fact_deprecated: 847.572µs, S1024: 804.642µs, SA4019: 726.57µs, SA1019: 675.849µs, SA1012: 628.761µs, varcheck: 610.249µs, SA4003: 601.734µs
INFO [linters context/goanalysis] analyzers took 1.180983ms with top 10 stages: U1000: 656.494µs, buildir: 524.489µs
INFO [runner] processing took 3.378µs with stages: max_same_issues: 565ns, skip_dirs: 469ns, autogenerated_exclude: 294ns, cgo: 251ns, nolint: 235ns, filename_unadjuster: 221ns, max_from_linter: 208ns, uniq_by_line: 152ns, path_prettifier: 144ns, diff: 142ns, skip_files: 134ns, identifier_marker: 128ns, sort_results: 59ns, max_per_file_from_linter: 57ns, path_shortener: 55ns, source_code: 54ns, exclude: 54ns, exclude-rules: 53ns, severity-rules: 52ns, path_prefixer: 51ns
INFO [runner] linters took 107.323844ms with stages: goanalysis_metalinter: 70.414496ms, unused: 36.866632ms
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 5 samples, avg is 72.0MB, max is 72.6MB
INFO Execution took 398.634797ms

Description of problem

The problem: gofumpt conflicts with wsl lister for empty lines before if statement with several assignments.

Example of code

package main

import (
	"encoding/base64"
	"log"
)

func main() {
	str := "just example of code for test"
	encodedStr := base64.StdEncoding.EncodeToString([]byte(str))
	_, err := base64.StdEncoding.DecodeString(encodedStr)
	if err != nil {
		log.Fatalf("Decodding error: %s", err)
	}
}

How to reproduce:

1. run linting

$ golangci-lint run --enable wsl,gofumpt
main.go:12:2: only one cuddle assignment allowed before if statement (wsl)
	if err != nil {
	^

2. add new line before if statement to fix wsl error:

package main

import (
	"encoding/base64"
	"log"
)

func main() {
	str := "just example of code for test"
	encodedStr := base64.StdEncoding.EncodeToString([]byte(str))
	_, err := base64.StdEncoding.DecodeString(encodedStr)

	if err != nil {
		log.Fatalf("Decodding error: %s", err)
	}
}

3. run linting again:

$ golangci-lint run --enable wsl,gofumpt
main.go:12: File is not `gofumpt`-ed (gofumpt)

4. run gofumpt -w main.go to fix this problem:

$ gofumpt -w main.go
$

5. run linting again:

$ golangci-lint run --enable wsl,gofumpt
main.go:12:2: only one cuddle assignment allowed before if statement (wsl)
	if err != nil {
	^

Ooops! WSL returns the error, again.

[boring-cyborg[bot]]

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

[bombsimon]

Hi! I'm the author of wsl and I'm not sure exactly how this should be tackled. In the mind of wsl, we should only cuddle one assignment with an if statement and we should encourage users to cuddle error checks. So to solve this, there's a third alternative which is what wsl would've made if the fixer was merged.

package main

import (
	"encoding/base64"
	"log"
)

func main() {
	str := "just example of code for test"
	encodedStr := base64.StdEncoding.EncodeToString([]byte(str))

	_, err := base64.StdEncoding.DecodeString(encodedStr)
	if err != nil {
		log.Fatalf("Decodding error: %s", err)
	}
}

I agree that it's not obvious and I understand that it's hard to argue about using wsl when there's no fixer. One of the reason the work for the fixer got stuck was because gofumpt did such a good job. Anyway, sorry for the inconvenience but at lest the code above will satisfy both linters. Feel free to raise an issue in the wsl.

EDIT not really the same but this is also valid according to both gofumpt and wsl:

Source code

package main

import (
	"encoding/base64"
	"log"
)

func main() {
	str := "just example of code for test"

	encodedStr := base64.StdEncoding.EncodeToString([]byte(str))
	if _, err := base64.StdEncoding.DecodeString(encodedStr); err != nil {
		log.Fatalf("Decodding error: %s", err)
	}
}

[velp]

This code is just an example to describe the problem.

Hm, looks like gofumpt -w ... should add new line before nearest (for if statement) assignment (in my example it's _, err := base64.StdEncoding.DecodeString(encodedStr)) if found several assignments.

Let's ask gofumpt's author @mvdan

[mvdan]

I don't think gofumpt should fix anything here. The original source code is fine, as far as I can tell, and forcing the addition of an empty line feels out of scope for a generic formatter. If someone really wants to go the extra mile with empty lines, they can already use wsl. It seems like if that tool slightly improves its error message, or suggests the right change to make, then it won't enter into a conflict loop with gofumpt.

[mvdan]

I'm also not sure how useful it is to run two opinionated formatting/style tools at once. They'll probably run into conflicts sooner or later, unless both tools are compatible by design - just like gofumpt by design never fights gofmt.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[mvdan]

I really disagree with the use of the stale bot. This bug is still very much valid, and time passing has not magically made it go away.

[ldez]

I will re-open. What is your solution to this problem?

[mvdan]

Running both wsl and gofumpt at the same time is likely a bad idea; see https://github.com/golangci/golangci-lint/issues/1510#issuecomment-728279315.

[vitsadecky]

I have same issue.

[bombsimon]

I feel like I'm the one that should try to sort this out. Personally I use gofumpt and wsl together so it shouldn't be a big problem but I agree that the error messages might be confusing and hard to know how to solve without a fixer. I can try to free up some time to get back to implementing the fixer but sadly I don't know when that would be.

I'm not sure what happens when two linters with a fixer reports a suggested fix on the same line but I guess worst case a solution would be to run the fixer one by one.