Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unparam false positive since upgrading to 1.13 #367

Closed
cloudlena opened this issue Jan 21, 2019 · 10 comments
Closed

unparam false positive since upgrading to 1.13 #367

cloudlena opened this issue Jan 21, 2019 · 10 comments
Labels
false positive An error is reported when one does not exist

Comments

@cloudlena
Copy link

cloudlena commented Jan 21, 2019

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
  1. 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
  1. 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"
  1. 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
Copy link
Member

jirfag commented Jan 21, 2019

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

@cloudlena
Copy link
Author

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
Copy link

mvdan commented Jan 24, 2019

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
Copy link
Author

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

@mvdan
Copy link

mvdan commented Jan 24, 2019

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

@cloudlena
Copy link
Author

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
Copy link
Member

jirfag commented Jan 26, 2019

@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
Copy link
Author

@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
Copy link
Member

jirfag commented Jan 26, 2019

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

jirfag added a commit that referenced this issue Feb 11, 2019
$ git cherry -v cc9d2fb52971 fbb59629db34
+ 7362051ae01a0e35956c077c3be5505c70edd200 testdata: add more regression
tests
+ a88ca0234e2c3732a53cd49514fb3877a5d9f1f5 properly record which methods
implement interfaces
+ b762b0b27fa23ebbdfc31df1af4097cfd89a17f6 work properly with method
wrapper functions
+ f59bb08c5c7429b88e0c6e2399b12e71e8d950db testdata: consistently use
tabs
+ 46a5101c55d03117b263b4c5161e5d01353311c1 replace more callgraph code
with plain SSA
+ 1679b9996abdc6431c2147a133e5223ebb86ea60 rewrite foo(bar()) code to
work without callgraph
+ 71b5df77c291f8c5ead5f9ff0a69e0eebf3ae5b2 rewrite "called in return"
code without callgraph
+ fc5b1c74f563d4f2f9ee7d91eda648fc7baebf67 check: replace last use of
callgraph
+ 229ad68a599e2622cf9d3bb87a385b158efe736f fix a minor false negative in
callExtract
+ fbb59629db34a0f69275bc336cc8c3a4dd9fbe5d fix up another false negative
in testdata
@tpounds tpounds added the false positive An error is reported when one does not exist label Sep 25, 2019
@jirfag
Copy link
Member

jirfag commented Oct 15, 2019

unparam was updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
false positive An error is reported when one does not exist
Projects
None yet
Development

No branches or pull requests

4 participants