[gopls-release-branch.0.17] 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.

Also, add back the replace directive for the release branch.

Fixes golang/go#70744

Change-Id: Ife9ae9a0fedf5a9d6b321cfb959f0d16eeec2986
Reviewed-on: https://go-review.googlesource.com/c/tools/+/634923
Reviewed-by: Alan Donovan <adonovan@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
(cherry picked from commit 82b6f75)
Reviewed-on: https://go-review.googlesource.com/c/tools/+/635215

gopls/go.mod

@@ -26,3 +26,5 @@ require (
 	golang.org/x/exp/typeparams v0.0.0-20231108232855-2478ac86f678 // indirect
 	gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 // indirect
 )
+
+replace golang.org/x/tools => ../

gopls/go.sum

@@ -14,22 +14,38 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/rogpeppe/go-internal v1.12.0 h1:exVL4IDcn6na9z1rAb56Vxr+CgyK3nn3O+epU5NdKM8=
 github.com/rogpeppe/go-internal v1.12.0/go.mod h1:E+RYuTGaKKdloAfM02xzb0FW3Paa99yedzYV+kq4uf4=
+github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
+golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU=
+golang.org/x/crypto v0.29.0/go.mod h1:+F4F4N5hv6v38hfeYwTdx20oUvLLc+QfrE9Ax9HtgRg=
 golang.org/x/exp/typeparams v0.0.0-20231108232855-2478ac86f678 h1:1P7xPZEwZMoBoz0Yze5Nx2/4pxj6nw9ZqHWXqP0iRgQ=
 golang.org/x/exp/typeparams v0.0.0-20231108232855-2478ac86f678/go.mod h1:AbB0pIl9nAr9wVwH+Z2ZpaocVmF5I4GyWCDIsVjR0bk=
+golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c=
 golang.org/x/mod v0.22.0 h1:D4nJWe9zXqHOmWqj4VMOJhvzj7bEZg4wEYa759z1pH4=
 golang.org/x/mod v0.22.0/go.mod h1:6SkKJ3Xj0I0BrPOZoBy3bdMptDDU9oJrpohJ3eWZ1fY=
+golang.org/x/net v0.10.0/go.mod h1:0qNGK6F8kojg2nk9dLZ2mShWaEBan6FAoqfSigmmuDg=
+golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44=
+golang.org/x/net v0.31.0/go.mod h1:P4fl1q7dY2hnZFxEk4pPSkDHF+QqjitcnDjUQyMM+pM=
+golang.org/x/sync v0.7.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
 golang.org/x/sync v0.9.0 h1:fEo0HyrW1GIgZdpbhCRO0PkJajUS5H9IFUztCgEo2jQ=
 golang.org/x/sync v0.9.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=
+golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
+golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.27.0 h1:wBqf8DvsY9Y/2P8gAfPDEYNuS30J4lPHJxXSb/nJZ+s=
 golang.org/x/sys v0.27.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/telemetry v0.0.0-20240521205824-bda55230c457/go.mod h1:pRgIJT+bRLFKnoM1ldnzKoxTIn14Yxz928LQRYYgIN0=
 golang.org/x/telemetry v0.0.0-20241106142447-58a1122356f5 h1:TCDqnvbBsFapViksHcHySl/sW4+rTGNIAoJJesHRuMM=
 golang.org/x/telemetry v0.0.0-20241106142447-58a1122356f5/go.mod h1:8nZWdGp9pq73ZI//QJyckMQab3yq7hoWi7SI0UIusVI=
+golang.org/x/term v0.8.0/go.mod h1:xPskH00ivmX89bAKVGSKKtLOWNx2+17Eiy94tnKShWo=
+golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk=
+golang.org/x/term v0.26.0/go.mod h1:Si5m1o57C5nBNQo5z1iq+XDijt21BDBDp2bK0QI8e3E=
 golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
+golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
+golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 golang.org/x/text v0.20.0 h1:gK/Kv2otX8gz+wn7Rmb3vT96ZwuoxnQlY+HlJVj7Qug=
 golang.org/x/text v0.20.0/go.mod h1:D4IsuqiFMhST5bX19pQ9ikHC2GsaKyk/oF+pn3ducp4=
-golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
-golang.org/x/tools v0.27.1-0.20241204210344-1eb875fe7736 h1:FbrmQg54JnK6XU/imxsp4zJQgBNXbUEu4pAjQHxfEQQ=
-golang.org/x/tools v0.27.1-0.20241204210344-1eb875fe7736/go.mod h1:sUi0ZgbwW9ZPAq26Ekut+weQPR5eIM6GQLQ1Yjm1H0Q=
 golang.org/x/vuln v1.0.4 h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I=
 golang.org/x/vuln v1.0.4/go.mod h1:NbJdUQhX8jY++FtuhrXs2Eyx0yePo9pF7nPlIjo9aaQ=
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

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))
 		}