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

[georgysavva]

Hi. In the previous issue #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  

[georgysavva]

Hi. Any update on this?

[sayboras]

Will spend sometimes this week to check on this.

[sayboras]

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

$ 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                  

[georgysavva]

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 #1375

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

unparam -exported ./...  

[sayboras]

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

$ 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                  

[georgysavva]

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:

$ 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!

[sayboras]

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

[georgysavva]

Hi. Any update on this?

[sayboras]

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.