3 files changed, 86 insertions, 113 deletions

diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index 606a6064b..92b5adaa2 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -4,22 +4,48 @@
 //! macro-expanded files, but we need to present them to the users in terms of
 //! original files. So we need to map the ranges.
 
-use std::{cell::RefCell, collections::HashSet};
+mod fixes;
+
+use std::cell::RefCell;
 
 use base_db::SourceDatabase;
 use hir::{diagnostics::DiagnosticSinkBuilder, Semantics};
 use ide_db::RootDatabase;
 use itertools::Itertools;
+use rustc_hash::FxHashSet;
 use syntax::{
     ast::{self, AstNode},
     SyntaxNode, TextRange, T,
 };
 use text_edit::TextEdit;
 
-use crate::{Diagnostic, FileId, Fix, SourceFileEdit};
+use crate::{FileId, Label, SourceChange, SourceFileEdit};
+
+use self::fixes::DiagnosticWithFix;
+
+#[derive(Debug)]
+pub struct Diagnostic {
+    // pub name: Option<String>,
+    pub message: String,
+    pub range: TextRange,
+    pub severity: Severity,
+    pub fix: Option<Fix>,
+}
 
-mod diagnostics_with_fix;
-use diagnostics_with_fix::DiagnosticWithFix;
+#[derive(Debug)]
+pub struct Fix {
+    pub label: Label,
+    pub source_change: SourceChange,
+    /// Allows to trigger the fix only when the caret is in the range given
+    pub fix_trigger_range: TextRange,
+}
+
+impl Fix {
+    fn new(label: &str, source_change: SourceChange, fix_trigger_range: TextRange) -> Self {
+        let label = Label::new(label);
+        Self { label, source_change, fix_trigger_range }
+    }
+}
 
 #[derive(Debug, Copy, Clone)]
 pub enum Severity {
@@ -27,11 +53,16 @@ pub enum Severity {
     WeakWarning,
 }
 
+#[derive(Default, Debug, Clone)]
+pub struct DiagnosticsConfig {
+    pub disable_experimental: bool,
+    pub disabled: FxHashSet<String>,
+}
+
 pub(crate) fn diagnostics(
     db: &RootDatabase,
+    config: &DiagnosticsConfig,
     file_id: FileId,
-    enable_experimental: bool,
-    disabled_diagnostics: Option<HashSet<String>>,
 ) -> Vec<Diagnostic> {
     let _p = profile::span("diagnostics");
     let sema = Semantics::new(db);
@@ -40,7 +71,7 @@ pub(crate) fn diagnostics(
 
     // [#34344] Only take first 128 errors to prevent slowing down editor/ide, the number 128 is chosen arbitrarily.
     res.extend(parse.errors().iter().take(128).map(|err| Diagnostic {
-        name: None,
+        // name: None,
         range: err.range(),
         message: format!("Syntax Error: {}", err),
         severity: Severity::Error,
@@ -52,7 +83,7 @@ pub(crate) fn diagnostics(
         check_struct_shorthand_initialization(&mut res, file_id, &node);
     }
     let res = RefCell::new(res);
-    let mut sink_builder = DiagnosticSinkBuilder::new()
+    let sink_builder = DiagnosticSinkBuilder::new()
         .on::<hir::diagnostics::UnresolvedModule, _>(|d| {
             res.borrow_mut().push(diagnostic_with_fix(d, &sema));
         })
@@ -66,19 +97,15 @@ pub(crate) fn diagnostics(
             res.borrow_mut().push(diagnostic_with_fix(d, &sema));
         })
         // Only collect experimental diagnostics when they're enabled.
-        .filter(|diag| !diag.is_experimental() || enable_experimental);
-
-    if let Some(disabled_diagnostics) = disabled_diagnostics {
-        // Do not collect disabled diagnostics.
-        sink_builder = sink_builder.filter(move |diag| !disabled_diagnostics.contains(diag.name()));
-    }
+        .filter(|diag| !(diag.is_experimental() && config.disable_experimental))
+        .filter(|diag| !config.disabled.contains(diag.code().as_str()));
 
     // Finalize the `DiagnosticSink` building process.
     let mut sink = sink_builder
         // Diagnostics not handled above get no fix and default treatment.
         .build(|d| {
             res.borrow_mut().push(Diagnostic {
-                name: Some(d.name().into()),
+                // name: Some(d.name().into()),
                 message: d.message(),
                 range: sema.diagnostics_display_range(d).range,
                 severity: Severity::Error,
@@ -95,7 +122,7 @@ pub(crate) fn diagnostics(
 
 fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic {
     Diagnostic {
-        name: Some(d.name().into()),
+        // name: Some(d.name().into()),
         range: sema.diagnostics_display_range(d).range,
         message: d.message(),
         severity: Severity::Error,
@@ -122,7 +149,7 @@ fn check_unnecessary_braces_in_use_statement(
                 });
 
         acc.push(Diagnostic {
-            name: None,
+            // name: None,
             range: use_range,
             message: "Unnecessary braces in use statement".to_string(),
             severity: Severity::WeakWarning,
@@ -169,7 +196,7 @@ fn check_struct_shorthand_initialization(
 
                 let field_range = record_field.syntax().text_range();
                 acc.push(Diagnostic {
-                    name: None,
+                    // name: None,
                     range: field_range,
                     message: "Shorthand struct initialization".to_string(),
                     severity: Severity::WeakWarning,
@@ -187,12 +214,14 @@ fn check_struct_shorthand_initialization(
 
 #[cfg(test)]
 mod tests {
-    use std::collections::HashSet;
+    use expect::{expect, Expect};
     use stdx::trim_indent;
     use test_utils::assert_eq_text;
 
-    use crate::mock_analysis::{analysis_and_position, single_file, MockAnalysis};
-    use expect::{expect, Expect};
+    use crate::{
+        mock_analysis::{analysis_and_position, single_file, MockAnalysis},
+        DiagnosticsConfig,
+    };
 
     /// Takes a multi-file input fixture with annotated cursor positions,
     /// and checks that:
@@ -203,8 +232,11 @@ mod tests {
         let after = trim_indent(ra_fixture_after);
 
         let (analysis, file_position) = analysis_and_position(ra_fixture_before);
-        let diagnostic =
-            analysis.diagnostics(file_position.file_id, true, None).unwrap().pop().unwrap();
+        let diagnostic = analysis
+            .diagnostics(&DiagnosticsConfig::default(), file_position.file_id)
+            .unwrap()
+            .pop()
+            .unwrap();
         let mut fix = diagnostic.fix.unwrap();
         let edit = fix.source_change.source_file_edits.pop().unwrap().edit;
         let target_file_contents = analysis.file_text(file_position.file_id).unwrap();
@@ -230,7 +262,11 @@ mod tests {
         let ra_fixture_after = &trim_indent(ra_fixture_after);
         let (analysis, file_pos) = analysis_and_position(ra_fixture_before);
         let current_file_id = file_pos.file_id;
-        let diagnostic = analysis.diagnostics(current_file_id, true, None).unwrap().pop().unwrap();
+        let diagnostic = analysis
+            .diagnostics(&DiagnosticsConfig::default(), current_file_id)
+            .unwrap()
+            .pop()
+            .unwrap();
         let mut fix = diagnostic.fix.unwrap();
         let edit = fix.source_change.source_file_edits.pop().unwrap();
         let changed_file_id = edit.file_id;
@@ -251,58 +287,16 @@ mod tests {
         let analysis = mock.analysis();
         let diagnostics = files
             .into_iter()
-            .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap())
-            .collect::<Vec<_>>();
-        assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics);
-    }
-
-    /// Takes a multi-file input fixture with annotated cursor position and the list of disabled diagnostics,
-    /// and checks that provided diagnostics aren't spawned during analysis.
-    fn check_disabled_diagnostics(ra_fixture: &str, disabled_diagnostics: &[&'static str]) {
-        let disabled_diagnostics: HashSet<_> =
-            disabled_diagnostics.into_iter().map(|diag| diag.to_string()).collect();
-
-        let mock = MockAnalysis::with_files(ra_fixture);
-        let files = mock.files().map(|(it, _)| it).collect::<Vec<_>>();
-        let analysis = mock.analysis();
-
-        let diagnostics = files
-            .clone()
-            .into_iter()
             .flat_map(|file_id| {
-                analysis.diagnostics(file_id, true, Some(disabled_diagnostics.clone())).unwrap()
+                analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap()
             })
             .collect::<Vec<_>>();
-
-        // First, we have to check that diagnostic is not emitted when it's added to the disabled diagnostics list.
-        for diagnostic in diagnostics {
-            if let Some(name) = diagnostic.name {
-                assert!(!disabled_diagnostics.contains(&name), "Diagnostic {} is disabled", name);
-            }
-        }
-
-        // Then, we must reset the config and repeat the check, so that we'll be sure that without
-        // config these diagnostics are emitted.
-        // This is required for tests to not become outdated if e.g. diagnostics name changes:
-        // without this additional run the test will pass simply because a diagnostic with an old name
-        // will no longer exist.
-        let diagnostics = files
-            .into_iter()
-            .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap())
-            .collect::<Vec<_>>();
-
-        assert!(
-            diagnostics
-                .into_iter()
-                .filter_map(|diag| diag.name)
-                .any(|name| disabled_diagnostics.contains(&name)),
-            "At least one of the diagnostics was not emitted even without config; are the diagnostics names correct?"
-        );
+        assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics);
     }
 
     fn check_expect(ra_fixture: &str, expect: Expect) {
         let (analysis, file_id) = single_file(ra_fixture);
-        let diagnostics = analysis.diagnostics(file_id, true, None).unwrap();
+        let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap();
         expect.assert_debug_eq(&diagnostics)
     }
 
@@ -562,9 +556,6 @@ fn test_fn() {
             expect![[r#"
                 [
                     Diagnostic {
-                        name: Some(
-                            "unresolved-module",
-                        ),
                         message: "unresolved module",
                         range: 0..8,
                         severity: Error,
@@ -741,6 +732,15 @@ struct Foo {
 
     #[test]
     fn test_disabled_diagnostics() {
-        check_disabled_diagnostics(r#"mod foo;"#, &["unresolved-module"]);
+        let mut config = DiagnosticsConfig::default();
+        config.disabled.insert("unresolved-module".into());
+
+        let (analysis, file_id) = single_file(r#"mod foo;"#);
+
+        let diagnostics = analysis.diagnostics(&config, file_id).unwrap();
+        assert!(diagnostics.is_empty());
+
+        let diagnostics = analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap();
+        assert!(!diagnostics.is_empty());
     }
 }
diff --git a/crates/ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ide/src/diagnostics/fixes.rs
index 85b46c995..68ae1c239 100644
--- a/crates/ide/src/diagnostics/diagnostics_with_fix.rs
+++ b/crates/ide/src/diagnostics/fixes.rs
@@ -1,7 +1,5 @@
 //! Provides a way to attach fixes to the diagnostics.
 //! The same module also has all curret custom fixes for the diagnostics implemented.
-use crate::Fix;
-use ast::{edit::IndentLevel, make};
 use base_db::FileId;
 use hir::{
     db::AstDatabase,
@@ -12,9 +10,15 @@ use ide_db::{
     source_change::{FileSystemEdit, SourceFileEdit},
     RootDatabase,
 };
-use syntax::{algo, ast, AstNode};
+use syntax::{
+    algo,
+    ast::{self, edit::IndentLevel, make},
+    AstNode,
+};
 use text_edit::TextEdit;
 
+use crate::diagnostics::Fix;
+
 /// A [Diagnostic] that potentially has a fix available.
 ///
 /// [Diagnostic]: hir::diagnostics::Diagnostic
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index 4b797f374..e3af6d5bc 100644
--- a/crates/ide/src/lib.rs
+++ b/crates/ide/src/lib.rs
@@ -44,7 +44,7 @@ mod syntax_highlighting;
 mod syntax_tree;
 mod typing;
 
-use std::{collections::HashSet, sync::Arc};
+use std::sync::Arc;
 
 use base_db::{
     salsa::{self, ParallelDatabase},
@@ -65,7 +65,7 @@ pub use crate::{
     completion::{
         CompletionConfig, CompletionItem, CompletionItemKind, CompletionScore, InsertTextFormat,
     },
-    diagnostics::Severity,
+    diagnostics::{Diagnostic, DiagnosticsConfig, Fix, Severity},
     display::NavigationTarget,
     expand_macro::ExpandedMacro,
     file_structure::StructureNode,
@@ -88,6 +88,7 @@ pub use base_db::{
 pub use hir::{Documentation, Semantics};
 pub use ide_db::{
     change::AnalysisChange,
+    label::Label,
     line_index::{LineCol, LineIndex},
     search::SearchScope,
     source_change::{FileSystemEdit, SourceChange, SourceFileEdit},
@@ -99,35 +100,6 @@ pub use text_edit::{Indel, TextEdit};
 
 pub type Cancelable<T> = Result<T, Canceled>;
 
-#[derive(Debug)]
-pub struct Diagnostic {
-    pub name: Option<String>,
-    pub message: String,
-    pub range: TextRange,
-    pub severity: Severity,
-    pub fix: Option<Fix>,
-}
-
-#[derive(Debug)]
-pub struct Fix {
-    pub label: String,
-    pub source_change: SourceChange,
-    /// Allows to trigger the fix only when the caret is in the range given
-    pub fix_trigger_range: TextRange,
-}
-
-impl Fix {
-    pub fn new(
-        label: impl Into<String>,
-        source_change: SourceChange,
-        fix_trigger_range: TextRange,
-    ) -> Self {
-        let label = label.into();
-        assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.'));
-        Self { label, source_change, fix_trigger_range }
-    }
-}
-
 /// Info associated with a text range.
 #[derive(Debug)]
 pub struct RangeInfo<T> {
@@ -148,7 +120,7 @@ pub struct AnalysisHost {
 }
 
 impl AnalysisHost {
-    pub fn new(lru_capacity: Option<usize>) -> Self {
+    pub fn new(lru_capacity: Option<usize>) -> AnalysisHost {
         AnalysisHost { db: RootDatabase::new(lru_capacity) }
     }
 
@@ -495,13 +467,10 @@ impl Analysis {
     /// Computes the set of diagnostics for the given file.
     pub fn diagnostics(
         &self,
+        config: &DiagnosticsConfig,
         file_id: FileId,
-        enable_experimental: bool,
-        disabled_diagnostics: Option<HashSet<String>>,
     ) -> Cancelable<Vec<Diagnostic>> {
-        self.with_db(|db| {
-            diagnostics::diagnostics(db, file_id, enable_experimental, disabled_diagnostics)
-        })
+        self.with_db(|db| diagnostics::diagnostics(db, config, file_id))
     }
 
     /// Returns the edit required to rename reference at the position to the new