6 files changed, 78 insertions, 74 deletions

diff --git a/crates/ra_hir/src/diagnostics.rs b/crates/ra_hir/src/diagnostics.rs
index 266b513dc..564f6a5db 100644
--- a/crates/ra_hir/src/diagnostics.rs
+++ b/crates/ra_hir/src/diagnostics.rs
@@ -1,7 +1,7 @@
 //! FIXME: write short doc here
 pub use hir_def::diagnostics::UnresolvedModule;
 pub use hir_expand::diagnostics::{
-    AstDiagnostic, Diagnostic, DiagnosticSink, DiagnosticSinkBuilder,
+    Diagnostic, DiagnosticSink, DiagnosticSinkBuilder, DiagnosticWithFix,
 };
 pub use hir_ty::diagnostics::{
     MismatchedArgCount, MissingFields, MissingMatchArms, MissingOkInTailExpr, NoSuchField,
diff --git a/crates/ra_hir/src/semantics.rs b/crates/ra_hir/src/semantics.rs
index b4420d378..c5bc2baff 100644
--- a/crates/ra_hir/src/semantics.rs
+++ b/crates/ra_hir/src/semantics.rs
@@ -8,7 +8,7 @@ use hir_def::{
     resolver::{self, HasResolver, Resolver},
     AsMacroCall, FunctionId, TraitId, VariantId,
 };
-use hir_expand::{diagnostics::AstDiagnostic, hygiene::Hygiene, name::AsName, ExpansionInfo};
+use hir_expand::{diagnostics::DiagnosticWithFix, hygiene::Hygiene, name::AsName, ExpansionInfo};
 use hir_ty::associated_type_shorthand_candidates;
 use itertools::Itertools;
 use ra_db::{FileId, FileRange};
@@ -109,12 +109,12 @@ impl<'db, DB: HirDatabase> Semantics<'db, DB> {
         self.imp.parse(file_id)
     }
 
-    pub fn diagnostic_fix_source<T: AstDiagnostic + Diagnostic>(
+    pub fn diagnostic_fix_source<T: DiagnosticWithFix + Diagnostic>(
         &self,
         d: &T,
-    ) -> <T as AstDiagnostic>::AST {
+    ) -> Option<<T as DiagnosticWithFix>::AST> {
         let file_id = d.presentation().file_id;
-        let root = self.db.parse_or_expand(file_id).unwrap();
+        let root = self.db.parse_or_expand(file_id)?;
         self.imp.cache(root, file_id);
         d.fix_source(self.db.upcast())
     }
diff --git a/crates/ra_hir_def/src/diagnostics.rs b/crates/ra_hir_def/src/diagnostics.rs
index be9612846..033be683c 100644
--- a/crates/ra_hir_def/src/diagnostics.rs
+++ b/crates/ra_hir_def/src/diagnostics.rs
@@ -2,7 +2,7 @@
 
 use std::any::Any;
 
-use hir_expand::diagnostics::{AstDiagnostic, Diagnostic};
+use hir_expand::diagnostics::{Diagnostic, DiagnosticWithFix};
 use ra_syntax::{ast, AstPtr, SyntaxNodePtr};
 
 use hir_expand::{HirFileId, InFile};
@@ -26,10 +26,10 @@ impl Diagnostic for UnresolvedModule {
     }
 }
 
-impl AstDiagnostic for UnresolvedModule {
+impl DiagnosticWithFix for UnresolvedModule {
     type AST = ast::Module;
-    fn fix_source(&self, db: &dyn hir_expand::db::AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.file).unwrap();
-        self.decl.to_node(&root)
+    fn fix_source(&self, db: &dyn hir_expand::db::AstDatabase) -> Option<Self::AST> {
+        let root = db.parse_or_expand(self.file)?;
+        Some(self.decl.to_node(&root))
     }
 }
diff --git a/crates/ra_hir_expand/src/diagnostics.rs b/crates/ra_hir_expand/src/diagnostics.rs
index 2b74473ce..62a09a73a 100644
--- a/crates/ra_hir_expand/src/diagnostics.rs
+++ b/crates/ra_hir_expand/src/diagnostics.rs
@@ -29,15 +29,9 @@ pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static {
     }
 }
 
-pub trait AstDiagnostic {
+pub trait DiagnosticWithFix {
     type AST;
-    fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST;
-}
-
-impl dyn Diagnostic {
-    pub fn downcast_ref<D: Diagnostic>(&self) -> Option<&D> {
-        self.as_any().downcast_ref()
-    }
+    fn fix_source(&self, db: &dyn AstDatabase) -> Option<Self::AST>;
 }
 
 pub struct DiagnosticSink<'a> {
@@ -83,12 +77,9 @@ impl<'a> DiagnosticSinkBuilder<'a> {
         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);
-                Ok(())
-            }
+    pub fn on<D: Diagnostic, F: FnMut(&D) -> Option<()> + 'a>(mut self, mut cb: F) -> Self {
+        let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::<D>() {
+            Some(d) => cb(d).ok_or(()),
             None => Err(()),
         };
         self.callbacks.push(Box::new(cb));
diff --git a/crates/ra_hir_ty/src/diagnostics.rs b/crates/ra_hir_ty/src/diagnostics.rs
index 1e3a44637..b34ba5bfc 100644
--- a/crates/ra_hir_ty/src/diagnostics.rs
+++ b/crates/ra_hir_ty/src/diagnostics.rs
@@ -6,7 +6,7 @@ mod unsafe_check;
 use std::any::Any;
 
 use hir_def::DefWithBodyId;
-use hir_expand::diagnostics::{AstDiagnostic, Diagnostic, DiagnosticSink};
+use hir_expand::diagnostics::{Diagnostic, DiagnosticSink, DiagnosticWithFix};
 use hir_expand::{db::AstDatabase, name::Name, HirFileId, InFile};
 use ra_prof::profile;
 use ra_syntax::{ast, AstPtr, SyntaxNodePtr};
@@ -46,12 +46,12 @@ impl Diagnostic for NoSuchField {
     }
 }
 
-impl AstDiagnostic for NoSuchField {
+impl DiagnosticWithFix for NoSuchField {
     type AST = ast::RecordExprField;
 
-    fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.file).unwrap();
-        self.field.to_node(&root)
+    fn fix_source(&self, db: &dyn AstDatabase) -> Option<Self::AST> {
+        let root = db.parse_or_expand(self.file)?;
+        Some(self.field.to_node(&root))
     }
 }
 
@@ -88,12 +88,12 @@ impl Diagnostic for MissingFields {
     }
 }
 
-impl AstDiagnostic for MissingFields {
+impl DiagnosticWithFix for MissingFields {
     type AST = ast::RecordExpr;
 
-    fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.file).unwrap();
-        self.field_list_parent.to_node(&root)
+    fn fix_source(&self, db: &dyn AstDatabase) -> Option<Self::AST> {
+        let root = db.parse_or_expand(self.file)?;
+        Some(self.field_list_parent.to_node(&root))
     }
 }
 
@@ -163,12 +163,12 @@ impl Diagnostic for MissingOkInTailExpr {
     }
 }
 
-impl AstDiagnostic for MissingOkInTailExpr {
+impl DiagnosticWithFix for MissingOkInTailExpr {
     type AST = ast::Expr;
 
-    fn fix_source(&self, db: &dyn AstDatabase) -> Self::AST {
-        let root = db.parse_or_expand(self.file).unwrap();
-        self.expr.to_node(&root)
+    fn fix_source(&self, db: &dyn AstDatabase) -> Option<Self::AST> {
+        let root = db.parse_or_expand(self.file)?;
+        Some(self.expr.to_node(&root))
     }
 }
 
diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs
index 55593a8cb..043ce357b 100644
--- a/crates/ra_ide/src/diagnostics.rs
+++ b/crates/ra_ide/src/diagnostics.rs
@@ -62,12 +62,18 @@ pub(crate) fn diagnostics(
                 }
                 .into(),
             );
+            let fix = sema
+                .diagnostic_fix_source(d)
+                .map(|unresolved_module| unresolved_module.syntax().text_range())
+                .map(|fix_range| (fix, fix_range));
+
             res.borrow_mut().push(Diagnostic {
                 range: sema.diagnostics_presentation_range(d).range,
                 message: d.message(),
                 severity: Severity::Error,
-                fix: Some((fix, sema.diagnostic_fix_source(d).syntax().text_range())),
-            })
+                fix,
+            });
+            Some(())
         })
         .on::<hir::diagnostics::MissingFields, _>(|d| {
             // Note that although we could add a diagnostics to
@@ -78,30 +84,29 @@ pub(crate) fn diagnostics(
             let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
                 None
             } else {
-                let record_expr = sema.diagnostic_fix_source(d);
-                if let Some(old_field_list) = record_expr.record_expr_field_list() {
-                    let mut new_field_list = old_field_list.clone();
-                    for f in d.missed_fields.iter() {
-                        let field = make::record_expr_field(
-                            make::name_ref(&f.to_string()),
-                            Some(make::expr_unit()),
-                        );
-                        new_field_list = new_field_list.append_field(&field);
-                    }
-
-                    let edit = {
-                        let mut builder = TextEditBuilder::default();
-                        algo::diff(&old_field_list.syntax(), &new_field_list.syntax())
-                            .into_text_edit(&mut builder);
-                        builder.finish()
-                    };
-                    Some((
-                        Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()),
-                        sema.original_range(&old_field_list.syntax()).range,
-                    ))
-                } else {
-                    None
-                }
+                sema.diagnostic_fix_source(d)
+                    .and_then(|record_expr| record_expr.record_expr_field_list())
+                    .map(|old_field_list| {
+                        let mut new_field_list = old_field_list.clone();
+                        for f in d.missed_fields.iter() {
+                            let field = make::record_expr_field(
+                                make::name_ref(&f.to_string()),
+                                Some(make::expr_unit()),
+                            );
+                            new_field_list = new_field_list.append_field(&field);
+                        }
+
+                        let edit = {
+                            let mut builder = TextEditBuilder::default();
+                            algo::diff(&old_field_list.syntax(), &new_field_list.syntax())
+                                .into_text_edit(&mut builder);
+                            builder.finish()
+                        };
+                        (
+                            Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()),
+                            sema.original_range(&old_field_list.syntax()).range,
+                        )
+                    })
             };
 
             res.borrow_mut().push(Diagnostic {
@@ -109,28 +114,36 @@ pub(crate) fn diagnostics(
                 message: d.message(),
                 severity: Severity::Error,
                 fix,
-            })
+            });
+            Some(())
         })
         .on::<hir::diagnostics::MissingOkInTailExpr, _>(|d| {
-            let tail_expr = sema.diagnostic_fix_source(d);
-            let tail_expr_range = tail_expr.syntax().text_range();
-            let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax()));
-            let source_change = SourceFileEdit { file_id, edit }.into();
+            let fix = sema.diagnostic_fix_source(d).map(|tail_expr| {
+                let tail_expr_range = tail_expr.syntax().text_range();
+                let edit =
+                    TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax()));
+                let source_change = SourceFileEdit { file_id, edit }.into();
+                (Fix::new("Wrap with ok", source_change), tail_expr_range)
+            });
+
             res.borrow_mut().push(Diagnostic {
                 range: sema.diagnostics_presentation_range(d).range,
                 message: d.message(),
                 severity: Severity::Error,
-                fix: Some((Fix::new("Wrap with ok", source_change), tail_expr_range)),
-            })
+                fix,
+            });
+            Some(())
         })
         .on::<hir::diagnostics::NoSuchField, _>(|d| {
             res.borrow_mut().push(Diagnostic {
                 range: sema.diagnostics_presentation_range(d).range,
                 message: d.message(),
                 severity: Severity::Error,
-                fix: missing_struct_field_fix(&sema, file_id, d)
-                    .map(|fix| (fix, sema.diagnostic_fix_source(d).syntax().text_range())),
-            })
+                fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| {
+                    Some((fix, sema.diagnostic_fix_source(d)?.syntax().text_range()))
+                }),
+            });
+            Some(())
         })
         // Only collect experimental diagnostics when they're enabled.
         .filter(|diag| !diag.is_experimental() || enable_experimental)
@@ -156,7 +169,7 @@ fn missing_struct_field_fix(
     usage_file_id: FileId,
     d: &hir::diagnostics::NoSuchField,
 ) -> Option<Fix> {
-    let record_expr_field = sema.diagnostic_fix_source(d);
+    let record_expr_field = sema.diagnostic_fix_source(d)?;
 
     let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?;
     let def_id = sema.resolve_variant(record_lit)?;