internal/typesinternal: support union types in func TypeExpr

- Updates "typesinternal.TypeExpr" to support the representation
of union types. Union types are now expressed as nested binary
expressions connected by `|`, mirroring the structure used in
"go/ast". The last term in the union corresponds to the topmost
level of the binary expression.
- Updates "typesinternal.ZeroValue" & "ZeroString" function
returnning both the zero value and a boolean indicating whether
the input type is valid or not.
- Removes the possibility of func TypeExpr returning nil,
eliminating the need for nil checks.
- Updates "typesinternal.ZeroValue" to support alias type param.
- Adds marker tests (type parameter, named type, alias type) for
fillStruct.

Fixes:
- Fill struct code action returns &*new(T) when input type is
pointer to type parameter.
- TypeExpr return expression without type params when input
type is instantiated type.
- ZeroString and ZeroExpr now return new(T) for type parameter
aliases. Previously, t.Underlying() on an alias returned the
underlying interface, resulting in nil.

Change-Id: Ib433d9ddca3dd329f950ca328455c4397d8c68bb
Reviewed-on: https://go-review.googlesource.com/c/tools/+/631995
LUCI-TryBot-Result: Go LUCI <[email protected]>
Auto-Submit: Hongxiang Jiang <[email protected]>
Reviewed-by: Alan Donovan <[email protected]>

Author: h9jiang

go/ssa/const.go

@@ -78,7 +78,7 @@ func zeroConst(t types.Type) *Const {
 func (c *Const) RelString(from *types.Package) string {
 	var s string
 	if c.Value == nil {
-		s = typesinternal.ZeroString(c.typ, types.RelativeTo(from))
+		s, _ = typesinternal.ZeroString(c.typ, types.RelativeTo(from))
 	} else if c.Value.Kind() == constant.String {
 		s = constant.StringVal(c.Value)
 		const max = 20

go/ssa/const_test.go

@@ -58,10 +58,10 @@ func TestConstString(t *testing.T) {
 		{"interface{string}", nil, `"":interface{string}`},
 		{"interface{int|int64}", nil, "0:interface{int|int64}"},
 		{"interface{bool}", nil, "false:interface{bool}"},
-		{"interface{bool|int}", nil, "nil:interface{bool|int}"},
-		{"interface{int|string}", nil, "nil:interface{int|string}"},
-		{"interface{bool|string}", nil, "nil:interface{bool|string}"},
-		{"interface{struct{x string}}", nil, "nil:interface{struct{x string}}"},
+		{"interface{bool|int}", nil, "invalid:interface{bool|int}"},
+		{"interface{int|string}", nil, "invalid:interface{int|string}"},
+		{"interface{bool|string}", nil, "invalid:interface{bool|string}"},
+		{"interface{struct{x string}}", nil, "invalid:interface{struct{x string}}"},
 		{"interface{int|int64}", int64(1), "1:interface{int|int64}"},
 		{"interface{~bool}", true, "true:interface{~bool}"},
 		{"interface{Named}", "lorem ipsum", `"lorem ipsum":interface{P.Named}`},

gopls/internal/analysis/fillreturns/fillreturns.go

@@ -184,7 +184,7 @@ outer:
 				// If no identifier matches the pattern, generate a zero value.
 				if best := fuzzy.BestMatch(retTyp.String(), names); best != "" {
 					fixed[i] = ast.NewIdent(best)
-				} else if zero := typesinternal.ZeroExpr(file, pass.Pkg, retTyp); zero != nil {
+				} else if zero, isValid := typesinternal.ZeroExpr(file, pass.Pkg, retTyp); isValid {
 					fixed[i] = zero
 				} else {
 					return nil, nil

gopls/internal/analysis/fillstruct/fillstruct.go

@@ -232,8 +232,8 @@ func SuggestedFix(fset *token.FileSet, start, end token.Pos, content []byte, fil
 			// NOTE: We currently match on the name of the field key rather than the field type.
 			if best := fuzzy.BestMatch(fieldName, names); best != "" {
 				kv.Value = ast.NewIdent(best)
-			} else if v := populateValue(file, pkg, fieldTyp); v != nil {
-				kv.Value = v
+			} else if expr, isValid := populateValue(file, pkg, fieldTyp); isValid {
+				kv.Value = expr
 			} else {
 				return nil, nil, nil // no fix to suggest
 			}
@@ -328,170 +328,109 @@ func indent(str, ind []byte) []byte {
 //
 // The reasoning here is that users will call fillstruct with the intention of
 // initializing the struct, in which case setting these fields to nil has no effect.
-//
-// populateValue returns nil if the value cannot be filled.
-func populateValue(f *ast.File, pkg *types.Package, typ types.Type) ast.Expr {
-	switch u := typ.Underlying().(type) {
-	case *types.Basic:
-		switch {
-		case u.Info()&types.IsNumeric != 0:
-			return &ast.BasicLit{Kind: token.INT, Value: "0"}
-		case u.Info()&types.IsBoolean != 0:
-			return &ast.Ident{Name: "false"}
-		case u.Info()&types.IsString != 0:
-			return &ast.BasicLit{Kind: token.STRING, Value: `""`}
-		case u.Kind() == types.UnsafePointer:
-			return ast.NewIdent("nil")
-		case u.Kind() == types.Invalid:
-			return nil
+func populateValue(f *ast.File, pkg *types.Package, typ types.Type) (_ ast.Expr, isValid bool) {
+	switch t := typ.(type) {
+	case *types.TypeParam, *types.Interface, *types.Struct, *types.Basic:
+		return typesinternal.ZeroExpr(f, pkg, t)
+
+	case *types.Alias, *types.Named:
+		switch t.Underlying().(type) {
+		// Avoid typesinternal.ZeroExpr here as we don't want to return nil.
+		case *types.Map, *types.Slice:
+			return &ast.CompositeLit{
+				Type: typesinternal.TypeExpr(f, pkg, t),
+			}, true
 		default:
-			panic(fmt.Sprintf("unknown basic type %v", u))
+			return typesinternal.ZeroExpr(f, pkg, t)
 		}
 
-	case *types.Map:
-		k := typesinternal.TypeExpr(f, pkg, u.Key())
-		v := typesinternal.TypeExpr(f, pkg, u.Elem())
-		if k == nil || v == nil {
-			return nil
-		}
-		return &ast.CompositeLit{
-			Type: &ast.MapType{
-				Key:   k,
-				Value: v,
-			},
-		}
-	case *types.Slice:
-		s := typesinternal.TypeExpr(f, pkg, u.Elem())
-		if s == nil {
-			return nil
-		}
+	// Avoid typesinternal.ZeroExpr here as we don't want to return nil.
+	case *types.Map, *types.Slice:
 		return &ast.CompositeLit{
-			Type: &ast.ArrayType{
-				Elt: s,
-			},
-		}
+			Type: typesinternal.TypeExpr(f, pkg, t),
+		}, true
 
 	case *types.Array:
-		a := typesinternal.TypeExpr(f, pkg, u.Elem())
-		if a == nil {
-			return nil
-		}
 		return &ast.CompositeLit{
 			Type: &ast.ArrayType{
-				Elt: a,
+				Elt: typesinternal.TypeExpr(f, pkg, t.Elem()),
 				Len: &ast.BasicLit{
-					Kind: token.INT, Value: fmt.Sprintf("%v", u.Len()),
+					Kind: token.INT, Value: fmt.Sprintf("%v", t.Len()),
 				},
 			},
-		}
+		}, true
 
 	case *types.Chan:
-		v := typesinternal.TypeExpr(f, pkg, u.Elem())
-		if v == nil {
-			return nil
-		}
-		dir := ast.ChanDir(u.Dir())
-		if u.Dir() == types.SendRecv {
+		dir := ast.ChanDir(t.Dir())
+		if t.Dir() == types.SendRecv {
 			dir = ast.SEND | ast.RECV
 		}
 		return &ast.CallExpr{
 			Fun: ast.NewIdent("make"),
 			Args: []ast.Expr{
 				&ast.ChanType{
 					Dir:   dir,
-					Value: v,
+					Value: typesinternal.TypeExpr(f, pkg, t.Elem()),
 				},
 			},
-		}
-
-	case *types.Struct:
-		s := typesinternal.TypeExpr(f, pkg, typ)
-		if s == nil {
-			return nil
-		}
-		return &ast.CompositeLit{
-			Type: s,
-		}
+		}, true
 
 	case *types.Signature:
-		var params []*ast.Field
-		for i := 0; i < u.Params().Len(); i++ {
-			p := typesinternal.TypeExpr(f, pkg, u.Params().At(i).Type())
-			if p == nil {
-				return nil
-			}
-			params = append(params, &ast.Field{
-				Type: p,
-				Names: []*ast.Ident{
-					{
-						Name: u.Params().At(i).Name(),
-					},
-				},
-			})
-		}
-		var returns []*ast.Field
-		for i := 0; i < u.Results().Len(); i++ {
-			r := typesinternal.TypeExpr(f, pkg, u.Results().At(i).Type())
-			if r == nil {
-				return nil
-			}
-			returns = append(returns, &ast.Field{
-				Type: r,
-			})
-		}
 		return &ast.FuncLit{
-			Type: &ast.FuncType{
-				Params: &ast.FieldList{
-					List: params,
-				},
-				Results: &ast.FieldList{
-					List: returns,
+			Type: typesinternal.TypeExpr(f, pkg, t).(*ast.FuncType),
+			// The body of the function literal contains a panic statement to
+			// avoid type errors.
+			Body: &ast.BlockStmt{
+				List: []ast.Stmt{
+					&ast.ExprStmt{
+						X: &ast.CallExpr{
+							Fun: ast.NewIdent("panic"),
+							Args: []ast.Expr{
+								&ast.BasicLit{
+									Kind:  token.STRING,
+									Value: `"TODO"`,
+								},
+							},
+						},
+					},
 				},
 			},
-			Body: &ast.BlockStmt{},
-		}
+		}, true
 
 	case *types.Pointer:
-		switch types.Unalias(u.Elem()).(type) {
+		switch tt := types.Unalias(t.Elem()).(type) {
 		case *types.Basic:
 			return &ast.CallExpr{
 				Fun: &ast.Ident{
 					Name: "new",
 				},
 				Args: []ast.Expr{
 					&ast.Ident{
-						Name: u.Elem().String(),
+						Name: t.Elem().String(),
 					},
 				},
-			}
+			}, true
+		// Pointer to type parameter should return new(T) instead of &*new(T).
+		case *types.TypeParam:
+			return &ast.CallExpr{
+				Fun: &ast.Ident{
+					Name: "new",
+				},
+				Args: []ast.Expr{
+					&ast.Ident{
+						Name: tt.Obj().Name(),
+					},
+				},
+			}, true
 		default:
-			x := populateValue(f, pkg, u.Elem())
-			if x == nil {
-				return nil
-			}
+			// TODO(hxjiang): & prefix only works if populateValue returns a
+			// composite literal T{} or the expression new(T).
+			expr, isValid := populateValue(f, pkg, t.Elem())
 			return &ast.UnaryExpr{
 				Op: token.AND,
-				X:  x,
-			}
-		}
-
-	case *types.Interface:
-		if param, ok := types.Unalias(typ).(*types.TypeParam); ok {
-			// *new(T) is the zero value of a type parameter T.
-			// TODO(adonovan): one could give a more specific zero
-			// value if the type has a core type that is, say,
-			// always a number or a pointer. See go/ssa for details.
-			return &ast.StarExpr{
-				X: &ast.CallExpr{
-					Fun: ast.NewIdent("new"),
-					Args: []ast.Expr{
-						ast.NewIdent(param.Obj().Name()),
-					},
-				},
-			}
+				X:  expr,
+			}, isValid
 		}
-
-		return ast.NewIdent("nil")
 	}
-	return nil
+	return nil, false
 }

gopls/internal/golang/addtest.go

@@ -497,7 +497,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
 		if i == 0 && isContextType(typ) {
 			f.Value = qf(types.NewPackage("context", "context")) + ".Background()"
 		} else if name == "" || name == "_" {
-			f.Value = typesinternal.ZeroString(typ, qf)
+			f.Value, _ = typesinternal.ZeroString(typ, qf)
 		} else {
 			f.Name = name
 		}
@@ -631,7 +631,7 @@ func AddTestForFunc(ctx context.Context, snapshot *cache.Snapshot, loc protocol.
 				if i == 0 && isContextType(typ) {
 					f.Value = qf(types.NewPackage("context", "context")) + ".Background()"
 				} else if name == "" || name == "_" {
-					f.Value = typesinternal.ZeroString(typ, qf)
+					f.Value, _ = typesinternal.ZeroString(typ, qf)
 				} else {
 					f.Name = name
 				}

gopls/internal/golang/completion/postfix_snippets.go

@@ -442,8 +442,8 @@ func (a *postfixTmplArgs) TypeName(t types.Type) (string, error) {
 
 // Zero return the zero value representation of type t
 func (a *postfixTmplArgs) Zero(t types.Type) string {
-	// TODO: use typesinternal.ZeroString, once it supports invalid types.
-	return formatZeroValue(t, a.qf)
+	zero, _ := typesinternal.ZeroString(t, a.qf)
+	return zero
 }
 
 func (a *postfixTmplArgs) IsIdent() bool {

gopls/internal/golang/completion/statements.go

@@ -15,6 +15,7 @@ import (
 	"golang.org/x/tools/gopls/internal/golang"
 	"golang.org/x/tools/gopls/internal/golang/completion/snippet"
 	"golang.org/x/tools/gopls/internal/protocol"
+	"golang.org/x/tools/internal/typesinternal"
 )
 
 // addStatementCandidates adds full statement completion candidates
@@ -294,7 +295,9 @@ func (c *completer) addErrCheck() {
 	} else {
 		snip.WriteText("return ")
 		for i := 0; i < result.Len()-1; i++ {
-			snip.WriteText(formatZeroValue(result.At(i).Type(), c.qf))
+			if zero, isValid := typesinternal.ZeroString(result.At(i).Type(), c.qf); isValid {
+				snip.WriteText(zero)
+			}
 			snip.WriteText(", ")
 		}
 		snip.WritePlaceholder(func(b *snippet.Builder) {
@@ -404,7 +407,10 @@ func (c *completer) addReturnZeroValues() {
 			fmt.Fprintf(&label, ", ")
 		}
 
-		zero := formatZeroValue(result.At(i).Type(), c.qf)
+		zero, isValid := typesinternal.ZeroString(result.At(i).Type(), c.qf)
+		if !isValid {
+			zero = ""
+		}
 		snip.WritePlaceholder(func(b *snippet.Builder) {
 			b.WriteText(zero)
 		})

gopls/internal/golang/completion/util.go

@@ -277,31 +277,6 @@ func prevStmt(pos token.Pos, path []ast.Node) ast.Stmt {
 	return nil
 }
 
-// formatZeroValue produces Go code representing the zero value of T. It
-// returns the empty string if T is invalid.
-//
-// TODO(rfindley): use typesinternal.ZeroString once we can sort out how to
-// propagate invalid types (see golang/go#70744).
-func formatZeroValue(T types.Type, qf types.Qualifier) string {
-	switch u := T.Underlying().(type) {
-	case *types.Basic:
-		switch {
-		case u.Info()&types.IsNumeric > 0:
-			return "0"
-		case u.Info()&types.IsString > 0:
-			return `""`
-		case u.Info()&types.IsBoolean > 0:
-			return "false"
-		default:
-			return ""
-		}
-	case *types.Pointer, *types.Interface, *types.Chan, *types.Map, *types.Slice, *types.Signature:
-		return "nil"
-	default:
-		return types.TypeString(T, qf) + "{}"
-	}
-}
-
 // isBasicKind returns whether t is a basic type of kind k.
 func isBasicKind(t types.Type, k types.BasicInfo) bool {
 	b, _ := t.Underlying().(*types.Basic)