x/tools/gopls: extract variable returns an extra lhs expression and no template edit support is available

[gayanper]

go version go1.20.1 darwin/arm64
golang.org/x/tools/gopls v0.14.2
golang.org/x/tools/gopls@v0.14.2 h1:sIw6vjZiuQ9S7s0auUUkHlWgsCkKZFWDHmrge8LYsnc=
github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW6Y=
golang.org/x/mod@v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0=
golang.org/x/sync@v0.4.0 h1:zxkM55ReGkDlKSM+Fu41A+zmbZuaPVbGMzvvdUPznYQ=
golang.org/x/sys@v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=
golang.org/x/telemetry@v0.0.0-20231114163143-69313e640400 h1:brbkEFfGwNGAEkykUOcryE/JiHUMMJouzE0fWWmz/QU=
golang.org/x/text@v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k=
golang.org/x/tools@v0.14.1-0.20231114185516-c9d3e7de13fd h1:Oku7E+OCrXHyst1dG1z10etCTxewCHXNFLRlyMPbh3w=
golang.org/x/vuln@v1.0.1 h1:KUas02EjQK5LTuIx1OylBQdKKZ9jeugs+HiqO5HormU=
honnef.co/go/tools@v0.4.5 h1:YGD4H+SuIOOqsyoLOpZDWcieM28W47/zRO7f+9V3nvo=
mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
mvdan.cc/xurls/v2@v2.4.0 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

[gayanper]

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]

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).

[gayanper]

@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]

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

[gayanper]

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.