internal/typesinternal: rollback of Zero{Value,Expr} for gopls

In CL 626537, the gopls behavior of formatting or generating a type
expression for the zero value of a types.Type was changed to panic in
the presence of invalid types. This leads to multiple panics in gopls
(see tests). Fix by selectively rolling back this aspect of the change,
as it relates to gopls.

Fixes golang/go#70744

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

Author: findleyr

gopls/internal/golang/completion/postfix_snippets.go

@@ -442,7 +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 {
-	return typesinternal.ZeroString(t, a.qf)
+	// TODO: use typesinternal.ZeroString, once it supports invalid types.
+	return formatZeroValue(t, a.qf)
 }
 
 func (a *postfixTmplArgs) IsIdent() bool {

gopls/internal/golang/completion/statements.go

@@ -15,7 +15,6 @@ 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
@@ -295,7 +294,7 @@ func (c *completer) addErrCheck() {
 	} else {
 		snip.WriteText("return ")
 		for i := 0; i < result.Len()-1; i++ {
-			snip.WriteText(typesinternal.ZeroString(result.At(i).Type(), c.qf))
+			snip.WriteText(formatZeroValue(result.At(i).Type(), c.qf))
 			snip.WriteText(", ")
 		}
 		snip.WritePlaceholder(func(b *snippet.Builder) {
@@ -405,7 +404,7 @@ func (c *completer) addReturnZeroValues() {
 			fmt.Fprintf(&label, ", ")
 		}
 
-		zero := typesinternal.ZeroString(result.At(i).Type(), c.qf)
+		zero := formatZeroValue(result.At(i).Type(), c.qf)
 		snip.WritePlaceholder(func(b *snippet.Builder) {
 			b.WriteText(zero)
 		})

gopls/internal/golang/completion/util.go

@@ -277,6 +277,31 @@ 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)

gopls/internal/test/marker/testdata/completion/statements.txt

@@ -98,6 +98,20 @@ func two() error {
 	//@snippet("", stmtTwoIfErrReturn, "if s.err != nil {\n\treturn ${1:s.err}\n\\}")
 }
 
+-- if_err_check_return3.go --
+package statements
+
+import "os"
+
+// Check that completion logic handles an invalid return type.
+func badReturn() (NotAType, error) {
+	_, err := os.Open("foo")
+	//@snippet("", stmtOneIfErrReturn, "if err != nil {\n\treturn , ${1:err}\n\\}")
+
+	_, err = os.Open("foo")
+	if er //@snippet(" //", stmtOneErrReturn, "err != nil {\n\treturn , ${1:err}\n\\}")
+}
+
 -- if_err_check_test.go --
 package statements
 
@@ -132,3 +146,10 @@ func foo() (int, string, error) {
 func bar() (int, string, error) {
 	return //@snippet(" ", stmtReturnZeroValues, "return ${1:0}, ${2:\"\"}, ${3:nil}")
 }
+
+
+//@item(stmtReturnInvalidValues, `return `)
+
+func invalidReturnStatement() NotAType {
+	return //@snippet(" ", stmtReturnInvalidValues, "return ${1:}")
+}

internal/typesinternal/zerovalue.go

@@ -80,6 +80,11 @@ func ZeroString(t types.Type, qf types.Qualifier) string {
 // ZeroExpr is defined for types that are suitable for variables.
 // It may panic for other types such as Tuple or Union.
 // See [ZeroString] for a variant that returns a string.
+//
+// TODO(rfindley): improve the behavior of this function in the presence of
+// invalid types. It should probably not return nil for
+// types.Typ[types.Invalid], but must to preserve previous behavior.
+// (golang/go#70744)
 func ZeroExpr(f *ast.File, pkg *types.Package, typ types.Type) ast.Expr {
 	switch t := typ.(type) {
 	case *types.Basic:
@@ -94,6 +99,8 @@ func ZeroExpr(f *ast.File, pkg *types.Package, typ types.Type) ast.Expr {
 			fallthrough
 		case t.Kind() == types.UntypedNil:
 			return ast.NewIdent("nil")
+		case t == types.Typ[types.Invalid]:
+			return nil
 		default:
 			panic(fmt.Sprint("ZeroExpr for unexpected type:", t))
 		}