Golangci-lint: Golang compilation errors still result in golangci-lint success result (0 exit)

[rfay]

golanci-lint v1.11.3 can run into a compilation failure and still return a 0 exit code, which is clearly the opposite of what we want.

• mkdir junk
• Place this file in the junk directory: https://gist.github.com/rfay/36d05e93d4ec41340eac8ab998d091c0#file-auth-ssh_test-go
• golangci-lint run --out-format=line-number --disable-all --enable=gofmt --enable=govet --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode ./junk/...
• Result is just a warning, with a zero (success) result from the run: WARN [runner/staticcheck] Can't run megacheck because of compilation errors in packages [github.com/drud/junk.test]: -: junk/auth-ssh_test.go:15:2: cannot find package "github.com/stretchr/testify/assert" in any of: /usr/local/Cellar/go/1.11.2/libexec/src/github.com/stretchr/testify/assert (from $GOROOT) /Users/rfay/go/src/github.com/stretchr/testify/assert (from $GOPATH)

1. golangci-lint v1.11.3**
2. No golangci.yml in use
3. go version && go env:

go version && go env
go version go1.11.2 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/rfay/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/rfay/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.2/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
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/q7/gr99xnnd67bg00r_sb75rhtm0000gn/T/go-build189930056=/tmp/go-build -gno-record-gcc-switches -fno-common"

1. Verbose output of running: golangci-lint run -v (actually golangci-lint run -v --out-format=line-number --disable-all --enable=gofmt --enable=govet --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode ./junk/...

golangci-lint run -v --out-format=line-number --disable-all --enable=gofmt --enable=govet  --enable=errcheck --enable=staticcheck --enable=ineffassign --enable=varcheck --enable=deadcode ./junk/...
INFO [config_reader] Config search paths: [./ /Users/rfay/go/src/github.com/drud/junk /Users/rfay/go/src/github.com/drud /Users/rfay/go/src/github.com /Users/rfay/go/src /Users/rfay/go /Users/rfay /Users /]
INFO [lintersdb] Active 7 linters: [deadcode errcheck gofmt govet ineffassign staticcheck varcheck]
INFO [loader] Go packages loading at mode load deps types and syntax took 965.550759ms
INFO [loader] SSA repr building took 1.360452ms
INFO [loader/astcache] Parsed AST of all pkg.GoFiles: [] for 391ns
INFO [loader/astcache] Parsed AST of all pkg.GoFiles: [] for 404ns
INFO [loader] Packages that do not compile: [github.com/drud/junk.test]
INFO [runner/skip dirs] sorted abs args: [/Users/rfay/go/src/github.com/drud/junk]
INFO [runner] worker.1 took 5.12µs
INFO [runner] worker.4 took 855.351µs with stages: errcheck: 474.426µs, govet: 20.958µs, ineffassign: 15.461µs, gofmt: 2.307µs
INFO [runner] worker.3 took 893.772µs with stages: deadcode: 418.973µs, varcheck: 25.19µs
WARN [runner/staticcheck] Can't run megacheck because of compilation errors in packages [github.com/drud/junk.test]: -: junk/auth-ssh_test.go:15:2: cannot find package "github.com/stretchr/testify/assert" in any of:
    /usr/local/Cellar/go/1.11.2/libexec/src/github.com/stretchr/testify/assert (from $GOROOT)
    /Users/rfay/go/src/github.com/stretchr/testify/assert (from $GOPATH)
INFO [runner] worker.2 took 1.370267ms with stages: staticcheck: 1.016251ms
INFO [runner] processing took 7.278µs with stages: nolint: 2.988µs, max_same_issues: 741ns, skip_dirs: 483ns, path_prettifier: 434ns, autogenerated_exclude: 366ns, max_from_linter: 305ns, diff: 287ns, source_code: 285ns, uniq_by_line: 284ns, max_per_file_from_linter: 248ns, skip_files: 231ns, exclude: 225ns, cgo: 223ns, path_shortener: 178ns
INFO [runner] Workers idle times: #1: 669.676µs, #3: 421.356µs, #4: 470.798µs
INFO Memory: 13 samples, avg is 68.9MB, max is 68.9MB
INFO Execution took 1.222214863s
$ echo $?
0

(Exit value is 0 even though compilation failed)

[fho]

issue still exist in version 1.18.0

[tpounds]

@rfay, @fho Can either of you provide a minimal example that exhibits the undesirable behavior?

[dcu]

@tpounds full example here:

bash-3.2$ cat main.go
package main

import "fmt"

func main() {
    broken
}
bash-3.2$ golangci-lint run --print-issued-lines=false --build-tags  --disable-all --exclude-use-default=false --enable=govet --enable=golint --enable=errcheck --enable=gocyclo --enable=deadcode --enable=gosimple --enable=staticcheck --deadline=60s .
WARN [runner] Can't run linter unused: buildssa: analysis skipped: errors in package: [/tmp/src/broken-code/main.go:6:2: undeclared name: broken /tmp/src/broken-code/main.go:3:8: "fmt" imported but not used]
WARN [runner] Can't run linter goanalysis_metalinter: vrp: failed prerequisites: buildssa@broken-code
bash-3.2$ echo $?
0

[dcu]

according to this issue (https://github.com/fatih/vim-go/issues/2533) vim-go was previously using the typecheck linter to prevent this behavior but that workaround is no longer working, since 95ec0cf21ea4720ec9555c43176ab8b1d1b035f2

[tpounds]

@dcu Thanks for providing an example! I think there are a couple of areas where we could improve things.

1. Provide an option to fail on compilation errors (e.g. --fail-on-compile, --fail-on-all-errors)
2. Improve the compiler error output when the above flags are present or the linter is run with a --debug or --verbose=N flag.

Thoughts?

/cc @jirfag

[rfay]

I actually can't imagine that a compilation error isn't a lint error.

My experience is that I use golangci-lint as a git pre-push hook. Sadly, I might push something that doesn't compile. When that happens, golangci-lint fails with non-zero exit code and the push happens anyway.

[dcu]

I agree that a compilation error must result in error by default

[tpounds]

@rfay, @dcu Agreed, however, this is a breaking change in current behavior so it's likely we would want to bump major versions to introduce this change. Regardless I think it makes sense to expose this as a configurable option to allow current users to opt-in to the behavior now (1.x) and opt-out in the future (2.x). Thoughts?

[Gradecak]

I know this is a relatively fresh issue, but Is there any approximate timeline for when such a feature would be available on 2.X? This bug makes it very difficult to use golangci-lint as an editor integration.

[ernado]

This should be already fixed when we switched from warnings to errors for this. Feel free to reopen.