govet requires go install to work better

[alecthomas]

I know this report won't be that helpful, but I don't have much information.

I have this configuration:

# Exhaustive list of options here:
# https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
run:
  deadline: 5m
  tests: true
  skip-dirs:
    - mock # Remove once https://github.com/golangci/golangci-lint/issues/86 is fixed

output:
  print-issued-lines: false

linters:
  enable-all: true
  disable:
    - maligned

linters-settings:
  govet:
    check-shadowing: true
  golint:
    min-confidence: 0
  gocyclo:
    min-complexity: 10
  dupl:
    threshold: 100
  goconst:
    min-len: 2
    min-occurrences: 2
  gocyclo:
    min-complexity: 20

issues:
  max-per-linter: 0
  max-same: 0
  exclude-use-default: false
  exclude:
    # Captured by errcheck.
    - '^(G104|G204):'
    # Very commonly not checked.
    - 'Error return value of .(.*\.Help|.*\.MarkFlagRequired|(os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*printf?|os\.(Un)?Setenv). is not checked'
    # TODO(aat): remove this once docs are added.
    - '.*should have.*comment.*'
    # Weird error only seen on CI...
    - 'internal error: no range for'

Running golangci-lint run locally on my laptop results in the following error:

pkg/provision/iam/policies.go:133:5: declaration of "err" shadows declaration at pkg/provision/iam/policies.go:115 (govet)

But when this runs under CI (on Kochiku), no error is reported. I've run it on CI with -v and it gives the following:

time="2018-06-12T00:27:06Z" level=info msg="Config search paths: [./ /data/app/kochiku-worker/shared/build-partition/cash/squaremason/src/stash.corp.squareup.com/cash/squaremason /data/app/kochiku-worker/shared/build-partition/cash/squaremason/src/stash.corp.squareup.com/cash /data/app/kochiku-worker/shared/build-partition/cash/squaremason/src/stash.corp.squareup.com /data/app/kochiku-worker/shared/build-partition/cash/squaremason/src /data/app/kochiku-worker/shared/build-partition/cash/squaremason /data/app/kochiku-worker/shared/build-partition/cash /data/app/kochiku-worker/shared/build-partition /data/app/kochiku-worker/shared /data/app/kochiku-worker /data/app /data /]"
time="2018-06-12T00:27:06Z" level=info msg="Used config file .golangci.yml"
time="2018-06-12T00:27:06Z" level=info msg="Active 18 linters: [deadcode depguard dupl errcheck gas goconst gocyclo gofmt goimports golint govet ineffassign interfacer megacheck structcheck typecheck unconvert varcheck]"
time="2018-06-12T00:27:06Z" level=info msg="Skipped dirs: [mocks pkg/awsenv/testdata vendor]"
time="2018-06-12T00:27:06Z" level=info msg="Paths resolving took 3.812451ms: [cmd/sqm cmd/sqm/env cmd/sqm/service internal/registry pkg/awsenv pkg/awsenv/iampolicy pkg/k8senv pkg/provision pkg/provision/database pkg/provision/iam pkg/provision/service server]"
time="2018-06-12T00:27:13Z" level=info msg="Program loading took 7.027793144s"
time="2018-06-12T00:27:17Z" level=info msg="SSA repr building took 3.545941115s"
time="2018-06-12T00:27:17Z" level=info msg="worker.3 took 358.10083ms with stages: goimports: 239.360565ms, dupl: 98.248495ms, ineffassign: 16.93739ms, gocyclo: 1.830568ms, unconvert: 1.651006ms"
time="2018-06-12T00:27:17Z" level=info msg="worker.4 took 360.277981ms with stages: golint: 237.566721ms, gofmt: 57.642512ms, gas: 45.870413ms, varcheck: 7.755287ms, deadcode: 3.634114ms, structcheck: 3.598876ms, errcheck: 2.459229ms, goconst: 1.650107ms, typecheck: 948ns"
time="2018-06-12T00:27:18Z" level=info msg="worker.1 took 928.704012ms with stages: interfacer: 790.557348ms, govet: 138.112655ms, depguard: 7.709µs"
time="2018-06-12T00:28:08Z" level=info msg="worker.2 took 51.523991372s with stages: megacheck: 51.523955449s"
time="2018-06-12T00:28:08Z" level=info msg="Workers idle times: #1: 50.595252839s, #3: 51.165207269s, #4: 51.163674384s"
time="2018-06-12T00:28:08Z" level=info msg="processing took 1.665043ms with stages: nolint: 824.966µs, exclude: 571.104µs, path_prettifier: 149.593µs, autogenerated_exclude: 78.38µs, cgo: 13.559µs, max_same_issues: 4.953µs, uniq_by_line: 4.94µs, diff: 4.718µs, max_per_file_from_linter: 4.331µs, skip_files: 4.306µs, max_from_linter: 4.193µs"
Congrats! No issues were found.

The CI gets the latest golangci-lint with:

go get -u github.com/golangci/golangci-lint/cmd/golangci-lint

And I'm using the same version locally.

Sorry I can't be of more help, but I can't get any more information out of golangci-lint or the CI.

[alecthomas]

I have a bit more information. I added a manual run of go vet -shadow ./... to the CI, and also ran it locally.

Locally:

$ go vet -shadow ./...
$

CI:

+ go vet -shadow ./...
+ golangci-lint run
client/protocol.go:102:2: declaration of "entity" shadows declaration at client/protocol.go:11 (govet)
store/db.go:242:2: declaration of "entity" shadows declaration at store/db.go:14 (govet)
store/db.go:267:3: declaration of "entity" shadows declaration at store/db.go:14 (govet)
store/fake.go:117:2: declaration of "entity" shadows declaration at store/fake.go:9 (govet)

I'm not sure what's going on. These reports are around a variable shadowing a package import, so they are correct.

I'm not sure why go vet isn't reporting them either :\

[jirfag]

hi!

1. try go install -i ./... (depends on project, maybe go install -i ./cmd/...) first in CI, does it help?
2. give a feedback please: why don't you consider golangci.com instead of running golangci-lint in custom CI?

[alecthomas]

1. You gave me an idea; I did the opposite. I removed my local pkg dir and got the following:

$ rm -rf $GOPATH/pkg
$ golangci-lint run --fast
cmd/sotw/main.go:13:2: declaration of "entity" shadows declaration at cmd/sotw/main.go:5 (govet)
$ go install ./...
$ golangci-lint run --fast
Congrats! No issues were found.

I don't know why that would be the case...

2. We can't use any SAAS, including GitHub, Circle, golangci.com, etc.

[jirfag]

which version of go do you use?

[alecthomas]

I've tried 1.9, 1.9.2, 1.10 and 1.10.2

[jirfag]

ok, thank you.
I will try to reproduce, it's a weird bug.

[jirfag]

1. On your local machine is golangci-lint built locally (binary made by go get) or you use prebuilt binary (curl ... | bash -s ...)? And how it's built/installed on CI and which version of go do you use in CI machine?
2. run locally, please

go version
head -5 cmd/sotw/main.go | tail -1
head -13 cmd/sotw/main.go | tail -1
rm -rf $GOPATH/pkg
golangci-lint run --no-config --disable-all -Egovet cmd/sotw/main.go
go vet -shadow ./...
go install -i ./cmd/sotw/
golangci-lint run --no-config --disable-all -Egovet cmd/sotw/main.go
go vet -shadow ./...

[alecthomas]

It's built by go get.

$ go version
go version go1.10.2 darwin/amd64
$ head -5 cmd/sotw/main.go | tail -1
	"stash.corp.squareup.com/cash/squaresotw/entity"
$ head -13 cmd/sotw/main.go | tail -1
	entity, err := poller.Poll(entity.ID("arn:aws:eks:us-east-1:934409966526:cluster/eks-us-east-1"))
$ rm -rf $GOPATH/pkg
$ golangci-lint run --no-config --disable-all -Egovet cmd/sotw/main.go
Congrats! No issues were found.
$ golangci-lint run --no-config --disable-all -Egovet
Congrats! No issues were found.
$ go vet -shadow ./...
$ go install -i ./cmd/sotw
$ golangci-lint run --no-config --disable-all -Egovet cmd/sotw/main.go
Congrats! No issues were found.
$ go vet -shadow ./...

So it looks like I can only reproduce the warnings when I am using the configuration file AND have cleared the pkg directory...that is truly baffling.

[alecthomas]

I just tried it with the curled version 1.6.1 and I can reproduce it:

$ rm -rf $GOPATH/pkg
$ ./bin/golangci-lint run --fast
cmd/sotw/main.go:13:2: declaration of "entity" shadows declaration at cmd/sotw/main.go:5 (govet)

[jirfag]

in environment where you've reproduced it run, please:

go vet -x -v -shadow ./cmd/sotw/main.go
go tool vet -v -shadow ./cmd/sotw/main.go

[jirfag]

Got it. For go version < 1.10 we need to manually go install project to go vet work properly, it's a known issue. In go 1.10 it was fixed: go vet now rebuilds project on each invocation using build cache. But golangci-lint doesn't do it.

I have 3 choices how to fix it:

1. Run go install from golangci-lint if we are in fast mode and govet is enabled. It won't slow down go 1.10 builds and can slow down go < 1.10 builds, but will make them correct. The disadvantage is in need of go binary, but it looks ok.

• In slow mode govet must consume source-loaded data, not from an object files.

2. Make govet a "slow" linter: it starts to consume source-loaded (not object files loaded) information about types.
3. Make type information loading incremental: use separate cache for it. It will deprecate --fast and all linters can consume type information from source code.

I can't guarantee that fix will be in a near future. The fastest solution now is to run go install in CI.