Skip to content

gopls/internal/mcp: add go_rename tool #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
140 changes: 140 additions & 0 deletions gopls/internal/cmd/mcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,146 @@ func getRandomPort() int {
return listener.Addr().(*net.TCPAddr).Port
}

func TestMCPRename(t *testing.T) {
// Test the go_rename tool via MCP.
if !(supportsFsnotify(runtime.GOOS)) {
// See golang/go#74580
t.Skipf(`skipping on %s; fsnotify is not supported`, runtime.GOOS)
}
testenv.NeedsExec(t) // stdio transport uses execve(2)

tests := []struct {
name string
dryRunValue interface{} // nil, true, or false
wantStrings []string
}{
{
name: "default_dry_run",
dryRunValue: nil, // omit dryRun parameter
wantStrings: []string{"a.go", "b.go", "c.go", "NewName", "OldName", "Rename would make", "6 changes", "3 files"},
},
{
name: "explicit_dry_run",
dryRunValue: true,
wantStrings: []string{"a.go", "b.go", "c.go", "NewName", "OldName", "Rename would make", "6 changes", "3 files"},
},
{
name: "apply_changes",
dryRunValue: false,
wantStrings: []string{"Successfully applied", "6 changes", "3 files"},
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
// Test cross-file rename with: function definition (a.go), function calls (b.go, c.go),
// and variable assignments (all files) = 6 total references across 3 files
tree := writeTree(t, `
-- go.mod --
module example.com
go 1.18

-- a.go --
package main

func OldName() {
}

var globalVar = OldName

-- b.go --
package main

func TestFunc() {
OldName()
x := OldName
_ = x
}

-- c.go --
package main

func AnotherFunc() {
OldName()
result := OldName
_ = result
}
`)

goplsCmd := exec.Command(os.Args[0], "mcp")
goplsCmd.Env = append(os.Environ(), "ENTRYPOINT=goplsMain")
goplsCmd.Dir = tree

ctx := t.Context()
client := mcp.NewClient("client", "v0.0.1", nil)
mcpSession, err := client.Connect(ctx, mcp.NewCommandTransport(goplsCmd))
if err != nil {
t.Fatal(err)
}
defer func() {
if err := mcpSession.Close(); err != nil {
t.Errorf("closing MCP connection: %v", err)
}
}()

tool := "go_rename"
args := map[string]any{
"file": filepath.Join(tree, "a.go"),
"line": 2,
"column": 5,
"newName": "NewName",
}

// Add dryRun parameter if specified
if tt.dryRunValue != nil {
args["dryRun"] = tt.dryRunValue
}

res, err := mcpSession.CallTool(ctx, &mcp.CallToolParams{Name: tool, Arguments: args})
if err != nil {
t.Fatal(err)
}
got := resultText(t, res)

// Check expected strings in output
for _, want := range tt.wantStrings {
if !strings.Contains(got, want) {
t.Errorf("CallTool(%s, %v) = %v, want containing %q", tool, args, got, want)
}
}

// Check file modification based on dryRun value across all files
isDryRun := tt.dryRunValue == nil || tt.dryRunValue == true

for _, filename := range []string{"a.go", "b.go", "c.go"} {
content, err := os.ReadFile(filepath.Join(tree, filename))
if err != nil {
t.Fatal(err)
}

if isDryRun {
// Files should NOT be modified in dry run
if strings.Contains(string(content), "NewName") {
t.Errorf("Expected %s to NOT contain 'NewName' in dry run, but it was modified: %s", filename, string(content))
}
if !strings.Contains(string(content), "OldName") {
t.Errorf("Expected %s to still contain 'OldName' in dry run, got: %s", filename, string(content))
}
continue
}

// Files should be modified when applying changes - verify cross-file rename
if !strings.Contains(string(content), "NewName") {
t.Errorf("Expected %s to contain 'NewName' after cross-file rename, got: %s", filename, string(content))
}
if strings.Contains(string(content), "OldName") {
t.Errorf("Expected %s to no longer contain 'OldName' after rename, but found: %s", filename, string(content))
}
}
})
}
}

// supportsFsnotify returns true if fsnotify supports the os.
func supportsFsnotify(os string) bool {
return os == "darwin" || os == "linux" || os == "windows"
Expand Down
12 changes: 9 additions & 3 deletions gopls/internal/mcp/instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,16 @@ The editing workflow is iterative. You should cycle through these steps until th

3. **Make edits**: Make the required edits, including edits to references you identified in the previous step. Don't proceed to the next step until all planned edits are complete.

4. **Check for errors**: After every code modification, you MUST call the `go_diagnostics` tool. Pass the paths of the files you have edited. This tool will report any build or analysis errors.
4. **Rename symbols**: For renaming symbols across the workspace, use the `go_rename` tool which will automatically find and rename all references. ALWAYS follow this workflow:
- First, run with `dryRun: true` (default) to preview changes: `go_rename({"file":"/path/to/server.go","line":10,"column":5,"newName":"NewServerName"})`
- Show the user what changes would be made and ask for confirmation
- Only if the user approves, apply the changes: `go_rename({"file":"/path/to/server.go","line":10,"column":5,"newName":"NewServerName","dryRun":false})`
- Exception: Only skip the preview step if the user explicitly requests to apply changes immediately

5. **Check for errors**: After every code modification, you MUST call the `go_diagnostics` tool. Pass the paths of the files you have edited. This tool will report any build or analysis errors.
EXAMPLE: `go_diagnostics({"files":["/path/to/server.go"]})`

5. **Fix errors**: If `go_diagnostics` reports any errors, fix them. The tool may provide suggested quick fixes in the form of diffs. You should review these diffs and apply them if they are correct. Once you've applied a fix, re-run `go_diagnostics` to confirm that the issue is resolved. It is OK to ignore 'hint' or 'info' diagnostics if they are not relevant to the current task. Note that Go diagnostic messages may contain a summary of the source code, which may not match its exact text.
6. **Fix errors**: If `go_diagnostics` reports any errors, fix them. The tool may provide suggested quick fixes in the form of diffs. You should review these diffs and apply them if they are correct. Once you've applied a fix, re-run `go_diagnostics` to confirm that the issue is resolved. It is OK to ignore 'hint' or 'info' diagnostics if they are not relevant to the current task. Note that Go diagnostic messages may contain a summary of the source code, which may not match its exact text.

6. **Run tests**: Once `go_diagnostics` reports no errors (and ONLY once there are no errors), run the tests for the packages you have changed. You can do this with `go test [packagePath...]`. Don't run `go test ./...` unless the user explicitly requests it, as doing so may slow down the iteration loop.
7. **Run tests**: Once `go_diagnostics` reports no errors (and ONLY once there are no errors), run the tests for the packages you have changed. You can do this with `go test [packagePath...]`. Don't run `go test ./...` unless the user explicitly requests it, as doing so may slow down the iteration loop.

13 changes: 10 additions & 3 deletions gopls/internal/mcp/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ func Serve(ctx context.Context, address string, sessions Sessions, isDaemon bool
}

// StartStdIO starts an MCP server over stdio.
func StartStdIO(ctx context.Context, session *cache.Session, server protocol.Server, rpcLog io.Writer) error {
func StartStdIO(
ctx context.Context, session *cache.Session, server protocol.Server, rpcLog io.Writer,
) error {
transport := mcp.NewStdioTransport()
var t mcp.Transport = transport
if rpcLog != nil {
Expand Down Expand Up @@ -166,6 +168,7 @@ func newServer(session *cache.Session, lspServer protocol.Server) *mcp.Server {
h.symbolReferencesTool(),
h.searchTool(),
h.fileContextTool(),
h.renameTool(),
}
disabledTools := append(defaultTools,
// The fileMetadata tool is redundant with fileContext.
Expand Down Expand Up @@ -222,7 +225,9 @@ func (h *handler) snapshot() (*cache.Snapshot, func(), error) {
//
// This helps avoid stale packages, but is not a substitute for real file
// watching, as it misses things like files being added to a package.
func (h *handler) fileOf(ctx context.Context, file string) (file.Handle, *cache.Snapshot, func(), error) {
func (h *handler) fileOf(
ctx context.Context, file string,
) (file.Handle, *cache.Snapshot, func(), error) {
uri := protocol.URIFromPath(file)
fh, snapshot, release, err := h.session.FileOf(ctx, uri)
if err != nil {
Expand Down Expand Up @@ -260,7 +265,9 @@ func (h *handler) fileOf(ctx context.Context, file string) (file.Handle, *cache.
//
// It also doesn't catch package changes that occur due to added files or
// changes to the go.mod file.
func checkForFileChanges(ctx context.Context, snapshot *cache.Snapshot, id metadata.PackageID) ([]protocol.FileEvent, error) {
func checkForFileChanges(
ctx context.Context, snapshot *cache.Snapshot, id metadata.PackageID,
) ([]protocol.FileEvent, error) {
var events []protocol.FileEvent

seen := make(map[metadata.PackageID]struct{})
Expand Down
Loading