`unused` linter in golangci-lint reports unused var for named interface param

[kpitt]

When declaring an interface method, providing names for the parameters can act as a form of documentation, and is necessary if you want to refer to a parameter in the doc comments. However, it is not possible for the parameter to be "used" because an interface method declaration has no body, so these parameters should not be marked as unused even if the ParametersAreUsed option is false. I did not see this issue in earlier versions, so it may be a new behavior since the AST-based rewrite.

Note that I am running this through golangci-lint because I couldn't figure out a way to configure the ParametersAreUsed option when calling staticcheck directly. The version of golangci-lint I am using has a dependency on the honnef.co/go/tools v0.4.7 module.

Lint Command Output

$ golangci-lint run -c golangci.yml main.go
main.go:7:7: var `testCase` is unused (unused)
	Test(testCase string)
	    ^

Test Code

main.go Source Code

package main

import "fmt"

type Tester interface {
	// Test runs the test case specified by testCase.
	Test(testCase string)
}

type TesterImpl struct{}

func (t *TesterImpl) Test(testCase string) {
	fmt.Println(testCase)
}

func main() {
	var t Tester = &TesterImpl{}
	t.Test("NamedInterfaceParams")
}

golangci-list Config

linters:
  disable-all: true
  enable:
    - unused

linters-settings:
  unused:
    parameters-are-used: false

Environment Info

$ golangci-lint version
golangci-lint has version v1.59.1 built with go1.22.3 from (unknown, modified: ?, mod sum: "h1:CRRLu1JbhK5avLABFJ/OHVSQ0Ie5c4ulsOId1h3TTks=") on (unknown)

$ go version
go version go1.22.3 darwin/arm64

$ go env
GO111MODULE=''
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/kenny/Library/Caches/go-build'
GOENV='/Users/kenny/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/kenny/.go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/kenny/.go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/opt/homebrew/Cellar/go/1.22.3/libexec'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/opt/homebrew/Cellar/go/1.22.3/libexec/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='cc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/tmp/test/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/n9/s58k73qj3vg713wlsv2tr3nh0000gn/T/go-build1263784959=/tmp/go-build -gno-record-gcc-switches -fno-common'

[dominikh]

parameters-are-used is an experimental, unsupported option. The only valid use of unused currently is as used by staticcheck.

[dominikh]

That said, this one might be easy enough to fix.

[kpitt]

That said, this one might be easy enough to fix.

I already took a look at the code before filing an issue, and it seems pretty straight-forward. I can submit a pull request if you'd like.

[dominikh]

That said, this one might be easy enough to fix.

I already took a look at the code before filing an issue, and it seems pretty straight-forward. I can submit a pull request if you'd like.

Thank you, but I'm significantly faster at making changes than reviewing and merging PRs.

[kpitt]

I'm significantly faster at making changes than reviewing and merging PRs.

:+1: I thought that might be the case, but figured I would offer anyway. Just trying to be a good open-source citizen. :grin:

[krhubert]

This feature would be great. unused is much more powerful with parameters-are-used set to false.

When I think about this feature, I also think about type in the error output. Let me demonstrate the usecase.

Problem

There are common arguments like context.Context in interface methods that are not used by all types of implementations. It would be great to have a way to do not mark those as unused.

type I interface {
  Do(ctx context.Context, i int, s string) error
}

type Mock struct {}

func (Mock) Do(ctx context.Context, i int, s string) error {
  fmt.Println(i, s)
  return nil
}

the output of unused is:

var ctx is unused (U1000)

Solution

The desired output returns no errors in the example above. Let me share my 2 solutions to this issue:

1. Along with parameters-are-used: false, pass types-are-used: ["context.Context" ....] which takes precedence over parameters-are-used: false

2. Make output move verbose: var ctx context.Context is unused (U1000) - this approach allows to have more control over what should be excluded from linting by tools like golangci-lint. I might write a rule to exclude var ctx is unused but having not only a name but type as well reduce probability of missing a valid unused argument.

[yan-kalc]

That said, this one might be easy enough to fix.

I already took a look at the code before filing an issue, and it seems pretty straight-forward. I can submit a pull request if you'd like.

Thank you, but I'm significantly faster at making changes than reviewing and merging PRs.

Hi, @dominikh , any chances that you'll work on this one? I also find ParametersAreUsed quite useful, but this case with interfaces doesn't allow to enable it.

[dominikh]

any chances that you'll work on this one?

Eventually, yes. It's not high on my list of priorities, however.