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: extract variable returns an extra lhs expression and no template edit support is available #65944

Open
gayanper opened this issue Feb 22, 2024 · 5 comments
Labels
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.
Milestone

Comments

@gayanper
Copy link

go version go1.20.1 darwin/arm64
golang.org/x/tools/gopls v0.14.2
golang.org/x/tools/[email protected] h1:sIw6vjZiuQ9S7s0auUUkHlWgsCkKZFWDHmrge8LYsnc=
github.com/BurntSushi/[email protected] h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/[email protected] h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/sergi/[email protected] h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
golang.org/x/exp/[email protected] h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/[email protected] h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=
golang.org/x/[email protected] h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
golang.org/x/[email protected] h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=
golang.org/x/[email protected] h1:brbkEFfGwNGAEkykUOcryE/JiHUMMJouzE0fWWmz/QU=
golang.org/x/[email protected] h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
golang.org/x/[email protected] h1:Oku7E+OCrXHyst1dG1z10etCTxewCHXNFLRlyMPbh3w=
golang.org/x/[email protected] h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU=
honnef.co/go/[email protected] h1:YGD4H+SuIOOqsyoLOpZDWcieM28W47/zRO7f+9V3nvo=
mvdan.cc/[email protected] h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
mvdan.cc/xurls/[email protected] h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.20.1
vscode 1.86.2

Describe the bug

Given the following code

func hello() (string, int) {
	return "", 1
}

func call() {
	hello()
}

if try to extract variable for function hello, I will get the following with no proper template edits so I can rename both variable tuples

image
@gayanper
Copy link
Author

Could the problem be because of the code here https://github.com/golang/tools/blob/054c06df410b0347e908c647ef441733aea4a8df/gopls/internal/golang/extract.go#L85

where both the assignment statement and the calculated LHS is sent as edits ?

@suzmue
Copy link
Contributor

suzmue commented Feb 26, 2024

Thanks for filing this bug. In this scenario it probably makes sense to skip emitting the expression that causes a syntax error, and leave the variable declarations as unused (especially since the extra expression does not qualify as a use anyway).

We are planning to look into improving our refactoring code actions in the near future, and all of the extract code actions suffer from a lack of support for customizing the names of new symbols. AFAICT LSP does not support snippets for codeactions, only for completions (see microsoft/language-server-protocol#592).

@suzmue suzmue changed the title Extract variable returns an extra lhs expression and no template edit support is available x/tools/gopls: extract variable returns an extra lhs expression and no template edit support is available Feb 26, 2024
@suzmue suzmue transferred this issue from golang/vscode-go Feb 26, 2024
@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 Feb 26, 2024
@gopherbot gopherbot added this to the Unreleased milestone Feb 26, 2024
@suzmue suzmue modified the milestones: Unreleased, gopls/backlog Feb 26, 2024
@suzmue suzmue added the Refactoring Issues related to refactoring tools label Feb 26, 2024
@gayanper
Copy link
Author

@suzmue You are correct. I think in other languages it is done by a custom action which consists of edits followed by a rename.

@gayanper
Copy link
Author

gayanper commented Mar 2, 2024

@suzmue I tried to improve the extract variable with the current features we have in LSP, Made the PR under golang/tools#482

@gayanper
Copy link
Author

gayanper commented Mar 2, 2024

Also as part of the CodeAction we can send a command which will run after the edits are applied. But we will only be able to rename one variable when there are multiple return values. So I felt that I should just check here before continuing on that path since I didn't feel that the effort to implement it brings much value. of course it might in majority of scenarios where the return values are value, error where we can prompt to rename the value variable. But then I would guess we need to suggest a better name for error variable, like err with a suffix if there is a name conflict.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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

3 participants