gopls/internal/lsp/analysis: fix stubmethods with variadic parameters

Fix the stubmethods code action when matching against calls of variadic
functions, and add a regression test.

Additionally, reinstate panic recovery for code actions that were
migrated from convenience analyzers. I do not have confidence that this
is the only such panic.

Fixes golang/go#61693

Change-Id: Id2642a13d3f793de27c0d7ebfa665428d671c56f
Reviewed-on: https://go-review.googlesource.com/c/tools/+/514755
Run-TryBot: Robert Findley <[email protected]>
gopls-CI: kokoro <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Peter Weinberger <[email protected]>

Author: findleyr

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