aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2021-02-01 16:41:52 +0000
committerGitHub <[email protected]>2021-02-01 16:41:52 +0000
commit4131e88bb7b62db5743cdf5a9ed01e3cababc5ea (patch)
tree593813edafdf3dba5655a321391cbd028d7e649e
parent2d9bb69990b866bad0b4300972f1706d38329ad3 (diff)
parentb072bbed2a770a87ce6b2e919f1401e3021aef1d (diff)
Merge #7508
7508: Don't filter code suggestions on Applicability r=lnicola a=CryZe 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`, [Source](https://github.com/rust-lang/rust/blob/b3897e3d1302391ed02efbac1dce8073646b8173/compiler/rustc_lint_defs/src/lib.rs#L27-L29) ```rust /// The suggestion is definitely what the user intended. This suggestion should be /// automatically applied. MachineApplicable, ``` 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 command 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. [Source](https://github.com/rust-lang/rust/blob/b3897e3d1302391ed02efbac1dce8073646b8173/compiler/rustc_lint_defs/src/lib.rs#L22-L24) ```rust /// All suggestions are marked with an `Applicability`. Tools use the applicability of a suggestion /// to determine whether it should be automatically applied or if the user should be consulted /// before applying the suggestion. ``` 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. Co-authored-by: Christopher Serr <[email protected]>
-rw-r--r--crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt47
-rw-r--r--crates/rust-analyzer/src/diagnostics/to_proto.rs6
2 files changed, 48 insertions, 5 deletions
diff --git a/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt b/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
index 7576097b3..ce80476fb 100644
--- a/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
+++ b/crates/rust-analyzer/src/diagnostics/test_data/clippy_pass_by_ref.txt
@@ -266,6 +266,51 @@
266 tags: None, 266 tags: None,
267 data: None, 267 data: None,
268 }, 268 },
269 fixes: [], 269 fixes: [
270 CodeAction {
271 title: "consider passing by value instead",
272 group: None,
273 kind: Some(
274 CodeActionKind(
275 "quickfix",
276 ),
277 ),
278 edit: Some(
279 SnippetWorkspaceEdit {
280 changes: Some(
281 {
282 Url {
283 scheme: "file",
284 host: None,
285 port: None,
286 path: "/test/compiler/mir/tagset.rs",
287 query: None,
288 fragment: None,
289 }: [
290 TextEdit {
291 range: Range {
292 start: Position {
293 line: 41,
294 character: 23,
295 },
296 end: Position {
297 line: 41,
298 character: 28,
299 },
300 },
301 new_text: "self",
302 },
303 ],
304 },
305 ),
306 document_changes: None,
307 },
308 ),
309 is_preferred: Some(
310 true,
311 ),
312 data: None,
313 },
314 ],
270 }, 315 },
271] 316]
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.
3use std::{collections::HashMap, path::Path}; 3use std::{collections::HashMap, path::Path};
4 4
5use flycheck::{Applicability, DiagnosticLevel, DiagnosticSpan}; 5use flycheck::{DiagnosticLevel, DiagnosticSpan};
6use stdx::format_to; 6use stdx::format_to;
7 7
8use crate::{lsp_ext, to_proto::url_from_abs_path}; 8use 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);