x/tools/gopls: filter noisy code actions when triggerKind is automatic

[hyangah]

gopls version: built @ c467be3

What did I do?

Triggered printf diagnostics and SA1006 diagnostics

	fmt.Fprintln(os.Stderr, "run uploading\n")

Settings:

    "gopls": {
        "ui.diagnostic.staticcheck": true,
    },
    "go.languageServerFlags": ["-rpc.trace"]

What did you expect

Gopls detects the issue and provides quick fixes for these diagnostics.

What did you see instead

Gopls detected the issues, but code actions are not available from the editor (neither with the light bulb nor the Quick Fix hover)

• Light bulb on editor and Hover on editor - don't work.
  
  

• Light bulb on PROBLEMS panel - sort of worked
  

• Lightbulb for SA1006 worked (except the duplicated code actions).
• Lightbulb for printf analysis is not present.

Traces

It looks like VS Code triggers code actions in different ways.
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_codeAction

1. Light bulb on editor

VS Code sent a request asking for quick fixes for detected problems. (See "context" filled with the diagnostic&its source, and triggerKind:2 (TriggerKind: "Automatic"). Gopls returned "inline call to Fprintln" code action, which doesn't make sense.

[Trace - 10:28:39.053 AM] Sending request 'textDocument/codeAction - (93)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":1}},"context":{"diagnostics":[{"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":43}},"message":"fmt.Fprintln arg list ends with redundant newline","code":"default","codeDescription":{"href":"https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf"},"severity":2,"source":"printf"}],"triggerKind":2}}


[Trace - 10:28:39.055 AM] Received response 'textDocument/codeAction - (93)' in 1ms.
Result: [{"title":"Inline call to Fprintln","kind":"refactor.inline","command":{"title":"Inline call to Fprintln","command":"gopls.apply_fix","arguments":[{"Fix":"inline_call","URI":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go","Range":{"start":{"line":291,"character":1},"end":{"line":291,"character":1}},"ResolveEdits":false}]}}]

Note that VS Code asked quick fixes for "printf" analyzer, but not for SA1006 analyzer's report. This can be a client-side issue and I will investigate it separately.

2. Hover on editor

VS Code sent the request asking for quick fixes for detected problems. (See "context" filled with the diagnostic&its source, and triggerKind:1 (TriggerKind "Invoked"). Gopls returned nothing.

[Trace - 10:31:05.080 AM] Sending request 'textDocument/codeAction - (97)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":43}},"context":{"diagnostics":[{"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":43}},"message":"fmt.Fprintln arg list ends with redundant newline","code":"default","codeDescription":{"href":"https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf"},"severity":2,"source":"printf"}],"only":["quickfix"],"triggerKind":1}}


[Trace - 10:31:05.081 AM] Received response 'textDocument/codeAction - (97)' in 0ms.
Result: null

3. Light bulb on PROBLEMS panel

VS Code sent request with "quickfix" and "triggerKind: 1". Gopls returned some code actions for SA1006 diagnostic (but with duplications)

[Trace - 10:46:11.180 AM] Sending request 'textDocument/codeAction - (125)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"range":{"start":{"line":162,"character":1},"end":{"line":162,"character":53}},"context":{"diagnostics":[{"range":{"start":{"line":162,"character":1},"end":{"line":162,"character":53}},"message":"printf-style function with dynamic format string and no further arguments should use print-style function instead","code":"default","severity":2,"source":"SA1006"}],"only":["quickfix"],"triggerKind":1}}


[Trace - 10:46:11.181 AM] Received response 'textDocument/codeAction - (125)' in 0ms.
Result: [{"title":"use fmt.Fprint instead of fmt.Fprintf","kind":"quickfix","diagnostics":[{"range":{"start":{"line":162,"character":1},"end":{"line":162,"character":53}},"severity":2,"code":"default","source":"SA1006","message":"printf-style function with dynamic format string and no further arguments should use print-style function instead"}],"edit":{"documentChanges":[{"textDocument":{"version":407,"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"edits":[{"range":{"start":{"line":162,"character":1},"end":{"line":162,"character":12}},"newText":"fmt.Fprint"}]}]}},{"title":"use fmt.Fprint instead of fmt.Fprintf","kind":"quickfix","diagnostics":[{"range":{"start":{"line":162,"character":1},"end":{"line":162,"character":53}},"severity":2,"code":"default","source":"SA1006","message":"printf-style function with dynamic format string and no further arguments should use print-style function instead"}],"edit":{"documentChanges":[{"textDocument":{"version":407,"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"edits":[{"range":{"start":{"line":162,"character":1},"end":{"line":162,"character":12}},"newText":"fmt.Fprint"}]}]}}]

But didn't return codeactions for printf diagnostic.

[Trace - 10:47:04.164 AM] Sending request 'textDocument/codeAction - (126)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":43}},"context":{"diagnostics":[{"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":43}},"message":"fmt.Fprintln arg list ends with redundant newline","code":"default","codeDescription":{"href":"https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf"},"severity":2,"source":"printf"}],"only":["quickfix"],"triggerKind":1}}


[Trace - 10:47:04.165 AM] Received response 'textDocument/codeAction - (126)' in 1ms.
Result: null

[hyangah]

The behavior is non-deterministic. Sometimes for the same request (case 1 Light bulb on editor), gopls returns the quickfix for printf analyzer. During the same session (without editing...):

[Trace - 11:03:34.199 AM] Sending request 'textDocument/codeAction - (62)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"range":{"start":{"line":291,"character":26},"end":{"line":291,"character":26}},"context":{"diagnostics":[{"range":{"start":{"line":291,"character":1},"end":{"line":291,"character":43}},"message":"fmt.Fprintln arg list ends with redundant newline","code":"default","codeDescription":{"href":"https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf"},"severity":2,"source":"printf"}],"triggerKind":2}}


[Trace - 11:03:34.202 AM] Received response 'textDocument/codeAction - (62)' in 2ms.
Result: [{"title":"Convert to raw string literal","kind":"refactor.rewrite","edit":{"documentChanges":[{"textDocument":{"version":1,"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"edits":[{"range":{"start":{"line":291,"character":25},"end":{"line":291,"character":42}},"newText":"`run uploading\n`"}]}]}},{"title":"Inline call to Fprintln","kind":"refactor.inline","command":{"title":"Inline call to Fprintln","command":"gopls.apply_fix","arguments":[{"Fix":"inline_call","URI":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go","Range":{"start":{"line":291,"character":26},"end":{"line":291,"character":26}},"ResolveEdits":false}]}}]

vs

[Trace - 11:10:26.252 AM] Sending request 'textDocument/codeAction - (156)'.
Params: {"textDocument":{"uri":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go"},"range":{"start":{"line":296,"character":4},"end":{"line":296,"character":4}},"context":{"diagnostics":[{"range":{"start":{"line":296,"character":1},"end":{"line":296,"character":48}},"message":"fmt.Fprintln arg list ends with redundant newline","code":"default","codeDescription":{"href":"https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/printf"},"severity":2,"source":"printf"}],"triggerKind":2}}


[Trace - 11:10:26.254 AM] Received response 'textDocument/codeAction - (156)' in 2ms.
Result: [{"title":"Inline call to Fprintln","kind":"refactor.inline","command":{"title":"Inline call to Fprintln","command":"gopls.apply_fix","arguments":[{"Fix":"inline_call","URI":"file:///Users/hakim/projects/telemetry/cmd/gotelemetry/main.go","Range":{"start":{"line":296,"character":4},"end":{"line":296,"character":4}},"ResolveEdits":false}]}}]

[findleyr]

It looks like gopls doesn't return 'inline' when the request specifies "only":["quickfix"]. So it looks like the behavior is in fact deterministic.

Is the bug that VS Code should specify "quickfix"? Or should gopls not return inline when the action is automatic.

[findleyr]

Per discussion: the problem is simply that "inline" is often not useful, and so therefore should be filtered when the triggerKind is automatic.

[findleyr]

We should do this filtering for [email protected], since the inline lightbulb is distracting.

@adonovan do you want to do this, since you're currently working in the codeaction codebase?

[adonovan]

Sure, I can take this.

Selecting a statement triggers the "extract" lightbulb. Should it be treated similarly?
Or is inline special because it triggers any time the cursor is inside a function call, which is common?
Similarly, placing the cursor in a string literal (again, common) triggers the "flip quotes" action.

Should all of these actions appear only on the Refactor menu (no lightbulb)?
Or should the ones currently triggered by cursor position (no selection required) be so relegated?
Or should CodeAction stop offering commands based on mere cursor placement and require a selection (of a complete or partial string literal, or a function call, respectively)?
I'm not really sure what the ideal VS Code UX is here.

[findleyr]

I'll defer to @hyangah for what the VS Code UI should do, but it seems reasonable that if the region is a single point and trigger is automatic, there is no user intent.

[gopherbot]

Change https://go.dev/cl/556555 mentions this issue: gopls: consolidate analyzers, eliminating "convenience" analyzers

[findleyr]

I don't think this is going to make the v0.15.0 release. It's OK to move it to v0.16.0.

[gopherbot]

Change https://go.dev/cl/587555 mentions this issue: gopls/internal/server: avoid VS Code lightbulb

[gopherbot]

Change https://go.dev/cl/590935 mentions this issue: gopls/internal/server: fix regression in organize imports code action