From 6f02befee4249618a2a7858d27649fa389888ea8 Mon Sep 17 00:00:00 2001 From: Jonas Schievink Date: Fri, 24 Jul 2020 16:30:12 +0200 Subject: [PATCH] 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 | 137 ++++++++++++------------ 4 files changed, 101 insertions(+), 87 deletions(-) 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..6a5844f31f 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 d3ee9cf556..a9877d867f 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 e029af0dc9..8e715faa46 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); - } + .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), + }) }) - }); + .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);