gopls/internal/golang: allow "query" CodeActions on generated files

Previously, all CodeAction were assumed to be fixes or refactorings,
but GoTest and GoDoc are pure queries, and should be permitted even
in generated files. Unfortunately there's no systematic way to
distinguish, so for now we use an ad-hoc allow-list.

+ Test

Also:
- unexport golang.TestsAndBenchmarks
- clarify "want kind" logic in server.CodeAction

Change-Id: I78b991c68252505c962d22275a9e47bb1433ee40
Reviewed-on: https://go-review.googlesource.com/c/tools/+/577257
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/golang/code_lens.go

@@ -43,7 +43,7 @@ func runTestCodeLens(ctx context.Context, snapshot *cache.Snapshot, fh file.Hand
 	if err != nil {
 		return nil, err
 	}
-	fns, err := TestsAndBenchmarks(pkg, pgf)
+	fns, err := testsAndBenchmarks(pkg, pgf)
 	if err != nil {
 		return nil, err
 	}
@@ -99,7 +99,7 @@ type TestFns struct {
 	Benchmarks []TestFn
 }
 
-func TestsAndBenchmarks(pkg *cache.Package, pgf *parsego.File) (TestFns, error) {
+func testsAndBenchmarks(pkg *cache.Package, pgf *parsego.File) (TestFns, error) {
 	var out TestFns
 
 	if !strings.HasSuffix(pgf.URI.Path(), "_test.go") {

gopls/internal/golang/codeaction.go

@@ -33,6 +33,10 @@ func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle,
 	// Only compute quick fixes if there are any diagnostics to fix.
 	wantQuickFixes := want[protocol.QuickFix] && len(diagnostics) > 0
 
+	// Note: don't forget to update the allow-list in Server.CodeAction
+	// when adding new query operations like GoTest and GoDoc that
+	// are permitted even in generated source files
+
 	// Code actions requiring syntax information alone.
 	if wantQuickFixes || want[protocol.SourceOrganizeImports] || want[protocol.RefactorExtract] {
 		pgf, err := snapshot.ParseGo(ctx, fh, parsego.Full)
@@ -478,7 +482,7 @@ func getInlineCodeActions(pkg *cache.Package, pgf *parsego.File, rng protocol.Ra
 
 // getGoTestCodeActions returns any "run this test/benchmark" code actions for the selection.
 func getGoTestCodeActions(pkg *cache.Package, pgf *parsego.File, rng protocol.Range) ([]protocol.CodeAction, error) {
-	fns, err := TestsAndBenchmarks(pkg, pgf)
+	fns, err := testsAndBenchmarks(pkg, pgf)
 	if err != nil {
 		return nil, err
 	}

gopls/internal/server/code_action.go

@@ -16,6 +16,7 @@ import (
 	"golang.org/x/tools/gopls/internal/mod"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/protocol/command"
+	"golang.org/x/tools/gopls/internal/util/slices"
 	"golang.org/x/tools/internal/event"
 )
 
@@ -42,27 +43,24 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara
 
 	// The Only field of the context specifies which code actions the client wants.
 	// If Only is empty, assume that the client wants all of the non-explicit code actions.
-	var want map[protocol.CodeActionKind]bool
-	{
-		// Explicit Code Actions are opt-in and shouldn't be returned to the client unless
-		// requested using Only.
+	want := supportedCodeActions
+	if len(params.Context.Only) > 0 {
+		want = make(map[protocol.CodeActionKind]bool)
+
+		// Explicit Code Actions are opt-in and shouldn't be
+		// returned to the client unless requested using Only.
 		// TODO: Add other CodeLenses such as GoGenerate, RegenerateCgo, etc..
 		explicit := map[protocol.CodeActionKind]bool{
 			protocol.GoTest: true,
 		}
 
-		if len(params.Context.Only) == 0 {
-			want = supportedCodeActions
-		} else {
-			want = make(map[protocol.CodeActionKind]bool)
-			for _, only := range params.Context.Only {
-				for k, v := range supportedCodeActions {
-					if only == k || strings.HasPrefix(string(k), string(only)+".") {
-						want[k] = want[k] || v
-					}
+		for _, only := range params.Context.Only {
+			for k, v := range supportedCodeActions {
+				if only == k || strings.HasPrefix(string(k), string(only)+".") {
+					want[k] = want[k] || v
 				}
-				want[only] = want[only] || explicit[only]
 			}
+			want[only] = want[only] || explicit[only]
 		}
 	}
 	if len(want) == 0 {
@@ -103,12 +101,6 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara
 		return actions, nil
 
 	case file.Go:
-		// Don't suggest fixes for generated files, since they are generally
-		// not useful and some editors may apply them automatically on save.
-		if golang.IsGenerated(ctx, snapshot, uri) {
-			return nil, nil
-		}
-
 		actions, err := s.codeActionsMatchingDiagnostics(ctx, uri, snapshot, params.Context.Diagnostics, want)
 		if err != nil {
 			return nil, err
@@ -120,6 +112,20 @@ func (s *server) CodeAction(ctx context.Context, params *protocol.CodeActionPara
 		}
 		actions = append(actions, moreActions...)
 
+		// Don't suggest fixes for generated files, since they are generally
+		// not useful and some editors may apply them automatically on save.
+		// (Unfortunately there's no reliable way to distinguish fixes from
+		// queries, so we must list all kinds of queries here.)
+		if golang.IsGenerated(ctx, snapshot, uri) {
+			actions = slices.DeleteFunc(actions, func(a protocol.CodeAction) bool {
+				switch a.Kind {
+				case protocol.GoTest, protocol.GoDoc:
+					return false // read-only query
+				}
+				return true // potential write operation
+			})
+		}
+
 		return actions, nil
 
 	default: