golangci-lint returns false positive on unused-parameter

piyushdatazip commented 1 year ago

Welcome

[X] Yes, I'm using a binary release within 2 latest major releases. Only such installations are supported.
[X] Yes, I've searched similar issues on GitHub and didn't find any.
[X] Yes, I've included all information below (version, config, etc.).
[x] Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)

Description of the problem

func (a *AWSCloudProvider) UpdateNodeMachine(node NodeGroup, machineType string) error {
    if !isValidNodeGroup(ValidNodeGroups, node) {
        return fmt.Errorf("not valid nodegroup: %s", node)
    }

    if node == ClickhouseNG {
        return a.upgradeClickhouseNodes(machineType)
    }
    return fmt.Errorf("nodegroup upgrade not supported: %s", node)
}

Code sample is added above, golangci lint is reporting error

unused-parameter: parameter 'node' seems to be unused, consider removing or renaming it as _ (revive)
func (gcp *GoogleCloudProvider) UpdateNodeMachine(node NodeGroup, machineType string) error {
                                                  ^

as we can see node variable is being used here, would like to report this issue and get it fixed in next patch release

Version of golangci-lint

```console $ golangci-lint --version golangci-lint has version v1.52.0 built with go1.19.1 from (unknown, mod sum: "h1:T7w3tuF1goz64qGV+ML4MgysSl/yUfA3UZJK92oE48A=") on (unknown) ```

Configuration file

```console $ cat .golangci.yml linters-settings: depguard: # list-type: blacklist # packages: # # logging is allowed only by logutils.Log, logrus # # is allowed to use only in logutils package # - github.com/sirupsen/logrus # packages-with-error-message: # - github.com/sirupsen/logrus: "logging is allowed only by logutils.Log" dupl: threshold: 100 exhaustive: default-signifies-exhaustive: false funlen: lines: 100 statements: 50 gci: local-prefixes: github.com/golangci/golangci-lint goconst: min-len: 2 min-occurrences: 2 gocritic: enabled-tags: - diagnostic - experimental - opinionated - performance - style disabled-checks: - dupImport # https://github.com/go-critic/go-critic/issues/845 - ifElseChain - octalLiteral - whyNoLint - wrapperFunc gocyclo: min-complexity: 15 goimports: local-prefixes: github.com/golangci/golangci-lint golint: min-confidence: 0 gomnd: settings: mnd: # don't include the "operation" and "assign" checks: - argument - case - condition - return gosec: settings: exclude: -G204 govet: check-shadowing: false settings: printf: funcs: - (github.com/golangci/golangci-lint/pkg/logutils.Log).Infof - (github.com/golangci/golangci-lint/pkg/logutils.Log).Warnf - (github.com/golangci/golangci-lint/pkg/logutils.Log).Errorf - (github.com/golangci/golangci-lint/pkg/logutils.Log).Fatalf lll: line-length: 950 maligned: suggest-new: true misspell: # Correct spellings using locale preferences for US or UK. # Setting locale to US will correct the British spelling of 'colour' to 'color'. # Default is to use a neutral variety of English. locale: US ignore-words: - eles nolintlint: allow-leading-space: true # don't require machine-readable nolint directives (i.e. with no leading space) allow-unused: false # report any unused nolint directives require-explanation: false # don't require an explanation for nolint directives require-specific: false # don't require nolint directives to be specific about which linter is being skipped linters: # please, do not use `enable-all`: it's deprecated and will be removed soon. # inverted configuration with `enable-all` and `disable` is not scalable during updates of golangci-lint disable-all: true enable: - bodyclose # - dupl doesn't understand code properly # - deadcode - dogsled - errcheck # - exhaustive # - funlen # - goconst # - gocritic # - gocyclo - gofmt - goimports - revive - gosec # - gomnd # - goprintffuncname - gosimple - govet - ineffassign # - interfacer - lll - misspell # - nakedret # - nolintlint # - rowserrcheck # - scopelint - staticcheck # - structcheck - stylecheck - typecheck # - unconvert # - unparam - unused # - varcheck - whitespace - nilnil - nilerr # don't enable: # - asciicheck # - gochecknoglobals # - gocognit # - godot # - godox # - goerr113 # - maligned # - nestif # - prealloc # - testpackage # - wsl issues: # Excluding configuration per-path, per-linter, per-text and per-source exclude-rules: - path: _test\.go linters: - gomnd # https://github.com/go-critic/go-critic/issues/926 - linters: - gocritic text: "unnecessaryDefer:" run: skip-dirs: - test/testdata_etc - internal/cache - internal/renameio - internal/robustio timeout: 5m # golangci.com configuration # https://github.com/golangci/golangci/wiki/Configuration service: golangci-lint-version: 1.45.2 # use the fixed version to not introduce new linters unexpectedly prepare: - echo "here I can run custom commands, but no preparation needed for this repo" ```

Go environment

```console $ go version && go env go version go1.19.1 darwin/arm64 GO111MODULE="on" GOARCH="arm64" GOBIN="" GOCACHE="/Users/piyushsingariya/Library/Caches/go-build" GOENV="/Users/piyushsingariya/Library/Application Support/go/env" GOEXE="" GOEXPERIMENT="" GOFLAGS="" GOHOSTARCH="arm64" GOHOSTOS="darwin" GOINSECURE="" GOMODCACHE="/Users/piyushsingariya/go/pkg/mod" GONOPROXY="" GONOSUMDB="" GOOS="darwin" GOPATH="/Users/piyushsingariya/go" GOPRIVATE="" GOPROXY="https://proxy.golang.org,direct" GOROOT="/usr/local/go" GOSUMDB="sum.golang.org" GOTMPDIR="" GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64" GOVCS="" GOVERSION="go1.19.1" GCCGO="gccgo" AR="ar" CC="clang" CXX="clang++" CGO_ENABLED="1" GOMOD="/Users/piyushsingariya/code/cost-license/go.mod" GOWORK="" CGO_CFLAGS="-g -O2" CGO_CPPFLAGS="" CGO_CXXFLAGS="-g -O2" CGO_FFLAGS="-g -O2" CGO_LDFLAGS="-g -O2" PKG_CONFIG="pkg-config" GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/sb/zsz2k81j3l53wl8yl35nwnfw0000gn/T/go-build2312768786=/tmp/go-build -gno-record-gcc-switches -fno-common" ```

Verbose output of running

```console $ golangci-lint cache clean $ golangci-lint run -v INFO [config_reader] Config search paths: [./ /Users/piyushsingariya/code/cost-license /Users/piyushsingariya/code /Users/piyushsingariya /Users /] INFO [config_reader] Used config file .golangci.yml INFO [lintersdb] Active 19 linters: [bodyclose dogsled errcheck gofmt goimports gosec gosimple govet ineffassign lll misspell nilerr nilnil revive staticcheck stylecheck typecheck unused whitespace] INFO [loader] Go packages loading at mode 575 (exports_file|files|imports|name|compiled_files|deps|types_sizes) took 359.202208ms INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 2.122833ms INFO [linters_context/goanalysis] analyzers took 0s with no stages INFO [runner] Issues before processing: 211, after processing: 1 INFO [runner] Processors filtering stat (out/in): autogenerated_exclude: 211/211, max_from_linter: 1/1, source_code: 1/1, path_shortener: 1/1, cgo: 211/211, filename_unadjuster: 211/211, uniq_by_line: 1/2, diff: 1/1, exclude: 211/211, exclude-rules: 2/211, nolint: 2/2, max_per_file_from_linter: 1/1, severity-rules: 1/1, path_prefixer: 1/1, skip_files: 211/211, identifier_marker: 211/211, max_same_issues: 1/1, sort_results: 1/1, path_prettifier: 211/211, skip_dirs: 211/211 INFO [runner] processing took 8.26517ms with stages: exclude-rules: 4.3355ms, identifier_marker: 2.357834ms, path_prettifier: 659.959µs, autogenerated_exclude: 399.666µs, nolint: 375.792µs, skip_dirs: 83.875µs, source_code: 25.084µs, cgo: 14.334µs, filename_unadjuster: 5.833µs, max_same_issues: 2.624µs, uniq_by_line: 1.542µs, path_shortener: 1.084µs, max_from_linter: 542ns, exclude: 292ns, diff: 291ns, skip_files: 250ns, max_per_file_from_linter: 250ns, sort_results: 209ns, severity-rules: 167ns, path_prefixer: 42ns INFO [runner] linters took 139.394833ms with stages: goanalysis_metalinter: 131.046ms cloudproviders/google_cloud.go:26:51: unused-parameter: parameter 'node' seems to be unused, consider removing or renaming it as _ (revive) func (gcp *GoogleCloudProvider) UpdateNodeMachine(node NodeGroup, machineType string) error { ^ INFO File cache stats: 1 entries of total size 698B INFO Memory: 7 samples, avg is 51.2MB, max is 83.4MB INFO Execution took 507.952834ms ```

Code example or link to a public repository

```go // add your code here ```

boring-cyborg[bot] commented 1 year ago

Hey, thank you for opening your first Issue ! 🙂 If you would like to contribute we have a guide for contributors.

ldez commented 1 year ago

Hello,

You have checked the following box:

[x] Yes, I've tried with the standalone linter if available (e.g., gocritic, go vet, etc.). (https://golangci-lint.run/usage/linters/)

Can you give me the output of the run of the standalone binary of revive?

https://github.com/golangci/golangci-lint/blob/v1.52.0/go.mod#L70

piyushdatazip commented 1 year ago

Hi @ldez

$ /Users/piyushsingariya/go/bin/revive --version                                                                                                                                      
version 1.3.1

$ /Users/piyushsingariya/go/bin/revive -formatter friendly github.com/datazip/cost-license ./..                                                                                       
  ⚠  https://revive.run/r#package-comments  should have a package comment  
  /Users/piyushsingariya/code/cost-license/main.go:1:1

⚠ 1 problem (0 errors, 1 warning)

Warnings:
  1  package-comments

This is the output of latest review binary on the project. Which is not same as return by golangci-lint

ldez commented 1 year ago

Are you sure?

I created a working code from your snippet:

foo.go

```go // Package main foo package main import "fmt" func main() { } // ValidNodeGroups foo. var ValidNodeGroups []NodeGroup // ClickhouseNG foo. var ClickhouseNG = NodeGroup{} // NodeGroup foo. type NodeGroup struct { Name string } // AWSCloudProvider foo. type AWSCloudProvider struct{} // UpdateNodeMachine foo. func (a *AWSCloudProvider) UpdateNodeMachine(node NodeGroup, machineType string) error { if !isValidNodeGroup(ValidNodeGroups, node) { return fmt.Errorf("not valid nodegroup: %s", node) } if node == ClickhouseNG { return a.upgradeClickhouseNodes(machineType) } return fmt.Errorf("nodegroup upgrade not supported: %s", node) } func isValidNodeGroup(groups []NodeGroup, node NodeGroup) bool { for _, group := range groups { if group.Name == node.Name { return true } } return false } func (a *AWSCloudProvider) upgradeClickhouseNodes(machineType string) error { return nil } ```

And use your configuration.

$ revive ./...
foo.go:46:51: parameter 'machineType' seems to be unused, consider removing or renaming it as _

$ golangci-lint run 
foo.go:46:51: unused-parameter: parameter 'machineType' seems to be unused, consider removing or renaming it as _ (revive)
func (a *AWSCloudProvider) upgradeClickhouseNodes(machineType string) error {
                                                  ^

So it's not related to golangci-lint.

golangci / golangci-lint