diff --git a/gopls/internal/cmd/mcp_test.go b/gopls/internal/cmd/mcp_test.go index bdbc36ac951..eb3f68a649e 100644 --- a/gopls/internal/cmd/mcp_test.go +++ b/gopls/internal/cmd/mcp_test.go @@ -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" diff --git a/gopls/internal/mcp/instructions.md b/gopls/internal/mcp/instructions.md index 6a528696768..d07a051d747 100644 --- a/gopls/internal/mcp/instructions.md +++ b/gopls/internal/mcp/instructions.md @@ -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. diff --git a/gopls/internal/mcp/mcp.go b/gopls/internal/mcp/mcp.go index 9de7dfbb71f..a3bd3749e8d 100644 --- a/gopls/internal/mcp/mcp.go +++ b/gopls/internal/mcp/mcp.go @@ -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 { @@ -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. @@ -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 { @@ -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{}) diff --git a/gopls/internal/mcp/rename.go b/gopls/internal/mcp/rename.go new file mode 100644 index 00000000000..a749b571438 --- /dev/null +++ b/gopls/internal/mcp/rename.go @@ -0,0 +1,270 @@ +// Copyright 2025 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. + +package mcp + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strings" + + "golang.org/x/tools/gopls/internal/cache" + "golang.org/x/tools/gopls/internal/file" + "golang.org/x/tools/gopls/internal/protocol" + "golang.org/x/tools/internal/mcp" +) + +type renameParams struct { + File string `json:"file"` + Line uint32 `json:"line"` + Column uint32 `json:"column"` + NewName string `json:"newName"` + DryRun *bool `json:"dryRun,omitempty"` +} + +func (h *handler) renameTool() *mcp.ServerTool { + const desc = `Rename a Go symbol at the specified location. + +This tool renames a Go symbol (variable, function, type, etc.) at the given +file position. By default it shows what changes would be made (dry run). +Set dryRun to false to actually apply the changes. + +The line and column numbers are zero-based, following LSP conventions. +` + return mcp.NewServerTool( + "go_rename", + desc, + h.renameHandler, + mcp.Input( + mcp.Property("file", mcp.Description("the absolute path to the file containing the symbol")), + mcp.Property("line", mcp.Description("line number (zero-based)")), + mcp.Property("column", mcp.Description("column number (zero-based)")), + mcp.Property("newName", mcp.Description("the new name for the symbol")), + mcp.Property("dryRun", mcp.Description("if true (default), show changes without applying them; if false, apply the changes"), mcp.Required(false)), + ), + ) +} + +func (h *handler) renameHandler( + ctx context.Context, _ *mcp.ServerSession, params *mcp.CallToolParamsFor[renameParams], +) (*mcp.CallToolResultFor[any], error) { + fh, snapshot, release, err := h.fileOf(ctx, params.Arguments.File) + if err != nil { + return nil, err + } + defer release() + + if snapshot.FileKind(fh) != file.Go { + return nil, fmt.Errorf("can't rename symbols in non-Go files") + } + + if params.Arguments.NewName == "" { + return nil, fmt.Errorf("newName cannot be empty") + } + + uri := protocol.URIFromPath(params.Arguments.File) + position := protocol.Position{ + Line: params.Arguments.Line, + Character: params.Arguments.Column, + } + + renameParams := &protocol.RenameParams{ + TextDocument: protocol.TextDocumentIdentifier{URI: uri}, + Position: position, + NewName: params.Arguments.NewName, + } + + workspaceEdit, err := h.lspServer.Rename(ctx, renameParams) + if err != nil { + return nil, fmt.Errorf("failed to perform rename: %v", err) + } + + if workspaceEdit == nil { + return textResult("No changes would be made - symbol may not be renameable at this location."), nil + } + + // WorkspaceEdit can return changes in two formats: + // - Changes: simple map of file URI to text edits (legacy format) + // - DocumentChanges: array of document changes with versioning support (preferred format) + if len(workspaceEdit.Changes) == 0 && len(workspaceEdit.DocumentChanges) == 0 { + return textResult("No changes would be made."), nil + } + + // Check if this is a dry run (default to true if not specified) + dryRun := true + if params.Arguments.DryRun != nil { + dryRun = *params.Arguments.DryRun + } + + if dryRun { + return formatWorkspaceEdit(ctx, snapshot, workspaceEdit) + } + return applyWorkspaceEdit(ctx, snapshot, workspaceEdit) +} + +func formatWorkspaceEdit( + ctx context.Context, snapshot *cache.Snapshot, edit *protocol.WorkspaceEdit, +) (*mcp.CallToolResultFor[any], error) { + var builder strings.Builder + + totalChanges := 0 + fileCount := 0 + + // Count changes from legacy Changes format + for _, edits := range edit.Changes { + totalChanges += len(edits) + fileCount++ + } + + // Count changes from modern DocumentChanges format + for _, docChange := range edit.DocumentChanges { + if docChange.TextDocumentEdit != nil { + totalChanges += len(docChange.TextDocumentEdit.Edits) + fileCount++ + } + } + + if totalChanges == 0 { + return textResult("No changes would be made."), nil + } + + fmt.Fprintf(&builder, "Rename would make %d changes across %d files:\n\n", totalChanges, fileCount) + + // Process legacy Changes format + for uri, edits := range edit.Changes { + if err := formatFileChanges(&builder, ctx, snapshot, uri, edits); err != nil { + return nil, err + } + } + + // Process modern DocumentChanges format + for _, docChange := range edit.DocumentChanges { + if docChange.TextDocumentEdit != nil { + uri := docChange.TextDocumentEdit.TextDocument.URI + edits := protocol.AsTextEdits(docChange.TextDocumentEdit.Edits) + if err := formatFileChanges(&builder, ctx, snapshot, uri, edits); err != nil { + return nil, err + } + } + } + + return textResult(builder.String()), nil +} + +func applyWorkspaceEdit( + ctx context.Context, snapshot *cache.Snapshot, edit *protocol.WorkspaceEdit, +) (*mcp.CallToolResultFor[any], error) { + var builder strings.Builder + totalChanges := 0 + fileCount := 0 + + // Apply changes from legacy Changes format + for uri, edits := range edit.Changes { + if err := applyFileChanges(&builder, ctx, snapshot, uri, edits); err != nil { + return nil, err + } + totalChanges += len(edits) + fileCount++ + } + + // Apply changes from modern DocumentChanges format + for _, docChange := range edit.DocumentChanges { + if docChange.TextDocumentEdit != nil { + uri := docChange.TextDocumentEdit.TextDocument.URI + edits := protocol.AsTextEdits(docChange.TextDocumentEdit.Edits) + if err := applyFileChanges(&builder, ctx, snapshot, uri, edits); err != nil { + return nil, err + } + totalChanges += len(edits) + fileCount++ + } + } + + if totalChanges == 0 { + return textResult("No changes were applied."), nil + } + + fmt.Fprintf(&builder, "Successfully applied %d changes across %d files:\n\n", totalChanges, fileCount) + return textResult(builder.String()), nil +} + +func applyFileChanges( + builder *strings.Builder, + ctx context.Context, + snapshot *cache.Snapshot, + uri protocol.DocumentURI, + edits []protocol.TextEdit, +) error { + fh, err := snapshot.ReadFile(ctx, uri) + if err != nil { + return fmt.Errorf("failed to read file %s: %v", uri.Path(), err) + } + + content, err := fh.Content() + if err != nil { + return fmt.Errorf("failed to read file content %s: %v", uri.Path(), err) + } + + // Create a mapper for the file content + mapper := protocol.NewMapper(uri, content) + + // Apply the edits to get the new content + newContent, _, err := protocol.ApplyEdits(mapper, edits) + if err != nil { + return fmt.Errorf("failed to apply edits to %s: %v", uri.Path(), err) + } + + // Write the new content back to the file + if err := os.WriteFile(uri.Path(), newContent, 0644); err != nil { + return fmt.Errorf("failed to write file %s: %v", uri.Path(), err) + } + + fmt.Fprintf(builder, "✓ %s (%d changes)\n", filepath.ToSlash(uri.Path()), len(edits)) + return nil +} + +func formatFileChanges( + builder *strings.Builder, + ctx context.Context, + snapshot *cache.Snapshot, + uri protocol.DocumentURI, + edits []protocol.TextEdit, +) error { + fmt.Fprintf(builder, "File: %s\n", filepath.ToSlash(uri.Path())) + + fh, err := snapshot.ReadFile(ctx, uri) + if err != nil { + fmt.Fprintf(builder, " Error reading file: %v\n\n", err) + return nil + } + + content, err := fh.Content() + if err != nil { + fmt.Fprintf(builder, " Error reading file content: %v\n\n", err) + return nil + } + + lines := strings.Split(string(content), "\n") + + for i, edit := range edits { + fmt.Fprintf(builder, " Change %d (line %d, col %d-%d): ", i+1, edit.Range.Start.Line+1, edit.Range.Start.Character+1, edit.Range.End.Character+1) + + if int(edit.Range.Start.Line) < len(lines) { + oldText := "" + if edit.Range.Start.Line == edit.Range.End.Line { + line := lines[edit.Range.Start.Line] + if int(edit.Range.Start.Character) < len(line) && int(edit.Range.End.Character) <= len(line) { + oldText = line[edit.Range.Start.Character:edit.Range.End.Character] + } + } + fmt.Fprintf(builder, "'%s' → '%s'\n", oldText, edit.NewText) + } else { + fmt.Fprintf(builder, "→ '%s'\n", edit.NewText) + } + } + builder.WriteString("\n") + return nil +}