go-errorlint icon indicating copy to clipboard operation
go-errorlint copied to clipboard

disable comparison linter for `Is(target error) bool` methods

Open kolyshkin opened this issue 3 years ago • 2 comments

I wrote some code with a custom error type:

var ErrInvalidConfig = errors.New("invalid configuration")

// invalidConfig type is needed to make any error returned from Validator
// to be ErrInvalidConfig.
type invalidConfig struct {
	err error
}

func (e *invalidConfig) Is(target error) bool {
	return target == ErrInvalidConfig
}

...

Basically, the idea is to make errors.Is(err, ErrInvalidConfig) to return true for every error which is wrapped in invalidConfig type.

Now, the comparison linter complains:

validator.go:42:9: comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
	return target == ErrInvalidConfig
	       ^

Which is obviously incorrect because

  1. The argument of Is method, i.e. target, is not supposed to be unwrapped.
  2. Using errors.Is in the above example will result in infinite recursion.

There's a similar code in goland standard library (src/syscall/syscall_unix.go):

func (e Errno) Is(target error) bool {
          switch target {
          case oserror.ErrPermission:
                  return e == EACCES || e == EPERM
          case oserror.ErrExist:
                  return e == EEXIST || e == ENOTEMPTY
          case oserror.ErrNotExist:
                  return e == ENOENT
          }
          return false
  }

Note that target is compared with oserror.* directly.

kolyshkin avatar Jul 02 '21 01:07 kolyshkin

Yup, you should be able to make that comparison.

I think the best way to resolve this is to check whether the expression is in the scope of a function that adheres tot the Is signature. A linter exception is also possible, but I'd rather not make a roundtrip to the caveat section of the readme needed for something so common.

Unfortunately, I am very short on time and energy. If you need this to be resolved soon I recommend submitting a PR or adding a linter exception for now.

polyfloyd avatar Jul 02 '21 15:07 polyfloyd

or adding a linter exception for now

That is what I did for now. Might try to properly fix this in go-errorlint later...

kolyshkin avatar Jul 06 '21 22:07 kolyshkin