Merge #5796

5796: Align diagnostics config with the rest of rust-analyzer
 r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <[email protected]>

9 files changed, 84 insertions, 89 deletions

diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs
index 606a6064b..89ff55490 100644
--- a/crates/ide/src/diagnostics.rs
+++ b/crates/ide/src/diagnostics.rs
@@ -4,12 +4,15 @@
 //! 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 diagnostics_with_fix;
+
+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,
@@ -18,8 +21,7 @@ use text_edit::TextEdit;
 
 use crate::{Diagnostic, FileId, Fix, SourceFileEdit};
 
-mod diagnostics_with_fix;
-use diagnostics_with_fix::DiagnosticWithFix;
+use self::diagnostics_with_fix::DiagnosticWithFix;
 
 #[derive(Debug, Copy, Clone)]
 pub enum Severity {
@@ -27,11 +29,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);
@@ -52,7 +59,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,12 +73,8 @@ 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.name()));
 
     // Finalize the `DiagnosticSink` building process.
     let mut sink = sink_builder
@@ -187,12 +190,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 +208,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 +238,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,7 +263,9 @@ mod tests {
         let analysis = mock.analysis();
         let diagnostics = files
             .into_iter()
-            .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap())
+            .flat_map(|file_id| {
+                analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap()
+            })
             .collect::<Vec<_>>();
         assert_eq!(diagnostics.len(), 0, "unexpected diagnostics:\n{:#?}", diagnostics);
     }
@@ -259,8 +273,8 @@ mod tests {
     /// 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 mut config = DiagnosticsConfig::default();
+        config.disabled = 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<_>>();
@@ -269,15 +283,17 @@ mod tests {
         let diagnostics = files
             .clone()
             .into_iter()
-            .flat_map(|file_id| {
-                analysis.diagnostics(file_id, true, Some(disabled_diagnostics.clone())).unwrap()
-            })
+            .flat_map(|file_id| analysis.diagnostics(&config, 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);
+                assert!(
+                    !disabled_diagnostics.contains(&name.as_str()),
+                    "Diagnostic {} is disabled",
+                    name
+                );
             }
         }
 
@@ -288,21 +304,23 @@ mod tests {
         // will no longer exist.
         let diagnostics = files
             .into_iter()
-            .flat_map(|file_id| analysis.diagnostics(file_id, true, None).unwrap())
+            .flat_map(|file_id| {
+                analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap()
+            })
             .collect::<Vec<_>>();
 
         assert!(
             diagnostics
                 .into_iter()
                 .filter_map(|diag| diag.name)
-                .any(|name| disabled_diagnostics.contains(&name)),
+                .any(|name| disabled_diagnostics.contains(&name.as_str())),
             "At least one of the diagnostics was not emitted even without config; are the diagnostics names correct?"
         );
     }
 
     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)
     }
 
diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs
index 4b797f374..2a73abba2 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::{DiagnosticsConfig, Severity},
     display::NavigationTarget,
     expand_macro::ExpandedMacro,
     file_structure::StructureNode,
@@ -148,7 +148,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 +495,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
diff --git a/crates/rust-analyzer/src/cli/analysis_bench.rs b/crates/rust-analyzer/src/cli/analysis_bench.rs
index 43f0196af..c312e0a2e 100644
--- a/crates/rust-analyzer/src/cli/analysis_bench.rs
+++ b/crates/rust-analyzer/src/cli/analysis_bench.rs
@@ -7,7 +7,10 @@ use base_db::{
     salsa::{Database, Durability},
     FileId,
 };
-use ide::{Analysis, AnalysisChange, AnalysisHost, CompletionConfig, FilePosition, LineCol};
+use ide::{
+    Analysis, AnalysisChange, AnalysisHost, CompletionConfig, DiagnosticsConfig, FilePosition,
+    LineCol,
+};
 use vfs::AbsPathBuf;
 
 use crate::{
@@ -71,7 +74,7 @@ impl BenchCmd {
         match &self.what {
             BenchWhat::Highlight { .. } => {
                 let res = do_work(&mut host, file_id, |analysis| {
-                    analysis.diagnostics(file_id, true, None).unwrap();
+                    analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap();
                     analysis.highlight_as_html(file_id, false).unwrap()
                 });
                 if verbosity.is_verbose() {
diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs
index 31eb7ff3f..c424aa6e2 100644
--- a/crates/rust-analyzer/src/cli/diagnostics.rs
+++ b/crates/rust-analyzer/src/cli/diagnostics.rs
@@ -8,7 +8,7 @@ use rustc_hash::FxHashSet;
 
 use base_db::SourceDatabaseExt;
 use hir::Crate;
-use ide::Severity;
+use ide::{DiagnosticsConfig, Severity};
 
 use crate::cli::{load_cargo::load_cargo, Result};
 
@@ -47,7 +47,8 @@ pub fn diagnostics(
                 String::from("unknown")
             };
             println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id));
-            for diagnostic in analysis.diagnostics(file_id, true, None).unwrap() {
+            for diagnostic in analysis.diagnostics(&DiagnosticsConfig::default(), file_id).unwrap()
+            {
                 if matches!(diagnostic.severity, Severity::Error) {
                     found_error = true;
                 }
diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs
index 44fd7c286..99f7751ac 100644
--- a/crates/rust-analyzer/src/config.rs
+++ b/crates/rust-analyzer/src/config.rs
@@ -7,24 +7,25 @@
 //! configure the server itself, feature flags are passed into analysis, and
 //! tweak things like automatic insertion of `()` in completions.
 
-use std::{collections::HashSet, ffi::OsString, path::PathBuf};
+use std::{ffi::OsString, path::PathBuf};
 
 use flycheck::FlycheckConfig;
-use ide::{AssistConfig, CompletionConfig, HoverConfig, InlayHintsConfig};
+use ide::{AssistConfig, CompletionConfig, DiagnosticsConfig, HoverConfig, InlayHintsConfig};
 use lsp_types::ClientCapabilities;
 use project_model::{CargoConfig, ProjectJson, ProjectJsonData, ProjectManifest};
+use rustc_hash::FxHashSet;
 use serde::Deserialize;
 use vfs::AbsPathBuf;
 
-use crate::diagnostics::DiagnosticsConfig;
+use crate::diagnostics::DiagnosticsMapConfig;
 
 #[derive(Debug, Clone)]
 pub struct Config {
     pub client_caps: ClientCapsConfig,
 
     pub publish_diagnostics: bool,
-    pub experimental_diagnostics: bool,
     pub diagnostics: DiagnosticsConfig,
+    pub diagnostics_map: DiagnosticsMapConfig,
     pub lru_capacity: Option<usize>,
     pub proc_macro_srv: Option<(PathBuf, Vec<OsString>)>,
     pub files: FilesConfig,
@@ -45,14 +46,6 @@ pub struct Config {
     pub with_sysroot: bool,
     pub linked_projects: Vec<LinkedProject>,
     pub root_path: AbsPathBuf,
-
-    pub analysis: AnalysisConfig,
-}
-
-/// Configuration parameters for the analysis run.
-#[derive(Debug, Default, Clone)]
-pub struct AnalysisConfig {
-    pub disabled_diagnostics: HashSet<String>,
 }
 
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -146,8 +139,8 @@ impl Config {
 
             with_sysroot: true,
             publish_diagnostics: true,
-            experimental_diagnostics: true,
             diagnostics: DiagnosticsConfig::default(),
+            diagnostics_map: DiagnosticsMapConfig::default(),
             lru_capacity: None,
             proc_macro_srv: None,
             files: FilesConfig { watcher: FilesWatcher::Notify, exclude: Vec::new() },
@@ -184,8 +177,6 @@ impl Config {
             hover: HoverConfig::default(),
             linked_projects: Vec::new(),
             root_path,
-
-            analysis: AnalysisConfig::default(),
         }
     }
 
@@ -200,8 +191,11 @@ impl Config {
 
         self.with_sysroot = data.withSysroot;
         self.publish_diagnostics = data.diagnostics_enable;
-        self.experimental_diagnostics = data.diagnostics_enableExperimental;
         self.diagnostics = DiagnosticsConfig {
+            disable_experimental: !data.diagnostics_enableExperimental,
+            disabled: data.diagnostics_disabled,
+        };
+        self.diagnostics_map = DiagnosticsMapConfig {
             warnings_as_info: data.diagnostics_warningsAsInfo,
             warnings_as_hint: data.diagnostics_warningsAsHint,
         };
@@ -303,8 +297,6 @@ impl Config {
             goto_type_def: data.hoverActions_enable && data.hoverActions_gotoTypeDef,
         };
 
-        self.analysis = AnalysisConfig { disabled_diagnostics: data.analysis_disabledDiagnostics };
-
         log::info!("Config::update() = {:#?}", self);
     }
 
@@ -369,14 +361,6 @@ impl Config {
             self.client_caps.status_notification = get_bool("statusNotification");
         }
     }
-
-    pub fn disabled_diagnostics(&self) -> Option<HashSet<String>> {
-        if self.analysis.disabled_diagnostics.is_empty() {
-            None
-        } else {
-            Some(self.analysis.disabled_diagnostics.clone())
-        }
-    }
 }
 
 #[derive(Deserialize)]
@@ -434,6 +418,7 @@ config_data! {
 
         diagnostics_enable: bool                = true,
         diagnostics_enableExperimental: bool    = true,
+        diagnostics_disabled: FxHashSet<String> = FxHashSet::default(),
         diagnostics_warningsAsHint: Vec<String> = Vec::new(),
         diagnostics_warningsAsInfo: Vec<String> = Vec::new(),
 
@@ -464,7 +449,5 @@ config_data! {
         rustfmt_overrideCommand: Option<Vec<String>> = None,
 
         withSysroot: bool = true,
-
-        analysis_disabledDiagnostics: HashSet<String> = HashSet::new(),
     }
 }
diff --git a/crates/rust-analyzer/src/diagnostics.rs b/crates/rust-analyzer/src/diagnostics.rs
index 108df3eb0..ee6f2a867 100644
--- a/crates/rust-analyzer/src/diagnostics.rs
+++ b/crates/rust-analyzer/src/diagnostics.rs
@@ -11,7 +11,7 @@ use crate::lsp_ext;
 pub(crate) type CheckFixes = Arc<FxHashMap<FileId, Vec<Fix>>>;
 
 #[derive(Debug, Default, Clone)]
-pub struct DiagnosticsConfig {
+pub struct DiagnosticsMapConfig {
     pub warnings_as_info: Vec<String>,
     pub warnings_as_hint: Vec<String>,
 }
diff --git a/crates/rust-analyzer/src/diagnostics/to_proto.rs b/crates/rust-analyzer/src/diagnostics/to_proto.rs
index 6d5408156..df5583897 100644
--- a/crates/rust-analyzer/src/diagnostics/to_proto.rs
+++ b/crates/rust-analyzer/src/diagnostics/to_proto.rs
@@ -7,11 +7,11 @@ use stdx::format_to;
 
 use crate::{lsp_ext, to_proto::url_from_abs_path};
 
-use super::DiagnosticsConfig;
+use super::DiagnosticsMapConfig;
 
 /// Determines the LSP severity from a diagnostic
 fn diagnostic_severity(
-    config: &DiagnosticsConfig,
+    config: &DiagnosticsMapConfig,
     level: flycheck::DiagnosticLevel,
     code: Option<flycheck::DiagnosticCode>,
 ) -> Option<lsp_types::DiagnosticSeverity> {
@@ -141,7 +141,7 @@ pub(crate) struct MappedRustDiagnostic {
 ///
 /// If the diagnostic has no primary span this will return `None`
 pub(crate) fn map_rust_diagnostic_to_lsp(
-    config: &DiagnosticsConfig,
+    config: &DiagnosticsMapConfig,
     rd: &flycheck::Diagnostic,
     workspace_root: &Path,
 ) -> Vec<MappedRustDiagnostic> {
@@ -259,10 +259,10 @@ mod tests {
     use expect::{expect_file, ExpectFile};
 
     fn check(diagnostics_json: &str, expect: ExpectFile) {
-        check_with_config(DiagnosticsConfig::default(), diagnostics_json, expect)
+        check_with_config(DiagnosticsMapConfig::default(), diagnostics_json, expect)
     }
 
-    fn check_with_config(config: DiagnosticsConfig, diagnostics_json: &str, expect: ExpectFile) {
+    fn check_with_config(config: DiagnosticsMapConfig, diagnostics_json: &str, expect: ExpectFile) {
         let diagnostic: flycheck::Diagnostic = serde_json::from_str(diagnostics_json).unwrap();
         let workspace_root = Path::new("/test/");
         let actual = map_rust_diagnostic_to_lsp(&config, &diagnostic, workspace_root);
@@ -402,9 +402,9 @@ mod tests {
     #[cfg(not(windows))]
     fn rustc_unused_variable_as_info() {
         check_with_config(
-            DiagnosticsConfig {
+            DiagnosticsMapConfig {
                 warnings_as_info: vec!["unused_variables".to_string()],
-                ..DiagnosticsConfig::default()
+                ..DiagnosticsMapConfig::default()
             },
             r##"{
     "message": "unused variable: `foo`",
@@ -486,9 +486,9 @@ mod tests {
     #[cfg(not(windows))]
     fn rustc_unused_variable_as_hint() {
         check_with_config(
-            DiagnosticsConfig {
+            DiagnosticsMapConfig {
                 warnings_as_hint: vec!["unused_variables".to_string()],
-                ..DiagnosticsConfig::default()
+                ..DiagnosticsMapConfig::default()
             },
             r##"{
     "message": "unused variable: `foo`",
diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs
index 6510e8ccc..67ca9f055 100644
--- a/crates/rust-analyzer/src/handlers.rs
+++ b/crates/rust-analyzer/src/handlers.rs
@@ -775,11 +775,7 @@ fn handle_fixes(
         None => {}
     };
 
-    let diagnostics = snap.analysis.diagnostics(
-        file_id,
-        snap.config.experimental_diagnostics,
-        snap.config.disabled_diagnostics(),
-    )?;
+    let diagnostics = snap.analysis.diagnostics(&snap.config.diagnostics, file_id)?;
 
     for fix in diagnostics
         .into_iter()
@@ -1051,13 +1047,10 @@ pub(crate) fn publish_diagnostics(
 ) -> Result<Vec<Diagnostic>> {
     let _p = profile::span("publish_diagnostics");
     let line_index = snap.analysis.file_line_index(file_id)?;
+
     let diagnostics: Vec<Diagnostic> = snap
         .analysis
-        .diagnostics(
-            file_id,
-            snap.config.experimental_diagnostics,
-            snap.config.disabled_diagnostics(),
-        )?
+        .diagnostics(&snap.config.diagnostics, file_id)?
         .into_iter()
         .map(|d| Diagnostic {
             range: to_proto::range(&line_index, d.range),
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 66e04653a..f039cdc31 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -248,7 +248,7 @@ impl GlobalState {
             Event::Flycheck(task) => match task {
                 flycheck::Message::AddDiagnostic { workspace_root, diagnostic } => {
                     let diagnostics = crate::diagnostics::to_proto::map_rust_diagnostic_to_lsp(
-                        &self.config.diagnostics,
+                        &self.config.diagnostics_map,
                         &diagnostic,
                         &workspace_root,
                     );