diff options
author | bors[bot] <26634292+bors[bot]@users.noreply.github.com> | 2021-06-14 20:37:28 +0100 |
---|---|---|
committer | GitHub <[email protected]> | 2021-06-14 20:37:28 +0100 |
commit | ce8fdf3ab05b59b517dee985e4b3828c6e373a91 (patch) | |
tree | 8a498dc4c333f2f0e513a5b63a45c226d6e12279 /crates/ide_diagnostics/src/lib.rs | |
parent | 27a70492f7eb8181c90ed5aa62e8e8b7ce035c91 (diff) | |
parent | 4cfc767d7fadeab025227d67f104065c9e8a55d3 (diff) |
Merge #9276
9276: internal: refactor diagnostics more r=matklad a=matklad
bors r+
🤖
Co-authored-by: Aleksey Kladov <[email protected]>
Diffstat (limited to 'crates/ide_diagnostics/src/lib.rs')
-rw-r--r-- | crates/ide_diagnostics/src/lib.rs | 224 |
1 files changed, 34 insertions, 190 deletions
diff --git a/crates/ide_diagnostics/src/lib.rs b/crates/ide_diagnostics/src/lib.rs index 88037be5a..6ad1b4373 100644 --- a/crates/ide_diagnostics/src/lib.rs +++ b/crates/ide_diagnostics/src/lib.rs | |||
@@ -37,15 +37,17 @@ mod handlers { | |||
37 | pub(crate) mod remove_this_semicolon; | 37 | pub(crate) mod remove_this_semicolon; |
38 | pub(crate) mod replace_filter_map_next_with_find_map; | 38 | pub(crate) mod replace_filter_map_next_with_find_map; |
39 | pub(crate) mod unimplemented_builtin_macro; | 39 | pub(crate) mod unimplemented_builtin_macro; |
40 | pub(crate) mod unlinked_file; | ||
41 | pub(crate) mod unresolved_extern_crate; | 40 | pub(crate) mod unresolved_extern_crate; |
42 | pub(crate) mod unresolved_import; | 41 | pub(crate) mod unresolved_import; |
43 | pub(crate) mod unresolved_macro_call; | 42 | pub(crate) mod unresolved_macro_call; |
44 | pub(crate) mod unresolved_module; | 43 | pub(crate) mod unresolved_module; |
45 | pub(crate) mod unresolved_proc_macro; | 44 | pub(crate) mod unresolved_proc_macro; |
46 | } | ||
47 | 45 | ||
48 | mod field_shorthand; | 46 | // The handlers bellow are unusual, the implement the diagnostics as well. |
47 | pub(crate) mod field_shorthand; | ||
48 | pub(crate) mod useless_braces; | ||
49 | pub(crate) mod unlinked_file; | ||
50 | } | ||
49 | 51 | ||
50 | use hir::{diagnostics::AnyDiagnostic, Semantics}; | 52 | use hir::{diagnostics::AnyDiagnostic, Semantics}; |
51 | use ide_db::{ | 53 | use ide_db::{ |
@@ -55,15 +57,8 @@ use ide_db::{ | |||
55 | source_change::SourceChange, | 57 | source_change::SourceChange, |
56 | RootDatabase, | 58 | RootDatabase, |
57 | }; | 59 | }; |
58 | use itertools::Itertools; | ||
59 | use rustc_hash::FxHashSet; | 60 | use rustc_hash::FxHashSet; |
60 | use syntax::{ | 61 | use syntax::{ast::AstNode, TextRange}; |
61 | ast::{self, AstNode}, | ||
62 | SyntaxNode, TextRange, | ||
63 | }; | ||
64 | use text_edit::TextEdit; | ||
65 | |||
66 | use crate::handlers::unlinked_file::UnlinkedFile; | ||
67 | 62 | ||
68 | #[derive(Copy, Clone, Debug, PartialEq)] | 63 | #[derive(Copy, Clone, Debug, PartialEq)] |
69 | pub struct DiagnosticCode(pub &'static str); | 64 | pub struct DiagnosticCode(pub &'static str); |
@@ -123,6 +118,8 @@ impl Diagnostic { | |||
123 | #[derive(Debug, Copy, Clone)] | 118 | #[derive(Debug, Copy, Clone)] |
124 | pub enum Severity { | 119 | pub enum Severity { |
125 | Error, | 120 | Error, |
121 | // We don't actually emit this one yet, but we should at some point. | ||
122 | // Warning, | ||
126 | WeakWarning, | 123 | WeakWarning, |
127 | } | 124 | } |
128 | 125 | ||
@@ -157,21 +154,20 @@ pub fn diagnostics( | |||
157 | ); | 154 | ); |
158 | 155 | ||
159 | for node in parse.tree().syntax().descendants() { | 156 | for node in parse.tree().syntax().descendants() { |
160 | check_unnecessary_braces_in_use_statement(&mut res, file_id, &node); | 157 | handlers::useless_braces::useless_braces(&mut res, file_id, &node); |
161 | field_shorthand::check(&mut res, file_id, &node); | 158 | handlers::field_shorthand::field_shorthand(&mut res, file_id, &node); |
162 | } | 159 | } |
163 | 160 | ||
164 | let mut diags = Vec::new(); | ||
165 | let module = sema.to_module_def(file_id); | 161 | let module = sema.to_module_def(file_id); |
166 | if let Some(m) = module { | ||
167 | m.diagnostics(db, &mut diags) | ||
168 | } | ||
169 | 162 | ||
170 | let ctx = DiagnosticsContext { config, sema, resolve }; | 163 | let ctx = DiagnosticsContext { config, sema, resolve }; |
171 | if module.is_none() { | 164 | if module.is_none() { |
172 | let d = UnlinkedFile { file: file_id }; | 165 | handlers::unlinked_file::unlinked_file(&ctx, &mut res, file_id); |
173 | let d = handlers::unlinked_file::unlinked_file(&ctx, &d); | 166 | } |
174 | res.push(d) | 167 | |
168 | let mut diags = Vec::new(); | ||
169 | if let Some(m) = module { | ||
170 | m.diagnostics(db, &mut diags) | ||
175 | } | 171 | } |
176 | 172 | ||
177 | for diag in diags { | 173 | for diag in diags { |
@@ -211,61 +207,6 @@ pub fn diagnostics( | |||
211 | res | 207 | res |
212 | } | 208 | } |
213 | 209 | ||
214 | fn check_unnecessary_braces_in_use_statement( | ||
215 | acc: &mut Vec<Diagnostic>, | ||
216 | file_id: FileId, | ||
217 | node: &SyntaxNode, | ||
218 | ) -> Option<()> { | ||
219 | let use_tree_list = ast::UseTreeList::cast(node.clone())?; | ||
220 | if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { | ||
221 | // If there is a comment inside the bracketed `use`, | ||
222 | // assume it is a commented out module path and don't show diagnostic. | ||
223 | if use_tree_list.has_inner_comment() { | ||
224 | return Some(()); | ||
225 | } | ||
226 | |||
227 | let use_range = use_tree_list.syntax().text_range(); | ||
228 | let edit = | ||
229 | text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree) | ||
230 | .unwrap_or_else(|| { | ||
231 | let to_replace = single_use_tree.syntax().text().to_string(); | ||
232 | let mut edit_builder = TextEdit::builder(); | ||
233 | edit_builder.delete(use_range); | ||
234 | edit_builder.insert(use_range.start(), to_replace); | ||
235 | edit_builder.finish() | ||
236 | }); | ||
237 | |||
238 | acc.push( | ||
239 | Diagnostic::new( | ||
240 | "unnecessary-braces", | ||
241 | "Unnecessary braces in use statement".to_string(), | ||
242 | use_range, | ||
243 | ) | ||
244 | .severity(Severity::WeakWarning) | ||
245 | .with_fixes(Some(vec![fix( | ||
246 | "remove_braces", | ||
247 | "Remove unnecessary braces", | ||
248 | SourceChange::from_text_edit(file_id, edit), | ||
249 | use_range, | ||
250 | )])), | ||
251 | ); | ||
252 | } | ||
253 | |||
254 | Some(()) | ||
255 | } | ||
256 | |||
257 | fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement( | ||
258 | single_use_tree: &ast::UseTree, | ||
259 | ) -> Option<TextEdit> { | ||
260 | let use_tree_list_node = single_use_tree.syntax().parent()?; | ||
261 | if single_use_tree.path()?.segment()?.self_token().is_some() { | ||
262 | let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); | ||
263 | let end = use_tree_list_node.text_range().end(); | ||
264 | return Some(TextEdit::delete(TextRange::new(start, end))); | ||
265 | } | ||
266 | None | ||
267 | } | ||
268 | |||
269 | fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist { | 210 | fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist { |
270 | let mut res = unresolved_fix(id, label, target); | 211 | let mut res = unresolved_fix(id, label, target); |
271 | res.source_change = Some(source_change); | 212 | res.source_change = Some(source_change); |
@@ -294,7 +235,7 @@ mod tests { | |||
294 | use stdx::trim_indent; | 235 | use stdx::trim_indent; |
295 | use test_utils::{assert_eq_text, extract_annotations}; | 236 | use test_utils::{assert_eq_text, extract_annotations}; |
296 | 237 | ||
297 | use crate::DiagnosticsConfig; | 238 | use crate::{DiagnosticsConfig, Severity}; |
298 | 239 | ||
299 | /// Takes a multi-file input fixture with annotated cursor positions, | 240 | /// Takes a multi-file input fixture with annotated cursor positions, |
300 | /// and checks that: | 241 | /// and checks that: |
@@ -390,97 +331,29 @@ mod tests { | |||
390 | super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id); | 331 | super::diagnostics(&db, &config, &AssistResolveStrategy::All, file_id); |
391 | 332 | ||
392 | let expected = extract_annotations(&*db.file_text(file_id)); | 333 | let expected = extract_annotations(&*db.file_text(file_id)); |
393 | let mut actual = | 334 | let mut actual = diagnostics |
394 | diagnostics.into_iter().map(|d| (d.range, d.message)).collect::<Vec<_>>(); | 335 | .into_iter() |
336 | .map(|d| { | ||
337 | let mut annotation = String::new(); | ||
338 | if let Some(fixes) = &d.fixes { | ||
339 | assert!(!fixes.is_empty()); | ||
340 | annotation.push_str("💡 ") | ||
341 | } | ||
342 | annotation.push_str(match d.severity { | ||
343 | Severity::Error => "error", | ||
344 | Severity::WeakWarning => "weak", | ||
345 | }); | ||
346 | annotation.push_str(": "); | ||
347 | annotation.push_str(&d.message); | ||
348 | (d.range, annotation) | ||
349 | }) | ||
350 | .collect::<Vec<_>>(); | ||
395 | actual.sort_by_key(|(range, _)| range.start()); | 351 | actual.sort_by_key(|(range, _)| range.start()); |
396 | assert_eq!(expected, actual); | 352 | assert_eq!(expected, actual); |
397 | } | 353 | } |
398 | } | 354 | } |
399 | 355 | ||
400 | #[test] | 356 | #[test] |
401 | fn test_check_unnecessary_braces_in_use_statement() { | ||
402 | check_diagnostics( | ||
403 | r#" | ||
404 | use a; | ||
405 | use a::{c, d::e}; | ||
406 | |||
407 | mod a { | ||
408 | mod c {} | ||
409 | mod d { | ||
410 | mod e {} | ||
411 | } | ||
412 | } | ||
413 | "#, | ||
414 | ); | ||
415 | check_diagnostics( | ||
416 | r#" | ||
417 | use a; | ||
418 | use a::{ | ||
419 | c, | ||
420 | // d::e | ||
421 | }; | ||
422 | |||
423 | mod a { | ||
424 | mod c {} | ||
425 | mod d { | ||
426 | mod e {} | ||
427 | } | ||
428 | } | ||
429 | "#, | ||
430 | ); | ||
431 | check_fix( | ||
432 | r" | ||
433 | mod b {} | ||
434 | use {$0b}; | ||
435 | ", | ||
436 | r" | ||
437 | mod b {} | ||
438 | use b; | ||
439 | ", | ||
440 | ); | ||
441 | check_fix( | ||
442 | r" | ||
443 | mod b {} | ||
444 | use {b$0}; | ||
445 | ", | ||
446 | r" | ||
447 | mod b {} | ||
448 | use b; | ||
449 | ", | ||
450 | ); | ||
451 | check_fix( | ||
452 | r" | ||
453 | mod a { mod c {} } | ||
454 | use a::{c$0}; | ||
455 | ", | ||
456 | r" | ||
457 | mod a { mod c {} } | ||
458 | use a::c; | ||
459 | ", | ||
460 | ); | ||
461 | check_fix( | ||
462 | r" | ||
463 | mod a {} | ||
464 | use a::{self$0}; | ||
465 | ", | ||
466 | r" | ||
467 | mod a {} | ||
468 | use a; | ||
469 | ", | ||
470 | ); | ||
471 | check_fix( | ||
472 | r" | ||
473 | mod a { mod c {} mod d { mod e {} } } | ||
474 | use a::{c, d::{e$0}}; | ||
475 | ", | ||
476 | r" | ||
477 | mod a { mod c {} mod d { mod e {} } } | ||
478 | use a::{c, d::e}; | ||
479 | ", | ||
480 | ); | ||
481 | } | ||
482 | |||
483 | #[test] | ||
484 | fn test_disabled_diagnostics() { | 357 | fn test_disabled_diagnostics() { |
485 | let mut config = DiagnosticsConfig::default(); | 358 | let mut config = DiagnosticsConfig::default(); |
486 | config.disabled.insert("unresolved-module".into()); | 359 | config.disabled.insert("unresolved-module".into()); |
@@ -498,33 +371,4 @@ mod a { | |||
498 | ); | 371 | ); |
499 | assert!(!diagnostics.is_empty()); | 372 | assert!(!diagnostics.is_empty()); |
500 | } | 373 | } |
501 | |||
502 | #[test] | ||
503 | fn import_extern_crate_clash_with_inner_item() { | ||
504 | // This is more of a resolver test, but doesn't really work with the hir_def testsuite. | ||
505 | |||
506 | check_diagnostics( | ||
507 | r#" | ||
508 | //- /lib.rs crate:lib deps:jwt | ||
509 | mod permissions; | ||
510 | |||
511 | use permissions::jwt; | ||
512 | |||
513 | fn f() { | ||
514 | fn inner() {} | ||
515 | jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic | ||
516 | } | ||
517 | |||
518 | //- /permissions.rs | ||
519 | pub mod jwt { | ||
520 | pub struct Claims {} | ||
521 | } | ||
522 | |||
523 | //- /jwt/lib.rs crate:jwt | ||
524 | pub struct Claims { | ||
525 | field: u8, | ||
526 | } | ||
527 | "#, | ||
528 | ); | ||
529 | } | ||
530 | } | 374 | } |