Skip to content
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

x/tools/gopls: rename on a method receiver should rename all receivers for the type #41892

Open
bcmills opened this issue Oct 9, 2020 · 2 comments
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.

Comments

@bcmills
Copy link
Contributor

bcmills commented Oct 9, 2020

What version of Go are you using (go version)?

example.com$ go version
go version devel +2be7788f8 Fri Oct 9 02:49:19 2020 +0000 linux/amd64

example.com$ go version -m $(which gopls)
/usr/local/google/home/bcmills/bin/gopls: devel +186f0220d0 Mon Oct 5 11:12:24 2020 -0400
        path    golang.org/x/tools/gopls
        mod     golang.org/x/tools/gopls        v0.5.1  h1:AF3Uh7HF08SZpKFfgJO6zfF3bbxyDXWqdkK4kMXiQ1o=
        dep     github.com/BurntSushi/toml      v0.3.1  h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=
        dep     github.com/google/go-cmp        v0.5.1  h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=
        dep     github.com/sergi/go-diff        v1.1.0  h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
        dep     golang.org/x/mod        v0.3.0  h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=
        dep     golang.org/x/sync       v0.0.0-20200625203802-6e8e738ad208      h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=
        dep     golang.org/x/tools      v0.0.0-20200930165741-f1523d29dbb9      h1:1R38tQp22dcHpTKJPjgVa16FhlDy/kHEaCM/ndi/FIc=
        dep     golang.org/x/xerrors    v0.0.0-20200804184101-5ec99f83aff1      h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=
        dep     honnef.co/go/tools      v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=
        dep     mvdan.cc/gofumpt        v0.0.0-20200802201014-ab5a8192947d      h1:t8TAw9WgTLghti7RYkpPmqk4JtQ3+wcP5GgZqgWeWLQ=
        dep     mvdan.cc/xurls/v2       v2.2.0  h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=

example.com$ ls -d ~/.emacs.d/elpa/eglot-*
/usr/local/google/home/bcmills/.emacs.d/elpa/eglot-20200830.1254

example.com$ emacs --version
GNU Emacs 27.1

Does this issue reproduce with the latest release?

Yes.

What did you do?

Initial state:

-- go.mod --
module example.com

go 1.16
-- main.go --
package main

import "fmt"

type T struct{}

func (x *T) String() string {
	return fmt.Sprintf("%T{}", x)
}

func (x *T) Clone() *T {
	y := new(T)
	*y = *x
	return y
}

func main() {}
  1. emacs -nw main.go
  2. Move the point to the first method receiver (main.go:7:6).
  3. M-x eglot-rename
  4. Enter a new name (such as t) for the method receiver.

What did you expect to see?

Per http://golang.org/wiki/CodeReviewComments#receiver-names:

Be consistent, too: if you call the receiver "c" in one method, don't call it "cl" in another.

Since the file already satisfied that consistency rule before the rename, I expect it to satisfy that rule after the rename, too: all of the method receivers should have been renamed to the new name entered in step (4).

What did you see instead?

Only the receiver for the one method was renamed.

gopls log:

[Trace - 15:40:27.246 PM] Sending request 'initialize - (1)'.
Params: {"processId":712138,"rootPath":"/tmp/tmp.hs2uMVS9fG/example.com/","rootUri":"file:///tmp/tmp.hs2uMVS9fG/example.com/","initializationOptions":null,"capabilities":{"workspace":{"applyEdit":true,"executeCommand":{"dynamicRegistration":false},"workspaceEdit":{"documentChanges":false},"didChangeWatchedFiles":{"dynamicRegistration":true},"symbol":{"dynamicRegistration":false},"configuration":true},"textDocument":{"synchronization":{"dynamicRegistration":false,"willSave":true,"willSaveWaitUntil":true,"didSave":true},"completion":{"dynamicRegistration":false,"completionItem":{"snippetSupport":true},"contextSupport":true},"hover":{"dynamicRegistration":false,"contentFormat":["markdown","plaintext"]},"signatureHelp":{"dynamicRegistration":false,"signatureInformation":{"parameterInformation":{"labelOffsetSupport":true}}},"references":{"dynamicRegistration":false},"definition":{"dynamicRegistration":false},"declaration":{"dynamicRegistration":false},"implementation":{"dynamicRegistration":false},"typeDefinition":{"dynamicRegistration":false},"documentSymbol":{"dynamicRegistration":false,"hierarchicalDocumentSymbolSupport":true,"symbolKind":{"valueSet":[1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26]}},"documentHighlight":{"dynamicRegistration":false},"codeAction":{"dynamicRegistration":false,"codeActionLiteralSupport":{"codeActionKind":{"valueSet":["quickfix","refactor","refactor.extract","refactor.inline","refactor.rewrite","source","source.organizeImports"]}}},"formatting":{"dynamicRegistration":false},"rangeFormatting":{"dynamicRegistration":false},"rename":{"dynamicRegistration":false},"publishDiagnostics":{"relatedInformation":false}},"experimental":null}}


[Trace - 15:40:27.246 PM] Received response 'initialize - (1)' in 0ms.
Result: {"capabilities":{"textDocumentSync":{"openClose":true,"change":2,"save":{}},"completionProvider":{"triggerCharacters":["."]},"hoverProvider":true,"signatureHelpProvider":{"triggerCharacters":["(",","]},"definitionProvider":true,"typeDefinitionProvider":true,"implementationProvider":true,"referencesProvider":true,"documentHighlightProvider":true,"documentSymbolProvider":true,"codeActionProvider":{"codeActionKinds":["quickfix","refactor.extract","refactor.rewrite","source.fixAll","source.organizeImports"]},"codeLensProvider":{},"documentLinkProvider":{},"workspaceSymbolProvider":true,"documentFormattingProvider":true,"documentOnTypeFormattingProvider":{"firstTriggerCharacter":""},"renameProvider":true,"foldingRangeProvider":true,"executeCommandProvider":{"commands":["generate","fill_struct","regenerate_cgo","test","tidy","undeclared_name","upgrade_dependency","vendor","extract_variable","extract_function","gc_details","generate_gopls_mod"]},"callHierarchyProvider":true,"workspace":{"workspaceFolders":{"supported":true,"changeNotifications":"workspace/didChangeWorkspaceFolders"}}},"serverInfo":{"name":"gopls","version":"Build info\n----------\ngolang.org/x/tools/gopls v0.5.1\n    golang.org/x/tools/gopls@v0.5.1 h1:AF3Uh7HF08SZpKFfgJO6zfF3bbxyDXWqdkK4kMXiQ1o=\n    github.com/BurntSushi/toml@v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ=\n    github.com/google/go-cmp@v0.5.1 h1:JFrFEBb2xKufg6XkJsJr+WbKb4FQlURi5RUcBveYu9k=\n    github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=\n    golang.org/x/mod@v0.3.0 h1:RM4zey1++hCTbCVQfnWeKs9/IEsaBLA8vTkd0WVtmH4=\n    golang.org/x/sync@v0.0.0-20200625203802-6e8e738ad208 h1:qwRHBd0NqMbJxfbotnDhm2ByMI1Shq4Y6oRJo21SGJA=\n    golang.org/x/tools@v0.0.0-20200930165741-f1523d29dbb9 h1:1R38tQp22dcHpTKJPjgVa16FhlDy/kHEaCM/ndi/FIc=\n    golang.org/x/xerrors@v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE=\n    honnef.co/go/tools@v0.0.1-2020.1.5 h1:nI5egYTGJakVyOryqLs1cQO5dO0ksin5XXs2pspk75k=\n    mvdan.cc/gofumpt@v0.0.0-20200802201014-ab5a8192947d h1:t8TAw9WgTLghti7RYkpPmqk4JtQ3+wcP5GgZqgWeWLQ=\n    mvdan.cc/xurls/v2@v2.2.0 h1:NSZPykBXJFCetGZykLAxaL6SIpvbVy/UFEniIfHAa8A=\n"}}


[Trace - 15:40:27.248 PM] Sending notification 'initialized'.
Params: {}


[Trace - 15:40:27.248 PM] Received notification 'window/showMessage'.
Params: {"type":4,"message":"Loading packages..."}


[Trace - 15:40:27.248 PM] Received request 'workspace/configuration - (1)'.
Params: {"items":[{"scopeUri":"file:///tmp/tmp.hs2uMVS9fG/example.com/","section":"gopls"},{"scopeUri":"file:///tmp/tmp.hs2uMVS9fG/example.com/","section":"gopls-example.com"}]}


[Trace - 15:40:27.249 PM] Sending notification 'textDocument/didOpen'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go","version":0,"languageId":"go","text":"package main\n\nimport \"fmt\"\n\ntype T struct{}\n\nfunc (x *T) String() string {\n\treturn fmt.Sprintf(\"%T{}\", x)\n}\n\nfunc (x *T) Clone() *T {\n\ty := new(T)\n\t*y = *x\n\treturn y\n}\n\nfunc main() {}\n"}}


[Trace - 15:40:27.249 PM] Sending notification 'workspace/didChangeConfiguration'.
Params: {"settings":null}


[Trace - 15:40:27.250 PM] Sending request 'textDocument/documentSymbol - (2)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go"}}


[Trace - 15:40:27.254 PM] Sending response 'workspace/configuration - (1)' in 5ms.
Result: [null,null]


[Trace - 15:40:27.327 PM] Received request 'client/registerCapability - (2)'.
Params: {"registrations":[{"id":"workspace/didChangeWatchedFiles-0","method":"workspace/didChangeWatchedFiles","registerOptions":{"watchers":[{"globPattern":"**/*.{go,mod,sum}","kind":7},{"globPattern":"/tmp/tmp.hs2uMVS9fG/example.com/**/*.{go,mod,sum}","kind":7}]}}]}


[Trace - 15:40:27.327 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2020/10/09 15:40:27 go env for /tmp/tmp.hs2uMVS9fG/example.com/\n(root /tmp/tmp.hs2uMVS9fG/example.com)\n(go version go version devel +2be7788f8 Fri Oct 9 02:49:19 2020 +0000 linux/amd64\n)\n(valid build configuration = true)\n(build flags: [])\nGOINSECURE=\nGONOPROXY=\nGOMOD=/tmp/tmp.hs2uMVS9fG/example.com/go.mod\nGOPROXY=https://proxy.golang.org,direct\nGOPATH=/tmp/tmp.hs2uMVS9fG/_gopath\nGOPRIVATE=\nGOCACHE=/usr/local/google/home/bcmills/.cache/go-build\nGONOSUMDB=\nGO111MODULE=auto\nGOFLAGS=\nGOROOT=/usr/local/google/home/bcmills/sdk/gotip\nGOSUMDB=sum.golang.org\nGOMODCACHE=/tmp/tmp.hs2uMVS9fG/_gopath/pkg/mod\n\n"}


[Trace - 15:40:27.329 PM] Sending response 'client/registerCapability - (2)' in 1ms.
Result: 


[Trace - 15:40:27.588 PM] Received notification 'window/logMessage'.
Params: {"type":3,"message":"2020/10/09 15:40:27 go/packages.Load\n\tsnapshot=0\n\tdirectory=/tmp/tmp.hs2uMVS9fG/example.com\n\tquery=[./... builtin]\n\tpackages=2\n"}


[Trace - 15:40:27.628 PM] Received notification 'window/showMessage'.
Params: {"type":3,"message":"Finished loading packages."}


[Trace - 15:40:27.629 PM] Received request 'workspace/configuration - (3)'.
Params: {"items":[{"scopeUri":"file:///tmp/tmp.hs2uMVS9fG/example.com/","section":"gopls"},{"scopeUri":"file:///tmp/tmp.hs2uMVS9fG/example.com/","section":"gopls-example.com"}]}


[Trace - 15:40:27.671 PM] Sending response 'workspace/configuration - (3)' in 41ms.
Result: [null,null]


[Trace - 15:40:27.719 PM] Received response 'textDocument/documentSymbol - (2)' in 469ms.
Result: [{"name":"T","detail":"struct{...}","kind":23,"range":{"start":{"line":4,"character":5},"end":{"line":4,"character":15}},"selectionRange":{"start":{"line":4,"character":5},"end":{"line":4,"character":6}}},{"name":"(*T).String","detail":"()","kind":6,"range":{"start":{"line":6,"character":0},"end":{"line":8,"character":1}},"selectionRange":{"start":{"line":6,"character":12},"end":{"line":6,"character":18}}},{"name":"(*T).Clone","detail":"()","kind":6,"range":{"start":{"line":10,"character":0},"end":{"line":14,"character":1}},"selectionRange":{"start":{"line":10,"character":12},"end":{"line":10,"character":17}}},{"name":"main","detail":"()","kind":12,"range":{"start":{"line":16,"character":0},"end":{"line":16,"character":14}},"selectionRange":{"start":{"line":16,"character":5},"end":{"line":16,"character":9}}}]


[Trace - 15:40:30.981 PM] Sending request 'textDocument/signatureHelp - (3)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go"},"position":{"line":6,"character":6}}


[Trace - 15:40:30.981 PM] Sending request 'textDocument/hover - (4)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go"},"position":{"line":6,"character":6}}


[Trace - 15:40:30.981 PM] Received response 'textDocument/signatureHelp - (3)' in 0ms.
Result: null


[Trace - 15:40:30.982 PM] Sending request 'textDocument/documentHighlight - (5)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go"},"position":{"line":6,"character":6}}


[Trace - 15:40:30.982 PM] Received notification 'window/logMessage'.
Params: {"type":1,"message":"2020/10/09 15:40:30 no signature help: cannot find an enclosing function\n\tposition={6 6}\n"}


[Trace - 15:40:30.982 PM] Received response 'textDocument/hover - (4)' in 0ms.
Result: {"contents":{"kind":"markdown","value":"```go\nvar x *T\n```"},"range":{"start":{"line":6,"character":6},"end":{"line":6,"character":7}}}


[Trace - 15:40:30.982 PM] Received response 'textDocument/documentHighlight - (5)' in 0ms.
Result: [{"range":{"start":{"line":6,"character":6},"end":{"line":6,"character":7}},"kind":1},{"range":{"start":{"line":7,"character":28},"end":{"line":7,"character":29}},"kind":1}]


[Trace - 15:40:35.647 PM] Sending request 'textDocument/rename - (6)'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go"},"position":{"line":6,"character":6},"newName":"t"}


[Trace - 15:40:35.648 PM] Received response 'textDocument/rename - (6)' in 0ms.
Result: {"documentChanges":[{"textDocument":{"version":0,"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go"},"edits":[{"range":{"start":{"line":6,"character":6},"end":{"line":6,"character":7}},"newText":"t"},{"range":{"start":{"line":7,"character":28},"end":{"line":7,"character":29}},"newText":"t"}]}]}


[Trace - 15:40:36.160 PM] Sending notification 'textDocument/didChange'.
Params: {"textDocument":{"uri":"file:///tmp/tmp.hs2uMVS9fG/example.com/main.go","version":2},"contentChanges":[{"range":{"start":{"line":7,"character":28},"end":{"line":7,"character":29}},"rangeLength":1,"text":"t"},{"range":{"start":{"line":6,"character":6},"end":{"line":6,"character":7}},"rangeLength":1,"text":"t"}]}


@bcmills bcmills changed the title x/tools/gopls: rename on a method receiver does not rename all receivers for the type x/tools/gopls: rename on a method receiver should rename all receivers for the type Oct 9, 2020
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Oct 9, 2020
@gopherbot gopherbot added this to the Unreleased milestone Oct 9, 2020
@muirdm
Copy link

muirdm commented Nov 21, 2020

Some questions:

  1. If the receiver names aren't currently consistent, should we rename them all to be consistent?
  2. If we can't rename a receiver because the new name would conflict with another name in the method body, should we fail the entire rename operation or rename as many receivers as we can?

@mattheusv
Copy link

From a user perspective, I think it's better to rename even if the receiver names are not consistent at the moment, and rename as many receivers as possible.

@adonovan adonovan added the Refactoring Issues related to refactoring tools label Apr 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants