all: merge master (5b4d426) into gopls-release-branch.0.13

For golang/go#61675

Merge List:

+ 2023-08-01 5b4d426 gopls/internal/hooks: clean language version before passing to gofumpt
+ 2023-08-01 2160c5f gopls/internal/lsp/analysis: fix stubmethods with variadic parameters

Change-Id: I5a27fc0307d7f032b6f06d677a29a757a3c7f758

Author: findleyr

gopls/internal/hooks/gofumpt_118.go

@@ -9,16 +9,70 @@ package hooks
 
 import (
 	"context"
+	"fmt"
 
 	"golang.org/x/tools/gopls/internal/lsp/source"
 	"mvdan.cc/gofumpt/format"
 )
 
 func updateGofumpt(options *source.Options) {
 	options.GofumptFormat = func(ctx context.Context, langVersion, modulePath string, src []byte) ([]byte, error) {
+		fixedVersion, err := fixLangVersion(langVersion)
+		if err != nil {
+			return nil, err
+		}
 		return format.Source(src, format.Options{
-			LangVersion: langVersion,
+			LangVersion: fixedVersion,
 			ModulePath:  modulePath,
 		})
 	}
 }
+
+// fixLangVersion function cleans the input so that gofumpt doesn't panic. It is
+// rather permissive, and accepts version strings that aren't technically valid
+// in a go.mod file.
+//
+// More specifically, it looks for an optional 'v' followed by 1-3
+// '.'-separated numbers. The resulting string is stripped of any suffix beyond
+// this expected version number pattern.
+//
+// See also golang/go#61692: gofumpt does not accept the new language versions
+// appearing in go.mod files (e.g. go1.21rc3).
+func fixLangVersion(input string) (string, error) {
+	bad := func() (string, error) {
+		return "", fmt.Errorf("invalid language version syntax %q", input)
+	}
+	if input == "" {
+		return input, nil
+	}
+	i := 0
+	if input[0] == 'v' { // be flexible about 'v'
+		i++
+	}
+	// takeDigits consumes ascii numerals 0-9 and reports if at least one was
+	// consumed.
+	takeDigits := func() bool {
+		found := false
+		for ; i < len(input) && '0' <= input[i] && input[i] <= '9'; i++ {
+			found = true
+		}
+		return found
+	}
+	if !takeDigits() { // versions must start with at least one number
+		return bad()
+	}
+
+	// Accept optional minor and patch versions.
+	for n := 0; n < 2; n++ {
+		if i < len(input) && input[i] == '.' {
+			// Look for minor/patch version.
+			i++
+			if !takeDigits() {
+				i--
+				break
+			}
+		}
+	}
+	// Accept any suffix.
+	return input[:i], nil
+}

gopls/internal/hooks/gofumpt_118_test.go

@@ -0,0 +1,53 @@
+// Copyright 2022 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.
+
+//go:build go1.18
+// +build go1.18
+
+package hooks
+
+import "testing"
+
+func TestFixLangVersion(t *testing.T) {
+	tests := []struct {
+		input, want string
+		wantErr     bool
+	}{
+		{"", "", false},
+		{"1.18", "1.18", false},
+		{"v1.18", "v1.18", false},
+		{"1.21", "1.21", false},
+		{"1.21rc3", "1.21", false},
+		{"1.21.0", "1.21.0", false},
+		{"1.21.1", "1.21.1", false},
+		{"v1.21.1", "v1.21.1", false},
+		{"v1.21.0rc1", "v1.21.0", false}, // not technically valid, but we're flexible
+		{"v1.21.0.0", "v1.21.0", false},  // also technically invalid
+		{"1.1", "1.1", false},
+		{"v1", "v1", false},
+		{"1", "1", false},
+		{"v1.21.", "v1.21", false}, // also invalid
+		{"1.21.", "1.21", false},
+
+		// Error cases.
+		{"rc1", "", true},
+		{"x1.2.3", "", true},
+	}
+
+	for _, test := range tests {
+		got, err := fixLangVersion(test.input)
+		if test.wantErr {
+			if err == nil {
+				t.Errorf("fixLangVersion(%q) succeeded unexpectedly", test.input)
+			}
+			continue
+		}
+		if err != nil {
+			t.Fatalf("fixLangVersion(%q) failed: %v", test.input, err)
+		}
+		if got != test.want {
+			t.Errorf("fixLangVersion(%q) = %s, want %s", test.input, got, test.want)
+		}
+	}
+}

gopls/internal/lsp/analysis/stubmethods/stubmethods.go

@@ -174,8 +174,19 @@ func fromCallExpr(fset *token.FileSet, ti *types.Info, pos token.Pos, ce *ast.Ca
 	if !ok {
 		return nil
 	}
-	sigVar := sig.Params().At(paramIdx)
-	iface := ifaceObjFromType(sigVar.Type())
+	var paramType types.Type
+	if sig.Variadic() && paramIdx >= sig.Params().Len()-1 {
+		v := sig.Params().At(sig.Params().Len() - 1)
+		if s, _ := v.Type().(*types.Slice); s != nil {
+			paramType = s.Elem()
+		}
+	} else if paramIdx < sig.Params().Len() {
+		paramType = sig.Params().At(paramIdx).Type()
+	}
+	if paramType == nil {
+		return nil // A type error prevents us from determining the param type.
+	}
+	iface := ifaceObjFromType(paramType)
 	if iface == nil {
 		return nil
 	}

gopls/internal/lsp/code_action.go

@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"golang.org/x/tools/go/ast/inspector"
+	"golang.org/x/tools/gopls/internal/bug"
 	"golang.org/x/tools/gopls/internal/lsp/analysis/fillstruct"
 	"golang.org/x/tools/gopls/internal/lsp/analysis/infertypeargs"
 	"golang.org/x/tools/gopls/internal/lsp/analysis/stubmethods"
@@ -198,26 +199,46 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara
 			if err != nil {
 				return nil, err
 			}
-			for _, pd := range diagnostics {
+			for _, pd := range stubMethodsDiagnostics {
 				start, end, err := pgf.RangePos(pd.Range)
 				if err != nil {
 					return nil, err
 				}
-				if d, ok := stubmethods.DiagnosticForError(pkg.FileSet(), pgf.File, start, end, pd.Message, pkg.GetTypesInfo()); ok {
+				action, ok, err := func() (_ protocol.CodeAction, _ bool, rerr error) {
+					// golang/go#61693: code actions were refactored to run outside of the
+					// analysis framework, but as a result they lost their panic recovery.
+					//
+					// Stubmethods "should never fail"", but put back the panic recovery as a
+					// defensive measure.
+					defer func() {
+						if r := recover(); r != nil {
+							rerr = bug.Errorf("stubmethods panicked: %v", r)
+						}
+					}()
+					d, ok := stubmethods.DiagnosticForError(pkg.FileSet(), pgf.File, start, end, pd.Message, pkg.GetTypesInfo())
+					if !ok {
+						return protocol.CodeAction{}, false, nil
+					}
 					cmd, err := command.NewApplyFixCommand(d.Message, command.ApplyFixArgs{
 						URI:   protocol.URIFromSpanURI(pgf.URI),
 						Fix:   source.StubMethods,
 						Range: pd.Range,
 					})
 					if err != nil {
-						return nil, err
+						return protocol.CodeAction{}, false, err
 					}
-					actions = append(actions, protocol.CodeAction{
+					return protocol.CodeAction{
 						Title:       d.Message,
 						Kind:        protocol.QuickFix,
 						Command:     &cmd,
 						Diagnostics: []protocol.Diagnostic{pd},
-					})
+					}, true, nil
+				}()
+				if err != nil {
+					return nil, err
+				}
+				if ok {
+					actions = append(actions, action)
 				}
 			}
 
@@ -387,7 +408,17 @@ func refactorExtract(ctx context.Context, snapshot source.Snapshot, pgf *source.
 	return actions, nil
 }
 
-func refactorRewrite(ctx context.Context, snapshot source.Snapshot, pkg source.Package, pgf *source.ParsedGoFile, fh source.FileHandle, rng protocol.Range) ([]protocol.CodeAction, error) {
+func refactorRewrite(ctx context.Context, snapshot source.Snapshot, pkg source.Package, pgf *source.ParsedGoFile, fh source.FileHandle, rng protocol.Range) (_ []protocol.CodeAction, rerr error) {
+	// golang/go#61693: code actions were refactored to run outside of the
+	// analysis framework, but as a result they lost their panic recovery.
+	//
+	// These code actions should never fail, but put back the panic recovery as a
+	// defensive measure.
+	defer func() {
+		if r := recover(); r != nil {
+			rerr = bug.Errorf("refactor.rewrite code actions panicked: %v", r)
+		}
+	}()
 	start, end, err := pgf.RangePos(rng)
 	if err != nil {
 		return nil, err

gopls/internal/regtest/marker/testdata/stubmethods/issue61693.txt

@@ -0,0 +1,35 @@
+This test exercises stub methods functionality with variadic parameters.
+
+In golang/go#61693 stubmethods was panicking in this case.
+
+-- go.mod --
+module mod.com
+
+go 1.18
+-- main.go --
+package main
+
+type C int
+
+func F(err ...error) {}
+
+func _() {
+	var x error
+	F(x, C(0)) //@suggestedfix(re"C.0.", re"missing method Error", "quickfix", stub)
+}
+-- @stub/main.go --
+package main
+
+type C int
+
+// Error implements error.
+func (C) Error() string {
+	panic("unimplemented")
+}
+
+func F(err ...error) {}
+
+func _() {
+	var x error
+	F(x, C(0)) //@suggestedfix(re"C.0.", re"missing method Error", "quickfix", stub)
+}

gopls/internal/regtest/misc/formatting_test.go

@@ -366,3 +366,30 @@ const Bar = 42
 		}
 	})
 }
+
+func TestGofumpt_Issue61692(t *testing.T) {
+	testenv.NeedsGo1Point(t, 21)
+
+	const input = `
+-- go.mod --
+module foo
+
+go 1.21rc3
+-- foo.go --
+package foo
+
+func _() {
+	foo :=
+		"bar"
+}
+`
+
+	WithOptions(
+		Settings{
+			"gofumpt": true,
+		},
+	).Run(t, input, func(t *testing.T, env *Env) {
+		env.OpenFile("foo.go")
+		env.FormatBuffer("foo.go") // golang/go#61692: must not panic
+	})
+}