diff options
author | Christopher Serr <[email protected]> | 2021-02-01 15:57:04 +0000 |
---|---|---|
committer | Christopher Serr <[email protected]> | 2021-02-01 15:57:04 +0000 |
commit | 2e8c1d13ad75ac569a707be05eff535f1894a23e (patch) | |
tree | 27f7d1e82e1415be1b0fc4e359160d3181bfb13e | |
parent | 2d9bb69990b866bad0b4300972f1706d38329ad3 (diff) |
Don't filter code suggestions on Applicability
I've noticed that there are various suggestions that rust-analyzer seems
to filter out, even if they make sense.
Here's an example of where it seems like there should be a suggestion,
but there isn't:
![https://i.imgur.com/wsjM6iz.png](https://i.imgur.com/wsjM6iz.png)
It turns out that this specific suggestion is not considered
`MachineApplicable`, which are the only suggestions that rust-analyzer
accepts. However if you read the documentation for `MachineApplicable`,
https://github.com/rust-lang/rust/blob/b3897e3d1302391ed02efbac1dce8073646b8173/compiler/rustc_lint_defs/src/lib.rs#L27-L29
then you realize that these are specifically only those suggestions that
rust-analyzer could even automatically apply (in some distant future,
behind some setting or so). Other suggestions that may have some
semantic impact do not use `MachineApplicable`. So all other suggestions
are still intended to be suggested to the user, just not automatically
applied without the user being consulted.
https://github.com/rust-lang/rust/blob/b3897e3d1302391ed02efbac1dce8073646b8173/compiler/rustc_lint_defs/src/lib.rs#L22-L24
So with that in mind, rust-analyzer should almost definitely not filter
out `MaybeIncorrect` (which honestly is named horribly, it just means
that it's a semantic change, not just a syntactical one).
Then there's `HasPlaceholders` which basically is just another semantic
one, but with placeholders. The user will have to make some adjustments,
but the suggestion still is perfectly valid. rust-analyzer could
probably detect those placeholders and put proper "tab through" markers
there for the IDE, but that's not necessary for now.
Then the last one is `Unspecified` which is so unknown that I don't even
know how to judge it, meaning that the suggestion should probably also
just be suggested to the user and then they can decide.
So with all that in mind, I'm proposing to get rid of the check for
Applicability entirely.
-rw-r--r-- | crates/rust-analyzer/src/diagnostics/to_proto.rs | 6 |
1 files changed, 2 insertions, 4 deletions
diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs index 757899484..0ed87fe3e 100644 --- a/crates/rust-analyzer/src/diagnostics/to_proto.rs +++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs | |||
@@ -2,7 +2,7 @@ | |||
2 | //! `cargo check` json format to the LSP diagnostic format. | 2 | //! `cargo check` json format to the LSP diagnostic format. |
3 | use std::{collections::HashMap, path::Path}; | 3 | use std::{collections::HashMap, path::Path}; |
4 | 4 | ||
5 | use flycheck::{Applicability, DiagnosticLevel, DiagnosticSpan}; | 5 | use flycheck::{DiagnosticLevel, DiagnosticSpan}; |
6 | use stdx::format_to; | 6 | use stdx::format_to; |
7 | 7 | ||
8 | use crate::{lsp_ext, to_proto::url_from_abs_path}; | 8 | use crate::{lsp_ext, to_proto::url_from_abs_path}; |
@@ -97,9 +97,7 @@ fn map_rust_child_diagnostic( | |||
97 | 97 | ||
98 | let mut edit_map: HashMap<lsp_types::Url, Vec<lsp_types::TextEdit>> = HashMap::new(); | 98 | let mut edit_map: HashMap<lsp_types::Url, Vec<lsp_types::TextEdit>> = HashMap::new(); |
99 | for &span in &spans { | 99 | for &span in &spans { |
100 | if let (Some(Applicability::MachineApplicable), Some(suggested_replacement)) = | 100 | if let Some(suggested_replacement) = &span.suggested_replacement { |
101 | (&span.suggestion_applicability, &span.suggested_replacement) | ||
102 | { | ||
103 | let location = location(workspace_root, span); | 101 | let location = location(workspace_root, span); |
104 | let edit = lsp_types::TextEdit::new(location.range, suggested_replacement.clone()); | 102 | let edit = lsp_types::TextEdit::new(location.range, suggested_replacement.clone()); |
105 | edit_map.entry(location.uri).or_default().push(edit); | 103 | edit_map.entry(location.uri).or_default().push(edit); |