feat: enable automatically fix diagnostics

[Young-Flash]

Change

• server side: add specified_diagnostic_code field into AssistConfig, which use to filter out unreleated fixes to the diagnostic. add handle_code_action_for_specified_diagnostic based on the implementation of handle_code_action, which use to handle the rust-analyzer/codeActionForDiagnostic LSP request, it will return the only code action fix corrsponding to the specified diagnostic
• client side: listen the onWillSaveTextDocument event and apply the fix corrsponding to the diagnostics which config by rust-analyzer.autoFixDiagnostics

Demo

based on the following config in settings.josn

"rust-analyzer.autoFixDiagnostics": [
    "unused_braces",
    "redundant_field_names",
    "non_snake_case",
],

Todo

• server side: currently we only support native diagnostic fix from rust-analyzer(crates/ide-diagnostics/src/handlers/), ideally we should support fix from cargo check also. we should also add more native diagnostics and fix in rust-analyzer, but it require a lot of work
• client side: expose a config for other event to trigger autoFixDiagnostics, like format file?

close #12960

[Veykril]

Some thoughts, specified_diagnostic_code seems unnecessary. Instead of picking the first quickfix when there are multiple, autofixing shouldn't be applicable imo. If there are multiple options we can't reason about which one to pick so that's left to the user.

As such I don't believe implementing this needs to touch the server code at all.

With that out of the way, I feel like this isn't necessarily rust-analyzer specific but could be a generic configurable vscode extension that consumes a config of diagnostics with quickfixes that should be autofixed on save that is language agnostic?

[Young-Flash]

Some thoughts, specified_diagnostic_code seems unnecessary. Instead of picking the first quickfix when there are multiple, autofixing shouldn't be applicable imo. If there are multiple options we can't reason about which one to pick so that's left to the user.

As such I don't believe implementing this needs to touch the server code at all.

The reason why I pick the first one in the returned CodeAction list is(for simplicity) that I reuse the response format of textDocument/codeAction as rust-analyzer/codeActionForDiagnostic's response format.

In fact, the response for rust-analyzer/codeActionForDiagnostic LSP request will contains at most one CodeAction, since we use specified_diagnostic_code to filter out diagnostic which is unrelated

.filter(|it| {
    specific_diagnostic_code.map_or(true, |specific_diagnostic_code| {
        &it.code.as_str() == specific_diagnostic_code
    })
})

and use CodeActionParams.range to filter out CodeAction which is out of the currentDiagnostic's range

.filter(|it| it.target.intersect(frange.range).is_some())

[Young-Flash]

With that out of the way, I feel like this isn't necessarily rust-analyzer specific but could be a generic configurable vscode extension that consumes a config of diagnostics with quickfixes that should be autofixed on save that is language agnostic?

It may be, but that generic configurable vscode extension for all kind of language requires more work(since we cannot guarantee that the diagnosis and CodeAction of every language will respect the LSP protocol and vscode api?), currently I just want to support auto fix diagnostic for rust-analyzer

[Veykril]

It may be, but that generic configurable vscode extension for all kind of language requires more work(since we cannot guarantee that the diagnosis and CodeAction of every language will respect the LSP protocol and vscode api?), currently I just want to support auto fix diagnostic for rust-analyzer

The LSP shouldn't really matter here at all. I'd expect that this functionality should be completely implementable with just vscode's diagnostics API

[bors]

☔ The latest upstream changes (presumably #16662) made this pull request unmergeable. Please resolve the merge conflicts.

[Young-Flash]

I'd expect that this functionality should be completely implementable with just vscode's diagnostics API

Is it really possible to make this without touching rust-analyzer server side? If we don't filter out some unreleated CodeAction in server side, we will always get more than one CodeAction everytime we emit textDocument/codeAction, thus can't determine which to apply.

#16715 (comment)

[Veykril]

The client should be able to associate code actions with diagnostics, so that filtering should happen on the client side, see https://code.visualstudio.com/api/references/vscode-api#CodeAction having a

diagnostics field:

Diagnostics that this code action resolves.

[Young-Flash]

The client should be able to associate code actions with diagnostics

yeah, but that diagnostics?: [Diagnostic] field is allowed to undefined, and rust-analyzer server side don't attach diagnostics info for CodeAction

[Veykril]

and rust-analyzer server side don't attach diagnostics info for CodeAction

Then we should fix that (that probably didn't exist back when code actions were implemented in r-a).

And them being possibly undefined comes from code actions not necessarily being associated with a diagnostic, so those are irrelevant to the feature here in the first place. Note that I am fine with us having this for r-a specifically for now, but my requirement is that this should not require changes to the server (aside from implementing the diagnostics field for CodeActions)

[Young-Flash]

seems tricky
we have ide_db::assists::Assist in ide_diagnostics::Diagnostic

rust-analyzer/crates/ide-diagnostics/src/lib.rs

Lines 136 to 146 in a2e2741

	pub struct Diagnostic {
	pub code: DiagnosticCode,
	pub message: String,
	pub range: FileRange,
	pub severity: Severity,
	pub unused: bool,
	pub experimental: bool,
	pub fixes: Option<Vec<Assist>>,
	// The node that will be affected by `#[allow]` and similar attributes.
	pub main_node: Option<InFile<SyntaxNodePtr>>,
	}

attach Diagnostic in Assist casue cycle dependency ?

[Veykril]

We have it inverted in comparison to the LSP (that is our diagnostics contain the assist that resolves them), but that is fine. We can flip that in the protocol conversion layer. We probably just need to restructure things slightly there so we don't lose the information before