From 426d098bd6a032cb03e61d4b3d091caeaecbd4d0 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 12 Apr 2021 17:58:01 +0300 Subject: internal: prepare for lazy diagnostics --- crates/ide/src/diagnostics.rs | 83 ++++++++++++++------------- crates/ide/src/diagnostics/field_shorthand.rs | 8 ++- crates/ide/src/diagnostics/fixes.rs | 42 ++++++++------ crates/ide/src/diagnostics/unlinked_file.rs | 14 +++-- crates/ide/src/lib.rs | 2 +- crates/rust-analyzer/src/handlers.rs | 24 ++++---- 6 files changed, 96 insertions(+), 77 deletions(-) diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 0ace80a1e..4f0b4a62e 100644 --- a/crates/ide/src/diagnostics.rs +++ b/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, + pub fix: Option, pub unused: bool, pub code: Option, } @@ -56,7 +56,7 @@ impl Diagnostic { } } - fn with_fix(self, fix: Option) -> Self { + fn with_fix(self, fix: Option) -> 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, diff --git a/crates/ide/src/diagnostics/field_shorthand.rs b/crates/ide/src/diagnostics/field_shorthand.rs index 5c89e2170..2b1787f9b 100644 --- a/crates/ide/src/diagnostics/field_shorthand.rs +++ b/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, 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, diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 5fb3e2d91..69cf5288c 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/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) -> Option; + fn fix(&self, sema: &Semantics) -> Option; } impl DiagnosticWithFix for UnresolvedModule { - fn fix(&self, sema: &Semantics) -> Option { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option { + fn fix(&self, sema: &Semantics) -> Option { // 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) -> Option { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option { + fn fix(&self, sema: &Semantics) -> Option { 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, usage_file_id: FileId, record_expr_field: &ast::RecordExprField, -) -> Option { +) -> Option { 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 { match field_def_list { diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index e174fb767..5482b7287 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/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) -> Option { + fn fix(&self, sema: &hir::Semantics) -> Option { // 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 { +) -> Option { 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, diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index 3f73c0632..0615b26d3 100644 --- a/crates/ide/src/lib.rs +++ b/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}, diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index edfa42eb5..107685c63 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/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() { -- cgit v1.2.3