go/types: report detailed reason in error messages for invalid assignments

Fixes #10260.

Change-Id: I52d059144608912e6f7f9516e4961a75e9463355
Reviewed-on: https://go-review.googlesource.com/14644
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/go/types/api.go b/src/go/types/api.go
index b3bf6f01476dbf25d533dfb8993e915d7529f679..21f885d4044002f996731a479c8292ec279572b7 100644 (file)
--- a/src/go/types/api.go
+++ b/src/go/types/api.go
@@ -320,7 +320,7 @@ func AssertableTo(V *Interface, T Type) bool {
 // AssignableTo reports whether a value of type V is assignable to a variable of type T.
 func AssignableTo(V, T Type) bool {
        x := operand{mode: value, typ: V}
-       return x.assignableTo(nil, T) // config not needed for non-constant x
+       return x.assignableTo(nil, T, nil) // config not needed for non-constant x
 }
 
 // ConvertibleTo reports whether a value of type V is convertible to a value of type T.

diff --git a/src/go/types/assignments.go b/src/go/types/assignments.go
index e88de56a0daf79c94b49207bf10dad5ef77dfe3d..4231196b2ddf16c2409bf683b1d95aca82ce0b27 100644 (file)
--- a/src/go/types/assignments.go
+++ b/src/go/types/assignments.go
@@ -16,10 +16,9 @@ import (
 // type. If x.mode == invalid upon return, then assignment has already
 // issued an error message and the caller doesn't have to report another.
 // Use T == nil to indicate assignment to an untyped blank identifier.
-//
-// TODO(gri) Should find a better way to handle in-band errors.
-//
-func (check *Checker) assignment(x *operand, T Type) bool {
+// If the result is false and a non-nil reason is provided, it may be set
+// to a more detailed explanation of the failure (result != "").
+func (check *Checker) assignment(x *operand, T Type, reason *string) bool {
        switch x.mode {
        case invalid:
                return true // error reported before
@@ -58,11 +57,12 @@ func (check *Checker) assignment(x *operand, T Type) bool {
                        return false
                }
        }
+       // x.typ is typed
 
        // spec: "If a left-hand side is the blank identifier, any typed or
        // non-constant value except for the predeclared identifier nil may
        // be assigned to it."
-       return T == nil || x.assignableTo(check.conf, T)
+       return T == nil || x.assignableTo(check.conf, T, reason)
 }
 
 func (check *Checker) initConst(lhs *Const, x *operand) {
@@ -88,9 +88,9 @@ func (check *Checker) initConst(lhs *Const, x *operand) {
                lhs.typ = x.typ
        }
 
-       if !check.assignment(x, lhs.typ) {
+       if reason := ""; !check.assignment(x, lhs.typ, &reason) {
                if x.mode != invalid {
-                       check.errorf(x.pos(), "cannot define constant %s (type %s) as %s", lhs.Name(), lhs.typ, x)
+                       check.xerrorf(x.pos(), reason, "cannot define constant %s (type %s) as %s", lhs.Name(), lhs.typ, x)
                }
                return
        }
@@ -122,13 +122,13 @@ func (check *Checker) initVar(lhs *Var, x *operand, result bool) Type {
                lhs.typ = typ
        }
 
-       if !check.assignment(x, lhs.typ) {
+       if reason := ""; !check.assignment(x, lhs.typ, &reason) {
                if x.mode != invalid {
                        if result {
                                // don't refer to lhs.name because it may be an anonymous result parameter
-                               check.errorf(x.pos(), "cannot return %s as value of type %s", x, lhs.typ)
+                               check.xerrorf(x.pos(), reason, "cannot return %s as value of type %s", x, lhs.typ)
                        } else {
-                               check.errorf(x.pos(), "cannot initialize %s with %s", lhs, x)
+                               check.xerrorf(x.pos(), reason, "cannot initialize %s with %s", lhs, x)
                        }
                }
                return nil
@@ -148,7 +148,7 @@ func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type {
        // Don't evaluate lhs if it is the blank identifier.
        if ident != nil && ident.Name == "_" {
                check.recordDef(ident, nil)
-               if !check.assignment(x, nil) {
+               if !check.assignment(x, nil, nil) {
                        assert(x.mode == invalid)
                        x.typ = nil
                }
@@ -191,9 +191,9 @@ func (check *Checker) assignVar(lhs ast.Expr, x *operand) Type {
                return nil
        }
 
-       if !check.assignment(x, z.typ) {
+       if reason := ""; !check.assignment(x, z.typ, &reason) {
                if x.mode != invalid {
-                       check.errorf(x.pos(), "cannot assign %s to %s", x, &z)
+                       check.xerrorf(x.pos(), reason, "cannot assign %s to %s", x, &z)
                }
                return nil
        }

diff --git a/src/go/types/builtins.go b/src/go/types/builtins.go
index 7857a446a9fffea9214bb3862f9829119dc0b7e6..a879c8164db5e0c05eec2c207a7f9069ecf52ca1 100644 (file)
--- a/src/go/types/builtins.go
+++ b/src/go/types/builtins.go
@@ -95,7 +95,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                // spec: "As a special case, append also accepts a first argument assignable
                // to type []byte with a second argument of string type followed by ... .
                // This form appends the bytes of the string.
-               if nargs == 2 && call.Ellipsis.IsValid() && x.assignableTo(check.conf, NewSlice(universeByte)) {
+               if nargs == 2 && call.Ellipsis.IsValid() && x.assignableTo(check.conf, NewSlice(universeByte), nil) {
                        arg(x, 1)
                        if x.mode == invalid {
                                return
@@ -341,7 +341,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               if !x.assignableTo(check.conf, m.key) {
+               if !x.assignableTo(check.conf, m.key, nil) {
                        check.invalidArg(x.pos(), "%s is not assignable to %s", x, m.key)
                        return
                }
@@ -471,7 +471,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
        case _Panic:
                // panic(x)
                T := new(Interface)
-               if !check.assignment(x, T) {
+               if !check.assignment(x, T, nil) {
                        assert(x.mode == invalid)
                        return
                }
@@ -491,7 +491,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                                if i > 0 {
                                        arg(x, i) // first argument already evaluated
                                }
-                               if !check.assignment(x, nil) {
+                               if !check.assignment(x, nil, nil) {
                                        assert(x.mode == invalid)
                                        return
                                }
@@ -514,7 +514,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 
        case _Alignof:
                // unsafe.Alignof(x T) uintptr
-               if !check.assignment(x, nil) {
+               if !check.assignment(x, nil, nil) {
                        assert(x.mode == invalid)
                        return
                }
@@ -571,7 +571,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 
        case _Sizeof:
                // unsafe.Sizeof(x T) uintptr
-               if !check.assignment(x, nil) {
+               if !check.assignment(x, nil, nil) {
                        assert(x.mode == invalid)
                        return
                }

diff --git a/src/go/types/call.go b/src/go/types/call.go
index 62cefc047ef4f93ae76f3e48d0d85a05e167da34..c3ed0778e9500dcab611c10ea5fde94c85d4eb5f 100644 (file)
--- a/src/go/types/call.go
+++ b/src/go/types/call.go
@@ -261,8 +261,8 @@ func (check *Checker) argument(sig *Signature, i int, x *operand, ellipsis token
                typ = typ.(*Slice).elem
        }
 
-       if !check.assignment(x, typ) && x.mode != invalid {
-               check.errorf(x.pos(), "cannot pass argument %s to parameter of type %s", x, typ)
+       if reason := ""; !check.assignment(x, typ, &reason) && x.mode != invalid {
+               check.xerrorf(x.pos(), reason, "cannot pass argument %s to parameter of type %s", x, typ)
        }
 }
 

diff --git a/src/go/types/conversions.go b/src/go/types/conversions.go
index 74826ce93451f8a80b4132347971c02c471c3ccb..33e8930fbb630f08a3e9667526b94d8b9ac0a8da 100644 (file)
--- a/src/go/types/conversions.go
+++ b/src/go/types/conversions.go
@@ -65,7 +65,7 @@ func (check *Checker) conversion(x *operand, T Type) {
 
 func (x *operand) convertibleTo(conf *Config, T Type) bool {
        // "x is assignable to T"
-       if x.assignableTo(conf, T) {
+       if x.assignableTo(conf, T, nil) {
                return true
        }
 

diff --git a/src/go/types/errors.go b/src/go/types/errors.go
index 0c0049b1f3e21185374a736203b30a37711e21a8..7c81b129a4a1597d5e21f9d750899159a1591fd2 100644 (file)
--- a/src/go/types/errors.go
+++ b/src/go/types/errors.go
@@ -86,6 +86,14 @@ func (check *Checker) errorf(pos token.Pos, format string, args ...interface{})
        check.err(pos, check.sprintf(format, args...), false)
 }
 
+func (check *Checker) xerrorf(pos token.Pos, reason, format string, args ...interface{}) {
+       if reason != "" {
+               format += ": %s"
+               args = append(args, reason)
+       }
+       check.err(pos, check.sprintf(format, args...), true)
+}
+
 func (check *Checker) softErrorf(pos token.Pos, format string, args ...interface{}) {
        check.err(pos, check.sprintf(format, args...), true)
 }

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index e26607b5329b478603709fd8278de1b02a54d8b0..9d2331a1ad5d8db5ad526433ac3917cfe09c1da2 100644 (file)
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -570,7 +570,7 @@ func (check *Checker) comparison(x, y *operand, op token.Token) {
        // spec: "In any comparison, the first operand must be assignable
        // to the type of the second operand, or vice versa."
        err := ""
-       if x.assignableTo(check.conf, y.typ) || y.assignableTo(check.conf, x.typ) {
+       if x.assignableTo(check.conf, y.typ, nil) || y.assignableTo(check.conf, x.typ, nil) {
                defined := false
                switch op {
                case token.EQL, token.NEQ:
@@ -898,8 +898,8 @@ func (check *Checker) indexedElts(elts []ast.Expr, typ Type, length int64) int64
                // check element against composite literal element type
                var x operand
                check.exprWithHint(&x, eval, typ)
-               if !check.assignment(&x, typ) && x.mode != invalid {
-                       check.errorf(x.pos(), "cannot use %s as %s value in array or slice literal", &x, typ)
+               if reason := ""; !check.assignment(&x, typ, &reason) && x.mode != invalid {
+                       check.xerrorf(x.pos(), reason, "cannot use %s as %s value in array or slice literal", &x, typ)
                }
        }
        return max
@@ -1062,9 +1062,9 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                                        visited[i] = true
                                        check.expr(x, kv.Value)
                                        etyp := fld.typ
-                                       if !check.assignment(x, etyp) {
+                                       if reason := ""; !check.assignment(x, etyp, &reason) {
                                                if x.mode != invalid {
-                                                       check.errorf(x.pos(), "cannot use %s as %s value in struct literal", x, etyp)
+                                                       check.xerrorf(x.pos(), reason, "cannot use %s as %s value in struct literal", x, etyp)
                                                }
                                                continue
                                        }
@@ -1088,9 +1088,9 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                                                continue
                                        }
                                        etyp := fld.typ
-                                       if !check.assignment(x, etyp) {
+                                       if reason := ""; !check.assignment(x, etyp, &reason) {
                                                if x.mode != invalid {
-                                                       check.errorf(x.pos(), "cannot use %s as %s value in struct literal", x, etyp)
+                                                       check.xerrorf(x.pos(), reason, "cannot use %s as %s value in struct literal", x, etyp)
                                                }
                                                continue
                                        }
@@ -1120,9 +1120,9 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                                        continue
                                }
                                check.exprWithHint(x, kv.Key, utyp.key)
-                               if !check.assignment(x, utyp.key) {
+                               if reason := ""; !check.assignment(x, utyp.key, &reason) {
                                        if x.mode != invalid {
-                                               check.errorf(x.pos(), "cannot use %s as %s key in map literal", x, utyp.key)
+                                               check.xerrorf(x.pos(), reason, "cannot use %s as %s key in map literal", x, utyp.key)
                                        }
                                        continue
                                }
@@ -1147,9 +1147,9 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                                        }
                                }
                                check.exprWithHint(x, kv.Value, utyp.elem)
-                               if !check.assignment(x, utyp.elem) {
+                               if reason := ""; !check.assignment(x, utyp.elem, &reason) {
                                        if x.mode != invalid {
-                                               check.errorf(x.pos(), "cannot use %s as %s value in map literal", x, utyp.elem)
+                                               check.xerrorf(x.pos(), reason, "cannot use %s as %s value in map literal", x, utyp.elem)
                                        }
                                        continue
                                }
@@ -1220,9 +1220,9 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                case *Map:
                        var key operand
                        check.expr(&key, e.Index)
-                       if !check.assignment(&key, typ.key) {
+                       if reason := ""; !check.assignment(&key, typ.key, &reason) {
                                if key.mode != invalid {
-                                       check.invalidOp(key.pos(), "cannot use %s as map index of type %s", &key, typ.key)
+                                       check.xerrorf(key.pos(), reason, "cannot use %s as map index of type %s", &key, typ.key)
                                }
                                goto Error
                        }

diff --git a/src/go/types/operand.go b/src/go/types/operand.go
index d3bab51b04a0bfbd11c302fe0b4b33b13f8ea18f..09eac8354dce1f2de61d8c899ba09b66b2750250 100644 (file)
--- a/src/go/types/operand.go
+++ b/src/go/types/operand.go
@@ -202,7 +202,9 @@ func (x *operand) isNil() bool {
 //           overlapping in functionality. Need to simplify and clean up.
 
 // assignableTo reports whether x is assignable to a variable of type T.
-func (x *operand) assignableTo(conf *Config, T Type) bool {
+// If the result is false and a non-nil reason is provided, it may be set
+// to a more detailed explanation of the failure (result != "").
+func (x *operand) assignableTo(conf *Config, T Type, reason *string) bool {
        if x.mode == invalid || T == Typ[Invalid] {
                return true // avoid spurious errors
        }
@@ -217,49 +219,15 @@ func (x *operand) assignableTo(conf *Config, T Type) bool {
        Vu := V.Underlying()
        Tu := T.Underlying()
 
-       // T is an interface type and x implements T
-       // (Do this check first as it might succeed early.)
-       if Ti, ok := Tu.(*Interface); ok {
-               if Implements(x.typ, Ti) {
-                       return true
-               }
-       }
-
-       // x's type V and T have identical underlying types
-       // and at least one of V or T is not a named type
-       if Identical(Vu, Tu) && (!isNamed(V) || !isNamed(T)) {
-               return true
-       }
-
-       // x is a bidirectional channel value, T is a channel
-       // type, x's type V and T have identical element types,
-       // and at least one of V or T is not a named type
-       if Vc, ok := Vu.(*Chan); ok && Vc.dir == SendRecv {
-               if Tc, ok := Tu.(*Chan); ok && Identical(Vc.elem, Tc.elem) {
-                       return !isNamed(V) || !isNamed(T)
-               }
-       }
-
-       // x is the predeclared identifier nil and T is a pointer,
-       // function, slice, map, channel, or interface type
-       if x.isNil() {
-               switch t := Tu.(type) {
-               case *Basic:
-                       if t.kind == UnsafePointer {
-                               return true
-                       }
-               case *Pointer, *Signature, *Slice, *Map, *Chan, *Interface:
-                       return true
-               }
-               return false
-       }
-
-       // x is an untyped constant representable by a value of type T
+       // x is an untyped value representable by a value of type T
        // TODO(gri) This is borrowing from checker.convertUntyped and
        //           checker.representable. Need to clean up.
        if isUntyped(Vu) {
                switch t := Tu.(type) {
                case *Basic:
+                       if x.isNil() && t.kind == UnsafePointer {
+                               return true
+                       }
                        if x.mode == constant_ {
                                return representableConst(x.val, conf, t.kind, nil)
                        }
@@ -274,6 +242,37 @@ func (x *operand) assignableTo(conf *Config, T Type) bool {
                        return x.isNil()
                }
        }
+       // Vu is typed
+
+       // x's type V and T have identical underlying types
+       // and at least one of V or T is not a named type
+       if Identical(Vu, Tu) && (!isNamed(V) || !isNamed(T)) {
+               return true
+       }
+
+       // T is an interface type and x implements T
+       if Ti, ok := Tu.(*Interface); ok {
+               if m, wrongType := MissingMethod(x.typ, Ti, true); m != nil /* Implements(x.typ, Ti) */ {
+                       if reason != nil {
+                               if wrongType {
+                                       *reason = "wrong type for method " + m.Name()
+                               } else {
+                                       *reason = "missing method " + m.Name()
+                               }
+                       }
+                       return false
+               }
+               return true
+       }
+
+       // x is a bidirectional channel value, T is a channel
+       // type, x's type V and T have identical element types,
+       // and at least one of V or T is not a named type
+       if Vc, ok := Vu.(*Chan); ok && Vc.dir == SendRecv {
+               if Tc, ok := Tu.(*Chan); ok && Identical(Vc.elem, Tc.elem) {
+                       return !isNamed(V) || !isNamed(T)
+               }
+       }
 
        return false
 }

diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go
index 062b767c1ad488e9133b8a7a68b672e60e7afcdb..50efc1fc9974d64f9d1976340a4a0657a08f9f76 100644 (file)
--- a/src/go/types/stmt.go
+++ b/src/go/types/stmt.go
@@ -321,9 +321,10 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                if ch.mode == invalid || x.mode == invalid {
                        return
                }
-               if tch, ok := ch.typ.Underlying().(*Chan); !ok || tch.dir == RecvOnly || !check.assignment(&x, tch.elem) {
+               reason := ""
+               if tch, ok := ch.typ.Underlying().(*Chan); !ok || tch.dir == RecvOnly || !check.assignment(&x, tch.elem, &reason) {
                        if x.mode != invalid {
-                               check.invalidOp(ch.pos(), "cannot send %s to channel %s", &x, &ch)
+                               check.xerrorf(x.pos(), reason, "cannot send %s to channel %s", &x, &ch)
                        }
                }
 
@@ -464,7 +465,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                        check.expr(&x, s.Tag)
                        // By checking assignment of x to an invisible temporary
                        // (as a compiler would), we get all the relevant checks.
-                       check.assignment(&x, nil)
+                       check.assignment(&x, nil, nil)
                } else {
                        // spec: "A missing switch expression is
                        // equivalent to the boolean value true."

diff --git a/src/go/types/testdata/issues.src b/src/go/types/testdata/issues.src
index 9e2c901a28228b54738e1cade0203e98e890658b..1e2f43b7284d707f4785f0a63091324fc44bd937 100644 (file)
--- a/src/go/types/testdata/issues.src
+++ b/src/go/types/testdata/issues.src
@@ -101,3 +101,56 @@ func issue10979() {
 var a1, b1 /* ERROR cycle */ , c1 /* ERROR cycle */ b1 = 0 > 0<<""[""[c1]]>c1
 var a2, b2 /* ERROR cycle */ = 0 /* ERROR mismatch */ /* ERROR mismatch */ > 0<<""[b2]
 var a3, b3 /* ERROR cycle */ = int /* ERROR mismatch */ /* ERROR mismatch */ (1<<""[b3])
+
+// issue10260
+// Check that error messages explain reason for interface assignment failures.
+type (
+       I0 interface{}
+       I1 interface{ foo() }
+       I2 interface{ foo(x int) }
+       T0 struct{}
+       T1 struct{}
+       T2 struct{}
+)
+
+func (*T1) foo() {}
+func (*T2) foo(x int) {}
+
+func issue10260() {
+       var (
+               i0 I0
+               i1 I1
+               i2 I2
+               t0 *T0
+               t1 *T1
+               t2 *T2
+       )
+       i1 = i0 /* ERROR cannot assign .* missing method foo */
+       i1 = t0 /* ERROR cannot assign .* missing method foo */
+       i1 = i2 /* ERROR cannot assign .* wrong type for method foo */
+       i1 = t2 /* ERROR cannot assign .* wrong type for method foo */
+       i2 = i1 /* ERROR cannot assign .* wrong type for method foo */
+       i2 = t1 /* ERROR cannot assign .* wrong type for method foo */
+
+       _ = func() I1 { return i0 /* ERROR cannot return .* missing method foo */ }
+       _ = func() I1 { return t0 /* ERROR cannot return .* missing method foo */ }
+       _ = func() I1 { return i2 /* ERROR cannot return .* wrong type for method foo */ }
+       _ = func() I1 { return t2 /* ERROR cannot return .* wrong type for method foo */ }
+       _ = func() I2 { return i1 /* ERROR cannot return .* wrong type for method foo */ }
+       _ = func() I2 { return t1 /* ERROR cannot return .* wrong type for method foo */ }
+
+       // a few more - less exhaustive now
+
+       f := func(I1, I2){}
+       f(i0 /* ERROR cannot pass .* missing method foo */ , i1 /* ERROR cannot pass .* wrong type for method foo */)
+
+       _ = [...]I1{i0 /* ERROR cannot use .* missing method foo */ }
+       _ = [...]I1{i2 /* ERROR cannot use .* wrong type for method foo */ }
+       _ = []I1{i0 /* ERROR cannot use .* missing method foo */ }
+       _ = []I1{i2 /* ERROR cannot use .* wrong type for method foo */ }
+       _ = map[int]I1{0: i0 /* ERROR cannot use .* missing method foo */ }
+       _ = map[int]I1{0: i2 /* ERROR cannot use .* wrong type for method foo */ }
+
+       make(chan I1) <- i0 /* ERROR cannot send .* missing method foo */
+       make(chan I1) <- i2 /* ERROR cannot send .* wrong type for method foo */
+}

diff --git a/src/go/types/testdata/stmt0.src b/src/go/types/testdata/stmt0.src
index e946066c4965a8e55320de5ce5449255df266b1b..80abbd1d96f67bda60f3a9543da69ac9c04dcdd1 100644 (file)
--- a/src/go/types/testdata/stmt0.src
+++ b/src/go/types/testdata/stmt0.src
@@ -180,8 +180,8 @@ func sends() {
        var ch chan int
        var rch <-chan int
        var x int
-       x /* ERROR "cannot send" */ <- x
-       rch /* ERROR "cannot send" */ <- x
+       x <- x /* ERROR "cannot send" */
+       rch <- x /* ERROR "cannot send" */
        ch <- "foo" /* ERROR "cannot convert" */
        ch <- x
 }