-
Notifications
You must be signed in to change notification settings - Fork 17.8k
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
cmd/vet: ensure errors implement Is, As, and Unwrap anonymous interfaces #40356
Comments
@ianlancetaylor: can you add this to the proposals project? |
@carnott-snap Done. |
did this get lost in the incoming stack? |
The incoming stack has more than 200 issues in it, so it's going to take a while to get through them all. See https://github.com/golang/go/projects/1 . |
This proposal has been added to the active column of the proposals project |
I don't object to |
Change https://golang.org/cl/312829 mentions this issue: |
Based on the discussion above, this proposal seems like a likely accept. |
Tested golang.org/cl/312829 against a large Go codebase that uses Is, As and Unwrap. No findings so no false positives. |
No change in consensus, so accepted. 🎉 |
…ture. Extend stdmethods to warn when a type implementing error has an Is, As, or Unwrap method with a different signature than the one expected by the errors package. For golang/go#40356 Change-Id: Ia196bb83a685340d6dab216b87c8a4aa2846f785 Reviewed-on: https://go-review.googlesource.com/c/tools/+/312829 Run-TryBot: Tim King <[email protected]> gopls-CI: kokoro <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Michael Matloob <[email protected]> Trust: Bryan C. Mills <[email protected]>
Change https://golang.org/cl/321389 mentions this issue: |
The interfaces for
errors.Is
anderrors.As
seem stable and complete. Unfortunately, it requires reading through the godoc to ensure that your custom error type correctly implementsinterface{ As(interface{}) bool }
,interface{ Is(error) bool }
, orinterface{ Unwrap() error }
. This can lead to implementation bugs that fail pathologically.Instead of exposing these symbols, as was proposed in #39539, we can add a vet check to ensure that
error
s withIs
,As
, orUnwrap
methods implement the correct interfaces. This is not a requirement that allerror
s implementIs
,As
, andUnwrap
, but simply that if the symbol exists, the syntax matches.Technically, this is not required, as you can use an anonymous type to perform this check at compile time. But this is ungainly, not automatic, and fails to the same typographical issues due to its distributed implementation:
The text was updated successfully, but these errors were encountered: