x/tools/gopls: add support for 'refactor.extract.constant' code actions

[stamblerre]

See https://github.com/microsoft/vscode-go/issues/3040.

https://microsoft.github.io/language-server-protocol/specifications/specification-3-15/#textDocument_codeAction has more details on the possibilities here.

The function refactoring can be implemented by making use of guru's freevars code.

[gopherbot]

Change https://golang.org/cl/221917 mentions this issue: internal/lsp: add code action to extract local variable

[gopherbot]

Change https://golang.org/cl/241957 mentions this issue: internal/lsp: support extract function

[joshbaum]

https://golang.org/cl/241957 and https://golang.org/cl/240182 support extracting a function and variable, respectively, as a code action. The following are open issues with function extraction:

• [ ] Extraction does not include comments. Comments are replaced with newlines in the extracted function.
• [ ] You cannot extract a selection that begins with or ends with a comment.
• [ ] Control flow statements (break, continue, goto, etc.) are not handled specially. This can produce bad code if extracted poorly.
• [ ] Defer statements are not handled specially. This can also produce bad code if extracted poorly.
• [x] Identifiers can be on the left-hand side of an assignment statement even if they are never used again. This produces a yellow line with the message "variable is never used." This occurs when a variable name is used later in scope, but the first time it is used after the selection, it is defined again (using :=) rather than assigned. This does not produce incorrect code, but should be fixed.

[gopherbot]

Change https://golang.org/cl/243650 mentions this issue: internal/lsp/source: support return statements in extract function

[gopherbot]

Change https://golang.org/cl/312469 mentions this issue: internal/lsp: add support for extracting non-nested returns

[gopherbot]

Change https://golang.org/cl/313211 mentions this issue: internal/lsp: print comments that would be lost during extract func

[gopherbot]

Change https://golang.org/cl/351989 mentions this issue: internal/lsp: allow extract func ranges to begin/end with comments

[suerta-git]

Hi there, have these code actions been released or not?

Currently when I do the refactoring there is: Is this normal behavior？

[stamblerre]

@suerta-git: What refactoring would you like to see there?

[suerta-git]

@stamblerre I am expecting to see extract variable. It works in quick fix like this:

But not works on my hotkey:

[stamblerre]

Ok, if you are seeing a refactoring there but it doesn't work with the hotkey, then this is probably a VS Code bug, not a bug in the extension. Please file an issue with https://github.com/microsoft/vscode.

[suerta-git]

@stamblerre so you mean this extension already provides the funtionality for 'refactor.extract.constant/variable', but the hotkey doesn't work due to some reason like a vscode bug, right?

[stamblerre]

Oh actually- now I see the issue. We don't actually offer refactor.extract.constant, but we do offer refactor.extract.variable. We can make this fix on our side.

[suerta-git]

Hi @stamblerre thanks for your reply. I tried to reinstall my vscode and restore to the initial state. Then set one customized shortcut like this in keybindings.json (refer to official doc):

// Place your key bindings in this file to override the defaults
[
    {
        "key": "alt+cmd+v",
        "command": "editor.action.codeAction",
        "args": {
          "kind": "refactor.extract.variable"
        }
    }
]

The issue still exists:

Using quick fix (cmd + . in mac os):

Using shortcut (alt+cmd+v defined above):

As a reference, for other languages,refactor.extract.constant shortcut is avaiable on TypeScript (doesn't support refactor.extract.variable), but both of two shortcuts aren't avaiable on Python and C#.

Seems even though vscode provides the ability for customizing shortcuts, this feature is not well supported by language extensions. Not sure if it is due to some bugs in vscode.

[suzmue]

gopls sets the codeActionKind for Extract variable to be refactor.extract. So the keybinding would need to be

{
        "key": "alt+cmd+v",
        "command": "editor.action.codeAction",
        "args": {
          "kind": "refactor.extract"
        }
    }

We should definitely consider being more specific for the code action kinds to enable keybindings like this.

[gopherbot]

Change https://go.dev/cl/564338 mentions this issue: gopls: Enable refactor.extract.constant/variable/function CodeActionKinds

[NateWilliams2]

I've started attacking this issue at https://go.dev/cl/564338 and added tests/enabled the usage of refactor.extract.constant, refactor.extract.function, and refactor.extract.variable.

One possible issue is that I have not implemented a new type of code action for constants- the refactor.extract.constant still calls internal/golang/extract.go:extractVariable. The issue here is if someone calls refactor.extract.constant on a non-constant variable, the typical variable extraction will occur rather than an error stating that the target must be a constant. I am not well-versed in the ast package that extractVariable uses to analyze the type of variable, but I imagine it would be possible to confirm that the target is a constant and then run the extraction. Does anyone have advice here? Thanks in advance.

[findleyr]

Thanks, @NateWilliams2. I commented on your CL. In order to check if an expression is constant, we need type information. We can discuss further on the CL, if you want.

[gopherbot]

Change https://go.dev/cl/565235 mentions this issue: gopls: Enable refactor.extract.constant/variable/function CodeActionKinds

[gopherbot]

Change https://go.dev/cl/567135 mentions this issue: gopls: Enable refactor.extract.constant/variable/function CodeActionKinds