unparam false positive since upgrading to 1.13

[cloudlena]

unparam reports false positives since upgrading to 1.13. Probably they are related to mvdan/unparam#36.

Please include the following information:

1. Version of golangci-lint: golangci-lint --version (or git commit if you don't use binary distribution)

golangci-lint has version 1.13 built from 2192097 on 2019-01-21T07:58:24Z

2. Config file: cat .golangci.yml

linters:
  enable-all: true
  disable:
    - lll
    - dupl
    - scopelint

linters-settings:
  govet:
    check-shadowing: true
  maligned:
    suggest-new: true
  misspell:
    locale: US

3. Go environment: go version && go env

go version go1.11.4 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/tobi/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/tobi/go"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/Cellar/go/1.11.4/libexec"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.11.4/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/tobi/go/src/bitbucket.org/medisante/device-metrics/go.mod"
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/vl/n1hn_wj96dx0v7l8nm0cgbwr0000gn/T/go-build062638386=/tmp/go-build -gno-record-gcc-switches -fno-common"

4. Verbose output of running: golangci-lint run -v

INFO [config_reader] Config search paths: [./ /Users/tobi/go/src/bitbucket.org/medisante/device-metrics /Users/tobi/go/src/bitbucket.org/medisante /Users/tobi/go/src/bitbucket.org /Users/tobi/go/src /Users/tobi/go /Users/tobi /Users /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 28 linters: [deadcode depguard errcheck gochecknoglobals gochecknoinits goconst gocritic gocyclo gofmt goimports golint gosec gosimple govet ineffassign interfacer maligned misspell nakedret prealloc staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck]
INFO [lintersdb] Optimized sublinters [staticcheck gosimple unused stylecheck] into metalinter megacheck
INFO [loader] Go packages loading at mode load deps types and syntax took 1.373139169s
INFO [loader] SSA repr building timing: packages building 28.149496ms, total 236.15878ms
INFO [runner] worker.1 took 308.128396ms with stages: misspell: 209.977185ms, govet: 44.505369ms, errcheck: 34.833948ms, varcheck: 17.391558ms, nakedret: 1.357527ms, typecheck: 12.819µs
INFO [runner] worker.8 took 310.680197ms with stages: gosec: 155.974053ms, unconvert: 94.616965ms, gocritic: 46.960677ms, deadcode: 6.779298ms, goconst: 2.127511ms, gocyclo: 1.595161ms, prealloc: 1.409989ms, gochecknoglobals: 1.109995ms, gochecknoinits: 28.531µs
INFO [runner] worker.3 took 314.359824ms with stages: gofmt: 204.843355ms, ineffassign: 97.918687ms, maligned: 7.484319ms, structcheck: 3.950368ms, depguard: 90.386µs
INFO [runner] worker.7 took 461.617471ms with stages: interfacer: 461.581805ms
INFO [runner] worker.4 took 471.670256ms with stages: goimports: 471.638271ms
INFO [runner] worker.2 took 663.610761ms with stages: golint: 663.592887ms
INFO [runner] worker.5 took 1.191572259s with stages: unparam: 1.191549754s
cmd/apigw-getmetrics/main_test.go:124:52: TestGetMetricsHandler$1$2 - result 1 (error) is always nil (unparam)
                        resolver := func(string, string) (aws.Endpoint, error) {
                                                                        ^
cmd/cloudwatch-generatedemodata/main_test.go:87:88: TestGetMetricsHandler$1$2 - result 1 (error) is always nil (unparam)
                        cfg.EndpointResolver = aws.EndpointResolverFunc(func(string, string) (aws.Endpoint, error) {
                                                                                                            ^
INFO [runner] worker.6 took 1.431312672s with stages: megacheck: 1.431304621s
INFO [runner] Workers idle times: #1: 1.123044284s, #2: 767.667422ms, #3: 1.114808806s, #4: 959.420943ms, #5: 239.740515ms, #7: 969.466583ms, #8: 1.120378496s
INFO [runner] Issues before processing: 83, after processing: 2
INFO [runner] processing took 28.026308ms with stages: path_prettifier: 19.198798ms, skip_dirs: 4.357334ms, autogenerated_exclude: 2.822555ms, cgo: 1.329711ms, source_code: 146.089µs, exclude: 125.742µs, nolint: 31.519µs, max_same_issues: 4.415µs, uniq_by_line: 2.552µs, path_shortener: 2.418µs, max_from_linter: 1.902µs, max_per_file_from_linter: 1.371µs, diff: 1.091µs, skip_files: 811ns
INFO Memory: 31 samples, avg is 427.5MB, max is 878.0MB
INFO Execution took 3.168527503s

In this case, the function signature is given by an external package (i.e. https://github.com/aws/aws-sdk-go-v2) and cannot be changed. In my view, this is a false positive.

[jirfag]

Hi, thank you!
Can you check does it reproduce with the latest unparam only (without golangci-lint)?

[cloudlena]

It does:

$ unparam ./...
cmd/apigw-getmetrics/main_test.go:124:52: TestGetMetricsHandler$1$2 - result 1 (error) is always nil
cmd/cloudwatch-generatedemodata/main_test.go:87:88: TestGetMetricsHandler$1$2 - result 1 (error) is always nil

[mvdan]

Note that I've just fixed three false positive issues in unparam; please try the latest master. If there's still a bug, please open an issue.

[cloudlena]

Thanks, @mvdan. Unfortunately, I'm still getting the above problem. I have created mvdan/unparam#37.

[mvdan]

For what it's worth, the upstream issue is now fixed. Grabbing the latest unparam should get rid of the false positive.

[cloudlena]

I can confirm that the upstream issue it's fixed now. Thanks so much for the great work and short response times, @mvdan!

@jirfag, can we get this into a new release of golangci-lint?

[jirfag]

@mastertinner unparam contains too many changes, including breaking change with an inability to select a callgraph algorithm. I can include it only into the next minor release v1.14.0 but not into v1.13.2.

[cloudlena]

@jirfag, can we create that release then? For me this is blocking because I cannot upgrade to 1.13 as long as that unparam bug is still in there.

[jirfag]

Sure, I’m planning v1.14 in 1-2 weeks. Before that I would like to make only patch-release fixes