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>

go/analysis/passes/errorsas/errorsas.go

@@ -7,6 +7,7 @@
 package errorsas
 
 import (
+	"errors"
 	"go/ast"
 	"go/types"
 
@@ -50,26 +51,39 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		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")
 }

go/analysis/passes/errorsas/testdata/src/a/a.go

@@ -28,10 +28,10 @@ func _() {
 		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`