Skip to content

Commit 444aadd

Browse files
committed
gopls/internal/cmd: redesign codeaction subcommand
This CL redesigns the codeaction subcommand (formerly named "fix") to make it more regular and useful: - By default, filtered (matching) code actions are merely printed unless the -exec flag is specified, as with 'gopls codelens'. In particular, this means that the tests no longer have the side effect of executing any action that is always offered, such as the forthcoming "open gopls documentation" action. - By default, all kinds of actions are returned unless explicitly filtered with a -kind flag. - The -all flag, which prevented discarding of non-preferred actions, has gone away. (All gopls actions are non-preferred, at least for now, so no flag is needed; if things change, the flag should be a tristate -preferred=auto|true|bool.) - The "fix" command is retained as a trivial stub for clarity. - Actions may be filtered by -title=regexp. - Disabled actions are discarded. Change-Id: Ic88c1232bbc7ff24ae33e6427c3773cb2564eb06 Reviewed-on: https://go-review.googlesource.com/c/tools/+/596797 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent cad260e commit 444aadd

File tree

11 files changed

+353
-238
lines changed

11 files changed

+353
-238
lines changed

gopls/doc/features/transformation.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ Client support for code actions:
103103
available (if there are multiple) and execute it.
104104
Some action kinds have filtering shortcuts,
105105
e.g. [`M-x eglot-code-action-{inline,extract,rewrite}`](https://joaotavora.github.io/eglot/#index-M_002dx-eglot_002dcode_002daction_002dinline).
106-
- **CLI**: `gopls fix -a file.go:#123-#456 kinds...` executes code actions of the specified
107-
kinds (e.g. `refactor.inline`) on the selected range, specified using zero-based byte offsets.
106+
- **CLI**: `gopls codeaction -exec -kind k,... -diff file.go:#123-#456` executes code actions of the specified
107+
kinds (e.g. `refactor.inline`) on the selected range, specified using zero-based byte offsets, and displays the diff.
108108

109109
## Formatting
110110

gopls/internal/cmd/cmd.go

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ type Application struct {
7575
editFlags *EditFlags
7676
}
7777

78-
// EditFlags defines flags common to {fix,format,imports,rename}
78+
// EditFlags defines flags common to {code{action,lens},format,imports,rename}
7979
// that control how edits are applied to the client's files.
8080
//
8181
// The type is exported for flag reflection.
@@ -131,6 +131,10 @@ It is typically used with an editor to provide language features. When no
131131
command is specified, gopls will default to the 'serve' command. The language
132132
features can also be accessed via the gopls command-line interface.
133133
134+
For documentation of all its features, see:
135+
136+
https://github.com/golang/tools/blob/master/gopls/doc/features
137+
134138
Usage:
135139
gopls help [<subject>]
136140
@@ -280,9 +284,11 @@ func (app *Application) featureCommands() []tool.Application {
280284
return []tool.Application{
281285
&callHierarchy{app: app},
282286
&check{app: app},
287+
&codeaction{app: app},
283288
&codelens{app: app},
284289
&definition{app: app},
285290
&execute{app: app},
291+
&fix{app: app}, // (non-functional)
286292
&foldingRanges{app: app},
287293
&format{app: app},
288294
&highlight{app: app},
@@ -297,7 +303,6 @@ func (app *Application) featureCommands() []tool.Application {
297303
&semtok{app: app},
298304
&signature{app: app},
299305
&stats{app: app},
300-
&suggestedFix{app: app},
301306
&symbols{app: app},
302307

303308
&workspaceSymbol{app: app},
@@ -931,3 +936,17 @@ func pointPosition(m *protocol.Mapper, p point) (protocol.Position, error) {
931936
}
932937
return protocol.Position{}, fmt.Errorf("point has neither offset nor line/column")
933938
}
939+
940+
// TODO(adonovan): delete in 2025.
941+
type fix struct{ app *Application }
942+
943+
func (*fix) Name() string { return "fix" }
944+
func (cmd *fix) Parent() string { return cmd.app.Name() }
945+
func (*fix) Usage() string { return "" }
946+
func (*fix) ShortHelp() string { return "apply suggested fixes (obsolete)" }
947+
func (*fix) DetailedHelp(flags *flag.FlagSet) {
948+
fmt.Fprintf(flags.Output(), `No longer supported; use "gopls codeaction" instead.`)
949+
}
950+
func (*fix) Run(ctx context.Context, args ...string) error {
951+
return tool.CommandLineErrorf(`no longer supported; use "gopls codeaction" instead`)
952+
}

gopls/internal/cmd/codeaction.go

Lines changed: 210 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,210 @@
1+
// Copyright 2024 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package cmd
6+
7+
import (
8+
"context"
9+
"flag"
10+
"fmt"
11+
"regexp"
12+
"strings"
13+
14+
"golang.org/x/tools/gopls/internal/protocol"
15+
"golang.org/x/tools/gopls/internal/util/slices"
16+
"golang.org/x/tools/internal/tool"
17+
)
18+
19+
// codeaction implements the codeaction verb for gopls.
20+
type codeaction struct {
21+
EditFlags
22+
Kind string `flag:"kind" help:"comma-separated list of code action kinds to filter"`
23+
Title string `flag:"title" help:"regular expression to match title"`
24+
Exec bool `flag:"exec" help:"execute the first matching code action"`
25+
26+
app *Application
27+
}
28+
29+
func (cmd *codeaction) Name() string { return "codeaction" }
30+
func (cmd *codeaction) Parent() string { return cmd.app.Name() }
31+
func (cmd *codeaction) Usage() string { return "[codeaction-flags] filename[:line[:col]]" }
32+
func (cmd *codeaction) ShortHelp() string { return "list or execute code actions" }
33+
func (cmd *codeaction) DetailedHelp(f *flag.FlagSet) {
34+
fmt.Fprintf(f.Output(), `
35+
36+
The codeaction command lists or executes code actions for the
37+
specified file or range of a file. Each code action contains
38+
either an edit to be directly applied to the file, or a command
39+
to be executed by the server, which may have an effect such as:
40+
- requesting that the client apply an edit;
41+
- changing the state of the server; or
42+
- requesting that the client open a document.
43+
44+
The -kind and and -title flags filter the list of actions.
45+
46+
The -kind flag specifies a comma-separated list of LSP CodeAction kinds.
47+
Only actions of these kinds will be requested from the server.
48+
Valid kinds include:
49+
50+
quickfix
51+
refactor
52+
refactor.extract
53+
refactor.inline
54+
refactor.rewrite
55+
source.organizeImports
56+
source.fixAll
57+
source.assembly
58+
source.doc
59+
source.freesymbols
60+
goTest
61+
62+
Kinds are hierarchical, so "refactor" includes "refactor.inline".
63+
(Note: actions of kind "goTest" are not returned unless explicitly
64+
requested.)
65+
66+
The -title flag specifies a regular expression that must match the
67+
action's title. (Ideally kinds would be specific enough that this
68+
isn't necessary; we really need to subdivide refactor.rewrite; see
69+
gopls/internal/settings/codeactionkind.go.)
70+
71+
The -exec flag causes the first matching code action to be executed.
72+
Without the flag, the matching actions are merely listed.
73+
74+
It is not currently possible to execute more than one action,
75+
as that requires a way to detect and resolve conflicts.
76+
TODO(adonovan): support it when golang/go#67049 is resolved.
77+
78+
If executing an action causes the server to send a patch to the
79+
client, the usual -write, -preserve, -diff, and -list flags govern how
80+
the client deals with the patch.
81+
82+
Example: execute the first "quick fix" in the specified file and show the diff:
83+
84+
$ gopls codeaction -kind=quickfix -exec -diff ./gopls/main.go
85+
86+
codeaction-flags:
87+
`)
88+
printFlagDefaults(f)
89+
}
90+
91+
func (cmd *codeaction) Run(ctx context.Context, args ...string) error {
92+
if len(args) < 1 {
93+
return tool.CommandLineErrorf("codeaction expects at least 1 argument")
94+
}
95+
cmd.app.editFlags = &cmd.EditFlags
96+
conn, err := cmd.app.connect(ctx)
97+
if err != nil {
98+
return err
99+
}
100+
defer conn.terminate(ctx)
101+
102+
from := parseSpan(args[0])
103+
uri := from.URI()
104+
file, err := conn.openFile(ctx, uri)
105+
if err != nil {
106+
return err
107+
}
108+
rng, err := file.spanRange(from)
109+
if err != nil {
110+
return err
111+
}
112+
113+
titleRE, err := regexp.Compile(cmd.Title)
114+
if err != nil {
115+
return err
116+
}
117+
118+
// Get diagnostics, as they may encode various lazy code actions.
119+
if err := conn.diagnoseFiles(ctx, []protocol.DocumentURI{uri}); err != nil {
120+
return err
121+
}
122+
diagnostics := []protocol.Diagnostic{} // LSP wants non-nil slice
123+
conn.client.filesMu.Lock()
124+
diagnostics = append(diagnostics, file.diagnostics...)
125+
conn.client.filesMu.Unlock()
126+
127+
// Request code actions of the desired kinds.
128+
var kinds []protocol.CodeActionKind
129+
if cmd.Kind != "" {
130+
for _, kind := range strings.Split(cmd.Kind, ",") {
131+
kinds = append(kinds, protocol.CodeActionKind(kind))
132+
}
133+
}
134+
actions, err := conn.CodeAction(ctx, &protocol.CodeActionParams{
135+
TextDocument: protocol.TextDocumentIdentifier{URI: uri},
136+
Range: rng,
137+
Context: protocol.CodeActionContext{
138+
Only: kinds,
139+
Diagnostics: diagnostics,
140+
},
141+
})
142+
if err != nil {
143+
return fmt.Errorf("%v: %v", from, err)
144+
}
145+
146+
// Gather edits from matching code actions.
147+
var edits []protocol.TextEdit
148+
for _, act := range actions {
149+
if act.Disabled != nil {
150+
continue
151+
}
152+
if !titleRE.MatchString(act.Title) {
153+
continue
154+
}
155+
156+
// If the provided span has a position (not just offsets),
157+
// and the action has diagnostics, the action must have a
158+
// diagnostic with the same range as it.
159+
if from.HasPosition() && len(act.Diagnostics) > 0 &&
160+
!slices.ContainsFunc(act.Diagnostics, func(diag protocol.Diagnostic) bool {
161+
return diag.Range.Start == rng.Start
162+
}) {
163+
continue
164+
}
165+
166+
if cmd.Exec {
167+
// -exec: run the first matching code action.
168+
if act.Command != nil {
169+
// This may cause the server to make
170+
// an ApplyEdit downcall to the client.
171+
if _, err := conn.executeCommand(ctx, act.Command); err != nil {
172+
return err
173+
}
174+
// The specification says that commands should
175+
// be executed _after_ edits are applied, not
176+
// instead of them, but we don't want to
177+
// duplicate edits.
178+
} else {
179+
// Partially apply CodeAction.Edit, a WorkspaceEdit.
180+
// (See also conn.Client.applyWorkspaceEdit(a.Edit)).
181+
for _, c := range act.Edit.DocumentChanges {
182+
tde := c.TextDocumentEdit
183+
if tde != nil && tde.TextDocument.URI == uri {
184+
// TODO(adonovan): this logic will butcher an edit that spans files.
185+
// It will also ignore create/delete/rename operations.
186+
// Fix or document. Need a three-way merge.
187+
edits = append(edits, protocol.AsTextEdits(tde.Edits)...)
188+
}
189+
}
190+
return applyTextEdits(file.mapper, edits, cmd.app.editFlags)
191+
}
192+
return nil
193+
} else {
194+
// No -exec: list matching code actions.
195+
action := "edit"
196+
if act.Command != nil {
197+
action = "command"
198+
}
199+
fmt.Printf("%s\t%q [%s]\n",
200+
action,
201+
act.Title,
202+
act.Kind)
203+
}
204+
}
205+
206+
if cmd.Exec {
207+
return fmt.Errorf("no matching code action at %s", from)
208+
}
209+
return nil
210+
}

gopls/internal/cmd/integration_test.go

Lines changed: 32 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -941,8 +941,8 @@ package foo
941941
}
942942
}
943943

944-
// TestFix tests the 'fix' subcommand (../suggested_fix.go).
945-
func TestFix(t *testing.T) {
944+
// TestCodeAction tests the 'codeaction' subcommand (../codeaction.go).
945+
func TestCodeAction(t *testing.T) {
946946
t.Parallel()
947947

948948
tree := writeTree(t, `
@@ -964,29 +964,46 @@ type C struct{}
964964

965965
// no arguments
966966
{
967-
res := gopls(t, tree, "fix")
967+
res := gopls(t, tree, "codeaction")
968968
res.checkExit(false)
969969
res.checkStderr("expects at least 1 argument")
970970
}
971-
// success with default kinds, {quickfix}.
972-
// -a is always required because no fix is currently "preferred" (!)
971+
// list code actions in file
973972
{
974-
res := gopls(t, tree, "fix", "-a", "a.go")
973+
res := gopls(t, tree, "codeaction", "a.go")
974+
res.checkExit(true)
975+
res.checkStdout(`edit "Fill in return values" \[quickfix\]`)
976+
res.checkStdout(`command "Browse documentation for package a" \[source.doc\]`)
977+
}
978+
// list code actions in file, filtering by title
979+
{
980+
res := gopls(t, tree, "codeaction", "-title=Br.wse", "a.go")
975981
res.checkExit(true)
976982
got := res.stdout
977-
want := `
978-
package a
979-
type T int
980-
func f() (int, string) { return 0, "" }
981-
982-
`[1:]
983+
want := `command "Browse documentation for package a" [source.doc]` + "\n"
984+
if got != want {
985+
t.Errorf("codeaction: got <<%s>>, want <<%s>>\nstderr:\n%s", got, want, res.stderr)
986+
}
987+
}
988+
// list code actions at position (of io.Reader)
989+
{
990+
res := gopls(t, tree, "codeaction", "b.go:#31")
991+
res.checkExit(true)
992+
res.checkStdout(`command "Browse documentation for type io.Reader" \[source.doc]`)
993+
}
994+
// list quick fixes at position (of type T)
995+
{
996+
res := gopls(t, tree, "codeaction", "-kind=quickfix", "a.go:#15")
997+
res.checkExit(true)
998+
got := res.stdout
999+
want := `edit "Fill in return values" [quickfix]` + "\n"
9831000
if got != want {
984-
t.Errorf("fix: got <<%s>>, want <<%s>>\nstderr:\n%s", got, want, res.stderr)
1001+
t.Errorf("codeaction: got <<%s>>, want <<%s>>\nstderr:\n%s", got, want, res.stderr)
9851002
}
9861003
}
9871004
// success, with explicit CodeAction kind and diagnostics span.
9881005
{
989-
res := gopls(t, tree, "fix", "-a", "b.go:#40", "quickfix")
1006+
res := gopls(t, tree, "codeaction", "-kind=quickfix", "-exec", "b.go:#40")
9901007
res.checkExit(true)
9911008
got := res.stdout
9921009
want := `
@@ -1004,7 +1021,7 @@ func (c C) Read(p []byte) (n int, err error) {
10041021
}
10051022
`[1:]
10061023
if got != want {
1007-
t.Errorf("fix: got <<%s>>, want <<%s>>\nstderr:\n%s", got, want, res.stderr)
1024+
t.Errorf("codeaction: got <<%s>>, want <<%s>>\nstderr:\n%s", got, want, res.stderr)
10081025
}
10091026
}
10101027
}

0 commit comments

Comments
 (0)