diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2020-08-18 13:04:49 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2020-08-18 13:04:49 +0100 |
commit | b8dfc331abbfce6aad0c248c91c57bd9890a668f (patch) | |
tree | 0c03882f96663e970c9d08637f677f6b037ddf6c /crates/ide | |
parent | 2252a65f238cac0ebf0c4c56d9b475fa0460d758 (diff) | |
parent | 34847c8d96ddf98c40117ebacaecec38b9b758fc (diff) |
Merge #5682
5682: Add an option to disable diagnostics r=matklad a=popzxc
As far as I know, currently it's not possible to disable a selected type of diagnostics provided by `rust-analyzer`.
This causes an inconvenient situation with a false-positive warnings: you either have to disable all the diagnostics, or you have to ignore these warnings.
There are some open issues related to this problem, e.g.: https://github.com/rust-analyzer/rust-analyzer/issues/5412, https://github.com/rust-analyzer/rust-analyzer/issues/5502
This PR attempts to make it possible to selectively disable some diagnostics on per-project basis.
Co-authored-by: Igor Aleksanov <[email protected]>
Diffstat (limited to 'crates/ide')
-rw-r--r-- | crates/ide/src/diagnostics.rs | 82 | ||||
-rw-r--r-- | crates/ide/src/lib.rs | 10 |
2 files changed, 82 insertions, 10 deletions
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index a3ec98178..606a6064b 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs | |||
@@ -4,7 +4,7 @@ | |||
4 | //! macro-expanded files, but we need to present them to the users in terms of | 4 | //! macro-expanded files, but we need to present them to the users in terms of |
5 | //! original files. So we need to map the ranges. | 5 | //! original files. So we need to map the ranges. |
6 | 6 | ||
7 | use std::cell::RefCell; | 7 | use std::{cell::RefCell, collections::HashSet}; |
8 | 8 | ||
9 | use base_db::SourceDatabase; | 9 | use base_db::SourceDatabase; |
10 | use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; | 10 | use hir::{diagnostics::DiagnosticSinkBuilder, Semantics}; |
@@ -31,6 +31,7 @@ pub(crate) fn diagnostics( | |||
31 | db: &RootDatabase, | 31 | db: &RootDatabase, |
32 | file_id: FileId, | 32 | file_id: FileId, |
33 | enable_experimental: bool, | 33 | enable_experimental: bool, |
34 | disabled_diagnostics: Option<HashSet<String>>, | ||
34 | ) -> Vec<Diagnostic> { | 35 | ) -> Vec<Diagnostic> { |
35 | let _p = profile::span("diagnostics"); | 36 | let _p = profile::span("diagnostics"); |
36 | let sema = Semantics::new(db); | 37 | let sema = Semantics::new(db); |
@@ -39,6 +40,7 @@ pub(crate) fn diagnostics( | |||
39 | 40 | ||
40 | // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. | 41 | // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. |
41 | res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { | 42 | res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { |
43 | name: None, | ||
42 | range: err.range(), | 44 | range: err.range(), |
43 | message: format!("Syntax Error: {}", err), | 45 | message: format!("Syntax Error: {}", err), |
44 | severity: Severity::Error, | 46 | severity: Severity::Error, |
@@ -50,7 +52,7 @@ pub(crate) fn diagnostics( | |||
50 | check_struct_shorthand_initialization(&mut res, file_id, &node); | 52 | check_struct_shorthand_initialization(&mut res, file_id, &node); |
51 | } | 53 | } |
52 | let res = RefCell::new(res); | 54 | let res = RefCell::new(res); |
53 | let mut sink = DiagnosticSinkBuilder::new() | 55 | let mut sink_builder = DiagnosticSinkBuilder::new() |
54 | .on::<hir::diagnostics::UnresolvedModule, _>(|d| { | 56 | .on::<hir::diagnostics::UnresolvedModule, _>(|d| { |
55 | res.borrow_mut().push(diagnostic_with_fix(d, &sema)); | 57 | res.borrow_mut().push(diagnostic_with_fix(d, &sema)); |
56 | }) | 58 | }) |
@@ -64,10 +66,19 @@ pub(crate) fn diagnostics( | |||
64 | res.borrow_mut().push(diagnostic_with_fix(d, &sema)); | 66 | res.borrow_mut().push(diagnostic_with_fix(d, &sema)); |
65 | }) | 67 | }) |
66 | // Only collect experimental diagnostics when they're enabled. | 68 | // Only collect experimental diagnostics when they're enabled. |
67 | .filter(|diag| !diag.is_experimental() || enable_experimental) | 69 | .filter(|diag| !diag.is_experimental() || enable_experimental); |
70 | |||
71 | if let Some(disabled_diagnostics) = disabled_diagnostics { | ||
72 | // Do not collect disabled diagnostics. | ||
73 | sink_builder = sink_builder.filter(move |diag| !disabled_diagnostics.contains(diag.name())); | ||
74 | } | ||
75 | |||
76 | // Finalize the `DiagnosticSink` building process. | ||
77 | let mut sink = sink_builder | ||
68 | // Diagnostics not handled above get no fix and default treatment. | 78 | // Diagnostics not handled above get no fix and default treatment. |
69 | .build(|d| { | 79 | .build(|d| { |
70 | res.borrow_mut().push(Diagnostic { | 80 | res.borrow_mut().push(Diagnostic { |
81 | name: Some(d.name().into()), | ||
71 | message: d.message(), | 82 | message: d.message(), |
72 | range: sema.diagnostics_display_range(d).range, | 83 | range: sema.diagnostics_display_range(d).range, |
73 | severity: Severity::Error, | 84 | severity: Severity::Error, |
@@ -84,6 +95,7 @@ pub(crate) fn diagnostics( | |||
84 | 95 | ||
85 | fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic { | 96 | fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic { |
86 | Diagnostic { | 97 | Diagnostic { |
98 | name: Some(d.name().into()), | ||
87 | range: sema.diagnostics_display_range(d).range, | 99 | range: sema.diagnostics_display_range(d).range, |
88 | message: d.message(), | 100 | message: d.message(), |
89 | severity: Severity::Error, | 101 | severity: Severity::Error, |
@@ -110,6 +122,7 @@ fn check_unnecessary_braces_in_use_statement( | |||
110 | }); | 122 | }); |
111 | 123 | ||
112 | acc.push(Diagnostic { | 124 | acc.push(Diagnostic { |
125 | name: None, | ||
113 | range: use_range, | 126 | range: use_range, |
114 | message: "Unnecessary braces in use statement".to_string(), | 127 | message: "Unnecessary braces in use statement".to_string(), |
115 | severity: Severity::WeakWarning, | 128 | severity: Severity::WeakWarning, |
@@ -156,6 +169,7 @@ fn check_struct_shorthand_initialization( | |||
156 | 169 | ||
157 | let field_range = record_field.syntax().text_range(); | 170 | let field_range = record_field.syntax().text_range(); |
158 | acc.push(Diagnostic { | 171 | acc.push(Diagnostic { |
172 | name: None, | ||
159 | range: field_range, | 173 | range: field_range, |
160 | message: "Shorthand struct initialization".to_string(), | 174 | message: "Shorthand struct initialization".to_string(), |
161 | severity: Severity::WeakWarning, | 175 | severity: Severity::WeakWarning, |
@@ -173,6 +187,7 @@ fn check_struct_shorthand_initialization( | |||
173 | 187 | ||
174 | #[cfg(test)] | 188 | #[cfg(test)] |
175 | mod tests { | 189 | mod tests { |
190 | use std::collections::HashSet; | ||
176 | use stdx::trim_indent; | 191 | use stdx::trim_indent; |
177 | use test_utils::assert_eq_text; | 192 | use test_utils::assert_eq_text; |
178 | 193 | ||
@@ -188,7 +203,8 @@ mod tests { | |||
188 | let after = trim_indent(ra_fixture_after); | 203 | let after = trim_indent(ra_fixture_after); |
189 | 204 | ||
190 | let (analysis, file_position) = analysis_and_position(ra_fixture_before); | 205 | let (analysis, file_position) = analysis_and_position(ra_fixture_before); |
191 | let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap(); | 206 | let diagnostic = |
207 | analysis.diagnostics(file_position.file_id, true, None).unwrap().pop().unwrap(); | ||
192 | let mut fix = diagnostic.fix.unwrap(); | 208 | let mut fix = diagnostic.fix.unwrap(); |
193 | let edit = fix.source_change.source_file_edits.pop().unwrap().edit; | 209 | let edit = fix.source_change.source_file_edits.pop().unwrap().edit; |
194 | let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); | 210 | let target_file_contents = analysis.file_text(file_position.file_id).unwrap(); |
@@ -214,7 +230,7 @@ mod tests { | |||
214 | let ra_fixture_after = &trim_indent(ra_fixture_after); | 230 | let ra_fixture_after = &trim_indent(ra_fixture_after); |
215 | let (analysis, file_pos) = analysis_and_position(ra_fixture_before); | 231 | let (analysis, file_pos) = analysis_and_position(ra_fixture_before); |
216 | let current_file_id = file_pos.file_id; | 232 | let current_file_id = file_pos.file_id; |
217 | let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap(); | 233 | let diagnostic = analysis.diagnostics(current_file_id, true, None).unwrap().pop().unwrap(); |
218 | let mut fix = diagnostic.fix.unwrap(); | 234 | let mut fix = diagnostic.fix.unwrap(); |
219 | let edit = fix.source_change.source_file_edits.pop().unwrap(); | 235 | let edit = fix.source_change.source_file_edits.pop().unwrap(); |
220 | let changed_file_id = edit.file_id; | 236 | let changed_file_id = edit.file_id; |
@@ -235,14 +251,58 @@ mod tests { | |||
235 | let analysis = mock.analysis(); | 251 | let analysis = mock.analysis(); |
236 | let diagnostics = files | 252 | let diagnostics = files |
237 | .into_iter() | 253 | .into_iter() |
238 | .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap()) | 254 | .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) |
239 | .collect::<Vec<_>>(); | 255 | .collect::<Vec<_>>(); |
240 | assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); | 256 | assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); |
241 | } | 257 | } |
242 | 258 | ||
259 | /// Takes a multi-file input fixture with annotated cursor position and the list of disabled diagnostics, | ||
260 | /// and checks that provided diagnostics aren't spawned during analysis. | ||
261 | fn check_disabled_diagnostics(ra_fixture: &str, disabled_diagnostics: &[&'static str]) { | ||
262 | let disabled_diagnostics: HashSet<_> = | ||
263 | disabled_diagnostics.into_iter().map(|diag| diag.to_string()).collect(); | ||
264 | |||
265 | let mock = MockAnalysis::with_files(ra_fixture); | ||
266 | let files = mock.files().map(|(it, _)| it).collect::<Vec<_>>(); | ||
267 | let analysis = mock.analysis(); | ||
268 | |||
269 | let diagnostics = files | ||
270 | .clone() | ||
271 | .into_iter() | ||
272 | .flat_map(|file_id| { | ||
273 | analysis.diagnostics(file_id, true, Some(disabled_diagnostics.clone())).unwrap() | ||
274 | }) | ||
275 | .collect::<Vec<_>>(); | ||
276 | |||
277 | // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list. | ||
278 | for diagnostic in diagnostics { | ||
279 | if let Some(name) = diagnostic.name { | ||
280 | assert!(!disabled_diagnostics.contains(&name), "Diagnostic {} is disabled", name); | ||
281 | } | ||
282 | } | ||
283 | |||
284 | // Then, we must reset the config and repeat the check, so that we'll be sure that without | ||
285 | // config these diagnostics are emitted. | ||
286 | // This is required for tests to not become outdated if e.g. diagnostics name changes: | ||
287 | // without this additional run the test will pass simply because a diagnostic with an old name | ||
288 | // will no longer exist. | ||
289 | let diagnostics = files | ||
290 | .into_iter() | ||
291 | .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap()) | ||
292 | .collect::<Vec<_>>(); | ||
293 | |||
294 | assert!( | ||
295 | diagnostics | ||
296 | .into_iter() | ||
297 | .filter_map(|diag| diag.name) | ||
298 | .any(|name| disabled_diagnostics.contains(&name)), | ||
299 | "At least one of the diagnostics was not emitted even without config; are the diagnostics names correct?" | ||
300 | ); | ||
301 | } | ||
302 | |||
243 | fn check_expect(ra_fixture: &str, expect: Expect) { | 303 | fn check_expect(ra_fixture: &str, expect: Expect) { |
244 | let (analysis, file_id) = single_file(ra_fixture); | 304 | let (analysis, file_id) = single_file(ra_fixture); |
245 | let diagnostics = analysis.diagnostics(file_id, true).unwrap(); | 305 | let diagnostics = analysis.diagnostics(file_id, true, None).unwrap(); |
246 | expect.assert_debug_eq(&diagnostics) | 306 | expect.assert_debug_eq(&diagnostics) |
247 | } | 307 | } |
248 | 308 | ||
@@ -502,6 +562,9 @@ fn test_fn() { | |||
502 | expect![[r#" | 562 | expect![[r#" |
503 | [ | 563 | [ |
504 | Diagnostic { | 564 | Diagnostic { |
565 | name: Some( | ||
566 | "unresolved-module", | ||
567 | ), | ||
505 | message: "unresolved module", | 568 | message: "unresolved module", |
506 | range: 0..8, | 569 | range: 0..8, |
507 | severity: Error, | 570 | severity: Error, |
@@ -675,4 +738,9 @@ struct Foo { | |||
675 | ", | 738 | ", |
676 | ) | 739 | ) |
677 | } | 740 | } |
741 | |||
742 | #[test] | ||
743 | fn test_disabled_diagnostics() { | ||
744 | check_disabled_diagnostics(r#"mod foo;"#, &["unresolved-module"]); | ||
745 | } | ||
678 | } | 746 | } |
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index eb6389529..4b797f374 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs | |||
@@ -44,7 +44,7 @@ mod syntax_highlighting; | |||
44 | mod syntax_tree; | 44 | mod syntax_tree; |
45 | mod typing; | 45 | mod typing; |
46 | 46 | ||
47 | use std::sync::Arc; | 47 | use std::{collections::HashSet, sync::Arc}; |
48 | 48 | ||
49 | use base_db::{ | 49 | use base_db::{ |
50 | salsa::{self, ParallelDatabase}, | 50 | salsa::{self, ParallelDatabase}, |
@@ -101,6 +101,7 @@ pub type Cancelable<T> = Result<T, Canceled>; | |||
101 | 101 | ||
102 | #[derive(Debug)] | 102 | #[derive(Debug)] |
103 | pub struct Diagnostic { | 103 | pub struct Diagnostic { |
104 | pub name: Option<String>, | ||
104 | pub message: String, | 105 | pub message: String, |
105 | pub range: TextRange, | 106 | pub range: TextRange, |
106 | pub severity: Severity, | 107 | pub severity: Severity, |
@@ -147,7 +148,7 @@ pub struct AnalysisHost { | |||
147 | } | 148 | } |
148 | 149 | ||
149 | impl AnalysisHost { | 150 | impl AnalysisHost { |
150 | pub fn new(lru_capacity: Option<usize>) -> AnalysisHost { | 151 | pub fn new(lru_capacity: Option<usize>) -> Self { |
151 | AnalysisHost { db: RootDatabase::new(lru_capacity) } | 152 | AnalysisHost { db: RootDatabase::new(lru_capacity) } |
152 | } | 153 | } |
153 | 154 | ||
@@ -496,8 +497,11 @@ impl Analysis { | |||
496 | &self, | 497 | &self, |
497 | file_id: FileId, | 498 | file_id: FileId, |
498 | enable_experimental: bool, | 499 | enable_experimental: bool, |
500 | disabled_diagnostics: Option<HashSet<String>>, | ||
499 | ) -> Cancelable<Vec<Diagnostic>> { | 501 | ) -> Cancelable<Vec<Diagnostic>> { |
500 | self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental)) | 502 | self.with_db(|db| { |
503 | diagnostics::diagnostics(db, file_id, enable_experimental, disabled_diagnostics) | ||
504 | }) | ||
501 | } | 505 | } |
502 | 506 | ||
503 | /// Returns the edit required to rename reference at the position to the new | 507 | /// Returns the edit required to rename reference at the position to the new |