search

golangci / golangci-lint

Fast linters runner for Go
https://golangci-lint.run
GNU General Public License v3.0
15.69k stars 1.39k forks source link

go vet does not run the same in golangci-lint as it does by itself #450

Closed micahcoffman closed 5 years ago

micahcoffman commented 5 years ago

When I run go vet ./... I see:

# github.com/expel-io/nightingale/handlers_test [github.com/expel-io/nightingale/handlers.test]
handlers/health_check_handler_test.go:33:3: undefined: mockHttpClient

When I run golangci-lint run -v --no-config --disable-all --enable=govet I see:

INFO [lintersdb] Active 1 linters: [govet]
INFO [loader] Go packages loading at mode load types and syntax took 1.174175754s
INFO [loader] Packages that do not compile: [github.com/expel-io/nightingale/handlers_test [github.com/expel-io/nightingale/handlers.test]]
INFO [runner] worker.5 took 18.048µs
INFO [runner] worker.4 took 1.518µs
INFO [runner] worker.1 took 1.78µs
INFO [runner] worker.6 took 1.704µs
INFO [runner] worker.3 took 3.862µs
INFO [runner] worker.8 took 1.27µs
INFO [runner] worker.7 took 1.181µs
INFO [runner] worker.2 took 26.759643ms with stages: govet: 26.747457ms
INFO [runner] Workers idle times: #1: 26.688791ms, #3: 26.677114ms, #4: 26.687764ms, #5: 26.709212ms, #6: 26.681527ms, #7: 26.658573ms, #8: 26.669955ms
INFO [runner] processing took 28.64µs with stages: max_same_issues: 24.301µs, skip_dirs: 508ns, path_prettifier: 470ns, nolint: 443ns, diff: 311ns, autogenerated_exclude: 308ns, max_from_linter: 292ns, cgo: 274ns, identifier_marker: 235ns, exclude: 215ns, path_shortener: 209ns, skip_files: 201ns, replacement_builder: 197ns, exclude-rules: 194ns, uniq_by_line: 189ns, source_code: 149ns, max_per_file_from_linter: 144ns
INFO Memory: 14 samples, avg is 69.5MB, max is 69.9MB
INFO Execution took 1.392494272s

I would expect golangci-lint to report the vet error, but it doesn't at all. This is preventing me from switching to golangci-lint from gometalinter.

golangci-lint --version:

golangci-lint has version 1.15.0 built from 901cf25 on 2019-02-18T08:22:26Z

go version:

go version go1.12.1 darwin/amd64

go env:

GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/micahcoffman/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/micahcoffman/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/opt/go/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/opt/go/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/xr/7ppzw6ys2qn6x7rprbpg9zp00000gr/T/go-build630229549=/tmp/go-build -gno-record-gcc-switches -fno-common"
micahcoffman commented 5 years ago

Looking through the code for golangci-lint I see that some linters won't run on packages that don't compile. But I would fully expect vet to run and output errors.

micahcoffman commented 5 years ago

I see, the error i'm looking for is actually covered by typecheck, so i'm guessing govet is excluding the errors that typecheck would output by default.

micahcoffman commented 5 years ago

This can be closed if someone verifies that's how it is supposed to work.

geeknoid commented 5 years ago

I'm seeing similar behavior. Consider:

~/go/src/istio.io/istio/mixer/pkg/attribute$ golangci-lint run --no-config --disable-all --enable=govet
~/go/src/istio.io/istio/mixer/pkg/attribute$ golangci-lint run --no-config --disable-all --enable=typecheck
~/go/src/istio.io/istio/mixer/pkg/attribute$ go vet ./...
# istio.io/istio/mixer/pkg/attribute
./protoBag.go:80: Scope.Debugf format %s reads arg #2, but call has 1 arg

so the golangci-lint version of vet and typecheck don't find the extra %s present in the format string, but 'go vet' does.

micahcoffman commented 5 years ago

I'm not sure about that issue with typecheck you are seeing, but it looks like golangci-lint won't run govet on packages that don't compile which doesn't make sense to me. The runner for govet gets a list of packages from lintCtx.Program.InitialPackages() https://github.com/golangci/golangci-lint/blob/v1.15.0/pkg/golinters/govet.go#L52 which is populated in the loader, excluding packages that don't compile https://github.com/golangci/golangci-lint/blob/v1.15.0/pkg/lint/load.go#L91-L100.

This should be fixed in the newer runner for govet that was merged since it uses lintCtx.Packageshttps://github.com/golangci/golangci-lint/blob/master/pkg/golinters/goanalysis/linter.go#L118 which is populated with https://github.com/golangci/golangci-lint/blob/v1.15.0/pkg/lint/load.go#L321

micahcoffman commented 5 years ago

@jirfag do you have an eta on when you'll tag a new release? it would be nice to get those newer govet changes out so I can switch to using just govet instead of counting on typecheck etc to run on packages that don't compile.

geeknoid commented 5 years ago

Note that in the case I posted above, the code is fully compiled. The warning is not detected by golangci-lint using either typecheck or vet, but is detected by plain go vet.

jirfag commented 5 years ago

hi! Can't find the repository github.com/expel-io/nightingale to check. Also, can't reproduce in istio.io/istio/mixer. The new release 1.16.0 already contains new govet. Can you check it, please?

micahcoffman commented 5 years ago

Sure, i'll give it a go! FWIW the code is meant to be compiled for linux, and and setting the GOOS=linux causes a different error. So the issue may be more related to cross compilation. I've seen an older issue that is related, but it got closed out.

jirfag commented 5 years ago

compilation errors are reported by typecheck