diff options
author | Aleksey Kladov <[email protected]> | 2020-08-18 17:39:43 +0100 |
---|---|---|
committer | Aleksey Kladov <[email protected]> | 2020-08-18 17:39:43 +0100 |
commit | 8146669542dfc887956901b54a453c9a97fee7e3 (patch) | |
tree | 4408709203c98b63df20204a5630db86914f2e94 /crates/ide | |
parent | bbb1c617b9198c230d8331dc0ffc2affb01d2e7a (diff) |
Add type safety to diagnostic codes
Diffstat (limited to 'crates/ide')
-rw-r--r-- | crates/ide/src/diagnostics.rs | 76 |
1 files changed, 17 insertions, 59 deletions
diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 1f85805d2..6922034bc 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs | |||
@@ -25,7 +25,7 @@ use self::fixes::DiagnosticWithFix; | |||
25 | 25 | ||
26 | #[derive(Debug)] | 26 | #[derive(Debug)] |
27 | pub struct Diagnostic { | 27 | pub struct Diagnostic { |
28 | pub name: Option<String>, | 28 | // pub name: Option<String>, |
29 | pub message: String, | 29 | pub message: String, |
30 | pub range: TextRange, | 30 | pub range: TextRange, |
31 | pub severity: Severity, | 31 | pub severity: Severity, |
@@ -76,7 +76,7 @@ pub(crate) fn diagnostics( | |||
76 | 76 | ||
77 | // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. | 77 | // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily. |
78 | res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { | 78 | res.extend(parse.errors().iter().take(128).map(|err| Diagnostic { |
79 | name: None, | 79 | // name: None, |
80 | range: err.range(), | 80 | range: err.range(), |
81 | message: format!("Syntax Error: {}", err), | 81 | message: format!("Syntax Error: {}", err), |
82 | severity: Severity::Error, | 82 | severity: Severity::Error, |
@@ -103,14 +103,14 @@ pub(crate) fn diagnostics( | |||
103 | }) | 103 | }) |
104 | // Only collect experimental diagnostics when they're enabled. | 104 | // Only collect experimental diagnostics when they're enabled. |
105 | .filter(|diag| !(diag.is_experimental() && config.disable_experimental)) | 105 | .filter(|diag| !(diag.is_experimental() && config.disable_experimental)) |
106 | .filter(|diag| !config.disabled.contains(diag.name())); | 106 | .filter(|diag| !config.disabled.contains(diag.code().as_str())); |
107 | 107 | ||
108 | // Finalize the `DiagnosticSink` building process. | 108 | // Finalize the `DiagnosticSink` building process. |
109 | let mut sink = sink_builder | 109 | let mut sink = sink_builder |
110 | // Diagnostics not handled above get no fix and default treatment. | 110 | // Diagnostics not handled above get no fix and default treatment. |
111 | .build(|d| { | 111 | .build(|d| { |
112 | res.borrow_mut().push(Diagnostic { | 112 | res.borrow_mut().push(Diagnostic { |
113 | name: Some(d.name().into()), | 113 | // name: Some(d.name().into()), |
114 | message: d.message(), | 114 | message: d.message(), |
115 | range: sema.diagnostics_display_range(d).range, | 115 | range: sema.diagnostics_display_range(d).range, |
116 | severity: Severity::Error, | 116 | severity: Severity::Error, |
@@ -127,7 +127,7 @@ pub(crate) fn diagnostics( | |||
127 | 127 | ||
128 | fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic { | 128 | fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic { |
129 | Diagnostic { | 129 | Diagnostic { |
130 | name: Some(d.name().into()), | 130 | // name: Some(d.name().into()), |
131 | range: sema.diagnostics_display_range(d).range, | 131 | range: sema.diagnostics_display_range(d).range, |
132 | message: d.message(), | 132 | message: d.message(), |
133 | severity: Severity::Error, | 133 | severity: Severity::Error, |
@@ -154,7 +154,7 @@ fn check_unnecessary_braces_in_use_statement( | |||
154 | }); | 154 | }); |
155 | 155 | ||
156 | acc.push(Diagnostic { | 156 | acc.push(Diagnostic { |
157 | name: None, | 157 | // name: None, |
158 | range: use_range, | 158 | range: use_range, |
159 | message: "Unnecessary braces in use statement".to_string(), | 159 | message: "Unnecessary braces in use statement".to_string(), |
160 | severity: Severity::WeakWarning, | 160 | severity: Severity::WeakWarning, |
@@ -201,7 +201,7 @@ fn check_struct_shorthand_initialization( | |||
201 | 201 | ||
202 | let field_range = record_field.syntax().text_range(); | 202 | let field_range = record_field.syntax().text_range(); |
203 | acc.push(Diagnostic { | 203 | acc.push(Diagnostic { |
204 | name: None, | 204 | // name: None, |
205 | range: field_range, | 205 | range: field_range, |
206 | message: "Shorthand struct initialization".to_string(), | 206 | message: "Shorthand struct initialization".to_string(), |
207 | severity: Severity::WeakWarning, | 207 | severity: Severity::WeakWarning, |
@@ -299,54 +299,6 @@ mod tests { | |||
299 | assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); | 299 | assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics); |
300 | } | 300 | } |
301 | 301 | ||
302 | /// Takes a multi-file input fixture with annotated cursor position and the list of disabled diagnostics, | ||
303 | /// and checks that provided diagnostics aren't spawned during analysis. | ||
304 | fn check_disabled_diagnostics(ra_fixture: &str, disabled_diagnostics: &[&'static str]) { | ||
305 | let mut config = DiagnosticsConfig::default(); | ||
306 | config.disabled = disabled_diagnostics.into_iter().map(|diag| diag.to_string()).collect(); | ||
307 | |||
308 | let mock = MockAnalysis::with_files(ra_fixture); | ||
309 | let files = mock.files().map(|(it, _)| it).collect::<Vec<_>>(); | ||
310 | let analysis = mock.analysis(); | ||
311 | |||
312 | let diagnostics = files | ||
313 | .clone() | ||
314 | .into_iter() | ||
315 | .flat_map(|file_id| analysis.diagnostics(&config, file_id).unwrap()) | ||
316 | .collect::<Vec<_>>(); | ||
317 | |||
318 | // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list. | ||
319 | for diagnostic in diagnostics { | ||
320 | if let Some(name) = diagnostic.name { | ||
321 | assert!( | ||
322 | !disabled_diagnostics.contains(&name.as_str()), | ||
323 | "Diagnostic {} is disabled", | ||
324 | name | ||
325 | ); | ||
326 | } | ||
327 | } | ||
328 | |||
329 | // Then, we must reset the config and repeat the check, so that we'll be sure that without | ||
330 | // config these diagnostics are emitted. | ||
331 | // This is required for tests to not become outdated if e.g. diagnostics name changes: | ||
332 | // without this additional run the test will pass simply because a diagnostic with an old name | ||
333 | // will no longer exist. | ||
334 | let diagnostics = files | ||
335 | .into_iter() | ||
336 | .flat_map(|file_id| { | ||
337 | analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap() | ||
338 | }) | ||
339 | .collect::<Vec<_>>(); | ||
340 | |||
341 | assert!( | ||
342 | diagnostics | ||
343 | .into_iter() | ||
344 | .filter_map(|diag| diag.name) | ||
345 | .any(|name| disabled_diagnostics.contains(&name.as_str())), | ||
346 | "At least one of the diagnostics was not emitted even without config; are the diagnostics names correct?" | ||
347 | ); | ||
348 | } | ||
349 | |||
350 | fn check_expect(ra_fixture: &str, expect: Expect) { | 302 | fn check_expect(ra_fixture: &str, expect: Expect) { |
351 | let (analysis, file_id) = single_file(ra_fixture); | 303 | let (analysis, file_id) = single_file(ra_fixture); |
352 | let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); | 304 | let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); |
@@ -609,9 +561,6 @@ fn test_fn() { | |||
609 | expect![[r#" | 561 | expect![[r#" |
610 | [ | 562 | [ |
611 | Diagnostic { | 563 | Diagnostic { |
612 | name: Some( | ||
613 | "unresolved-module", | ||
614 | ), | ||
615 | message: "unresolved module", | 564 | message: "unresolved module", |
616 | range: 0..8, | 565 | range: 0..8, |
617 | severity: Error, | 566 | severity: Error, |
@@ -788,6 +737,15 @@ struct Foo { | |||
788 | 737 | ||
789 | #[test] | 738 | #[test] |
790 | fn test_disabled_diagnostics() { | 739 | fn test_disabled_diagnostics() { |
791 | check_disabled_diagnostics(r#"mod foo;"#, &["unresolved-module"]); | 740 | let mut config = DiagnosticsConfig::default(); |
741 | config.disabled.insert("unresolved-module".into()); | ||
742 | |||
743 | let (analysis, file_id) = single_file(r#"mod foo;"#); | ||
744 | |||
745 | let diagnostics = analysis.diagnostics(&config, file_id).unwrap(); | ||
746 | assert!(diagnostics.is_empty()); | ||
747 | |||
748 | let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap(); | ||
749 | assert!(!diagnostics.is_empty()); | ||
792 | } | 750 | } |
793 | } | 751 | } |