go/types, types2: better errors for non-existing fields or methods

This CL improves the error messages reported when a field or method
name is used that doesn't exist. It brings the error messges on par
(or better) with the respective errors reported before Go 1.18 (i.e.
before switching to the new type checker):

Make case distinctions based on whether a field/method is exported
and how it is spelled. Factor out that logic into a new function
(lookupError) in a new file (errsupport.go), which is generated for
go/types. Use lookupError when reporting selector lookup errors
and missing struct field keys.

Add a comprehensive set of tests (lookup2.go) and spot tests for
the two cases brought up by the issue at hand.

Adjusted existing tests as needed.

Fixes #49736.

Change-Id: I2f439948dcd12f9bd1a258367862d8ff96e32305
Reviewed-on: https://go-review.googlesource.com/c/go/+/560055
Run-TryBot: Robert Griesemer <gri@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>

src/cmd/compile/internal/types2/call.go

@@ -824,22 +824,8 @@ func (check *Checker) selector(x *operand, e *syntax.SelectorExpr, def *TypeName
 		if isInterfacePtr(x.typ) {
 			why = check.interfacePtrError(x.typ)
 		} else {
-			why = check.sprintf("type %s has no field or method %s", x.typ, sel)
+			alt, _, _ := lookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel, true)
-			// check if there's a field or method with different capitalization
+			why = check.lookupError(x.typ, sel, alt, false)
-			if obj, _, _ = lookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel, true); obj != nil {
-				var what string // empty or description with trailing space " " (default case, should never be reached)
-				switch obj.(type) {
-				case *Var:
-					what = "field "
-				case *Func:
-					what = "method "
-				}
-				if samePkg(obj.Pkg(), check.pkg) || obj.Exported() {
-					why = check.sprintf("%s, but does have %s%s", why, what, obj.Name())
-				} else if obj.Name() == sel {
-					why = check.sprintf("%s%s is not exported", what, obj.Name())
-				}
-			}
 		}
 		check.errorf(e.Sel, MissingFieldOrMethod, "%s.%s undefined (%s)", x.expr, sel, why)
 		goto Error

src/cmd/compile/internal/types2/errsupport.go

@@ -0,0 +1,113 @@
+// Copyright 2024 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.
+
+// This file implements support functions for error messages.
+
+package types2
+
+// lookupError returns a case-specific error when a lookup of selector sel in the
+// given type fails but an object with alternative spelling (case folding) is found.
+// If structLit is set, the error message is specifically for struct literal fields.
+func (check *Checker) lookupError(typ Type, sel string, obj Object, structLit bool) string {
+	// Provide more detail if there is an unexported object, or one with different capitalization.
+	// If selector and object are in the same package (==), export doesn't matter, otherwise (!=) it does.
+	// Messages depend on whether it's a general lookup or a field lookup in a struct literal.
+	//
+	// case           sel     pkg   have   message (examples for general lookup)
+	// ---------------------------------------------------------------------------------------------------------
+	// ok             x.Foo   ==    Foo
+	// misspelled     x.Foo   ==    FoO    type X has no field or method Foo, but does have field FoO
+	// misspelled     x.Foo   ==    foo    type X has no field or method Foo, but does have field foo
+	// misspelled     x.Foo   ==    foO    type X has no field or method Foo, but does have field foO
+	//
+	// misspelled     x.foo   ==    Foo    type X has no field or method foo, but does have field Foo
+	// misspelled     x.foo   ==    FoO    type X has no field or method foo, but does have field FoO
+	// ok             x.foo   ==    foo
+	// misspelled     x.foo   ==    foO    type X has no field or method foo, but does have field foO
+	//
+	// ok             x.Foo   !=    Foo
+	// misspelled     x.Foo   !=    FoO    type X has no field or method Foo, but does have field FoO
+	// unexported     x.Foo   !=    foo    type X has no field or method Foo, but does have unexported field foo
+	// missing        x.Foo   !=    foO    type X has no field or method Foo
+	//
+	// misspelled     x.foo   !=    Foo    type X has no field or method foo, but does have field Foo
+	// missing        x.foo   !=    FoO    type X has no field or method foo
+	// inaccessible   x.foo   !=    foo    cannot refer to unexported field foo
+	// missing        x.foo   !=    foO    type X has no field or method foo
+
+	const (
+		ok           = iota
+		missing      // no object found
+		misspelled   // found object with different spelling
+		unexported   // found object with name differing only in first letter
+		inaccessible // found object with matching name but inaccessible from the current package
+	)
+
+	// determine case
+	e := missing
+	var alt string // alternative spelling of selector; if any
+	if obj != nil {
+		alt = obj.Name()
+		if obj.Pkg() == check.pkg {
+			assert(alt != sel) // otherwise there is no lookup error
+			e = misspelled
+		} else if isExported(sel) {
+			if isExported(alt) {
+				e = misspelled
+			} else if tail(sel) == tail(alt) {
+				e = unexported
+			}
+		} else if isExported(alt) {
+			if tail(sel) == tail(alt) {
+				e = misspelled
+			}
+		} else if sel == alt {
+			e = inaccessible
+		}
+	}
+
+	if structLit {
+		switch e {
+		case missing:
+			return check.sprintf("unknown field %s in struct literal of type %s", sel, typ)
+		case misspelled:
+			return check.sprintf("unknown field %s in struct literal of type %s, but does have %s", sel, typ, alt)
+		case unexported:
+			return check.sprintf("unknown field %s in struct literal of type %s, but does have unexported %s", sel, typ, alt)
+		case inaccessible:
+			return check.sprintf("cannot refer to unexported field %s in struct literal of type %s", alt, typ)
+		}
+	} else {
+		what := "object"
+		switch obj.(type) {
+		case *Var:
+			what = "field"
+		case *Func:
+			what = "method"
+		}
+		switch e {
+		case missing:
+			return check.sprintf("type %s has no field or method %s", typ, sel)
+		case misspelled:
+			return check.sprintf("type %s has no field or method %s, but does have %s %s", typ, sel, what, alt)
+		case unexported:
+			return check.sprintf("type %s has no field or method %s, but does have unexported %s %s", typ, sel, what, alt)
+		case inaccessible:
+			return check.sprintf("cannot refer to unexported %s %s", what, alt)
+		}
+	}
+
+	panic("unreachable")
+}
+
+// tail returns the string s without its first (UTF-8) character.
+// If len(s) == 0, the result is s.
+func tail(s string) string {
+	for i, _ := range s {
+		if i > 0 {
+			return s[i:]
+		}
+	}
+	return s
+}

src/cmd/compile/internal/types2/expr.go

@@ -1184,9 +1184,14 @@ func (check *Checker) exprInternal(T *target, x *operand, e syntax.Expr, hint Ty
 						check.errorf(kv, InvalidLitField, "invalid field name %s in struct literal", kv.Key)
 						continue
 					}
-					i := fieldIndex(utyp.fields, check.pkg, key.Value, false)
+					i := fieldIndex(fields, check.pkg, key.Value, false)
 					if i < 0 {
-						check.errorf(kv.Key, MissingLitField, "unknown field %s in struct literal of type %s", key.Value, base)
+						var alt Object
+						if j := fieldIndex(fields, check.pkg, key.Value, true); j >= 0 {
+							alt = fields[j]
+						}
+						msg := check.lookupError(base, key.Value, alt, true)
+						check.error(kv.Key, MissingLitField, msg)
 						continue
 					}
 					fld := fields[i]

src/go/types/call.go

@@ -826,22 +826,8 @@ func (check *Checker) selector(x *operand, e *ast.SelectorExpr, def *TypeName, w
 		if isInterfacePtr(x.typ) {
 			why = check.interfacePtrError(x.typ)
 		} else {
-			why = check.sprintf("type %s has no field or method %s", x.typ, sel)
+			alt, _, _ := lookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel, true)
-			// check if there's a field or method with different capitalization
+			why = check.lookupError(x.typ, sel, alt, false)
-			if obj, _, _ = lookupFieldOrMethod(x.typ, x.mode == variable, check.pkg, sel, true); obj != nil {
-				var what string // empty or description with trailing space " " (default case, should never be reached)
-				switch obj.(type) {
-				case *Var:
-					what = "field "
-				case *Func:
-					what = "method "
-				}
-				if samePkg(obj.Pkg(), check.pkg) || obj.Exported() {
-					why = check.sprintf("%s, but does have %s%s", why, what, obj.Name())
-				} else if obj.Name() == sel {
-					why = check.sprintf("%s%s is not exported", what, obj.Name())
-				}
-			}
 		}
 		check.errorf(e.Sel, MissingFieldOrMethod, "%s.%s undefined (%s)", x.expr, sel, why)
 		goto Error

src/go/types/errsupport.go

@@ -0,0 +1,115 @@
+// Code generated by "go test -run=Generate -write=all"; DO NOT EDIT.
+
+// Copyright 2024 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.
+
+// This file implements support functions for error messages.
+
+package types
+
+// lookupError returns a case-specific error when a lookup of selector sel in the
+// given type fails but an object with alternative spelling (case folding) is found.
+// If structLit is set, the error message is specifically for struct literal fields.
+func (check *Checker) lookupError(typ Type, sel string, obj Object, structLit bool) string {
+	// Provide more detail if there is an unexported object, or one with different capitalization.
+	// If selector and object are in the same package (==), export doesn't matter, otherwise (!=) it does.
+	// Messages depend on whether it's a general lookup or a field lookup in a struct literal.
+	//
+	// case           sel     pkg   have   message (examples for general lookup)
+	// ---------------------------------------------------------------------------------------------------------
+	// ok             x.Foo   ==    Foo
+	// misspelled     x.Foo   ==    FoO    type X has no field or method Foo, but does have field FoO
+	// misspelled     x.Foo   ==    foo    type X has no field or method Foo, but does have field foo
+	// misspelled     x.Foo   ==    foO    type X has no field or method Foo, but does have field foO
+	//
+	// misspelled     x.foo   ==    Foo    type X has no field or method foo, but does have field Foo
+	// misspelled     x.foo   ==    FoO    type X has no field or method foo, but does have field FoO
+	// ok             x.foo   ==    foo
+	// misspelled     x.foo   ==    foO    type X has no field or method foo, but does have field foO
+	//
+	// ok             x.Foo   !=    Foo
+	// misspelled     x.Foo   !=    FoO    type X has no field or method Foo, but does have field FoO
+	// unexported     x.Foo   !=    foo    type X has no field or method Foo, but does have unexported field foo
+	// missing        x.Foo   !=    foO    type X has no field or method Foo
+	//
+	// misspelled     x.foo   !=    Foo    type X has no field or method foo, but does have field Foo
+	// missing        x.foo   !=    FoO    type X has no field or method foo
+	// inaccessible   x.foo   !=    foo    cannot refer to unexported field foo
+	// missing        x.foo   !=    foO    type X has no field or method foo
+
+	const (
+		ok           = iota
+		missing      // no object found
+		misspelled   // found object with different spelling
+		unexported   // found object with name differing only in first letter
+		inaccessible // found object with matching name but inaccessible from the current package
+	)
+
+	// determine case
+	e := missing
+	var alt string // alternative spelling of selector; if any
+	if obj != nil {
+		alt = obj.Name()
+		if obj.Pkg() == check.pkg {
+			assert(alt != sel) // otherwise there is no lookup error
+			e = misspelled
+		} else if isExported(sel) {
+			if isExported(alt) {
+				e = misspelled
+			} else if tail(sel) == tail(alt) {
+				e = unexported
+			}
+		} else if isExported(alt) {
+			if tail(sel) == tail(alt) {
+				e = misspelled
+			}
+		} else if sel == alt {
+			e = inaccessible
+		}
+	}
+
+	if structLit {
+		switch e {
+		case missing:
+			return check.sprintf("unknown field %s in struct literal of type %s", sel, typ)
+		case misspelled:
+			return check.sprintf("unknown field %s in struct literal of type %s, but does have %s", sel, typ, alt)
+		case unexported:
+			return check.sprintf("unknown field %s in struct literal of type %s, but does have unexported %s", sel, typ, alt)
+		case inaccessible:
+			return check.sprintf("cannot refer to unexported field %s in struct literal of type %s", alt, typ)
+		}
+	} else {
+		what := "object"
+		switch obj.(type) {
+		case *Var:
+			what = "field"
+		case *Func:
+			what = "method"
+		}
+		switch e {
+		case missing:
+			return check.sprintf("type %s has no field or method %s", typ, sel)
+		case misspelled:
+			return check.sprintf("type %s has no field or method %s, but does have %s %s", typ, sel, what, alt)
+		case unexported:
+			return check.sprintf("type %s has no field or method %s, but does have unexported %s %s", typ, sel, what, alt)
+		case inaccessible:
+			return check.sprintf("cannot refer to unexported %s %s", what, alt)
+		}
+	}
+
+	panic("unreachable")
+}
+
+// tail returns the string s without its first (UTF-8) character.
+// If len(s) == 0, the result is s.
+func tail(s string) string {
+	for i, _ := range s {
+		if i > 0 {
+			return s[i:]
+		}
+	}
+	return s
+}

src/go/types/expr.go

@@ -1166,7 +1166,12 @@ func (check *Checker) exprInternal(T *target, x *operand, e ast.Expr, hint Type)
 					}
 					i := fieldIndex(utyp.fields, check.pkg, key.Name, false)
 					if i < 0 {
-						check.errorf(kv, MissingLitField, "unknown field %s in struct literal of type %s", key.Name, base)
+						var alt Object
+						if j := fieldIndex(fields, check.pkg, key.Name, true); j >= 0 {
+							alt = fields[j]
+						}
+						msg := check.lookupError(base, key.Name, alt, true)
+						check.error(kv.Key, MissingLitField, msg)
 						continue
 					}
 					fld := fields[i]

src/go/types/generate_test.go

@@ -103,6 +103,7 @@ var filemap = map[string]action{
 	"const.go":          func(f *ast.File) { fixTokenPos(f) },
 	"context.go":        nil,
 	"context_test.go":   nil,
+	"errsupport.go":     nil,
 	"gccgosizes.go":     nil,
 	"gcsizes.go":        func(f *ast.File) { renameIdents(f, "IsSyncAtomicAlign64->_IsSyncAtomicAlign64") },
 	"hilbert_test.go":   func(f *ast.File) { renameImportPath(f, `"cmd/compile/internal/types2"`, `"go/types"`) },

src/internal/types/testdata/check/lookup1.go

@@ -62,12 +62,12 @@ func _() {
 func _() {
 	var x big.Float
 	_ = x.neg // ERROR "x.neg undefined (type big.Float has no field or method neg, but does have method Neg)"
-	_ = x.nEg // ERROR "x.nEg undefined (type big.Float has no field or method nEg, but does have method Neg)"
+	_ = x.nEg // ERROR "x.nEg undefined (type big.Float has no field or method nEg)"
 	_ = x.Neg
 	_ = x.NEg // ERROR "x.NEg undefined (type big.Float has no field or method NEg, but does have method Neg)"
 
-	_ = x.form // ERROR "x.form undefined (field form is not exported)"
+	_ = x.form // ERROR "x.form undefined (cannot refer to unexported field form)"
 	_ = x.fOrm // ERROR "x.fOrm undefined (type big.Float has no field or method fOrm)"
-	_ = x.Form // ERROR "x.Form undefined (type big.Float has no field or method Form)"
+	_ = x.Form // ERROR "x.Form undefined (type big.Float has no field or method Form, but does have unexported field form)"
 	_ = x.FOrm // ERROR "x.FOrm undefined (type big.Float has no field or method FOrm)"
 }

src/internal/types/testdata/check/lookup2.go

@@ -0,0 +1,94 @@
+// Copyright 2024 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
+
+import (
+	"go/ast"
+	"math/big"
+)
+
+// case           sel     pkg   have   message (examples for general lookup)
+// ---------------------------------------------------------------------------------------------------------
+// ok             x.Foo   ==    Foo
+// misspelled     x.Foo   ==    FoO    type X has no field or method Foo, but does have field FoO
+// misspelled     x.Foo   ==    foo    type X has no field or method Foo, but does have field foo
+// misspelled     x.Foo   ==    foO    type X has no field or method Foo, but does have field foO
+//
+// misspelled     x.foo   ==    Foo    type X has no field or method foo, but does have field Foo
+// misspelled     x.foo   ==    FoO    type X has no field or method foo, but does have field FoO
+// ok             x.foo   ==    foo
+// misspelled     x.foo   ==    foO    type X has no field or method foo, but does have field foO
+//
+// ok             x.Foo   !=    Foo
+// misspelled     x.Foo   !=    FoO    type X has no field or method Foo, but does have field FoO
+// unexported     x.Foo   !=    foo    type X has no field or method Foo, but does have unexported field foo
+// missing        x.Foo   !=    foO    type X has no field or method Foo
+//
+// misspelled     x.foo   !=    Foo    type X has no field or method foo, but does have field Foo
+// missing        x.foo   !=    FoO    type X has no field or method foo
+// inaccessible   x.foo   !=    foo    cannot refer to unexported field foo
+// missing        x.foo   !=    foO    type X has no field or method foo
+
+type S struct {
+	Foo1 int
+	FoO2 int
+	foo3 int
+	foO4 int
+}
+
+func _() {
+	var x S
+	_ = x.Foo1 // OK
+	_ = x.Foo2 // ERROR "x.Foo2 undefined (type S has no field or method Foo2, but does have field FoO2)"
+	_ = x.Foo3 // ERROR "x.Foo3 undefined (type S has no field or method Foo3, but does have field foo3)"
+	_ = x.Foo4 // ERROR "x.Foo4 undefined (type S has no field or method Foo4, but does have field foO4)"
+
+	_ = x.foo1 // ERROR "x.foo1 undefined (type S has no field or method foo1, but does have field Foo1)"
+	_ = x.foo2 // ERROR "x.foo2 undefined (type S has no field or method foo2, but does have field FoO2)"
+	_ = x.foo3 // OK
+	_ = x.foo4 // ERROR "x.foo4 undefined (type S has no field or method foo4, but does have field foO4)"
+}
+
+func _() {
+	_ = S{Foo1: 0} // OK
+	_ = S{Foo2 /* ERROR "unknown field Foo2 in struct literal of type S, but does have FoO2" */ : 0}
+	_ = S{Foo3 /* ERROR "unknown field Foo3 in struct literal of type S, but does have foo3" */ : 0}
+	_ = S{Foo4 /* ERROR "unknown field Foo4 in struct literal of type S, but does have foO4" */ : 0}
+
+	_ = S{foo1 /* ERROR "unknown field foo1 in struct literal of type S, but does have Foo1" */ : 0}
+	_ = S{foo2 /* ERROR "unknown field foo2 in struct literal of type S, but does have FoO2" */ : 0}
+	_ = S{foo3: 0} // OK
+	_ = S{foo4 /* ERROR "unknown field foo4 in struct literal of type S, but does have foO4" */ : 0}
+}
+
+// The following tests follow the same pattern as above but operate on an imported type instead of S.
+// Currently our testing framework doesn't make it easy to define an imported package for testing, so
+// instead we use the big.Float and ast.File types as they provide a suitable mix of exported and un-
+// exported fields and methods.
+
+func _() {
+	var x *big.Float
+	_ = x.Neg  // OK
+	_ = x.NeG  // ERROR "x.NeG undefined (type *big.Float has no field or method NeG, but does have method Neg)"
+	_ = x.Form // ERROR "x.Form undefined (type *big.Float has no field or method Form, but does have unexported field form)"
+	_ = x.ForM // ERROR "x.ForM undefined (type *big.Float has no field or method ForM)"
+
+	_ = x.abs  // ERROR "x.abs undefined (type *big.Float has no field or method abs, but does have method Abs)"
+	_ = x.abS  // ERROR "x.abS undefined (type *big.Float has no field or method abS)"
+	_ = x.form // ERROR "x.form undefined (cannot refer to unexported field form)"
+	_ = x.forM // ERROR "x.forM undefined (type *big.Float has no field or method forM)"
+}
+
+func _() {
+	_ = ast.File{Name: nil} // OK
+	_ = ast.File{NamE /* ERROR "unknown field NamE in struct literal of type ast.File, but does have Name" */ : nil}
+	_ = big.Float{Form /* ERROR "unknown field Form in struct literal of type big.Float, but does have unexported form" */ : 0}
+	_ = big.Float{ForM /* ERROR "unknown field ForM in struct literal of type big.Float" */ : 0}
+
+	_ = ast.File{name /* ERROR "unknown field name in struct literal of type ast.File, but does have Name" */ : nil}
+	_ = ast.File{namE /* ERROR "unknown field namE in struct literal of type ast.File" */ : nil}
+	_ = big.Float{form /* ERROR "cannot refer to unexported field form in struct literal of type big.Float" */ : 0}
+	_ = big.Float{forM /* ERROR "unknown field forM in struct literal of type big.Float" */ : 0}
+}

src/internal/types/testdata/fixedbugs/issue49736.go

@@ -0,0 +1,17 @@
+// Copyright 2024 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
+
+import "math/big"
+
+// From go.dev/issue/18419
+func _(x *big.Float) {
+	x.form /* ERROR "x.form undefined (cannot refer to unexported field form)" */ ()
+}
+
+// From go.dev/issue/31053
+func _() {
+	_ = big.Float{form /* ERROR "cannot refer to unexported field form in struct literal of type big.Float" */ : 0}
+}

test/fixedbugs/issue22794.go

@@ -15,7 +15,7 @@ func main() {
 	i1 := it{Floats: true}
 	if i1.floats { // ERROR "(type it .* field or method floats, but does have field Floats)|undefined field or method"
 	}
-	i2 := &it{floats: false} // ERROR "(but does have field Floats)|unknown field|declared and not used"
+	i2 := &it{floats: false} // ERROR "cannot refer to unexported field floats in struct literal|unknown field|declared and not used"
 	_ = &it{InneR: "foo"}    // ERROR "(but does have field inner)|unknown field"
 	_ = i2
 }

test/fixedbugs/issue25727.go

@@ -11,11 +11,11 @@ import "net/http"
 var s = http.Server{}
 var _ = s.doneChan                  // ERROR "s.doneChan undefined .cannot refer to unexported field or method doneChan.$|unexported field or method|s.doneChan undefined"
 var _ = s.DoneChan                  // ERROR "s.DoneChan undefined .type http.Server has no field or method DoneChan.$|undefined field or method"
-var _ = http.Server{tlsConfig: nil} // ERROR "unknown field tlsConfig in struct literal.+ .but does have TLSConfig.$|unknown field .?tlsConfig.? in .?http.Server|unknown field"
+var _ = http.Server{tlsConfig: nil} // ERROR "cannot refer to unexported field tlsConfig in struct literal|unknown field .?tlsConfig.? in .?http.Server|unknown field"
 var _ = http.Server{DoneChan: nil}  // ERROR "unknown field DoneChan in struct literal of type http.Server$|unknown field .?DoneChan.? in .?http.Server"
 
 type foo struct {
 	bar int
 }
 
-var _ = &foo{bAr: 10} // ERROR "unknown field bAr in struct literal.+ .but does have bar.$|unknown field .?bAr.? in .?foo|unknown field"
+var _ = &foo{bAr: 10} // ERROR "cannot refer to unexported field bAr in struct literal|unknown field .?bAr.? in .?foo|unknown field"