golangci-lint still reports false positives even with the latest unparam version

Created on 20 Sep 2020  Â·  9Comments  Â·  Source: golangci/golangci-lint

Hi. In the previous issue https://github.com/golangci/golangci-lint/issues/1375, I asked to update the unparam version to the latest one in golangci-lint dependencies. But it did help. it still reports the same false positives that unparam executed directly does not. I installed golangci-lint from the latest commit:

go get github.com/golangci/golangci-lint/cmd/golangci-lint@5efb842164996ba97d49d668cb4cb51c87810846
  • [+] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
  • [+] Yes, I've searched similar issues on GitHub and didn't find any.
  • [+] Yes, I've included all information below (version, config, etc).

Please include the following information:

Version of golangci-lint

$ golangci-lint --version
golangci-lint has version v1.31.1-0.20200919235159-5efb84216499 built from (unknown, mod sum: "h1:z7AU6wMUqgyuTEOPtbLarycm9FJh4JEaHGDybeVkahc=") on (unknown)

Config file

$ cat .golangci.yml
linters:
  disable-all: true
  enable:
    - bodyclose
    - deadcode
    - dogsled
    - dupl
    - errcheck
    - exhaustive
    - gochecknoinits
    - goconst
    - gocritic
    - gocyclo
    - godot
    - goerr113
    - gofumpt
    - goimports
    - golint
    - gomnd
    - gosec
    - gosimple
    - govet
    - ineffassign
    - lll
    - misspell
    - nakedret
    - noctx
    - nolintlint
    - scopelint
    - sqlclosecheck
    - staticcheck
    - structcheck
    - testpackage
    - unconvert
    - unparam
    - unused
    - varcheck

  # don't enable:
  # - asciicheck
  # - depguard
  # - exportloopref
  # - funlen
  # - gochecknoglobals
  # - gocognit
  # - godox
  # - gofmt
  # - goheader
  # - gomodguard
  # - goprintffuncname
  # - interfacer
  # - maligned
  # - nestif
  # - prealloc
  # - rowserrcheck
  # - stylecheck
  # - typecheck
  # - whitespace
  # - wsl

linters-settings:
  exhaustive:
    default-signifies-exhaustive: true
  goconst:
    min-occurrences: 2
  godot:
    check-all: true
  goimports:
    local-prefixes: github.com/georgysavva/news-app
  misspell:
    locale: US
  unparam:
    check-exported: true


issues:
  exclude-use-default: false
  exclude:
    - (comment on exported (method|function|type|const)|should have( a package)? comment|comment should be of the form)

  max-same-issues: 0

Go environment

$ go version && go env
go version go1.14 darwin/amd64
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/georgysavva/Library/Caches/go-build"
GOENV="/Users/georgysavva/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=" -mod="
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/georgysavva/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/georgysavva/repos/news-app/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/gd/lq1x2k352p3d74_5zby2jjbc0000gn/T/go-build280445042=/tmp/go-build -gno-record-gcc-switches -fno-common"

Verbose output of running

$ golangci-lint cache clean
$ golangci-lint run -v
INFO [config_reader] Config search paths: [./ /Users/georgysavva/repos/news-app /Users/georgysavva/repos /Users/georgysavva /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 34 linters: [bodyclose deadcode dogsled dupl errcheck exhaustive gochecknoinits goconst gocritic gocyclo godot goerr113 gofumpt goimports golint gomnd gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint scopelint sqlclosecheck staticcheck structcheck testpackage unconvert unparam unused varcheck] 
INFO [loader] Go packages loading at mode 575 (types_sizes|deps|exports_file|name|compiled_files|files|imports) took 246.016475ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 560.125µs 
INFO [linters context/goanalysis] analyzers took 10.318800553s with top 10 stages: buildir: 5.847617682s, exhaustive: 571.976189ms, inspect: 433.802669ms, fact_deprecated: 244.155438ms, printf: 196.807272ms, fact_purity: 196.373357ms, ctrlflow: 194.172168ms, buildssa: 154.770523ms, SA4011: 50.511795ms, unreachable: 46.892129ms 
INFO [linters context/goanalysis] analyzers took 49.158284ms with top 10 stages: buildir: 46.241318ms, U1000: 2.916966ms 
INFO [runner] Issues before processing: 30, after processing: 5 
INFO [runner] Processors filtering stat (out/in): path_prettifier: 30/30, exclude: 5/30, uniq_by_line: 5/5, path_shortener: 5/5, filename_unadjuster: 30/30, skip_dirs: 30/30, autogenerated_exclude: 30/30, source_code: 5/5, severity-rules: 5/5, skip_files: 30/30, identifier_marker: 30/30, diff: 5/5, max_same_issues: 5/5, path_prefixer: 5/5, sort_results: 5/5, cgo: 30/30, exclude-rules: 5/5, nolint: 5/5, max_per_file_from_linter: 5/5, max_from_linter: 5/5 
INFO [runner] processing took 1.356551ms with stages: identifier_marker: 485.031µs, path_prettifier: 278.252µs, autogenerated_exclude: 195.353µs, nolint: 154.38µs, exclude: 118.149µs, skip_dirs: 69.944µs, source_code: 30.071µs, cgo: 13.838µs, filename_unadjuster: 2.975µs, uniq_by_line: 2.148µs, path_shortener: 1.894µs, max_from_linter: 1.758µs, max_same_issues: 724ns, max_per_file_from_linter: 622ns, diff: 306ns, skip_files: 282ns, severity-rules: 238ns, exclude-rules: 238ns, sort_results: 211ns, path_prefixer: 137ns 
INFO [runner] linters took 3.540998041s with stages: goanalysis_metalinter: 3.446402066s, unused: 93.112493ms 
pkg/inmemory/inmemory.go:18:83: (*Storage).ReplaceArticles - result 0 (error) is always nil (unparam)
func (s *Storage) ReplaceArticles(_ context.Context, articles []*article.Article) error {
                                                                                  ^
pkg/inmemory/inmemory.go:37:103: (*Storage).GetArticles - result 1 (error) is always nil (unparam)
func (s *Storage) GetArticles(_ context.Context, categories, providers []string) ([]*article.Article, error) {
                                                                                                      ^
pkg/inmemory/inmemory.go:45:86: (*Storage).GetArticle - result 1 (error) is always nil (unparam)
func (s *Storage) GetArticle(_ context.Context, articleID string) (*article.Article, error) {
                                                                                     ^
pkg/inmemory/inmemory.go:52:63: (*Storage).GetCategories - result 1 (error) is always nil (unparam)
func (s *Storage) GetCategories(_ context.Context) ([]string, error) {
                                                              ^
pkg/inmemory/inmemory.go:62:62: (*Storage).GetProviders - result 1 (error) is always nil (unparam)
func (s *Storage) GetProviders(_ context.Context) ([]string, error) {
                                                             ^
INFO File cache stats: 10 entries of total size 15.0KiB 
INFO Memory: 40 samples, avg is 551.2MB, max is 941.2MB 
INFO Execution took 3.808125916s  

bug help wanted question

All 9 comments

Hi. Any update on this?

Will spend sometimes this week to check on this.

@georgysavva not sure if I missed something, I tried to run golangci-lint with your repo, seems fine

```shell script
$î‚° golangci-lint run -v
INFO [config_reader] Config search paths: [./ /home/tammach/go/src/github.com/georgysavva/news-app /home/tammach/go/src/github.com/georgysavva /home/tammach/go/src/github.com /home/tammach/go/src /home/tammach/go /home/tammach /home /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 34 linters: [bodyclose deadcode dogsled dupl errcheck exhaustive gochecknoinits goconst gocritic gocyclo godot goerr113 gofumpt goimports golint gomnd gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint scopelint sqlclosecheck staticcheck structcheck testpackage unconvert unparam unused varcheck]
INFO [loader] Go packages loading at mode 575 (compiled_files|deps|exports_file|files|name|imports|types_sizes) took 173.58131ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 837.935µs
INFO [linters context/goanalysis] analyzers took 0s with no stages
INFO [linters context/goanalysis] analyzers took 0s with no stages
INFO [runner] Issues before processing: 25, after processing: 0
INFO [runner] Processors filtering stat (out/in): filename_unadjuster: 25/25, autogenerated_exclude: 25/25, exclude: 0/25, skip_files: 25/25, skip_dirs: 25/25, cgo: 25/25, identifier_marker: 25/25, path_prettifier: 25/25
INFO [runner] processing took 733.159µs with stages: identifier_marker: 395.317µs, path_prettifier: 120.791µs, exclude: 77.428µs, autogenerated_exclude: 77.378µs, skip_dirs: 50.617µs, cgo: 5.58µs, filename_unadjuster: 1.833µs, nolint: 1.182µs, uniq_by_line: 600ns, max_same_issues: 411ns, diff: 321ns, source_code: 250ns, sort_results: 250ns, skip_files: 240ns, severity-rules: 220ns, exclude-rules: 200ns, max_from_linter: 160ns, max_per_file_from_linter: 160ns, path_shortener: 140ns, path_prefixer: 81ns
INFO [runner] linters took 57.73649ms with stages: goanalysis_metalinter: 56.45692ms, unused: 480.83µs
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 4 samples, avg is 73.0MB, max is 74.3MB
INFO Execution took 259.510592ms

$î‚° golangci-lint run -E unparam -v
INFO [config_reader] Config search paths: [./ /home/tammach/go/src/github.com/georgysavva/news-app /home/tammach/go/src/github.com/georgysavva /home/tammach/go/src/github.com /home/tammach/go/src /home/tammach/go /home/tammach /home /]
INFO [config_reader] Used config file .golangci.yml
INFO [lintersdb] Active 34 linters: [bodyclose deadcode dogsled dupl errcheck exhaustive gochecknoinits goconst gocritic gocyclo godot goerr113 gofumpt goimports golint gomnd gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint scopelint sqlclosecheck staticcheck structcheck testpackage unconvert unparam unused varcheck]
INFO [loader] Go packages loading at mode 575 (exports_file|files|name|types_sizes|compiled_files|deps|imports) took 178.930434ms
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 605.729µs
INFO [linters context/goanalysis] analyzers took 0s with no stages
INFO [linters context/goanalysis] analyzers took 0s with no stages
INFO [runner] Issues before processing: 25, after processing: 0
INFO [runner] Processors filtering stat (out/in): cgo: 25/25, filename_unadjuster: 25/25, path_prettifier: 25/25, exclude: 0/25, skip_dirs: 25/25, autogenerated_exclude: 25/25, skip_files: 25/25, identifier_marker: 25/25
INFO [runner] processing took 1.106657ms with stages: identifier_marker: 613.554µs, path_prettifier: 177.359µs, exclude: 125.31µs, autogenerated_exclude: 101.745µs, skip_dirs: 77.528µs, cgo: 3.887µs, filename_unadjuster: 2.124µs, uniq_by_line: 983ns, nolint: 912ns, max_same_issues: 631ns, diff: 410ns, source_code: 320ns, exclude-rules: 291ns, skip_files: 270ns, severity-rules: 261ns, sort_results: 261ns, max_from_linter: 250ns, path_shortener: 240ns, max_per_file_from_linter: 170ns, path_prefixer: 151ns
INFO [runner] linters took 61.450739ms with stages: goanalysis_metalinter: 59.883478ms, unused: 358.786µs
INFO File cache stats: 0 entries of total size 0B
INFO Memory: 4 samples, avg is 73.0MB, max is 73.6MB
INFO Execution took 267.821974ms

```

Hey! Sorry for the late response.
Did you run golangci-lint on this branch https://github.com/georgysavva/news-app/tree/unparam-false-positive ?
Because the false-positive errors don't exist in the master. I forgot to mention the separate branch in this new issue too and mentioned it only in https://github.com/golangci/golangci-lint/issues/1375

BTW here is how I run unparam independently on my project to ensure that it doesn't produce those false-positives:

unparam -exported ./...  

I just take a look, please re-try after toggling check-exported param in config. I didn't check unparam code, as behavior is consistent with check-exported value.

diff --git a/.golangci.yml b/.golangci.yml
index f82b39a..61688d5 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -70,7 +70,7 @@ linters-settings:
   misspell:
     locale: US
   unparam:
-    check-exported: true
+    check-exported: false

shell script $ golangci-lint run -E unparam -v INFO [config_reader] Config search paths: [./ /home/tammach/go/src/github.com/georgysavva/news-app /home/tammach/go/src/github.com/georgysavva /home/tammach/go/src/github.com /home/tammach/go/src /home/tammach/go /home/tammach /home /] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 34 linters: [bodyclose deadcode dogsled dupl errcheck exhaustive gochecknoinits goconst gocritic gocyclo godot goerr113 gofumpt goimports golint gomnd gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint scopelint sqlclosecheck staticcheck structcheck testpackage unconvert unparam unused varcheck] INFO [loader] Go packages loading at mode 575 (compiled_files|files|imports|types_sizes|deps|exports_file|name) took 169.074217ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 1.319319ms INFO [linters context/goanalysis] analyzers took 0s with no stages INFO [linters context/goanalysis] analyzers took 0s with no stages INFO [runner] Issues before processing: 25, after processing: 0 INFO [runner] Processors filtering stat (out/in): skip_files: 25/25, identifier_marker: 25/25, exclude: 0/25, skip_dirs: 25/25, autogenerated_exclude: 25/25, cgo: 25/25, path_prettifier: 25/25, filename_unadjuster: 25/25 INFO [runner] processing took 1.128828ms with stages: identifier_marker: 639.106µs, path_prettifier: 182.514µs, exclude: 131.087µs, autogenerated_exclude: 87.154µs, skip_dirs: 76.384µs, cgo: 5µs, filename_unadjuster: 2.354µs, nolint: 1.112µs, uniq_by_line: 641ns, max_same_issues: 621ns, diff: 490ns, source_code: 331ns, skip_files: 300ns, sort_results: 291ns, max_from_linter: 282ns, severity-rules: 280ns, exclude-rules: 260ns, path_prefixer: 250ns, path_shortener: 201ns, max_per_file_from_linter: 170ns INFO [runner] linters took 55.742688ms with stages: goanalysis_metalinter: 54.180742ms, unused: 346.984µs INFO File cache stats: 0 entries of total size 0B INFO Memory: 4 samples, avg is 73.0MB, max is 73.8MB INFO Execution took 232.822219ms

Sorry, I don't understand. Why do I need to toggle the check-exported param? I want unparam to check exported functions too, this is why I set check-exported to true, but it produces false-positives:

georgysavva news-app %golangci-lint run -E unparam -v
INFO [config_reader] Config search paths: [./ /Users/georgysavva/repos/news-app /Users/georgysavva/repos /Users/georgysavva /Users /] 
INFO [config_reader] Used config file .golangci.yml 
INFO [lintersdb] Active 34 linters: [bodyclose deadcode dogsled dupl errcheck exhaustive gochecknoinits goconst gocritic gocyclo godot goerr113 gofumpt goimports golint gomnd gosec gosimple govet ineffassign lll misspell nakedret noctx nolintlint scopelint sqlclosecheck staticcheck structcheck testpackage unconvert unparam unused varcheck] 
INFO [loader] Go packages loading at mode 575 (types_sizes|exports_file|imports|name|compiled_files|deps|files) took 203.945922ms 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 584.806µs 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [runner] Issues before processing: 30, after processing: 5 
INFO [runner] Processors filtering stat (out/in): sort_results: 5/5, diff: 5/5, max_per_file_from_linter: 5/5, path_prefixer: 5/5, max_from_linter: 5/5, filename_unadjuster: 30/30, skip_dirs: 30/30, autogenerated_exclude: 30/30, severity-rules: 5/5, skip_files: 30/30, identifier_marker: 30/30, exclude: 5/30, nolint: 5/5, uniq_by_line: 5/5, max_same_issues: 5/5, source_code: 5/5, path_shortener: 5/5, cgo: 30/30, path_prettifier: 30/30, exclude-rules: 5/5 
INFO [runner] processing took 1.308844ms with stages: identifier_marker: 486.739µs, path_prettifier: 254.059µs, nolint: 168.666µs, autogenerated_exclude: 160.438µs, exclude: 116.449µs, skip_dirs: 70.935µs, source_code: 32.573µs, cgo: 7.409µs, max_same_issues: 2.331µs, uniq_by_line: 2.23µs, max_from_linter: 1.973µs, filename_unadjuster: 1.595µs, path_shortener: 1.536µs, max_per_file_from_linter: 600ns, diff: 287ns, skip_files: 242ns, exclude-rules: 232ns, severity-rules: 210ns, path_prefixer: 185ns, sort_results: 155ns 
INFO [runner] linters took 96.117273ms with stages: goanalysis_metalinter: 94.168198ms, unused: 490.103µs 
pkg/inmemory/inmemory.go:18:83: (*Storage).ReplaceArticles - result 0 (error) is always nil (unparam)
func (s *Storage) ReplaceArticles(_ context.Context, articles []*article.Article) error {
                                                                                  ^
pkg/inmemory/inmemory.go:37:103: (*Storage).GetArticles - result 1 (error) is always nil (unparam)
func (s *Storage) GetArticles(_ context.Context, categories, providers []string) ([]*article.Article, error) {
                                                                                                      ^
pkg/inmemory/inmemory.go:45:86: (*Storage).GetArticle - result 1 (error) is always nil (unparam)
func (s *Storage) GetArticle(_ context.Context, articleID string) (*article.Article, error) {
                                                                                     ^
pkg/inmemory/inmemory.go:52:63: (*Storage).GetCategories - result 1 (error) is always nil (unparam)
func (s *Storage) GetCategories(_ context.Context) ([]string, error) {
                                                              ^
pkg/inmemory/inmemory.go:62:62: (*Storage).GetProviders - result 1 (error) is always nil (unparam)
func (s *Storage) GetProviders(_ context.Context) ([]string, error) {
                                                             ^
INFO File cache stats: 1 entries of total size 2.2KiB 
INFO Memory: 5 samples, avg is 70.9MB, max is 71.1MB 
INFO Execution took 313.905387ms   

And when I run unparam directly:

unparam -exported ./...

It also checks the exported functions (because of the -exported flag), but it doesn't produce those false-positives:

georgysavva news-app %unparam -exported ./...
georgysavva news-app %

Thanks!

This might be the gap based on the fact that I didn't check code for unpram, will give it another crack next week.

Hi. Any update on this?

Hi, sorry I am a little bit occupied at work recently, let me un-assign myself from this github issue, so that others can jump in and help.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kipply picture kipply  Â·  4Comments

lnshi picture lnshi  Â·  4Comments

alessio picture alessio  Â·  4Comments

cyriltovena picture cyriltovena  Â·  5Comments

liubog2008 picture liubog2008  Â·  3Comments