Simplify fix structure

3 files changed, 89 insertions, 92 deletions

diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs
index 165ff5249..757b76fd4 100644
--- a/crates/ra_ide/src/diagnostics.rs
+++ b/crates/ra_ide/src/diagnostics.rs
@@ -6,10 +6,7 @@
 
 use std::cell::RefCell;
 
-use hir::{
-    diagnostics::{Diagnostic as HirDiagnostics, DiagnosticSinkBuilder},
-    Semantics,
-};
+use hir::{diagnostics::DiagnosticSinkBuilder, Semantics};
 use itertools::Itertools;
 use ra_db::SourceDatabase;
 use ra_ide_db::RootDatabase;
@@ -73,7 +70,7 @@ pub(crate) fn diagnostics(
         .build(|d| {
             res.borrow_mut().push(Diagnostic {
                 message: d.message(),
-                range: sema.diagnostics_presentation_range(d).range,
+                range: sema.diagnostics_display_range(d).range,
                 severity: Severity::Error,
                 fix: None,
             })
@@ -86,12 +83,9 @@ pub(crate) fn diagnostics(
     res.into_inner()
 }
 
-fn diagnostic_with_fix<D: HirDiagnostics + DiagnosticWithFix>(
-    d: &D,
-    sema: &Semantics<RootDatabase>,
-) -> Diagnostic {
+fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> Diagnostic {
     Diagnostic {
-        range: sema.diagnostics_presentation_range(d).range,
+        range: sema.diagnostics_display_range(d).range,
         message: d.message(),
         severity: Severity::Error,
         fix: d.fix(&sema),
@@ -120,8 +114,9 @@ fn check_unnecessary_braces_in_use_statement(
             range: use_range,
             message: "Unnecessary braces in use statement".to_string(),
             severity: Severity::WeakWarning,
-            fix: Some((
-                Fix::new("Remove unnecessary braces", SourceFileEdit { file_id, edit }.into()),
+            fix: Some(Fix::new(
+                "Remove unnecessary braces",
+                SourceFileEdit { file_id, edit }.into(),
                 use_range,
             )),
         });
@@ -165,11 +160,9 @@ fn check_struct_shorthand_initialization(
                     range: field_range,
                     message: "Shorthand struct initialization".to_string(),
                     severity: Severity::WeakWarning,
-                    fix: Some((
-                        Fix::new(
-                            "Use struct shorthand initialization",
-                            SourceFileEdit { file_id, edit }.into(),
-                        ),
+                    fix: Some(Fix::new(
+                        "Use struct shorthand initialization",
+                        SourceFileEdit { file_id, edit }.into(),
                         field_range,
                     )),
                 });
@@ -197,7 +190,7 @@ mod tests {
 
         let (analysis, file_position) = analysis_and_position(ra_fixture_before);
         let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap();
-        let (mut fix, fix_range) = diagnostic.fix.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();
         let actual = {
@@ -208,9 +201,10 @@ mod tests {
 
         assert_eq_text!(&after, &actual);
         assert!(
-            fix_range.start() <= file_position.offset && fix_range.end() >= file_position.offset,
+            fix.fix_trigger_range.start() <= file_position.offset
+                && fix.fix_trigger_range.end() >= file_position.offset,
             "diagnostic fix range {:?} does not touch cursor position {:?}",
-            fix_range,
+            fix.fix_trigger_range,
             file_position.offset
         );
     }
@@ -222,7 +216,7 @@ mod tests {
         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, true).unwrap().pop().unwrap();
-        let mut fix = diagnostic.fix.unwrap().0;
+        let mut fix = diagnostic.fix.unwrap();
         let edit = fix.source_change.source_file_edits.pop().unwrap();
         let changed_file_id = edit.file_id;
         let before = analysis.file_text(changed_file_id).unwrap();
@@ -513,24 +507,22 @@ fn test_fn() {
                         range: 0..8,
                         severity: Error,
                         fix: Some(
-                            (
-                                Fix {
-                                    label: "Create module",
-                                    source_change: SourceChange {
-                                        source_file_edits: [],
-                                        file_system_edits: [
-                                            CreateFile {
-                                                anchor: FileId(
-                                                    1,
-                                                ),
-                                                dst: "foo.rs",
-                                            },
-                                        ],
-                                        is_snippet: false,
-                                    },
+                            Fix {
+                                label: "Create module",
+                                source_change: SourceChange {
+                                    source_file_edits: [],
+                                    file_system_edits: [
+                                        CreateFile {
+                                            anchor: FileId(
+                                                1,
+                                            ),
+                                            dst: "foo.rs",
+                                        },
+                                    ],
+                                    is_snippet: false,
                                 },
-                                0..8,
-                            ),
+                                fix_trigger_range: 0..8,
+                            },
                         ),
                     },
                 ]
diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs
index 1955e1521..57b54a61e 100644
--- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs
+++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs
@@ -1,9 +1,9 @@
-//! Provides a way to derive fixes based on the diagnostic data.
+//! Provides a way to attach fix actions to the
 use crate::Fix;
 use ast::{edit::IndentLevel, make};
 use hir::{
     db::AstDatabase,
-    diagnostics::{MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule},
+    diagnostics::{Diagnostic, MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule},
     HasSource, HirDisplay, Semantics, VariantDef,
 };
 use ra_db::FileId;
@@ -11,94 +11,90 @@ use ra_ide_db::{
     source_change::{FileSystemEdit, SourceFileEdit},
     RootDatabase,
 };
-use ra_syntax::{algo, ast, AstNode, TextRange};
+use ra_syntax::{algo, ast, AstNode};
 use ra_text_edit::{TextEdit, TextEditBuilder};
 
-/// A trait to implement fot the Diagnostic that has a fix available.
-pub trait DiagnosticWithFix {
-    /// Provides a fix with the fix range, if applicable in the current semantics.
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<(Fix, TextRange)>;
+/// A [Diagnostic] that potentially has a fix available.
+///
+/// [Diagnostic]: hir::diagnostics::Diagnostic
+pub trait DiagnosticWithFix: Diagnostic {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix>;
 }
 
 impl DiagnosticWithFix for UnresolvedModule {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<(Fix, TextRange)> {
-        let fix = Fix::new(
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+        let root = sema.db.parse_or_expand(self.file)?;
+        let unresolved_module = self.decl.to_node(&root);
+        Some(Fix::new(
             "Create module",
             FileSystemEdit::CreateFile {
                 anchor: self.file.original_file(sema.db),
                 dst: self.candidate.clone(),
             }
             .into(),
-        );
-
-        let root = sema.db.parse_or_expand(self.file)?;
-        let unresolved_module = self.decl.to_node(&root);
-        Some((fix, unresolved_module.syntax().text_range()))
+            unresolved_module.syntax().text_range(),
+        ))
     }
 }
 
 impl DiagnosticWithFix for NoSuchField {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<(Fix, TextRange)> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
         let root = sema.db.parse_or_expand(self.file)?;
-        let record_expr_field = self.field.to_node(&root);
-        let fix =
-            missing_struct_field_fix(&sema, self.file.original_file(sema.db), &record_expr_field)?;
-        Some((fix, record_expr_field.syntax().text_range()))
+        missing_record_expr_field_fix(
+            &sema,
+            self.file.original_file(sema.db),
+            &self.field.to_node(&root),
+        )
     }
 }
 
 impl DiagnosticWithFix for MissingFields {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<(Fix, TextRange)> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
         // 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.
         if self.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) {
-            None
-        } else {
-            let root = sema.db.parse_or_expand(self.file)?;
-            let old_field_list = self.field_list_parent.to_node(&root).record_expr_field_list()?;
-            let mut new_field_list = old_field_list.clone();
-            for f in self.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);
-            }
+            return None;
+        }
 
-            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: self.file.original_file(sema.db), edit }.into(),
-                ),
-                sema.original_range(&old_field_list.syntax()).range,
-                // old_field_list.syntax().text_range(),
-            ))
+        let root = sema.db.parse_or_expand(self.file)?;
+        let old_field_list = self.field_list_parent.to_node(&root).record_expr_field_list()?;
+        let mut new_field_list = old_field_list.clone();
+        for f in self.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: self.file.original_file(sema.db), edit }.into(),
+            sema.original_range(&old_field_list.syntax()).range,
+        ))
     }
 }
 
 impl DiagnosticWithFix for MissingOkInTailExpr {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<(Fix, TextRange)> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
         let root = sema.db.parse_or_expand(self.file)?;
         let tail_expr = self.expr.to_node(&root);
         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: self.file.original_file(sema.db), edit }.into();
-        Some((Fix::new("Wrap with ok", source_change), tail_expr_range))
+        Some(Fix::new("Wrap with ok", source_change, tail_expr_range))
     }
 }
 
-fn missing_struct_field_fix(
+fn missing_record_expr_field_fix(
     sema: &Semantics<RootDatabase>,
     usage_file_id: FileId,
     record_expr_field: &ast::RecordExprField,
@@ -159,8 +155,11 @@ fn missing_struct_field_fix(
         file_id: def_file_id,
         edit: TextEdit::insert(last_field_syntax.text_range().end(), new_field),
     };
-    let fix = Fix::new("Create field", source_change.into());
-    return Some(fix);
+    return Some(Fix::new(
+        "Create field",
+        source_change.into(),
+        record_expr_field.syntax().text_range(),
+    ));
 
     fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> {
         match field_def_list {
diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs
index 45a4b2421..89fcb6f17 100644
--- a/crates/ra_ide/src/lib.rs
+++ b/crates/ra_ide/src/lib.rs
@@ -105,20 +105,26 @@ pub struct Diagnostic {
     pub message: String,
     pub range: TextRange,
     pub severity: Severity,
-    pub fix: Option<(Fix, TextRange)>,
+    pub fix: Option<Fix>,
 }
 
 #[derive(Debug)]
 pub struct Fix {
     pub label: String,
     pub source_change: SourceChange,
+    /// Allows to trigger the fix only when the caret is in the range given
+    pub fix_trigger_range: TextRange,
 }
 
 impl Fix {
-    pub fn new(label: impl Into<String>, source_change: SourceChange) -> Self {
+    pub fn new(
+        label: impl Into<String>,
+        source_change: SourceChange,
+        fix_trigger_range: TextRange,
+    ) -> Self {
         let label = label.into();
         assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.'));
-        Self { label, source_change }
+        Self { label, source_change, fix_trigger_range }
     }
 }