From 6f02befee4249618a2a7858d27649fa389888ea8 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jul 2020 16:30:12 +0200 Subject: Add a builder for DiagnosticSink --- crates/ra_hir/src/diagnostics.rs | 4 +- crates/ra_hir_expand/src/diagnostics.rs | 43 ++++++---- crates/ra_hir_ty/src/diagnostics.rs | 4 +- crates/ra_ide/src/diagnostics.rs | 139 ++++++++++++++++---------------- 4 files changed, 102 insertions(+), 88 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs index 11a0ecb8b..266b513dc 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 545cff9bd..6a5844f31 100644 --- a/crates/ra_hir_expand/src/diagnostics.rs +++ b/crates/ra_hir_expand/src/diagnostics.rs @@ -48,23 +48,6 @@ pub struct DiagnosticSink<'a> { } 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 on(mut self, mut cb: F) -> DiagnosticSink<'a> { - let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { - Some(d) => { - cb(d); - Ok(()) - } - None => Err(()), - }; - self.callbacks.push(Box::new(cb)); - self - } - pub fn push(&mut self, d: impl Diagnostic) { let d: &dyn Diagnostic = &d; self._push(d); @@ -80,3 +63,29 @@ impl<'a> DiagnosticSink<'a> { (self.default_callback)(d) } } + +pub struct DiagnosticSinkBuilder<'a> { + callbacks: Vec Result<(), ()> + 'a>>, +} + +impl<'a> DiagnosticSinkBuilder<'a> { + pub fn new() -> Self { + Self { callbacks: Vec::new() } + } + + pub fn on(mut self, mut cb: F) -> Self { + let cb = move |diag: &dyn Diagnostic| match diag.downcast_ref::() { + Some(d) => { + cb(d); + Ok(()) + } + None => Err(()), + }; + self.callbacks.push(Box::new(cb)); + self + } + + pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { + DiagnosticSink { callbacks: self.callbacks, default_callback: Box::new(default_callback) } + } +} diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index d3ee9cf55..a9877d867 100644 --- a/crates/ra_hir_ty/src/diagnostics.rs +++ b/crates/ra_hir_ty/src/diagnostics.rs @@ -248,7 +248,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 +280,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 e029af0dc..8e715faa4 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; @@ -48,79 +48,82 @@ 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); - } - - let edit = { - let mut builder = TextEditBuilder::default(); - algo::diff(&d.ast(db).syntax(), &field_list.syntax()).into_text_edit(&mut builder); - builder.finish() + .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() + }; + 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), + }) }) - }); + .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); -- cgit v1.2.3 From 76509b3d2dde6770aa98d798ba8f50512825f266 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jul 2020 17:38:18 +0200 Subject: Allow filtering diagnostics --- crates/ra_hir_expand/src/diagnostics.rs | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs index 6a5844f31..84ba97b14 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,6 +47,7 @@ impl dyn Diagnostic { pub struct DiagnosticSink<'a> { callbacks: Vec Result<(), ()> + 'a>>, + filters: Vec bool + 'a>>, default_callback: Box, } @@ -54,7 +58,12 @@ impl<'a> DiagnosticSink<'a> { } fn _push(&mut self, d: &dyn Diagnostic) { - for cb in self.callbacks.iter_mut() { + for filter in &mut self.filters { + if !filter(d) { + return; + } + } + for cb in &mut self.callbacks { match cb(d) { Ok(()) => return, Err(()) => (), @@ -66,11 +75,17 @@ impl<'a> DiagnosticSink<'a> { pub struct DiagnosticSinkBuilder<'a> { callbacks: Vec Result<(), ()> + 'a>>, + filters: Vec bool + 'a>>, } impl<'a> DiagnosticSinkBuilder<'a> { pub fn new() -> Self { - Self { callbacks: Vec::new() } + 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 { @@ -86,6 +101,10 @@ impl<'a> DiagnosticSinkBuilder<'a> { } pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { - DiagnosticSink { callbacks: self.callbacks, default_callback: Box::new(default_callback) } + DiagnosticSink { + callbacks: self.callbacks, + filters: self.filters, + default_callback: Box::new(default_callback), + } } } -- cgit v1.2.3 From bec5123b0a688ad8d05531838cb9443ceac7ced3 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jul 2020 17:38:33 +0200 Subject: Mark MismatchedArgCount as experimental --- crates/ra_hir_ty/src/diagnostics.rs | 3 +++ 1 file changed, 3 insertions(+) (limited to 'crates') diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs index a9877d867..885abbaf2 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 { -- cgit v1.2.3 From f6f49735e8070789a970cfd534695a5f2be33624 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jul 2020 17:39:16 +0200 Subject: Add toggle for experimental diagnostics --- crates/rust-analyzer/src/config.rs | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'crates') diff --git a/crates/rust-analyzer/src/config.rs b/crates/rust-analyzer/src/config.rs index 8947ccf07..e11c8b909 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(), -- cgit v1.2.3 From 92a4ec80a0ce1dd834578f53ea3fd018530ec0e6 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jul 2020 17:39:44 +0200 Subject: Only display experimental diagnostics when enabled --- crates/ra_ide/src/diagnostics.rs | 17 ++++++++++++----- crates/ra_ide/src/lib.rs | 8 ++++++-- crates/rust-analyzer/src/cli/analysis_bench.rs | 2 +- crates/rust-analyzer/src/cli/diagnostics.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 4 ++-- 5 files changed, 22 insertions(+), 11 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 8e715faa4..897177d05 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -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); @@ -116,6 +120,9 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec 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(), @@ -301,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(); @@ -327,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; @@ -348,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 7356e947b..4c4d9f6fa 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 9299879b7..076184ad6 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 6f3c1c1f9..4ac8c8772 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/handlers.rs b/crates/rust-analyzer/src/handlers.rs index cad92c444..cd309ed74 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), -- cgit v1.2.3