Merge #5524

5524: Allow opting out of experimental diagnostics like MismatchedArgCount r=matklad a=jonas-schievink

Closes https://github.com/rust-analyzer/rust-analyzer/issues/5448
Closes https://github.com/rust-analyzer/rust-analyzer/issues/5419

Co-authored-by: Jonas Schievink <jonas.schievink@ferrous-systems.com>

crates/ra_hir/src/diagnostics.rs

@@ -1,6 +1,8 @@
 //! FIXME: write short doc here
 pub use hir_def::diagnostics::UnresolvedModule;
-pub use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
+pub use hir_expand::diagnostics::{
+    AstDiagnostic, Diagnostic, DiagnosticSink, DiagnosticSinkBuilder,
+};
 pub use hir_ty::diagnostics::{
     MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField,
 };

crates/ra_hir_expand/src/diagnostics.rs

@@ -24,6 +24,9 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
     fn message(&self) -> String;
     fn source(&self) -> InFile<SyntaxNodePtr>;
     fn as_any(&self) -> &(dyn Any + Send + 'static);
+    fn is_experimental(&self) -> bool {
+        false
+    }
 }
 
 pub trait AstDiagnostic {
@@ -44,16 +47,48 @@ impl dyn Diagnostic {
 
 pub struct DiagnosticSink<'a> {
     callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
+    filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
     default_callback: Box<dyn FnMut(&dyn Diagnostic) + 'a>,
 }
 
 impl<'a> DiagnosticSink<'a> {
-    /// FIXME: split `new` and `on` into a separate builder type
+    pub fn push(&mut self, d: impl Diagnostic) {
-    pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> {
+        let d: &dyn Diagnostic = &d;
-        DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) }
+        self._push(d);
     }
 
-    pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> DiagnosticSink<'a> {
+    fn _push(&mut self, d: &dyn Diagnostic) {
+        for filter in &mut self.filters {
+            if !filter(d) {
+                return;
+            }
+        }
+        for cb in &mut self.callbacks {
+            match cb(d) {
+                Ok(()) => return,
+                Err(()) => (),
+            }
+        }
+        (self.default_callback)(d)
+    }
+}
+
+pub struct DiagnosticSinkBuilder<'a> {
+    callbacks: Vec<Box<dyn FnMut(&dyn Diagnostic) -> Result<(), ()> + 'a>>,
+    filters: Vec<Box<dyn FnMut(&dyn Diagnostic) -> bool + 'a>>,
+}
+
+impl<'a> DiagnosticSinkBuilder<'a> {
+    pub fn new() -> Self {
+        Self { callbacks: Vec::new(), filters: Vec::new() }
+    }
+
+    pub fn filter<F: FnMut(&dyn Diagnostic) -> bool + 'a>(mut self, cb: F) -> Self {
+        self.filters.push(Box::new(cb));
+        self
+    }
+
+    pub fn on<D: Diagnostic, F: FnMut(&D) + 'a>(mut self, mut cb: F) -> Self {
         let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::<D>() {
             Some(d) => {
                 cb(d);
@@ -65,18 +100,11 @@ impl<'a> DiagnosticSink<'a> {
         self
     }
 
-    pub fn push(&mut self, d: impl Diagnostic) {
+    pub fn build<F: FnMut(&dyn Diagnostic) + 'a>(self, default_callback: F) -> DiagnosticSink<'a> {
-        let d: &dyn Diagnostic = &d;
+        DiagnosticSink {
-        self._push(d);
+            callbacks: self.callbacks,
-    }
+            filters: self.filters,
-
+            default_callback: Box::new(default_callback),
-    fn _push(&mut self, d: &dyn Diagnostic) {
-        for cb in self.callbacks.iter_mut() {
-            match cb(d) {
-                Ok(()) => return,
-                Err(()) => (),
-            }
         }
-        (self.default_callback)(d)
     }
 }

crates/ra_hir_ty/src/diagnostics.rs

@@ -234,6 +234,9 @@ impl Diagnostic for MismatchedArgCount {
     fn as_any(&self) -> &(dyn Any + Send + 'static) {
         self
     }
+    fn is_experimental(&self) -> bool {
+        true
+    }
 }
 
 impl AstDiagnostic for MismatchedArgCount {
@@ -248,7 +251,7 @@ impl AstDiagnostic for MismatchedArgCount {
 #[cfg(test)]
 mod tests {
     use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId};
-    use hir_expand::diagnostics::{Diagnostic, DiagnosticSink};
+    use hir_expand::diagnostics::{Diagnostic, DiagnosticSinkBuilder};
     use ra_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt};
     use ra_syntax::{TextRange, TextSize};
     use rustc_hash::FxHashMap;
@@ -280,7 +283,7 @@ mod tests {
                 }
 
                 for f in fns {
-                    let mut sink = DiagnosticSink::new(&mut cb);
+                    let mut sink = DiagnosticSinkBuilder::new().build(&mut cb);
                     validate_body(self, f.into(), &mut sink);
                 }
             }

crates/ra_ide/src/diagnostics.rs

@@ -7,7 +7,7 @@
 use std::cell::RefCell;
 
 use hir::{
-    diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSink},
+    diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSinkBuilder},
     HasSource, HirDisplay, Semantics, VariantDef,
 };
 use itertools::Itertools;
@@ -29,7 +29,11 @@ pub enum Severity {
     WeakWarning,
 }
 
-pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic> {
+pub(crate) fn diagnostics(
+    db: &RootDatabase,
+    file_id: FileId,
+    enable_experimental: bool,
+) -> Vec<Diagnostic> {
     let _p = profile("diagnostics");
     let sema = Semantics::new(db);
     let parse = db.parse(file_id);
@@ -48,79 +52,85 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec<Diagnostic>
         check_struct_shorthand_initialization(&mut res, file_id, &node);
     }
     let res = RefCell::new(res);
-    let mut sink = DiagnosticSink::new(|d| {
+    let mut sink = DiagnosticSinkBuilder::new()
-        res.borrow_mut().push(Diagnostic {
+        .on::<hir::diagnostics::UnresolvedModule, _>(|d| {
-            message: d.message(),
+            let original_file = d.source().file_id.original_file(db);
-            range: sema.diagnostics_range(d).range,
+            let fix = Fix::new(
-            severity: Severity::Error,
+                "Create module",
-            fix: None,
+                FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() }
+                    .into(),
+            );
+            res.borrow_mut().push(Diagnostic {
+                range: sema.diagnostics_range(d).range,
+                message: d.message(),
+                severity: Severity::Error,
+                fix: Some(fix),
+            })
         })
-    })
+        .on::<hir::diagnostics::MissingFields, _>(|d| {
-    .on::<hir::diagnostics::UnresolvedModule, _>(|d| {
+            // Note that although we could add a diagnostics to
-        let original_file = d.source().file_id.original_file(db);
+            // fill the missing tuple field, e.g :
-        let fix = Fix::new(
+            // `struct A(usize);`
-            "Create module",
+            // `let a = A { 0: () }`
-            FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() }.into(),
+            // but it is uncommon usage and it should not be encouraged.
-        );
+            let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
-        res.borrow_mut().push(Diagnostic {
+                None
-            range: sema.diagnostics_range(d).range,
+            } else {
-            message: d.message(),
+                let mut field_list = d.ast(db);
-            severity: Severity::Error,
+                for f in d.missed_fields.iter() {
-            fix: Some(fix),
+                    let field =
-        })
+                        make::record_field(make::name_ref(&f.to_string()), Some(make::expr_unit()));
-    })
+                    field_list = field_list.append_field(&field);
-    .on::<hir::diagnostics::MissingFields, _>(|d| {
+                }
-        // Note that although we could add a diagnostics to
-        // fill the missing tuple field, e.g :
-        // `struct A(usize);`
-        // `let a = A { 0: () }`
-        // but it is uncommon usage and it should not be encouraged.
-        let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
-            None
-        } else {
-            let mut field_list = d.ast(db);
-            for f in d.missed_fields.iter() {
-                let field =
-                    make::record_field(make::name_ref(&f.to_string()), Some(make::expr_unit()));
-                field_list = field_list.append_field(&field);
-            }
 
-            let edit = {
+                let edit = {
-                let mut builder = TextEditBuilder::default();
+                    let mut builder = TextEditBuilder::default();
-                algo::diff(&d.ast(db).syntax(), &field_list.syntax()).into_text_edit(&mut builder);
+                    algo::diff(&d.ast(db).syntax(), &field_list.syntax())
-                builder.finish()
+                        .into_text_edit(&mut builder);
+                    builder.finish()
+                };
+                Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()))
             };
-            Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()))
-        };
 
-        res.borrow_mut().push(Diagnostic {
+            res.borrow_mut().push(Diagnostic {
-            range: sema.diagnostics_range(d).range,
+                range: sema.diagnostics_range(d).range,
-            message: d.message(),
+                message: d.message(),
-            severity: Severity::Error,
+                severity: Severity::Error,
-            fix,
+                fix,
+            })
         })
-    })
+        .on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| {
-    .on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| {
+            let node = d.ast(db);
-        let node = d.ast(db);
+            let replacement = format!("Ok({})", node.syntax());
-        let replacement = format!("Ok({})", node.syntax());
+            let edit = TextEdit::replace(node.syntax().text_range(), replacement);
-        let edit = TextEdit::replace(node.syntax().text_range(), replacement);
+            let source_change = SourceFileEdit { file_id, edit }.into();
-        let source_change = SourceFileEdit { file_id, edit }.into();
+            let fix = Fix::new("Wrap with ok", source_change);
-        let fix = Fix::new("Wrap with ok", source_change);
+            res.borrow_mut().push(Diagnostic {
-        res.borrow_mut().push(Diagnostic {
+                range: sema.diagnostics_range(d).range,
-            range: sema.diagnostics_range(d).range,
+                message: d.message(),
-            message: d.message(),
+                severity: Severity::Error,
-            severity: Severity::Error,
+                fix: Some(fix),
-            fix: Some(fix),
+            })
         })
-    })
+        .on::<hir::diagnostics::NoSuchField, _>(|d| {
-    .on::<hir::diagnostics::NoSuchField, _>(|d| {
+            res.borrow_mut().push(Diagnostic {
-        res.borrow_mut().push(Diagnostic {
+                range: sema.diagnostics_range(d).range,
-            range: sema.diagnostics_range(d).range,
+                message: d.message(),
-            message: d.message(),
+                severity: Severity::Error,
-            severity: Severity::Error,
+                fix: missing_struct_field_fix(&sema, file_id, d),
-            fix: missing_struct_field_fix(&sema, file_id, d),
+            })
         })
-    });
+        // Only collect experimental diagnostics when they're enabled.
+        .filter(|diag| !diag.is_experimental() || enable_experimental)
+        // Diagnostics not handled above get no fix and default treatment.
+        .build(|d| {
+            res.borrow_mut().push(Diagnostic {
+                message: d.message(),
+                range: sema.diagnostics_range(d).range,
+                severity: Severity::Error,
+                fix: None,
+            })
+        });
 
     if let Some(m) = sema.to_module_def(file_id) {
         m.diagnostics(db, &mut sink);
@@ -298,7 +308,7 @@ 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).unwrap().pop().unwrap();
+        let diagnostic = analysis.diagnostics(file_position.file_id, true).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();
@@ -324,7 +334,7 @@ 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).unwrap().pop().unwrap();
+        let diagnostic = analysis.diagnostics(current_file_id, true).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;
@@ -345,14 +355,14 @@ mod tests {
         let analysis = mock.analysis();
         let diagnostics = files
             .into_iter()
-            .flat_map(|file_id| analysis.diagnostics(file_id).unwrap())
+            .flat_map(|file_id| analysis.diagnostics(file_id, true).unwrap())
             .collect::<Vec<_>>();
         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).unwrap();
+        let diagnostics = analysis.diagnostics(file_id, true).unwrap();
         expect.assert_debug_eq(&diagnostics)
     }
 

crates/ra_ide/src/lib.rs

@@ -487,8 +487,12 @@ impl Analysis {
     }
 
     /// Computes the set of diagnostics for the given file.
-    pub fn diagnostics(&self, file_id: FileId) -> Cancelable<Vec<Diagnostic>> {
+    pub fn diagnostics(
-        self.with_db(|db| diagnostics::diagnostics(db, file_id))
+        &self,
+        file_id: FileId,
+        enable_experimental: bool,
+    ) -> Cancelable<Vec<Diagnostic>> {
+        self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental))
     }
 
     /// Returns the edit required to rename reference at the position to the new

crates/rust-analyzer/src/cli/analysis_bench.rs

@@ -70,7 +70,7 @@ pub fn analysis_bench(
     match &what {
         BenchWhat::Highlight { .. } => {
             let res = do_work(&mut host, file_id, |analysis| {
-                analysis.diagnostics(file_id).unwrap();
+                analysis.diagnostics(file_id, true).unwrap();
                 analysis.highlight_as_html(file_id, false).unwrap()
             });
             if verbosity.is_verbose() {

crates/rust-analyzer/src/cli/diagnostics.rs

@@ -47,7 +47,7 @@ pub fn diagnostics(
                 String::from("unknown")
             };
             println!("processing crate: {}, module: {}", crate_name, _vfs.file_path(file_id));
-            for diagnostic in analysis.diagnostics(file_id).unwrap() {
+            for diagnostic in analysis.diagnostics(file_id, true).unwrap() {
                 if matches!(diagnostic.severity, Severity::Error) {
                     found_error = true;
                 }

crates/rust-analyzer/src/config.rs

@@ -23,6 +23,7 @@ pub struct Config {
     pub client_caps: ClientCapsConfig,
 
     pub publish_diagnostics: bool,
+    pub experimental_diagnostics: bool,
     pub diagnostics: DiagnosticsConfig,
     pub lru_capacity: Option<usize>,
     pub proc_macro_srv: Option<(PathBuf, Vec<OsString>)>,
@@ -137,6 +138,7 @@ impl Config {
 
             with_sysroot: true,
             publish_diagnostics: true,
+            experimental_diagnostics: true,
             diagnostics: DiagnosticsConfig::default(),
             lru_capacity: None,
             proc_macro_srv: None,
@@ -187,6 +189,7 @@ impl Config {
 
         self.with_sysroot = data.withSysroot;
         self.publish_diagnostics = data.diagnostics_enable;
+        self.experimental_diagnostics = data.diagnostics_enableExperimental;
         self.diagnostics = DiagnosticsConfig {
             warnings_as_info: data.diagnostics_warningsAsInfo,
             warnings_as_hint: data.diagnostics_warningsAsHint,
@@ -405,6 +408,7 @@ config_data! {
         completion_postfix_enable: bool          = true,
 
         diagnostics_enable: bool                = true,
+        diagnostics_enableExperimental: bool    = true,
         diagnostics_warningsAsHint: Vec<String> = Vec::new(),
         diagnostics_warningsAsInfo: Vec<String> = Vec::new(),
 

crates/rust-analyzer/src/handlers.rs

@@ -774,7 +774,7 @@ fn handle_fixes(
         None => {}
     };
 
-    let diagnostics = snap.analysis.diagnostics(file_id)?;
+    let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?;
 
     let fixes_from_diagnostics = diagnostics
         .into_iter()
@@ -1040,7 +1040,7 @@ pub(crate) fn publish_diagnostics(
     let line_index = snap.analysis.file_line_index(file_id)?;
     let diagnostics: Vec<Diagnostic> = snap
         .analysis
-        .diagnostics(file_id)?
+        .diagnostics(file_id, snap.config.experimental_diagnostics)?
         .into_iter()
         .map(|d| Diagnostic {
             range: to_proto::range(&line_index, d.range),

editors/code/package.json

@@ -200,11 +200,6 @@
             "type": "object",
             "title": "Rust Analyzer",
             "properties": {
-                "rust-analyzer.diagnostics.enable": {
-                    "type": "boolean",
-                    "default": true,
-                    "markdownDescription": "Whether to show native rust-analyzer diagnostics."
-                },
                 "rust-analyzer.lruCapacity": {
                     "type": [
                         "null",
@@ -579,6 +574,16 @@
                     "type": "boolean",
                     "default": true
                 },
+                "rust-analyzer.diagnostics.enable": {
+                    "type": "boolean",
+                    "default": true,
+                    "markdownDescription": "Whether to show native rust-analyzer diagnostics."
+                },
+                "rust-analyzer.diagnostics.enableExperimental": {
+                    "type": "boolean",
+                    "default": true,
+                    "markdownDescription": "Whether to show experimental rust-analyzer diagnostics that might have more false positives than usual."
+                },
                 "rust-analyzer.diagnostics.warningsAsInfo": {
                     "type": "array",
                     "uniqueItems": true,