go/types: change Checker.verify to return an error

This is a port of CL 342151 to go/types, adjusted for errors and
positions. Checker.sprintf was refactored to facilitate formatting
error messages with a nil Checker.

Change-Id: Ib2e5c942e55edaff7b5e77cf68a72bad70fea0b9
Reviewed-on: https://go-review.googlesource.com/c/go/+/342670
Trust: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Findley <rfindley@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@golang.org>

src/go/types/errors.go

@@ -63,6 +63,10 @@ func (check *Checker) markImports(pkg *Package) {
 }
 
 func (check *Checker) sprintf(format string, args ...interface{}) string {
+	return sprintf(check.fset, check.qualifier, format, args...)
+}
+
+func sprintf(fset *token.FileSet, qf Qualifier, format string, args ...interface{}) string {
 	for i, arg := range args {
 		switch a := arg.(type) {
 		case nil:
@@ -70,15 +74,17 @@ func (check *Checker) sprintf(format string, args ...interface{}) string {
 		case operand:
 			panic("got operand instead of *operand")
 		case *operand:
-			arg = operandString(a, check.qualifier)
+			arg = operandString(a, qf)
 		case token.Pos:
-			arg = check.fset.Position(a).String()
+			if fset != nil {
+				arg = fset.Position(a).String()
+			}
 		case ast.Expr:
 			arg = ExprString(a)
 		case Object:
-			arg = ObjectString(a, check.qualifier)
+			arg = ObjectString(a, qf)
 		case Type:
-			arg = TypeString(a, check.qualifier)
+			arg = TypeString(a, qf)
 		}
 		args[i] = arg
 	}

src/go/types/instantiate.go

@@ -8,6 +8,7 @@
 package types
 
 import (
+	"errors"
 	"fmt"
 	"go/token"
 )
@@ -73,7 +74,14 @@ func (check *Checker) Instantiate(pos token.Pos, typ Type, targs []Type, posList
 			// Avoid duplicate errors; instantiate will have complained if tparams
 			// and targs do not have the same length.
 			if len(tparams) == len(targs) {
-				check.verify(pos, tparams, targs, posList)
+				if i, err := check.verify(pos, tparams, targs); err != nil {
+					// best position for error reporting
+					pos := pos
+					if i < len(posList) {
+						pos = posList[i]
+					}
+					check.softErrorf(atPos(pos), _Todo, err.Error())
+				}
 			}
 		})
 	}
@@ -141,30 +149,36 @@ func (check *Checker) instantiateLazy(pos token.Pos, orig *Named, targs []Type)
 	return named
 }
 
-func (check *Checker) verify(pos token.Pos, tparams []*TypeName, targs []Type, posList []token.Pos) {
+func (check *Checker) verify(pos token.Pos, tparams []*TypeName, targs []Type) (int, error) {
 	smap := makeSubstMap(tparams, targs)
 	for i, tname := range tparams {
-		// best position for error reporting
-		pos := pos
-		if i < len(posList) {
-			pos = posList[i]
-		}
-
 		// stop checking bounds after the first failure
-		if !check.satisfies(pos, targs[i], tname.typ.(*TypeParam), smap) {
+		if err := check.satisfies(pos, targs[i], tname.typ.(*TypeParam), smap); err != nil {
-			break
+			return i, err
 		}
 	}
+	return -1, nil
 }
 
 // satisfies reports whether the type argument targ satisfies the constraint of type parameter
 // parameter tpar (after any of its type parameters have been substituted through smap).
 // A suitable error is reported if the result is false.
 // TODO(gri) This should be a method of interfaces or type sets.
-func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap substMap) bool {
+func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap substMap) error {
 	iface := tpar.iface()
 	if iface.Empty() {
-		return true // no type bound
+		return nil // no type bound
+	}
+
+	// TODO(rfindley): it would be great if users could pass in a qualifier here,
+	// rather than falling back to verbose qualification. Maybe this can be part
+	// of a the shared environment.
+	var qf Qualifier
+	if check != nil {
+		qf = check.qualifier
+	}
+	errorf := func(format string, args ...interface{}) error {
+		return errors.New(sprintf(nil, qf, format, args...))
 	}
 
 	// The type parameter bound is parameterized with the same type parameters
@@ -177,11 +191,9 @@ func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap
 	// TODO(gri) the error messages needs to be better, here
 	if iface.IsComparable() && !Comparable(targ) {
 		if tpar := asTypeParam(targ); tpar != nil && tpar.iface().typeSet().IsAll() {
-			check.softErrorf(atPos(pos), _Todo, "%s has no constraints", targ)
+			return errorf("%s has no constraints", targ)
-			return false
 		}
-		check.softErrorf(atPos(pos), _Todo, "%s does not satisfy comparable", targ)
+		return errorf("%s does not satisfy comparable", targ)
-		return false
 	}
 
 	// targ must implement iface (methods)
@@ -191,8 +203,7 @@ func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap
 		// method set is empty.
 		// TODO(gri) is this what we want? (spec question)
 		if base, isPtr := deref(targ); isPtr && asTypeParam(base) != nil {
-			check.errorf(atPos(pos), 0, "%s has no methods", targ)
+			return errorf("%s has no methods", targ)
-			return false
 		}
 		if m, wrong := check.missingMethod(targ, iface, true); m != nil {
 			// TODO(gri) needs to print updated name to avoid major confusion in error message!
@@ -203,20 +214,17 @@ func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap
 				// TODO(gri) This can still report uninstantiated types which makes the error message
 				//           more difficult to read then necessary.
 				// TODO(rFindley) should this use parentheses rather than ':' for qualification?
-				check.softErrorf(atPos(pos), _Todo,
+				return errorf("%s does not satisfy %s: wrong method signature\n\tgot  %s\n\twant %s",
-					"%s does not satisfy %s: wrong method signature\n\tgot  %s\n\twant %s",
 					targ, tpar.bound, wrong, m,
 				)
-			} else {
-				check.softErrorf(atPos(pos), 0, "%s does not satisfy %s (missing method %s)", targ, tpar.bound, m.name)
 			}
-			return false
+			return errorf("%s does not satisfy %s (missing method %s)", targ, tpar.bound, m.name)
 		}
 	}
 
 	// targ's underlying type must also be one of the interface types listed, if any
 	if !iface.typeSet().hasTerms() {
-		return true // nothing to do
+		return nil // nothing to do
 	}
 
 	// If targ is itself a type parameter, each of its possible types, but at least one, must be in the
@@ -224,23 +232,20 @@ func (check *Checker) satisfies(pos token.Pos, targ Type, tpar *TypeParam, smap
 	if targ := asTypeParam(targ); targ != nil {
 		targBound := targ.iface()
 		if !targBound.typeSet().hasTerms() {
-			check.softErrorf(atPos(pos), _Todo, "%s does not satisfy %s (%s has no type constraints)", targ, tpar.bound, targ)
+			return errorf("%s does not satisfy %s (%s has no type constraints)", targ, tpar.bound, targ)
-			return false
 		}
 		if !targBound.typeSet().subsetOf(iface.typeSet()) {
 			// TODO(gri) need better error message
-			check.softErrorf(atPos(pos), _Todo, "%s does not satisfy %s", targ, tpar.bound)
+			return errorf("%s does not satisfy %s", targ, tpar.bound)
-			return false
 		}
-		return true
+		return nil
 	}
 
 	// Otherwise, targ's type or underlying type must also be one of the interface types listed, if any.
 	if !iface.typeSet().includes(targ) {
 		// TODO(gri) better error message
-		check.softErrorf(atPos(pos), _Todo, "%s does not satisfy %s", targ, tpar.bound)
+		return errorf("%s does not satisfy %s", targ, tpar.bound)
-		return false
 	}
 
-	return true
+	return nil
 }