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

hyangah · 2024-01-19T15:49:22Z

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

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.

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

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

The text was updated successfully, but these errors were encountered:

hyangah · 2024-01-19T16:11:52Z

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 · 2024-01-19T16:56:18Z

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 · 2024-01-19T16:59:23Z

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

findleyr · 2024-01-19T17:31:08Z

We should do this filtering for gopls@v0.15.0, since the inline lightbulb is distracting.

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

adonovan · 2024-01-19T19:47:51Z

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 · 2024-01-19T22:30:09Z

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 · 2024-01-22T15:34:56Z

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

Make several mechanical clean-ups to analyzer implementations, reducing the complexity of their configuration and normalizing behavior. - Remove the "type error" analyzer category, since it is now an unused distinction. - Make "stubmethods" an ordinary analyzer, since it no longer needs special logic in the code action handler. - Revert infertypeargs to be an ordinary analyzer (golang/go#63821), but make its severity "hint", so that it doesn't show up in the problems tab. - Remove the "convenience" analyzer category, which only existed for the enabled/disabled configuration. The only effect of this is that the "fillstruct" analyzer can no longer be disabled, but since code actions should not be disruptive (golang/go#65167), it should be OK to have them all available. Notably, we don't allow disabling other code actions. Fixes golang/go#63821 Fixes golang/go#61559 Change-Id: I2a1cd01ea331c26fdbb35d6b5c97562c2a1f9240 Reviewed-on: https://go-review.googlesource.com/c/tools/+/556555 Reviewed-by: Alan Donovan <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Robert Findley <rfindley@google.com>

findleyr · 2024-02-06T20:26:59Z

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 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 Jan 19, 2024

gopherbot added this to the Unreleased milestone Jan 19, 2024

hyangah mentioned this issue Jan 19, 2024

x/tools/gopls: "source" and "code" in publishDiagnostics are reversed #65168

Closed

hyangah added the gopls/analysis Issues related to running analysis in gopls label Jan 19, 2024

findleyr modified the milestones: Unreleased, gopls/v0.15.0 Jan 19, 2024

findleyr changed the title ~~x/tools/gopls: code action for diagnostics do not include useful fixes~~ x/tools/gopls: filter noisy code actions when triggerKind is automatic Jan 19, 2024

findleyr modified the milestones: gopls/v0.15.0, gopls/v0.16.0 Feb 6, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hyangah commented Jan 19, 2024

hyangah commented Jan 19, 2024

findleyr commented Jan 19, 2024

findleyr commented Jan 19, 2024

findleyr commented Jan 19, 2024

adonovan commented Jan 19, 2024 •

edited

findleyr commented Jan 19, 2024

gopherbot commented Jan 22, 2024

findleyr commented Feb 6, 2024

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

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

Comments

hyangah commented Jan 19, 2024

What did I do?

What did you expect

What did you see instead

Traces

hyangah commented Jan 19, 2024

findleyr commented Jan 19, 2024

findleyr commented Jan 19, 2024

findleyr commented Jan 19, 2024

adonovan commented Jan 19, 2024 • edited

findleyr commented Jan 19, 2024

gopherbot commented Jan 22, 2024

findleyr commented Feb 6, 2024

adonovan commented Jan 19, 2024 •

edited