go-errorlint
go-errorlint copied to clipboard
polyfloyd
disable comparison linter for errors from golang.org/x/sys/unix
Functions from the golang.org/x/sys/unix package wraps its error, always returning bare errno, and so it's OK to use direct error comparison.
Example:
err := unix.Rmdir(path)
if err == nil || err == unix.ENOENT {
return nil
}
On the above code, the linter says:
comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
For a package that uses a lot of unix functions, it's a burden to annotate all such code as the above example with //nolint:errorlint // unix errors are bare, but this is exactly what I had to do in runc: https://github.com/opencontainers/runc/pull/3011/commits/56e478046af2f61e69a970148823b283ef88a768
Surely, the alternative is to switch to errors.Is(err, unix.ENOENT) and the like, but in the particular case it seems redudnant.
Would be nice to have comparison linter to add an exclusion for errors returned from golang.org/x/sys/unix.* functions.
https://pkg.go.dev/golang.org/x/sys/unix#section-documentation says:
These calls return err == nil to indicate success; otherwise err represents an operating system error describing the failure and holds a value of type syscall.Errno.
Summoning @tklauser to confirm (or deny) that there are no plans to change that, i.e. the errors returned from unix pkg will always be bare errnos.
https://pkg.go.dev/golang.org/x/sys/unix#section-documentation says:
These calls return err == nil to indicate success; otherwise err represents an operating system error describing the failure and holds a value of type syscall.Errno.
Summoning @tklauser to confirm (or deny) that there are no plans to change that, i.e. the errors returned from unix pkg will always be bare errnos.
That's correct. As far as I know there are no plans to change that. FWIW, the tests in the golang.org/x/sys/unix package use direct error comparison quite extensively too.
allowed.go in the errorlint source contains a set of errors and functions from pkg which are allowed to be compared. We can extend this list with the errors from golang.org/x/sys/unix.
One thing that does concern me about the current approach is that it uses only the package path basename which can become ambiguous with multiple packages. If the scope of the allowlist will be greater than just the standard library it is a good idea to match on absolute paths.
Based on https://github.com/golang/go/issues/40827, it seems like only io.EOF is really something which should not be wrapped while others might be?
@mitar Yes, you are right. I had taken the current approach because there is not really anything preventing people from still wrapping io.EOF in their own code. Ignoring io.EOF altogether should be complemented by a lint that detects wrapping io.EOF