go/analysis/passes/errorsas: warn if errors.As target is *error
Passing an *error as the second parameter to errors.As always matches,
making this test:
var e error
if errors.As(err, &e) { ... }
equivalent to:
e := err
if err != nil { ... }
Warn when the second parameter to errors.As has type *error.
Fixes golang/go#47528.
Change-Id: Ia0e25003493f3b349ab500f0b4d08c2acf88b328
Reviewed-on: https://go-review.googlesource.com/c/tools/+/339889
Reviewed-by: Tim King <taking@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Damien Neil <dneil@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
diff --git a/go/analysis/passes/errorsas/errorsas.go b/go/analysis/passes/errorsas/errorsas.go
index 384f025..96adad3 100644
--- a/go/analysis/passes/errorsas/errorsas.go
+++ b/go/analysis/passes/errorsas/errorsas.go
@@ -7,6 +7,7 @@
package errorsas
import (
+ "errors"
"go/ast"
"go/types"
@@ -50,26 +51,39 @@
if len(call.Args) < 2 {
return // not enough arguments, e.g. called with return values of another function
}
- if fn.FullName() == "errors.As" && !pointerToInterfaceOrError(pass, call.Args[1]) {
- pass.ReportRangef(call, "second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
+ if fn.FullName() != "errors.As" {
+ return
+ }
+ if err := checkAsTarget(pass, call.Args[1]); err != nil {
+ pass.ReportRangef(call, "%v", err)
}
})
return nil, nil
}
-var errorType = types.Universe.Lookup("error").Type().Underlying().(*types.Interface)
+var errorType = types.Universe.Lookup("error").Type()
// pointerToInterfaceOrError reports whether the type of e is a pointer to an interface or a type implementing error,
// or is the empty interface.
-func pointerToInterfaceOrError(pass *analysis.Pass, e ast.Expr) bool {
+
+// checkAsTarget reports an error if the second argument to errors.As is invalid.
+func checkAsTarget(pass *analysis.Pass, e ast.Expr) error {
t := pass.TypesInfo.Types[e].Type
if it, ok := t.Underlying().(*types.Interface); ok && it.NumMethods() == 0 {
- return true
+ // A target of interface{} is always allowed, since it often indicates
+ // a value forwarded from another source.
+ return nil
}
pt, ok := t.Underlying().(*types.Pointer)
if !ok {
- return false
+ return errors.New("second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
+ }
+ if pt.Elem() == errorType {
+ return errors.New("second argument to errors.As should not be *error")
}
_, ok = pt.Elem().Underlying().(*types.Interface)
- return ok || types.Implements(pt.Elem(), errorType)
+ if ok || types.Implements(pt.Elem(), errorType.Underlying().(*types.Interface)) {
+ return nil
+ }
+ return errors.New("second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type")
}
diff --git a/go/analysis/passes/errorsas/testdata/src/a/a.go b/go/analysis/passes/errorsas/testdata/src/a/a.go
index c987a8a..7a9ae89 100644
--- a/go/analysis/passes/errorsas/testdata/src/a/a.go
+++ b/go/analysis/passes/errorsas/testdata/src/a/a.go
@@ -28,10 +28,10 @@
f iface
ei interface{}
)
- errors.As(nil, &e) // *error
+ errors.As(nil, &e) // want `second argument to errors.As should not be \*error`
errors.As(nil, &m) // *T where T implemements error
errors.As(nil, &f) // *interface
- errors.As(nil, perr()) // *error, via a call
+ errors.As(nil, perr()) // want `second argument to errors.As should not be \*error`
errors.As(nil, ei) // empty interface
errors.As(nil, nil) // want `second argument to errors.As must be a non-nil pointer to either a type that implements error, or to any interface type`
