3

Q: How do I resolve this madness between the ireturn and nolintlint linters?

Details:

I have a Golang function with this signature

func NewClientCredentialsTokenSource(
    issuer string,
    clientId string,
    clientSecret string,
    scope []string,
) (oauth2.TokenSource, error) {

When I run golangci-lint v1.43.0 it reports

golangci-lint run
oidc/token_utils.go:19:1: NewClientCredentialsTokenSource returns interface (golang.org/x/oauth2.TokenSource) (ireturn)
func NewClientCredentialsTokenSource(

Since the function has only two return params it is easy to deduce it is complaining about oauth2.TokenSource and not error.

The downstream function called by NewClientCredentialsTokenSource returns an instance of oauth2.TokenSource so I don't have a concrete type to return. I have no choice but to return the oauth2.TokenSource interface.

So I add a lint exception to the function like this:

//nolint:ireturn
func NewClientCredentialsTokenSource(
    issuer string,
    clientId string,
    clientSecret string,
    scope []string,
) (oauth2.TokenSource, error) {

You'd think that should fix it but no! Now there's a new lint issue is reported:

golangci-lint run
oidc/token_utils.go:19:1: directive `//nolint:ireturn` is unused for linter "ireturn" (nolintlint)
//nolint:ireturn

So now I'm chasing my tail. ireturn complains I am returning an interface. I add an exception for that function only to have nolintlint complain I have an exception that doesn't apply.

What's a guy to do?

HairOfTheDog
  • 2,489
  • 2
  • 29
  • 35

3 Answers3

3

I tried adding the allow rule as suggested by Nico Huysamen but that opened a can of worms. In the end I still want to believe in the ireturn linter and don't want to disable it yet. I figure the cleanest way to resolve this issue is to add an exception for both linters, ireturn and nolintlint as shown here:

//nolint:nolintlint,ireturn
func NewClientCredentialsTokenSource(
    issuer string,
    clientId string,
    clientSecret string,
    scope []string,
) (oauth2.TokenSource, error) {

Update 5/25/2022:

Perhaps a better solution is shown below. For some reason the ireturn exception has to go inside the function signature.

func NewPasswordGrantTokenSource( //nolint:ireturn
    issuer string,
    clientId string,
    clientSecret string,
    username string,
    password string,
    scope []string,
) (oauth2.TokenSource, error) {
HairOfTheDog
  • 2,489
  • 2
  • 29
  • 35
2

You can allow this in .golangci.yml as mentioned above but as it replaces the standard defaults, you need to include them too so you would need:

  ireturn:
    # ireturn allows using `allow` and `reject` settings at the same time.
    # Both settings are lists of the keywords and regular expressions matched to interface or package names.
    # keywords:
    # - `empty` for `interface{}`
    # - `error` for errors
    # - `stdlib` for standard library
    # - `anon` for anonymous interfaces

    # By default, it allows using errors, empty interfaces, anonymous interfaces,
    # and interfaces provided by the standard library.
    allow:
      - anon
      - error
      - empty
      - stdlib
      # You can specify idiomatic endings for interface
      - (or|er)$
      # Your custom interfaces go here:
      - golang.org/x/oauth2.TokenSource
0

You can add the interface to ireturn's list of allowed interfaces.

ireturn --allow="golang.org/x/oauth2.TokenSource"

or via the linter-settings section in .golangci.yml

linters-settings:
  ireturn:
    allow:
      - golang.org/x/oauth2.TokenSource
Nico Huysamen
  • 10,217
  • 9
  • 62
  • 88
  • I tried your suggestion but it opened a can of worms. After adding that one interface as an exception in .golangci.yml the linter then complained about every method/function that returned `error` so I added an exception for `error`. Then it complained about every function that returns `net/http.Handler` (something it didn't previously do.) After that came the ultimate insult, it complained about functions that return `interface{}` and I couldn't find a way to add an exception for `interface{}` (`interface{}` has been used in limited cases due to the lack of generics in Go 1.17.3.) – HairOfTheDog May 26 '22 at 16:45
  • 1
    Ouch. Maybe then just disable ‘ireturn’ – Nico Huysamen May 26 '22 at 18:01