Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable comparison linter for Is(target error) bool methods #11

Closed
kolyshkin opened this issue Jul 2, 2021 · 2 comments
Closed

disable comparison linter for Is(target error) bool methods #11

kolyshkin opened this issue Jul 2, 2021 · 2 comments
Labels
bug Something isn't working help wanted Extra attention is needed

Comments

@kolyshkin
Copy link
Contributor

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.

@polyfloyd
Copy link
Owner

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 polyfloyd added bug Something isn't working help wanted Extra attention is needed labels Jul 2, 2021
@kolyshkin
Copy link
Contributor Author

or adding a linter exception for now

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants