go/types, types2: better error message when using *interface instead of interface

- detect *interface case and report specific error
- replaced switch with sequence of if's for more clarity
- fixed isInterfacePtr: it applies to all interfaces, incl.
  type parameters
- reviewed/fixed all uses of isInterfacePtr
- adjusted error messages to be consistently of the format
  "type %s is pointer to interface, not interface"

Fixes #48312.

Change-Id: Ic3c8cfcf93ad57ecdb60f6a727cce9e1aa4afb5d
Reviewed-on: https://go-review.googlesource.com/c/go/+/376914
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/call.go b/src/cmd/compile/internal/types2/call.go
index d93805e9c745e9639e2961b4b610a18e181fbff4..bd62e825af3d80c014a10bcb1f1cdbb34f3f362d 100644 (file)
--- a/src/cmd/compile/internal/types2/call.go
+++ b/src/cmd/compile/internal/types2/call.go
@@ -531,41 +531,51 @@ func (check *Checker) selector(x *operand, e *syntax.SelectorExpr) {
 
        obj, index, indirect = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel)
        if obj == nil {
-               switch {
-               case index != nil:
+               if index != nil {
                        // TODO(gri) should provide actual type where the conflict happens
                        check.errorf(e.Sel, "ambiguous selector %s.%s", x.expr, sel)
-               case indirect:
+                       goto Error
+               }
+
+               if indirect {
                        check.errorf(e.Sel, "cannot call pointer method %s on %s", sel, x.typ)
-               default:
-                       var why string
-                       if tpar, _ := x.typ.(*TypeParam); tpar != nil {
-                               // Type parameter bounds don't specify fields, so don't mention "field".
-                               if tname := tpar.iface().obj; tname != nil {
-                                       why = check.sprintf("interface %s has no method %s", tname.name, sel)
-                               } else {
-                                       why = check.sprintf("type bound for %s has no method %s", x.typ, sel)
-                               }
+                       goto Error
+               }
+
+               if isInterfacePtr(x.typ) {
+                       check.errorf(e.Sel, "%s.%s undefined (type %s is pointer to interface, not interface)", x.expr, sel, x.typ)
+                       goto Error
+               }
+
+               var why string
+               if tpar, _ := x.typ.(*TypeParam); tpar != nil {
+                       // Type parameter bounds don't specify fields, so don't mention "field".
+                       // TODO(gri) Type constraints may have accessible fields now. Revisit this.
+                       if tname := tpar.iface().obj; tname != nil {
+                               why = check.sprintf("interface %s has no method %s", tname.name, sel)
                        } else {
-                               why = check.sprintf("type %s has no field or method %s", x.typ, sel)
+                               why = check.sprintf("type bound for %s has no method %s", x.typ, sel)
                        }
+               } else {
+                       why = check.sprintf("type %s has no field or method %s", x.typ, sel)
+               }
 
-                       // Check if capitalization of sel matters and provide better error message in that case.
-                       if len(sel) > 0 {
-                               var changeCase string
-                               if r := rune(sel[0]); unicode.IsUpper(r) {
-                                       changeCase = string(unicode.ToLower(r)) + sel[1:]
-                               } else {
-                                       changeCase = string(unicode.ToUpper(r)) + sel[1:]
-                               }
-                               if obj, _, _ = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, changeCase); obj != nil {
-                                       why += ", but does have " + changeCase
-                               }
+               // Check if capitalization of sel matters and provide better error message in that case.
+               // TODO(gri) This code only looks at the first character but LookupFieldOrMethod has an
+               //           (internal) mechanism for case-insensitive lookup. Should use that instead.
+               if len(sel) > 0 {
+                       var changeCase string
+                       if r := rune(sel[0]); unicode.IsUpper(r) {
+                               changeCase = string(unicode.ToLower(r)) + sel[1:]
+                       } else {
+                               changeCase = string(unicode.ToUpper(r)) + sel[1:]
+                       }
+                       if obj, _, _ = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, changeCase); obj != nil {
+                               why += ", but does have " + changeCase
                        }
-
-                       check.errorf(e.Sel, "%s.%s undefined (%s)", x.expr, sel, why)
-
                }
+
+               check.errorf(e.Sel, "%s.%s undefined (%s)", x.expr, sel, why)
                goto Error
        }
 

diff --git a/src/cmd/compile/internal/types2/lookup.go b/src/cmd/compile/internal/types2/lookup.go
index 0cce3fdc3fcf2c1b8840ec49a7fe79a51a67f298..aa1ab8ac981e5b9cab70961895244ee45491fc98 100644 (file)
--- a/src/cmd/compile/internal/types2/lookup.go
+++ b/src/cmd/compile/internal/types2/lookup.go
@@ -448,12 +448,12 @@ func (check *Checker) missingMethodReason(V, T Type, m, wrongType *Func) string
                // an extra formatting option for types2.Type that doesn't print out
                // 'func'.
                r = strings.Replace(r, "^^func", "", -1)
-       } else if IsInterface(T) && !isTypeParam(T) {
+       } else if IsInterface(T) {
                if isInterfacePtr(V) {
-                       r = fmt.Sprintf("(%s is pointer to interface, not interface)", V)
+                       r = fmt.Sprintf("(type %s is pointer to interface, not interface)", V)
                }
-       } else if isInterfacePtr(T) && !isTypeParam(T) {
-               r = fmt.Sprintf("(%s is pointer to interface, not interface)", T)
+       } else if isInterfacePtr(T) {
+               r = fmt.Sprintf("(type %s is pointer to interface, not interface)", T)
        }
        if r == "" {
                r = fmt.Sprintf("(missing %s)", mname)
@@ -463,7 +463,7 @@ func (check *Checker) missingMethodReason(V, T Type, m, wrongType *Func) string
 
 func isInterfacePtr(T Type) bool {
        p, _ := under(T).(*Pointer)
-       return p != nil && IsInterface(p.base) && !isTypeParam(p.base)
+       return p != nil && IsInterface(p.base)
 }
 
 // assertableTo reports whether a value of type V can be asserted to have type T.

diff --git a/src/cmd/compile/internal/types2/operand.go b/src/cmd/compile/internal/types2/operand.go
index f6bd0291ece212a7f6b261883a7bd61a6d1004e7..69e3a0a8328f52d763db5f401059b3e0cba6d18c 100644 (file)
--- a/src/cmd/compile/internal/types2/operand.go
+++ b/src/cmd/compile/internal/types2/operand.go
@@ -317,7 +317,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) (bool, er
        if check != nil && check.conf.CompilerErrorMessages {
                if isInterfacePtr(Tu) {
                        if reason != nil {
-                               *reason = check.sprintf("%s does not implement %s (%s is pointer to interface, not interface)", x.typ, T, T)
+                               *reason = check.sprintf("%s does not implement %s (type %s is pointer to interface, not interface)", x.typ, T, T)
                        }
                        return false, _InvalidIfaceAssign
                }

diff --git a/src/cmd/compile/internal/types2/testdata/check/methodsets.src b/src/cmd/compile/internal/types2/testdata/check/methodsets.src
index 9fb10deb9a25662fe6294903098fe1ffd256219a..b0eb14cf50f94a298563c3f3c3e340b3809ef58c 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/check/methodsets.src
+++ b/src/cmd/compile/internal/types2/testdata/check/methodsets.src
@@ -196,9 +196,9 @@ func issue5918() {
                _ func(error) string = error.Error
 
                perr = &err
-               _ = perr.Error /* ERROR "no field or method" */ ()
-               _ func() string = perr.Error /* ERROR "no field or method" */
-               _ func(*error) string = (*error).Error /* ERROR "no field or method" */
+               _ = perr.Error /* ERROR "type \*error is pointer to interface, not interface" */ ()
+               _ func() string = perr.Error /* ERROR "type \*error is pointer to interface, not interface" */
+               _ func(*error) string = (*error).Error /* ERROR "type \*error is pointer to interface, not interface" */
        )
 
        type T *interface{ m() int }
@@ -207,8 +207,8 @@ func issue5918() {
                _ = (*x).m()
                _ = (*x).m
 
-               _ = x.m /* ERROR "no field or method" */ ()
-               _ = x.m /* ERROR "no field or method" */
-               _ = T.m /* ERROR "no field or method" */
+               _ = x.m /* ERROR "type T is pointer to interface, not interface" */ ()
+               _ = x.m /* ERROR "type T is pointer to interface, not interface" */
+               _ = T.m /* ERROR "type T is pointer to interface, not interface" */
        )
 }

diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47747.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47747.go2
index 6a2e787bf99848bc662654db7b6dd97b55bcec93..edde497f5aff1023644446e4c693be4193223b09 100644 (file)
--- a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47747.go2
+++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue47747.go2
@@ -20,7 +20,7 @@ func _[P interface{ m() }](x P) {
         x.m()
         // (&x).m doesn't exist because &x is of type *P
         // and pointers to type parameters don't have methods
-        (&x).m /* ERROR \*P has no field or method m */ ()
+        (&x).m /* ERROR type \*P is pointer to interface, not interface */ ()
 }
 
 
@@ -29,7 +29,7 @@ type T2 interface{ m() }
 func _(x *T2) {
         // x.m doesn't exists because x is of type *T2
         // and pointers to interfaces don't have methods
-        x.m /* ERROR \*T2 has no field or method m */()
+        x.m /* ERROR type \*T2 is pointer to interface, not interface */()
 }
 
 // Test case 1 from issue

diff --git a/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48312.go2 b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48312.go2
new file mode 100644 (file)
index 0000000..6e5911d
--- /dev/null
+++ b/src/cmd/compile/internal/types2/testdata/fixedbugs/issue48312.go2
@@ -0,0 +1,20 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type T interface{ m() }
+type P *T
+
+func _(p *T) {
+       p.m /* ERROR type \*T is pointer to interface, not interface */ ()
+}
+
+func _(p P) {
+       p.m /* ERROR type P is pointer to interface, not interface */ ()
+}
+
+func _[P T](p *P) {
+       p.m /* ERROR type \*P is pointer to interface, not interface */ ()
+}

diff --git a/src/go/types/call.go b/src/go/types/call.go
index ec6efd2379e358e7c1984bf48f716f5c2bbd1b80..a904b3df91aa6245abba4b3a15b9d9d5cb4e75d1 100644 (file)
--- a/src/go/types/call.go
+++ b/src/go/types/call.go
@@ -533,40 +533,52 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr) {
 
        obj, index, indirect = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel)
        if obj == nil {
-               switch {
-               case index != nil:
+               if index != nil {
                        // TODO(gri) should provide actual type where the conflict happens
                        check.errorf(e.Sel, _AmbiguousSelector, "ambiguous selector %s.%s", x.expr, sel)
-               case indirect:
+                       goto Error
+               }
+
+               if indirect {
                        check.errorf(e.Sel, _InvalidMethodExpr, "cannot call pointer method %s on %s", sel, x.typ)
-               default:
-                       var why string
-                       if tpar, _ := x.typ.(*TypeParam); tpar != nil {
-                               // Type parameter bounds don't specify fields, so don't mention "field".
-                               if tname := tpar.iface().obj; tname != nil {
-                                       why = check.sprintf("interface %s has no method %s", tname.name, sel)
-                               } else {
-                                       why = check.sprintf("type bound for %s has no method %s", x.typ, sel)
-                               }
+                       goto Error
+               }
+
+               if isInterfacePtr(x.typ) {
+                       check.errorf(e.Sel, _InvalidMethodExpr, "%s.%s undefined (type %s is pointer to interface, not interface)", x.expr, sel, x.typ)
+                       goto Error
+               }
+
+               var why string
+               if tpar, _ := x.typ.(*TypeParam); tpar != nil {
+                       // Type parameter bounds don't specify fields, so don't mention "field".
+                       // TODO(gri) Type constraints may have accessible fields now. Revisit this.
+                       if tname := tpar.iface().obj; tname != nil {
+                               why = check.sprintf("interface %s has no method %s", tname.name, sel)
                        } else {
-                               why = check.sprintf("type %s has no field or method %s", x.typ, sel)
+                               why = check.sprintf("type bound for %s has no method %s", x.typ, sel)
                        }
+               } else {
+                       why = check.sprintf("type %s has no field or method %s", x.typ, sel)
+               }
 
-                       // Check if capitalization of sel matters and provide better error message in that case.
-                       if len(sel) > 0 {
-                               var changeCase string
-                               if r := rune(sel[0]); unicode.IsUpper(r) {
-                                       changeCase = string(unicode.ToLower(r)) + sel[1:]
-                               } else {
-                                       changeCase = string(unicode.ToUpper(r)) + sel[1:]
-                               }
-                               if obj, _, _ = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, changeCase); obj != nil {
-                                       why += ", but does have " + changeCase
-                               }
+               // Check if capitalization of sel matters and provide better error message in that case.
+               // TODO(gri) This code only looks at the first character but LookupFieldOrMethod should
+               //           have an (internal) mechanism for case-insensitive lookup that we should use
+               //           instead (see types2).
+               if len(sel) > 0 {
+                       var changeCase string
+                       if r := rune(sel[0]); unicode.IsUpper(r) {
+                               changeCase = string(unicode.ToLower(r)) + sel[1:]
+                       } else {
+                               changeCase = string(unicode.ToUpper(r)) + sel[1:]
+                       }
+                       if obj, _, _ = LookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, changeCase); obj != nil {
+                               why += ", but does have " + changeCase
                        }
-
-                       check.errorf(e.Sel, _MissingFieldOrMethod, "%s.%s undefined (%s)", x.expr, sel, why)
                }
+
+               check.errorf(e.Sel, _MissingFieldOrMethod, "%s.%s undefined (%s)", x.expr, sel, why)
                goto Error
        }
 

diff --git a/src/go/types/lookup.go b/src/go/types/lookup.go
index e59335180460eed27d16129f61b9adb803e32c45..1b820d5403b65afe8872b8e0ac54939c1ccbad74 100644 (file)
--- a/src/go/types/lookup.go
+++ b/src/go/types/lookup.go
@@ -422,12 +422,12 @@ func (check *Checker) missingMethodReason(V, T Type, m, wrongType *Func) string
                // an extra formatting option for types2.Type that doesn't print out
                // 'func'.
                r = strings.Replace(r, "^^func", "", -1)
-       } else if IsInterface(T) && !isTypeParam(T) {
+       } else if IsInterface(T) {
                if isInterfacePtr(V) {
-                       r = fmt.Sprintf("(%s is pointer to interface, not interface)", V)
+                       r = fmt.Sprintf("(type %s is pointer to interface, not interface)", V)
                }
-       } else if isInterfacePtr(T) && !isTypeParam(T) {
-               r = fmt.Sprintf("(%s is pointer to interface, not interface)", T)
+       } else if isInterfacePtr(T) {
+               r = fmt.Sprintf("(type %s is pointer to interface, not interface)", T)
        }
        if r == "" {
                r = fmt.Sprintf("(missing %s)", mname)
@@ -437,7 +437,7 @@ func (check *Checker) missingMethodReason(V, T Type, m, wrongType *Func) string
 
 func isInterfacePtr(T Type) bool {
        p, _ := under(T).(*Pointer)
-       return p != nil && IsInterface(p.base) && !isTypeParam(T)
+       return p != nil && IsInterface(p.base)
 }
 
 // assertableTo reports whether a value of type V can be asserted to have type T.

diff --git a/src/go/types/operand.go b/src/go/types/operand.go
index 06ecbf14102efc9482d27ae2f9c69f0d28f77c31..d669981cf2c7dc2f0f8fd757593aa313bff024a5 100644 (file)
--- a/src/go/types/operand.go
+++ b/src/go/types/operand.go
@@ -308,7 +308,7 @@ func (x *operand) assignableTo(check *Checker, T Type, reason *string) (bool, er
        if check != nil && compilerErrorMessages {
                if isInterfacePtr(Tu) {
                        if reason != nil {
-                               *reason = check.sprintf("%s does not implement %s (%s is pointer to interface, not interface)", x.typ, T, T)
+                               *reason = check.sprintf("%s does not implement %s (type %s is pointer to interface, not interface)", x.typ, T, T)
                        }
                        return false, _InvalidIfaceAssign
                }

diff --git a/src/go/types/testdata/check/methodsets.src b/src/go/types/testdata/check/methodsets.src
index 9fb10deb9a25662fe6294903098fe1ffd256219a..b0eb14cf50f94a298563c3f3c3e340b3809ef58c 100644 (file)
--- a/src/go/types/testdata/check/methodsets.src
+++ b/src/go/types/testdata/check/methodsets.src
@@ -196,9 +196,9 @@ func issue5918() {
                _ func(error) string = error.Error
 
                perr = &err
-               _ = perr.Error /* ERROR "no field or method" */ ()
-               _ func() string = perr.Error /* ERROR "no field or method" */
-               _ func(*error) string = (*error).Error /* ERROR "no field or method" */
+               _ = perr.Error /* ERROR "type \*error is pointer to interface, not interface" */ ()
+               _ func() string = perr.Error /* ERROR "type \*error is pointer to interface, not interface" */
+               _ func(*error) string = (*error).Error /* ERROR "type \*error is pointer to interface, not interface" */
        )
 
        type T *interface{ m() int }
@@ -207,8 +207,8 @@ func issue5918() {
                _ = (*x).m()
                _ = (*x).m
 
-               _ = x.m /* ERROR "no field or method" */ ()
-               _ = x.m /* ERROR "no field or method" */
-               _ = T.m /* ERROR "no field or method" */
+               _ = x.m /* ERROR "type T is pointer to interface, not interface" */ ()
+               _ = x.m /* ERROR "type T is pointer to interface, not interface" */
+               _ = T.m /* ERROR "type T is pointer to interface, not interface" */
        )
 }

diff --git a/src/go/types/testdata/fixedbugs/issue47747.go2 b/src/go/types/testdata/fixedbugs/issue47747.go2
index 6a2e787bf99848bc662654db7b6dd97b55bcec93..edde497f5aff1023644446e4c693be4193223b09 100644 (file)
--- a/src/go/types/testdata/fixedbugs/issue47747.go2
+++ b/src/go/types/testdata/fixedbugs/issue47747.go2
@@ -20,7 +20,7 @@ func _[P interface{ m() }](x P) {
         x.m()
         // (&x).m doesn't exist because &x is of type *P
         // and pointers to type parameters don't have methods
-        (&x).m /* ERROR \*P has no field or method m */ ()
+        (&x).m /* ERROR type \*P is pointer to interface, not interface */ ()
 }
 
 
@@ -29,7 +29,7 @@ type T2 interface{ m() }
 func _(x *T2) {
         // x.m doesn't exists because x is of type *T2
         // and pointers to interfaces don't have methods
-        x.m /* ERROR \*T2 has no field or method m */()
+        x.m /* ERROR type \*T2 is pointer to interface, not interface */()
 }
 
 // Test case 1 from issue

diff --git a/src/go/types/testdata/fixedbugs/issue48312.go2 b/src/go/types/testdata/fixedbugs/issue48312.go2
new file mode 100644 (file)
index 0000000..6e5911d
--- /dev/null
+++ b/src/go/types/testdata/fixedbugs/issue48312.go2
@@ -0,0 +1,20 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package p
+
+type T interface{ m() }
+type P *T
+
+func _(p *T) {
+       p.m /* ERROR type \*T is pointer to interface, not interface */ ()
+}
+
+func _(p P) {
+       p.m /* ERROR type P is pointer to interface, not interface */ ()
+}
+
+func _[P T](p *P) {
+       p.m /* ERROR type \*P is pointer to interface, not interface */ ()
+}

diff --git a/test/fixedbugs/issue48471.go b/test/fixedbugs/issue48471.go
index 88caeede15ec3964482142d24ecffe7393834832..ba6245ab41dee2bc5bc36d78e40b5b2175df6798 100644 (file)
--- a/test/fixedbugs/issue48471.go
+++ b/test/fixedbugs/issue48471.go
@@ -32,7 +32,7 @@ func g() {
        i = new(T2)   // ERROR "cannot use new\(T2\) \(.*type \*T2\) as type I in assignment:\n\t\*T2 does not implement I \(missing M method\)\n\t\thave m\(int\)\n\t\twant M\(int\)"
        i = new(T3)   // ERROR "cannot use new\(T3\) \(.*type \*T3\) as type I in assignment:\n\t\*T3 does not implement I \(wrong type for M method\)\n\t\thave M\(string\)\n\t\twant M\(int\)"
        i = T4{}      // ERROR "cannot use T4\{\} \(.*type T4\) as type I in assignment:\n\tT4 does not implement I \(M method has pointer receiver\)"
-       i = new(I)    // ERROR "cannot use new\(I\) \(.*type \*I\) as type I in assignment:\n\t\*I does not implement I \(\*I is pointer to interface, not interface\)"
+       i = new(I)    // ERROR "cannot use new\(I\) \(.*type \*I\) as type I in assignment:\n\t\*I does not implement I \(type \*I is pointer to interface, not interface\)"
        _ = i.(*T2)   // ERROR "impossible type assertion: i.\(\*T2\)\n\t\*T2 does not implement I \(missing M method\)\n\t\thave m\(int\)\n\t\twant M\(int\)"
        _ = i.(*T3)   // ERROR "impossible type assertion: i.\(\*T3\)\n\t\*T3 does not implement I \(wrong type for M method\)\n\t\thave M\(string\)\n\t\twant M\(int\)"
        var t *T4

diff --git a/test/method2.go b/test/method2.go
index 2a92136d6c10baae3a901e61207f0f2a3e279406..0a497b4b84c942a3029af700f8c727907787d8ad 100644 (file)
--- a/test/method2.go
+++ b/test/method2.go
@@ -28,7 +28,7 @@ type Val interface {
        val() int
 }
 
-var _ = (*Val).val // ERROR "method"
+var _ = (*Val).val // ERROR "method|type \*Val is pointer to interface, not interface"
 
 var v Val
 var pv = &v