From 188ec3459e795732ad097758f7bf6b6b95bdbf5e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 17:13:40 +0300 Subject: Simplify fix structure --- crates/ra_ide/src/diagnostics.rs | 68 ++++++-------- .../ra_ide/src/diagnostics/diagnostics_with_fix.rs | 101 ++++++++++----------- crates/ra_ide/src/lib.rs | 12 ++- crates/rust-analyzer/src/handlers.rs | 7 +- 4 files changed, 92 insertions(+), 96 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: &D, - sema: &Semantics, -) -> Diagnostic { +fn diagnostic_with_fix(d: &D, sema: &Semantics) -> 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) -> Option<(Fix, TextRange)>; +/// A [Diagnostic] that potentially has a fix available. +/// +/// [Diagnostic]: hir::diagnostics::Diagnostic +pub trait DiagnosticWithFix: Diagnostic { + fn fix(&self, sema: &Semantics) -> Option; } impl DiagnosticWithFix for UnresolvedModule { - fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { - let fix = Fix::new( + 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( "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) -> Option<(Fix, TextRange)> { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option<(Fix, TextRange)> { + 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);` // `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) -> Option<(Fix, TextRange)> { + 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(); 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, 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 { 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, } #[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, source_change: SourceChange) -> Self { + pub fn new( + label: impl Into, + 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 } } } diff --git a/crates/rust-analyzer/src/handlers.rs b/crates/rust-analyzer/src/handlers.rs index 144c641b2..785dd2a26 100644 --- a/crates/rust-analyzer/src/handlers.rs +++ b/crates/rust-analyzer/src/handlers.rs @@ -773,12 +773,11 @@ fn handle_fixes( let diagnostics = snap.analysis.diagnostics(file_id, snap.config.experimental_diagnostics)?; - let fixes_from_diagnostics = diagnostics + for fix in diagnostics .into_iter() .filter_map(|d| d.fix) - .filter(|(_fix, fix_range)| fix_range.intersect(range).is_some()) - .map(|(fix, _range)| fix); - for fix in fixes_from_diagnostics { + .filter(|fix| fix.fix_trigger_range.intersect(range).is_some()) + { let title = fix.label; let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?; let action = lsp_ext::CodeAction { -- cgit v1.2.3