minor: make diagnostics more similar

crates/ide_diagnostics/src/handlers/field_shorthand.rs

@@ -7,7 +7,7 @@ use text_edit::TextEdit;
 
 use crate::{fix, Diagnostic, Severity};
 
-pub(super) fn check(acc: &mut Vec<Diagnostic>, file_id: FileId, node: &SyntaxNode) {
+pub(crate) fn field_shorthand(acc: &mut Vec<Diagnostic>, file_id: FileId, node: &SyntaxNode) {
     match_ast! {
         match node {
             ast::RecordExpr(it) => check_expr_field_shorthand(acc, file_id, it),

crates/ide_diagnostics/src/handlers/missing_fields.rs

@@ -323,4 +323,33 @@ fn f() {
 "#,
         );
     }
+
+    #[test]
+    fn import_extern_crate_clash_with_inner_item() {
+        // This is more of a resolver test, but doesn't really work with the hir_def testsuite.
+
+        check_diagnostics(
+            r#"
+//- /lib.rs crate:lib deps:jwt
+mod permissions;
+
+use permissions::jwt;
+
+fn f() {
+    fn inner() {}
+    jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic
+}
+
+//- /permissions.rs
+pub mod jwt  {
+    pub struct Claims {}
+}
+
+//- /jwt/lib.rs crate:jwt
+pub struct Claims {
+    field: u8,
+}
+        "#,
+        );
+    }
 }

crates/ide_diagnostics/src/handlers/unlinked_file.rs

@@ -14,32 +14,29 @@ use text_edit::TextEdit;
 
 use crate::{fix, Assist, Diagnostic, DiagnosticsContext};
 
-#[derive(Debug)]
-pub(crate) struct UnlinkedFile {
-    pub(crate) file: FileId,
-}
-
 // Diagnostic: unlinked-file
 //
 // This diagnostic is shown for files that are not included in any crate, or files that are part of
 // crates rust-analyzer failed to discover. The file will not have IDE features available.
-pub(crate) fn unlinked_file(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Diagnostic {
+pub(crate) fn unlinked_file(ctx: &DiagnosticsContext, acc: &mut Vec<Diagnostic>, file_id: FileId) {
     // Limit diagnostic to the first few characters in the file. This matches how VS Code
     // renders it with the full span, but on other editors, and is less invasive.
-    let range = ctx.sema.db.parse(d.file).syntax_node().text_range();
+    let range = ctx.sema.db.parse(file_id).syntax_node().text_range();
     // FIXME: This is wrong if one of the first three characters is not ascii: `//Ы`.
     let range = range.intersect(TextRange::up_to(TextSize::of("..."))).unwrap_or(range);
 
+    acc.push(
         Diagnostic::new("unlinked-file", "file not included in module tree", range)
-        .with_fixes(fixes(ctx, d))
+            .with_fixes(fixes(ctx, file_id)),
+    );
 }
 
-fn fixes(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Option<Vec<Assist>> {
+fn fixes(ctx: &DiagnosticsContext, file_id: FileId) -> Option<Vec<Assist>> {
     // If there's an existing module that could add `mod` or `pub mod` items to include the unlinked file,
     // suggest that as a fix.
 
-    let source_root = ctx.sema.db.source_root(ctx.sema.db.file_source_root(d.file));
+    let source_root = ctx.sema.db.source_root(ctx.sema.db.file_source_root(file_id));
-    let our_path = source_root.path_for_file(&d.file)?;
+    let our_path = source_root.path_for_file(&file_id)?;
     let module_name = our_path.name_and_extension()?.0;
 
     // Candidates to look for:
@@ -68,7 +65,7 @@ fn fixes(ctx: &DiagnosticsContext, d: &UnlinkedFile) -> Option<Vec<Assist>> {
                     }
 
                     if module.origin.file_id() == Some(*parent_id) {
-                        return make_fixes(ctx.sema.db, *parent_id, module_name, d.file);
+                        return make_fixes(ctx.sema.db, *parent_id, module_name, file_id);
                     }
                 }
             }

crates/ide_diagnostics/src/handlers/useless_braces.rs

@@ -0,0 +1,148 @@
+use ide_db::{base_db::FileId, source_change::SourceChange};
+use itertools::Itertools;
+use syntax::{ast, AstNode, SyntaxNode, TextRange};
+use text_edit::TextEdit;
+
+use crate::{fix, Diagnostic, Severity};
+
+// Diagnostic: unnecessary-braces
+//
+// Diagnostic for unnecessary braces in `use` items.
+pub(crate) fn useless_braces(
+    acc: &mut Vec<Diagnostic>,
+    file_id: FileId,
+    node: &SyntaxNode,
+) -> Option<()> {
+    let use_tree_list = ast::UseTreeList::cast(node.clone())?;
+    if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() {
+        // If there is a comment inside the bracketed `use`,
+        // assume it is a commented out module path and don't show diagnostic.
+        if use_tree_list.has_inner_comment() {
+            return Some(());
+        }
+
+        let use_range = use_tree_list.syntax().text_range();
+        let edit = remove_braces(&single_use_tree).unwrap_or_else(|| {
+            let to_replace = single_use_tree.syntax().text().to_string();
+            let mut edit_builder = TextEdit::builder();
+            edit_builder.delete(use_range);
+            edit_builder.insert(use_range.start(), to_replace);
+            edit_builder.finish()
+        });
+
+        acc.push(
+            Diagnostic::new(
+                "unnecessary-braces",
+                "Unnecessary braces in use statement".to_string(),
+                use_range,
+            )
+            .severity(Severity::WeakWarning)
+            .with_fixes(Some(vec![fix(
+                "remove_braces",
+                "Remove unnecessary braces",
+                SourceChange::from_text_edit(file_id, edit),
+                use_range,
+            )])),
+        );
+    }
+
+    Some(())
+}
+
+fn remove_braces(single_use_tree: &ast::UseTree) -> Option<TextEdit> {
+    let use_tree_list_node = single_use_tree.syntax().parent()?;
+    if single_use_tree.path()?.segment()?.self_token().is_some() {
+        let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start();
+        let end = use_tree_list_node.text_range().end();
+        return Some(TextEdit::delete(TextRange::new(start, end)));
+    }
+    None
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::tests::{check_diagnostics, check_fix};
+
+    #[test]
+    fn test_check_unnecessary_braces_in_use_statement() {
+        check_diagnostics(
+            r#"
+use a;
+use a::{c, d::e};
+
+mod a {
+    mod c {}
+    mod d {
+        mod e {}
+    }
+}
+"#,
+        );
+        check_diagnostics(
+            r#"
+use a;
+use a::{
+    c,
+    // d::e
+};
+
+mod a {
+    mod c {}
+    mod d {
+        mod e {}
+    }
+}
+"#,
+        );
+        check_fix(
+            r#"
+mod b {}
+use {$0b};
+"#,
+            r#"
+mod b {}
+use b;
+"#,
+        );
+        check_fix(
+            r#"
+mod b {}
+use {b$0};
+"#,
+            r#"
+mod b {}
+use b;
+"#,
+        );
+        check_fix(
+            r#"
+mod a { mod c {} }
+use a::{c$0};
+"#,
+            r#"
+mod a { mod c {} }
+use a::c;
+"#,
+        );
+        check_fix(
+            r#"
+mod a {}
+use a::{self$0};
+"#,
+            r#"
+mod a {}
+use a;
+"#,
+        );
+        check_fix(
+            r#"
+mod a { mod c {} mod d { mod e {} } }
+use a::{c, d::{e$0}};
+"#,
+            r#"
+mod a { mod c {} mod d { mod e {} } }
+use a::{c, d::e};
+"#,
+        );
+    }
+}

crates/ide_diagnostics/src/lib.rs

@@ -37,15 +37,17 @@ mod handlers {
     pub(crate) mod remove_this_semicolon;
     pub(crate) mod replace_filter_map_next_with_find_map;
     pub(crate) mod unimplemented_builtin_macro;
-    pub(crate) mod unlinked_file;
     pub(crate) mod unresolved_extern_crate;
     pub(crate) mod unresolved_import;
     pub(crate) mod unresolved_macro_call;
     pub(crate) mod unresolved_module;
     pub(crate) mod unresolved_proc_macro;
-}
 
-mod field_shorthand;
+    // The handlers bellow are unusual, the implement the diagnostics as well.
+    pub(crate) mod field_shorthand;
+    pub(crate) mod useless_braces;
+    pub(crate) mod unlinked_file;
+}
 
 use hir::{diagnostics::AnyDiagnostic, Semantics};
 use ide_db::{
@@ -55,15 +57,8 @@ use ide_db::{
     source_change::SourceChange,
     RootDatabase,
 };
-use itertools::Itertools;
 use rustc_hash::FxHashSet;
-use syntax::{
+use syntax::{ast::AstNode, TextRange};
-    ast::{self, AstNode},
-    SyntaxNode, TextRange,
-};
-use text_edit::TextEdit;
-
-use crate::handlers::unlinked_file::UnlinkedFile;
 
 #[derive(Copy, Clone, Debug, PartialEq)]
 pub struct DiagnosticCode(pub &'static str);
@@ -123,6 +118,8 @@ impl Diagnostic {
 #[derive(Debug, Copy, Clone)]
 pub enum Severity {
     Error,
+    // We don't actually emit this one yet, but we should at some point.
+    // Warning,
     WeakWarning,
 }
 
@@ -157,21 +154,20 @@ pub fn diagnostics(
     );
 
     for node in parse.tree().syntax().descendants() {
-        check_unnecessary_braces_in_use_statement(&mut res, file_id, &node);
+        handlers::useless_braces::useless_braces(&mut res, file_id, &node);
-        field_shorthand::check(&mut res, file_id, &node);
+        handlers::field_shorthand::field_shorthand(&mut res, file_id, &node);
     }
 
-    let mut diags = Vec::new();
     let module = sema.to_module_def(file_id);
-    if let Some(m) = module {
-        m.diagnostics(db, &mut diags)
-    }
 
     let ctx = DiagnosticsContext { config, sema, resolve };
     if module.is_none() {
-        let d = UnlinkedFile { file: file_id };
+        handlers::unlinked_file::unlinked_file(&ctx, &mut res, file_id);
-        let d = handlers::unlinked_file::unlinked_file(&ctx, &d);
+    }
-        res.push(d)
+
+    let mut diags = Vec::new();
+    if let Some(m) = module {
+        m.diagnostics(db, &mut diags)
     }
 
     for diag in diags {
@@ -211,61 +207,6 @@ pub fn diagnostics(
     res
 }
 
-fn check_unnecessary_braces_in_use_statement(
-    acc: &mut Vec<Diagnostic>,
-    file_id: FileId,
-    node: &SyntaxNode,
-) -> Option<()> {
-    let use_tree_list = ast::UseTreeList::cast(node.clone())?;
-    if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() {
-        // If there is a comment inside the bracketed `use`,
-        // assume it is a commented out module path and don't show diagnostic.
-        if use_tree_list.has_inner_comment() {
-            return Some(());
-        }
-
-        let use_range = use_tree_list.syntax().text_range();
-        let edit =
-            text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree)
-                .unwrap_or_else(|| {
-                    let to_replace = single_use_tree.syntax().text().to_string();
-                    let mut edit_builder = TextEdit::builder();
-                    edit_builder.delete(use_range);
-                    edit_builder.insert(use_range.start(), to_replace);
-                    edit_builder.finish()
-                });
-
-        acc.push(
-            Diagnostic::new(
-                "unnecessary-braces",
-                "Unnecessary braces in use statement".to_string(),
-                use_range,
-            )
-            .severity(Severity::WeakWarning)
-            .with_fixes(Some(vec![fix(
-                "remove_braces",
-                "Remove unnecessary braces",
-                SourceChange::from_text_edit(file_id, edit),
-                use_range,
-            )])),
-        );
-    }
-
-    Some(())
-}
-
-fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
-    single_use_tree: &ast::UseTree,
-) -> Option<TextEdit> {
-    let use_tree_list_node = single_use_tree.syntax().parent()?;
-    if single_use_tree.path()?.segment()?.self_token().is_some() {
-        let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start();
-        let end = use_tree_list_node.text_range().end();
-        return Some(TextEdit::delete(TextRange::new(start, end)));
-    }
-    None
-}
-
 fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist {
     let mut res = unresolved_fix(id, label, target);
     res.source_change = Some(source_change);
@@ -397,89 +338,6 @@ mod tests {
         }
     }
 
-    #[test]
-    fn test_check_unnecessary_braces_in_use_statement() {
-        check_diagnostics(
-            r#"
-use a;
-use a::{c, d::e};
-
-mod a {
-    mod c {}
-    mod d {
-        mod e {}
-    }
-}
-"#,
-        );
-        check_diagnostics(
-            r#"
-use a;
-use a::{
-    c,
-    // d::e
-};
-
-mod a {
-    mod c {}
-    mod d {
-        mod e {}
-    }
-}
-"#,
-        );
-        check_fix(
-            r"
-            mod b {}
-            use {$0b};
-            ",
-            r"
-            mod b {}
-            use b;
-            ",
-        );
-        check_fix(
-            r"
-            mod b {}
-            use {b$0};
-            ",
-            r"
-            mod b {}
-            use b;
-            ",
-        );
-        check_fix(
-            r"
-            mod a { mod c {} }
-            use a::{c$0};
-            ",
-            r"
-            mod a { mod c {} }
-            use a::c;
-            ",
-        );
-        check_fix(
-            r"
-            mod a {}
-            use a::{self$0};
-            ",
-            r"
-            mod a {}
-            use a;
-            ",
-        );
-        check_fix(
-            r"
-            mod a { mod c {} mod d { mod e {} } }
-            use a::{c, d::{e$0}};
-            ",
-            r"
-            mod a { mod c {} mod d { mod e {} } }
-            use a::{c, d::e};
-            ",
-        );
-    }
-
     #[test]
     fn test_disabled_diagnostics() {
         let mut config = DiagnosticsConfig::default();
@@ -498,33 +356,4 @@ mod a {
         );
         assert!(!diagnostics.is_empty());
     }
-
-    #[test]
-    fn import_extern_crate_clash_with_inner_item() {
-        // This is more of a resolver test, but doesn't really work with the hir_def testsuite.
-
-        check_diagnostics(
-            r#"
-//- /lib.rs crate:lib deps:jwt
-mod permissions;
-
-use permissions::jwt;
-
-fn f() {
-    fn inner() {}
-    jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic
-}
-
-//- /permissions.rs
-pub mod jwt  {
-    pub struct Claims {}
-}
-
-//- /jwt/lib.rs crate:jwt
-pub struct Claims {
-    field: u8,
-}
-        "#,
-        );
-    }
 }