diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 11a0ecb8b2..266b513dcf 100644 --- a/crates/ra_hir/src/diagnostics.rs +++ b/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, }; diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 545cff9bd1..84ba97b14a 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/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; 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 Result<(), ()> + 'a>>, + filters: Vec bool + 'a>>, default_callback: Box, } impl<'a> DiagnosticSink<'a> { - /// FIXME: split `new` and `on` into a separate builder type - pub fn new(cb: impl FnMut(&dyn Diagnostic) + 'a) -> DiagnosticSink<'a> { - DiagnosticSink { callbacks: Vec::new(), default_callback: Box::new(cb) } + pub fn push(&mut self, d: impl Diagnostic) { + let d: &dyn Diagnostic = &d; + self._push(d); } - pub fn on(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 Result<(), ()> + 'a>>, + filters: Vec bool + 'a>>, +} + +impl<'a> DiagnosticSinkBuilder<'a> { + pub fn new() -> Self { + Self { callbacks: Vec::new(), filters: Vec::new() } + } + + pub fn filter bool + 'a>(mut self, cb: F) -> Self { + self.filters.push(Box::new(cb)); + self + } + + pub fn on(mut self, mut cb: F) -> Self { let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { Some(d) => { cb(d); @@ -65,18 +100,11 @@ impl<'a> DiagnosticSink<'a> { self } - pub fn push(&mut self, d: impl Diagnostic) { - let d: &dyn Diagnostic = &d; - self._push(d); - } - - fn _push(&mut self, d: &dyn Diagnostic) { - for cb in self.callbacks.iter_mut() { - match cb(d) { - Ok(()) => return, - Err(()) => (), - } + pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { + DiagnosticSink { + callbacks: self.callbacks, + filters: self.filters, + default_callback: Box::new(default_callback), } - (self.default_callback)(d) } } diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index d3ee9cf556..885abbaf2d 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/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); } } diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index e029af0dc9..897177d05d 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/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 { +pub(crate) fn diagnostics( + db: &RootDatabase, + file_id: FileId, + enable_experimental: bool, +) -> Vec { 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 check_struct_shorthand_initialization(&mut res, file_id, &node); } let res = RefCell::new(res); - let mut sink = DiagnosticSink::new(|d| { - res.borrow_mut().push(Diagnostic { - message: d.message(), - range: sema.diagnostics_range(d).range, - severity: Severity::Error, - fix: None, + let mut sink = DiagnosticSinkBuilder::new() + .on::(|d| { + let original_file = d.source().file_id.original_file(db); + let fix = Fix::new( + "Create module", + 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::(|d| { - let original_file = d.source().file_id.original_file(db); - let fix = Fix::new( - "Create module", - 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::(|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); - } + .on::(|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 mut builder = TextEditBuilder::default(); - algo::diff(&d.ast(db).syntax(), &field_list.syntax()).into_text_edit(&mut builder); - builder.finish() + let edit = { + let mut builder = TextEditBuilder::default(); + algo::diff(&d.ast(db).syntax(), &field_list.syntax()) + .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 { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, + res.borrow_mut().push(Diagnostic { + range: sema.diagnostics_range(d).range, + message: d.message(), + severity: Severity::Error, + fix, + }) }) - }) - .on::(|d| { - let node = d.ast(db); - let replacement = format!("Ok({})", node.syntax()); - let edit = TextEdit::replace(node.syntax().text_range(), replacement); - let source_change = SourceFileEdit { file_id, edit }.into(); - let fix = Fix::new("Wrap with ok", source_change); - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: Some(fix), + .on::(|d| { + let node = d.ast(db); + let replacement = format!("Ok({})", node.syntax()); + let edit = TextEdit::replace(node.syntax().text_range(), replacement); + let source_change = SourceFileEdit { file_id, edit }.into(); + let fix = Fix::new("Wrap with ok", source_change); + res.borrow_mut().push(Diagnostic { + range: sema.diagnostics_range(d).range, + message: d.message(), + severity: Severity::Error, + fix: Some(fix), + }) }) - }) - .on::(|d| { - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d), + .on::(|d| { + res.borrow_mut().push(Diagnostic { + range: sema.diagnostics_range(d).range, + message: d.message(), + severity: Severity::Error, + 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::>(); 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) } diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 7356e947b9..4c4d9f6fa9 100644 --- a/crates/ra_ide/src/lib.rs +++ b/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> { - self.with_db(|db| diagnostics::diagnostics(db, file_id)) + pub fn diagnostics( + &self, + file_id: FileId, + enable_experimental: bool, + ) -> Cancelable> { + self.with_db(|db| diagnostics::diagnostics(db, file_id, enable_experimental)) } /// 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 9299879b75..076184ad6e 100644 --- a/crates/rust-analyzer/src/cli/analysis_bench.rs +++ b/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() { diff --git a/crates/rust-analyzer/src/cli/diagnostics.rs b/crates/rust-analyzer/src/cli/diagnostics.rs index 6f3c1c1f96..4ac8c8772e 100644 --- a/crates/rust-analyzer/src/cli/diagnostics.rs +++ b/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; } diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 8947ccf07e..e11c8b909a 100644 --- a/crates/rust-analyzer/src/config.rs +++ b/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, pub proc_macro_srv: Option<(PathBuf, Vec)>, @@ -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 = Vec::new(), diagnostics_warningsAsInfo: Vec = Vec::new(), diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index cad92c444e..cd309ed744 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/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 = 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), diff --git a/editors/code/package.json b/editors/code/package.json index 448e2269ff..658c913fdb 100644 --- a/editors/code/package.json +++ b/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,