Merge #8481

8481: internal: prepare for lazy diagnostics r=matklad a=matklad

bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ide/src/diagnostics.rs

@@ -25,7 +25,7 @@ use syntax::{
 use text_edit::TextEdit;
 use unlinked_file::UnlinkedFile;
 
-use crate::{FileId, Label, SourceChange};
+use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange};
 
 use self::fixes::DiagnosticWithFix;
 
@@ -35,7 +35,7 @@ pub struct Diagnostic {
     pub message: String,
     pub range: TextRange,
     pub severity: Severity,
-    pub fix: Option<Fix>,
+    pub fix: Option<Assist>,
     pub unused: bool,
     pub code: Option<DiagnosticCode>,
 }
@@ -56,7 +56,7 @@ impl Diagnostic {
         }
     }
 
-    fn with_fix(self, fix: Option<Fix>) -> Self {
+    fn with_fix(self, fix: Option<Assist>) -> Self {
         Self { fix, ..self }
     }
 
@@ -69,21 +69,6 @@ impl Diagnostic {
     }
 }
 
-#[derive(Debug)]
-pub struct Fix {
-    pub label: Label,
-    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 {
-    fn new(label: &str, source_change: SourceChange, fix_trigger_range: TextRange) -> Self {
-        let label = Label::new(label);
-        Self { label, source_change, fix_trigger_range }
-    }
-}
-
 #[derive(Debug, Copy, Clone)]
 pub enum Severity {
     Error,
@@ -261,7 +246,8 @@ fn check_unnecessary_braces_in_use_statement(
 
         acc.push(
             Diagnostic::hint(use_range, "Unnecessary braces in use statement".to_string())
-                .with_fix(Some(Fix::new(
+                .with_fix(Some(fix(
+                    "remove_braces",
                     "Remove unnecessary braces",
                     SourceChange::from_text_edit(file_id, edit),
                     use_range,
@@ -284,6 +270,17 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
     None
 }
 
+fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist {
+    assert!(!id.contains(' '));
+    Assist {
+        id: AssistId(id, AssistKind::QuickFix),
+        label: Label::new(label),
+        group: None,
+        target,
+        source_change: Some(source_change),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use expect_test::{expect, Expect};
@@ -308,10 +305,11 @@ mod tests {
             .unwrap();
         let fix = diagnostic.fix.unwrap();
         let actual = {
-            let file_id = *fix.source_change.source_file_edits.keys().next().unwrap();
+            let source_change = fix.source_change.unwrap();
+            let file_id = *source_change.source_file_edits.keys().next().unwrap();
             let mut actual = analysis.file_text(file_id).unwrap().to_string();
 
-            for edit in fix.source_change.source_file_edits.values() {
+            for edit in source_change.source_file_edits.values() {
                 edit.apply(&mut actual);
             }
             actual
@@ -319,9 +317,9 @@ mod tests {
 
         assert_eq_text!(&after, &actual);
         assert!(
-            fix.fix_trigger_range.contains_inclusive(file_position.offset),
+            fix.target.contains_inclusive(file_position.offset),
             "diagnostic fix range {:?} does not touch cursor position {:?}",
-            fix.fix_trigger_range,
+            fix.target,
             file_position.offset
         );
     }
@@ -665,24 +663,31 @@ fn test_fn() {
                         range: 0..8,
                         severity: Error,
                         fix: Some(
-                            Fix {
+                            Assist {
+                                id: AssistId(
+                                    "create_module",
+                                    QuickFix,
+                                ),
                                 label: "Create module",
-                                source_change: SourceChange {
-                                    source_file_edits: {},
-                                    file_system_edits: [
-                                        CreateFile {
-                                            dst: AnchoredPathBuf {
-                                                anchor: FileId(
-                                                    0,
-                                                ),
-                                                path: "foo.rs",
+                                group: None,
+                                target: 0..8,
+                                source_change: Some(
+                                    SourceChange {
+                                        source_file_edits: {},
+                                        file_system_edits: [
+                                            CreateFile {
+                                                dst: AnchoredPathBuf {
+                                                    anchor: FileId(
+                                                        0,
+                                                    ),
+                                                    path: "foo.rs",
+                                                },
+                                                initial_contents: "",
                                             },
-                                            initial_contents: "",
-                                        },
-                                    ],
-                                    is_snippet: false,
-                                },
-                                fix_trigger_range: 0..8,
+                                        ],
+                                        is_snippet: false,
+                                    },
+                                ),
                             },
                         ),
                         unused: false,

crates/ide/src/diagnostics/field_shorthand.rs

@@ -5,7 +5,7 @@ use ide_db::{base_db::FileId, source_change::SourceChange};
 use syntax::{ast, match_ast, AstNode, SyntaxNode};
 use text_edit::TextEdit;
 
-use crate::{Diagnostic, Fix};
+use crate::{diagnostics::fix, Diagnostic};
 
 pub(super) fn check(acc: &mut Vec<Diagnostic>, file_id: FileId, node: &SyntaxNode) {
     match_ast! {
@@ -47,7 +47,8 @@ fn check_expr_field_shorthand(
         let field_range = record_field.syntax().text_range();
         acc.push(
             Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()).with_fix(
-                Some(Fix::new(
+                Some(fix(
+                    "use_expr_field_shorthand",
                     "Use struct shorthand initialization",
                     SourceChange::from_text_edit(file_id, edit),
                     field_range,
@@ -86,7 +87,8 @@ fn check_pat_field_shorthand(
 
         let field_range = record_pat_field.syntax().text_range();
         acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fix(
-            Some(Fix::new(
+            Some(fix(
+                "use_pat_field_shorthand",
                 "Use struct field shorthand",
                 SourceChange::from_text_edit(file_id, edit),
                 field_range,

crates/ide/src/diagnostics/fixes.rs

@@ -20,20 +20,21 @@ use syntax::{
 };
 use text_edit::TextEdit;
 
-use crate::{diagnostics::Fix, references::rename::rename_with_semantics, FilePosition};
+use crate::{diagnostics::fix, references::rename::rename_with_semantics, Assist, FilePosition};
 
 /// A [Diagnostic] that potentially has a fix available.
 ///
 /// [Diagnostic]: hir::diagnostics::Diagnostic
 pub(crate) trait DiagnosticWithFix: Diagnostic {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix>;
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist>;
 }
 
 impl DiagnosticWithFix for UnresolvedModule {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         let root = sema.db.parse_or_expand(self.file)?;
         let unresolved_module = self.decl.to_node(&root);
-        Some(Fix::new(
+        Some(fix(
+            "create_module",
             "Create module",
             FileSystemEdit::CreateFile {
                 dst: AnchoredPathBuf {
@@ -49,7 +50,7 @@ impl DiagnosticWithFix for UnresolvedModule {
 }
 
 impl DiagnosticWithFix for NoSuchField {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         let root = sema.db.parse_or_expand(self.file)?;
         missing_record_expr_field_fix(
             &sema,
@@ -60,7 +61,7 @@ impl DiagnosticWithFix for NoSuchField {
 }
 
 impl DiagnosticWithFix for MissingFields {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         // Note that although we could add a diagnostics to
         // fill the missing tuple field, e.g :
         // `struct A(usize);`
@@ -86,7 +87,8 @@ impl DiagnosticWithFix for MissingFields {
                 .into_text_edit(&mut builder);
             builder.finish()
         };
-        Some(Fix::new(
+        Some(fix(
+            "fill_missing_fields",
             "Fill struct fields",
             SourceChange::from_text_edit(self.file.original_file(sema.db), edit),
             sema.original_range(&field_list_parent.syntax()).range,
@@ -95,7 +97,7 @@ impl DiagnosticWithFix for MissingFields {
 }
 
 impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         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();
@@ -103,12 +105,12 @@ impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
         let edit = TextEdit::replace(tail_expr_range, replacement);
         let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
         let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
-        Some(Fix::new(name, source_change, tail_expr_range))
+        Some(fix("wrap_tail_expr", name, source_change, tail_expr_range))
     }
 }
 
 impl DiagnosticWithFix for RemoveThisSemicolon {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         let root = sema.db.parse_or_expand(self.file)?;
 
         let semicolon = self
@@ -123,12 +125,12 @@ impl DiagnosticWithFix for RemoveThisSemicolon {
         let edit = TextEdit::delete(semicolon);
         let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
 
-        Some(Fix::new("Remove this semicolon", source_change, semicolon))
+        Some(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon))
     }
 }
 
 impl DiagnosticWithFix for IncorrectCase {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         let root = sema.db.parse_or_expand(self.file)?;
         let name_node = self.ident.to_node(&root);
 
@@ -140,12 +142,12 @@ impl DiagnosticWithFix for IncorrectCase {
             rename_with_semantics(sema, file_position, &self.suggested_text).ok()?;
 
         let label = format!("Rename to {}", self.suggested_text);
-        Some(Fix::new(&label, rename_changes, frange.range))
+        Some(fix("change_case", &label, rename_changes, frange.range))
     }
 }
 
 impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
-    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
         let root = sema.db.parse_or_expand(self.file)?;
         let next_expr = self.next_expr.to_node(&root);
         let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?;
@@ -163,7 +165,8 @@ impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
 
         let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
 
-        Some(Fix::new(
+        Some(fix(
+            "replace_with_find_map",
             "Replace filter_map(..).next() with find_map()",
             source_change,
             trigger_range,
@@ -175,7 +178,7 @@ fn missing_record_expr_field_fix(
     sema: &Semantics<RootDatabase>,
     usage_file_id: FileId,
     record_expr_field: &ast::RecordExprField,
-) -> Option<Fix> {
+) -> Option<Assist> {
     let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?;
     let def_id = sema.resolve_variant(record_lit)?;
     let module;
@@ -233,7 +236,12 @@ fn missing_record_expr_field_fix(
         def_file_id,
         TextEdit::insert(last_field_syntax.text_range().end(), new_field),
     );
-    return Some(Fix::new("Create field", source_change, record_expr_field.syntax().text_range()));
+    return Some(fix(
+        "create_field",
+        "Create field",
+        source_change,
+        record_expr_field.syntax().text_range(),
+    ));
 
     fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> {
         match field_def_list {

crates/ide/src/diagnostics/unlinked_file.rs

@@ -16,9 +16,10 @@ use syntax::{
 };
 use text_edit::TextEdit;
 
-use crate::Fix;
-
-use super::fixes::DiagnosticWithFix;
+use crate::{
+    diagnostics::{fix, fixes::DiagnosticWithFix},
+    Assist,
+};
 
 // Diagnostic: unlinked-file
 //
@@ -49,7 +50,7 @@ impl Diagnostic for UnlinkedFile {
 }
 
 impl DiagnosticWithFix for UnlinkedFile {
-    fn fix(&self, sema: &hir::Semantics<RootDatabase>) -> Option<Fix> {
+    fn fix(&self, sema: &hir::Semantics<RootDatabase>) -> Option<Assist> {
         // If there's an existing module that could add a `mod` item to include the unlinked file,
         // suggest that as a fix.
 
@@ -100,7 +101,7 @@ fn make_fix(
     parent_file_id: FileId,
     new_mod_name: &str,
     added_file_id: FileId,
-) -> Option<Fix> {
+) -> Option<Assist> {
     fn is_outline_mod(item: &ast::Item) -> bool {
         matches!(item, ast::Item::Module(m) if m.item_list().is_none())
     }
@@ -152,7 +153,8 @@ fn make_fix(
 
     let edit = builder.finish();
     let trigger_range = db.parse(added_file_id).tree().syntax().text_range();
-    Some(Fix::new(
+    Some(fix(
+        "add_mod_declaration",
         &format!("Insert `{}`", mod_decl),
         SourceChange::from_text_edit(parent_file_id, edit),
         trigger_range,

crates/ide/src/lib.rs

@@ -69,7 +69,7 @@ use crate::display::ToNav;
 pub use crate::{
     annotations::{Annotation, AnnotationConfig, AnnotationKind},
     call_hierarchy::CallItem,
-    diagnostics::{Diagnostic, DiagnosticsConfig, Fix, Severity},
+    diagnostics::{Diagnostic, DiagnosticsConfig, Severity},
     display::navigation_target::NavigationTarget,
     expand_macro::ExpandedMacro,
     file_structure::{StructureNode, StructureNodeKind},

crates/rust-analyzer/src/handlers.rs

@@ -1039,18 +1039,20 @@ fn add_quick_fixes(
     for fix in diagnostics
         .into_iter()
         .filter_map(|d| d.fix)
-        .filter(|fix| fix.fix_trigger_range.intersect(frange.range).is_some())
+        .filter(|fix| fix.target.intersect(frange.range).is_some())
     {
-        let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?;
-        let action = lsp_ext::CodeAction {
-            title: fix.label.to_string(),
-            group: None,
-            kind: Some(CodeActionKind::QUICKFIX),
-            edit: Some(edit),
-            is_preferred: Some(false),
-            data: None,
-        };
-        acc.push(action);
+        if let Some(source_change) = fix.source_change {
+            let edit = to_proto::snippet_workspace_edit(&snap, source_change)?;
+            let action = lsp_ext::CodeAction {
+                title: fix.label.to_string(),
+                group: None,
+                kind: Some(CodeActionKind::QUICKFIX),
+                edit: Some(edit),
+                is_preferred: Some(false),
+                data: None,
+            };
+            acc.push(action);
+        }
     }
 
     for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() {