go/analysis/passes/modernize: disable reflecttypefor modernizer for unnamed struct/interface

When a type contains an unnamed struct or an unnamed interface, we should
not suggest a fix because it will involve writing out the entire type and
will be more verbose than the existing code.

Fixes golang/go#76698

Change-Id: I5fdeb8dc5b7e70b2201fdf1fe714335aef439cc7
Reviewed-on: https://go-review.googlesource.com/c/tools/+/726941
Reviewed-by: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>

Author: madelinekalil

go/analysis/passes/modernize/reflect.go

@@ -102,6 +102,23 @@ func reflecttypefor(pass *analysis.Pass) (any, error) {
 			continue // e.g. reflect was dot-imported
 		}
 
+		// Don't offer a fix if the type contains an unnamed struct or unnamed
+		// interface because the replacement would be significantly more verbose.
+		// (See golang/go#76698)
+		if isComplicatedType(t) {
+			continue
+		}
+
+		// Don't offer the fix if the type string is too long. We define "too
+		// long" as more than three times the length of the original expression
+		// and at least 16 characters (a 3x length increase of a very
+		// short expression should not be cause for skipping the fix).
+		oldLen := int(expr.End() - expr.Pos())
+		newLen := len(tstr)
+		if newLen >= 16 && newLen > 3*oldLen {
+			continue
+		}
+
 		// If the call argument contains the last use
 		// of a variable, as in:
 		//	var zero T
@@ -137,3 +154,43 @@ func reflecttypefor(pass *analysis.Pass) (any, error) {
 
 	return nil, nil
 }
+
+// isComplicatedType reports whether type t is complicated, e.g. it is or contains an
+// unnamed struct, interface, or function signature.
+func isComplicatedType(t types.Type) bool {
+	var check func(typ types.Type) bool
+	check = func(typ types.Type) bool {
+		switch t := typ.(type) {
+		case typesinternal.NamedOrAlias:
+			for ta := range t.TypeArgs().Types() {
+				if check(ta) {
+					return true
+				}
+			}
+			return false
+		case *types.Struct, *types.Interface, *types.Signature:
+			// These are complex types with potentially many elements
+			// so we should avoid duplicating their definition.
+			return true
+		case *types.Pointer:
+			return check(t.Elem())
+		case *types.Slice:
+			return check(t.Elem())
+		case *types.Array:
+			return check(t.Elem())
+		case *types.Chan:
+			return check(t.Elem())
+		case *types.Map:
+			return check(t.Key()) || check(t.Elem())
+		case *types.Basic:
+			return false
+		case *types.TypeParam:
+			return false
+		default:
+			// Includes types.Union
+			return true
+		}
+	}
+
+	return check(t)
+}

go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go

@@ -6,8 +6,14 @@ import (
 	"time"
 )
 
+type A string
+
+type B[T any] int
+
 var (
 	x any
+	a A
+	b B[int]
 	_ = reflect.TypeOf(x)                 // nope (dynamic)
 	_ = reflect.TypeOf(0)                 // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = reflect.TypeOf(uint(0))           // want "reflect.TypeOf call can be simplified using TypeFor"
@@ -18,6 +24,8 @@ var (
 	_ = reflect.TypeOf(*new(time.Time))   // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = reflect.TypeOf(time.Time{})       // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = reflect.TypeOf(time.Duration(0))  // want "reflect.TypeOf call can be simplified using TypeFor"
+	_ = reflect.TypeOf(&a)                // want "reflect.TypeOf call can be simplified using TypeFor"
+	_ = reflect.TypeOf(&b)                // want "reflect.TypeOf call can be simplified using TypeFor"
 )
 
 // Eliminate local var if we deleted its last use.
@@ -29,3 +37,53 @@ func _() {
 	_ = reflect.TypeOf(z2) // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = z2                 // z2 has multiple uses
 }
+
+type T struct {
+	f struct {
+		A bool
+		B int
+		C string
+	}
+}
+
+type S struct {
+	f [2]struct {
+		A bool
+		B int
+		C string
+	}
+}
+
+type R []struct {
+	A int
+}
+
+type M[T struct{ F int }] int
+
+type P struct {
+	f interface {
+	}
+	g func() // fine length
+
+	long func(a int, b int, c int) (bool, string, int) // too long
+
+	s func(a struct{})
+
+	q func() struct{}
+}
+
+func f(t *T, r R, m *M[struct{ F int }], s *S, p *P) {
+	// No suggested fix for all of the following because the type is complicated -- e.g. has an unnamed struct,
+	// interface, or signature -- so the fix would be more verbose than the original expression.
+	// Also because structs and interfaces often acquire new fields and methods, and the type string
+	// produced by this modernizer won't get updated automatically, potentially causing a bug.
+	_ = reflect.TypeOf(&t.f)
+	_ = reflect.TypeOf(r[0])
+	_ = reflect.TypeOf(m)
+	_ = reflect.TypeOf(&s.f)
+	_ = reflect.TypeOf(&p.f)
+	_ = reflect.TypeOf(&p.g)
+	_ = reflect.TypeOf(&p.long)
+	_ = reflect.TypeOf(&p.q)
+	_ = reflect.TypeOf(&p.s)
+}

go/analysis/passes/modernize/testdata/src/reflecttypefor/reflecttypefor.go.golden

@@ -6,8 +6,14 @@ import (
 	"time"
 )
 
+type A string
+
+type B[T any] int
+
 var (
 	x any
+	a A
+	b B[int]
 	_ = reflect.TypeOf(x)                // nope (dynamic)
 	_ = reflect.TypeFor[int]()           // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = reflect.TypeFor[uint]()          // want "reflect.TypeOf call can be simplified using TypeFor"
@@ -18,6 +24,8 @@ var (
 	_ = reflect.TypeFor[time.Time]()     // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = reflect.TypeFor[time.Time]()     // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = reflect.TypeFor[time.Duration]() // want "reflect.TypeOf call can be simplified using TypeFor"
+	_ = reflect.TypeFor[*A]() // want "reflect.TypeOf call can be simplified using TypeFor"
+	_ = reflect.TypeFor[*B[int]]() // want "reflect.TypeOf call can be simplified using TypeFor"
 )
 
 // Eliminate local var if we deleted its last use.
@@ -28,3 +36,53 @@ func _() {
 	_ = reflect.TypeFor[string]() // want "reflect.TypeOf call can be simplified using TypeFor"
 	_ = z2                 // z2 has multiple uses
 }
+
+type T struct {
+	f struct {
+		A bool
+		B int
+		C string
+	}
+}
+
+type S struct {
+	f [2]struct {
+		A bool
+		B int
+		C string
+	}
+}
+
+type R []struct {
+	A int
+}
+
+type M[T struct{ F int }] int
+
+type P struct {
+	f interface {
+	}
+	g func() // fine length
+
+	long func(a int, b int, c int) (bool, string, int) // too long
+
+	s func(a struct{})
+
+	q func() struct{}
+}
+
+func f(t *T, r R, m *M[struct{ F int }], s *S, p *P) {
+	// No suggested fix for all of the following because the type is complicated -- e.g. has an unnamed struct,
+	// interface, or signature -- so the fix would be more verbose than the original expression.
+	// Also because structs and interfaces often acquire new fields and methods, and the type string
+	// produced by this modernizer won't get updated automatically, potentially causing a bug.
+	_ = reflect.TypeOf(&t.f)
+	_ = reflect.TypeOf(r[0])
+	_ = reflect.TypeOf(m)
+	_ = reflect.TypeOf(&s.f)
+	_ = reflect.TypeOf(&p.f)
+	_ = reflect.TypeOf(&p.g)
+	_ = reflect.TypeOf(&p.long)
+	_ = reflect.TypeOf(&p.q)
+	_ = reflect.TypeOf(&p.s)
+}