x/tools/gopls: revisit the set of 'quickfix' refactorings

[alanlwb]

Why can gopls@v0.16.2 do fill struct code action and gopls@v0.17.1 doesn't

feature or bug ?

gopls@v0.17.1

gopls@v0.16.2

[findleyr]

CC @adonovan

The option is still there. However, it has moved to the 'Refactor' menu.

[findleyr]

I'll note that this is an unclear decision. We used to serve all refactoring in the 'quick fix' menu, but some people noted that having lightbulbs for each cursor position was distracting.

Other editors make refactoring quick fixes more prominent than VS Code.

[findleyr]

@h9jiang is there a better way for us to surface these refactorings?

[h9jiang]

To provide some context, the lightbulb I believe is code actions response returned from gopls when you move the cursor. The lightbulb only shows up if the response is not empty. (meaning gopls think there is a code action available for place your cursor is selecting)

From the code perspective, the current behavior is intended. @adonovan recently changed the behavior of automatic code action response. (There are two kind, the first one is automatic where the vscode trigger code action automatically when you move the cursor. The second one is where the user trigger the code action manually by Right click + Code Action or Command + .).

Previously, both return the full list of code actions including fillStruct. That's why you are able to see this lightbulb in v0.16.2. In recent change, the automatic code action will return only code action of kind quickFix (and fillStruct does not belong to quickFix, it belongs to refactor.rewrite). So the response is empty for v0.17.0. However, you can still find all available code actions by triggering code action manually.

Re: findleyr@, I'm not sure if there is a better way of surfacing these code actions else where. I will keep reading.

However, I did notice a different behavior for TypeScript LSP.

Manual Code Action for async function Rewrite and Move:

Automatic Code Action for async function Rewrite only:

Maybe the more important code action is surfaced as lightbulb (automatic) and everything is surfaced as code action (manual). I will keep researching.

[alanlwb]

can I set the fillStruct to the automatic code action? Because it is indeed very useful. Now I have to downgrade the gopls version. thx.

[findleyr]

@alanlwb we will revisit this for v0.18.0, which is actually scheduled quite soon (end of Jan / early Feb). Unfortunately, for now if you need these to be available with the lightbulb, you must downgrade.

Promoting this to a gopls issue to make a decision about the behavior.

[findleyr]

Thanks @h9jiang for the analysis.

Sounds like we need to revisit the set of refactorings available in the quickfix menu, or consider changes to the VS Code extension to make refactorings more readily available.

[igor-spirin-indriver]

Filling the structure as a quick fix was very convenient, especially when using a mouse. I did it all the time.
Hopefully this important feature will return 🙏

[findleyr]

@igor-spirin-indriver yes, we'll fix this for gopsl@v0.18.0 (which is actually quite soon as we want to release this before Go 1.24).

We think we interpreted 'quick fix' too literally, only returning fixes for diagnostics.
How about the following heuristics:

• Quick fixes must be ~guaranteed to succeed. One of the main complaints that led us to constrain fixes was that 'inline this call' was provided in the quick fix menu but would frequently fail.
• Quick fixes must not break the build. This should really be a subset of the previous bullet, but must unfortunately be called out independently as not all of our code actions leave the workspace in a working state.
• Quick fixes must be frequently useful. This is of course a gray area, but we can use telemetry to determine which code actions our users are regularly invoking. I think 'generative' quick fixes, such as fill struct, are likely to be used frequently, as they are invoked as part of writing new code. Therefore, we want them to be easily accessible.

@adonovan @h9jiang what do you think? If we agree, we can document these heuristics and fix our dispatch.