From 2c04aad2d2a52ce52d6ea6452faf8d1788f0c83f Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Fri, 22 May 2020 18:03:08 +0200 Subject: KISS SourceChange The idea behind requiring the label is a noble one, but we are not really using it consistently anyway, and it should be easy to retrofit later, should we need it. --- crates/ra_assists/src/assist_context.rs | 10 +-- crates/ra_ide/src/diagnostics.rs | 101 +++++++++++++------------ crates/ra_ide/src/lib.rs | 18 ++++- crates/ra_ide/src/references/rename.rs | 11 +-- crates/ra_ide/src/typing.rs | 35 +++------ crates/ra_ide/src/typing/on_enter.rs | 5 +- crates/ra_ide_db/src/source_change.rs | 79 +++++-------------- crates/rust-analyzer/src/main_loop/handlers.rs | 6 +- 8 files changed, 112 insertions(+), 153 deletions(-) (limited to 'crates') diff --git a/crates/ra_assists/src/assist_context.rs b/crates/ra_assists/src/assist_context.rs index f3af70a3e..5b1a4680b 100644 --- a/crates/ra_assists/src/assist_context.rs +++ b/crates/ra_assists/src/assist_context.rs @@ -5,7 +5,7 @@ use hir::Semantics; use ra_db::{FileId, FileRange}; use ra_fmt::{leading_indent, reindent}; use ra_ide_db::{ - source_change::{SingleFileChange, SourceChange}, + source_change::{SourceChange, SourceFileEdit}, RootDatabase, }; use ra_syntax::{ @@ -150,11 +150,10 @@ impl Assists { self.add_impl(label, f) } fn add_impl(&mut self, label: Assist, f: impl FnOnce(&mut AssistBuilder)) -> Option<()> { - let change_label = label.label.clone(); let source_change = if self.resolve { let mut builder = AssistBuilder::new(self.file); f(&mut builder); - Some(builder.finish(change_label)) + Some(builder.finish()) } else { None }; @@ -246,9 +245,10 @@ impl AssistBuilder { &mut self.edit } - fn finish(self, change_label: String) -> SourceChange { + fn finish(self) -> SourceChange { let edit = self.edit.finish(); - let mut res = SingleFileChange { label: change_label, edit }.into_source_change(self.file); + let source_file_edit = SourceFileEdit { file_id: self.file, edit }; + let mut res: SourceChange = source_file_edit.into(); if self.is_snippet { res.is_snippet = true; } diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index c2819bbf7..3d83c0f71 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -21,7 +21,7 @@ use ra_syntax::{ }; use ra_text_edit::{TextEdit, TextEditBuilder}; -use crate::{Diagnostic, FileId, FileSystemEdit, SourceChange, SourceFileEdit}; +use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceChange, SourceFileEdit}; #[derive(Debug, Copy, Clone)] pub enum Severity { @@ -63,8 +63,8 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec .parent() .unwrap_or_else(|| RelativePath::new("")) .join(&d.candidate); - let create_file = FileSystemEdit::CreateFile { source_root, path }; - let fix = SourceChange::file_system_edit("Create module", create_file); + let fix = + Fix::new("Create module", FileSystemEdit::CreateFile { source_root, path }.into()); res.borrow_mut().push(Diagnostic { range: sema.diagnostics_range(d).range, message: d.message(), @@ -88,14 +88,12 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec field_list = field_list.append_field(&field); } - let mut builder = TextEditBuilder::default(); - algo::diff(&d.ast(db).syntax(), &field_list.syntax()).into_text_edit(&mut builder); - - Some(SourceChange::source_file_edit_from( - "Fill struct fields", - file_id, - builder.finish(), - )) + let edit = { + let mut builder = TextEditBuilder::default(); + algo::diff(&d.ast(db).syntax(), &field_list.syntax()).into_text_edit(&mut builder); + builder.finish() + }; + Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into())) }; res.borrow_mut().push(Diagnostic { @@ -117,7 +115,8 @@ pub(crate) fn diagnostics(db: &RootDatabase, file_id: FileId) -> Vec let node = d.ast(db); let replacement = format!("Ok({})", node.syntax()); let edit = TextEdit::replace(node.syntax().text_range(), replacement); - let fix = SourceChange::source_file_edit_from("Wrap with ok", file_id, edit); + let source_change = SourceChange::source_file_edit_from(file_id, edit); + let fix = Fix::new("Wrap with ok", source_change); res.borrow_mut().push(Diagnostic { range: sema.diagnostics_range(d).range, message: d.message(), @@ -154,9 +153,9 @@ fn check_unnecessary_braces_in_use_statement( range, message: "Unnecessary braces in use statement".to_string(), severity: Severity::WeakWarning, - fix: Some(SourceChange::source_file_edit( + fix: Some(Fix::new( "Remove unnecessary braces", - SourceFileEdit { file_id, edit }, + SourceFileEdit { file_id, edit }.into(), )), }); } @@ -198,9 +197,9 @@ fn check_struct_shorthand_initialization( range: record_field.syntax().text_range(), message: "Shorthand struct initialization".to_string(), severity: Severity::WeakWarning, - fix: Some(SourceChange::source_file_edit( + fix: Some(Fix::new( "Use struct shorthand initialization", - SourceFileEdit { file_id, edit }, + SourceFileEdit { file_id, edit }.into(), )), }); } @@ -240,7 +239,7 @@ mod tests { let diagnostic = diagnostics.pop().unwrap_or_else(|| panic!("no diagnostics for:\n{}\n", before)); let mut fix = diagnostic.fix.unwrap(); - let edit = fix.source_file_edits.pop().unwrap().edit; + let edit = fix.source_change.source_file_edits.pop().unwrap().edit; let actual = { let mut actual = before.to_string(); edit.apply(&mut actual); @@ -258,7 +257,7 @@ mod tests { let (analysis, file_position) = analysis_and_position(fixture); let diagnostic = analysis.diagnostics(file_position.file_id).unwrap().pop().unwrap(); let mut fix = diagnostic.fix.unwrap(); - let edit = fix.source_file_edits.pop().unwrap().edit; + 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 = { let mut actual = target_file_contents.to_string(); @@ -295,7 +294,7 @@ mod tests { let (analysis, file_id) = single_file(before); let diagnostic = analysis.diagnostics(file_id).unwrap().pop().unwrap(); let mut fix = diagnostic.fix.unwrap(); - let edit = fix.source_file_edits.pop().unwrap().edit; + let edit = fix.source_change.source_file_edits.pop().unwrap().edit; let actual = { let mut actual = before.to_string(); edit.apply(&mut actual); @@ -616,22 +615,24 @@ mod tests { Diagnostic { message: "unresolved module", range: 0..8, + severity: Error, fix: Some( - SourceChange { + Fix { label: "Create module", - source_file_edits: [], - file_system_edits: [ - CreateFile { - source_root: SourceRootId( - 0, - ), - path: "foo.rs", - }, - ], - is_snippet: false, + source_change: SourceChange { + source_file_edits: [], + file_system_edits: [ + CreateFile { + source_root: SourceRootId( + 0, + ), + path: "foo.rs", + }, + ], + is_snippet: false, + }, }, ), - severity: Error, }, ] "###); @@ -665,29 +666,31 @@ mod tests { Diagnostic { message: "Missing structure fields:\n- b", range: 224..233, + severity: Error, fix: Some( - SourceChange { + Fix { label: "Fill struct fields", - source_file_edits: [ - SourceFileEdit { - file_id: FileId( - 1, - ), - edit: TextEdit { - indels: [ - Indel { - insert: "{a:42, b: ()}", - delete: 3..9, - }, - ], + source_change: SourceChange { + source_file_edits: [ + SourceFileEdit { + file_id: FileId( + 1, + ), + edit: TextEdit { + indels: [ + Indel { + insert: "{a:42, b: ()}", + delete: 3..9, + }, + ], + }, }, - }, - ], - file_system_edits: [], - is_snippet: false, + ], + file_system_edits: [], + is_snippet: false, + }, }, ), - severity: Error, }, ] "###); diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 97ff67ee8..5ac002d82 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -97,8 +97,22 @@ pub type Cancelable = Result; pub struct Diagnostic { pub message: String, pub range: TextRange, - pub fix: Option, pub severity: Severity, + pub fix: Option, +} + +#[derive(Debug)] +pub struct Fix { + pub label: String, + pub source_change: SourceChange, +} + +impl Fix { + pub fn new(label: impl Into, source_change: SourceChange) -> Self { + let label = label.into(); + assert!(label.starts_with(char::is_uppercase) && !label.ends_with('.')); + Self { label, source_change } + } } /// Info associated with a text range. @@ -493,7 +507,7 @@ impl Analysis { ) -> Cancelable> { self.with_db(|db| { let edits = ssr::parse_search_replace(query, parse_only, db)?; - Ok(SourceChange::source_file_edits("Structural Search Replace", edits)) + Ok(SourceChange::source_file_edits(edits)) }) } diff --git a/crates/ra_ide/src/references/rename.rs b/crates/ra_ide/src/references/rename.rs index fd2163dad..28c6349b1 100644 --- a/crates/ra_ide/src/references/rename.rs +++ b/crates/ra_ide/src/references/rename.rs @@ -128,7 +128,7 @@ fn rename_mod( source_file_edits.extend(ref_edits); } - Some(SourceChange::from_edits("Rename", source_file_edits, file_system_edits)) + Some(SourceChange::from_edits(source_file_edits, file_system_edits)) } fn rename_to_self(db: &RootDatabase, position: FilePosition) -> Option> { @@ -171,7 +171,7 @@ fn rename_to_self(db: &RootDatabase, position: FilePosition) -> Option Option { +fn on_char_typed_inner(file: &SourceFile, offset: TextSize, char_typed: char) -> Option { assert!(TRIGGER_CHARS.contains(char_typed)); match char_typed { '.' => on_dot_typed(file, offset), @@ -61,7 +57,7 @@ fn on_char_typed_inner( /// Returns an edit which should be applied after `=` was typed. Primarily, /// this works when adding `let =`. // FIXME: use a snippet completion instead of this hack here. -fn on_eq_typed(file: &SourceFile, offset: TextSize) -> Option { +fn on_eq_typed(file: &SourceFile, offset: TextSize) -> Option { assert_eq!(file.syntax().text().char_at(offset), Some('=')); let let_stmt: ast::LetStmt = find_node_at_offset(file.syntax(), offset)?; if let_stmt.semicolon_token().is_some() { @@ -79,14 +75,11 @@ fn on_eq_typed(file: &SourceFile, offset: TextSize) -> Option return None; } let offset = let_stmt.syntax().text_range().end(); - Some(SingleFileChange { - label: "add semicolon".to_string(), - edit: TextEdit::insert(offset, ";".to_string()), - }) + Some(TextEdit::insert(offset, ";".to_string())) } /// Returns an edit which should be applied when a dot ('.') is typed on a blank line, indenting the line appropriately. -fn on_dot_typed(file: &SourceFile, offset: TextSize) -> Option { +fn on_dot_typed(file: &SourceFile, offset: TextSize) -> Option { assert_eq!(file.syntax().text().char_at(offset), Some('.')); let whitespace = file.syntax().token_at_offset(offset).left_biased().and_then(ast::Whitespace::cast)?; @@ -107,14 +100,11 @@ fn on_dot_typed(file: &SourceFile, offset: TextSize) -> Option return None; } - Some(SingleFileChange { - label: "reindent dot".to_string(), - edit: TextEdit::replace(TextRange::new(offset - current_indent_len, offset), target_indent), - }) + Some(TextEdit::replace(TextRange::new(offset - current_indent_len, offset), target_indent)) } /// Adds a space after an arrow when `fn foo() { ... }` is turned into `fn foo() -> { ... }` -fn on_arrow_typed(file: &SourceFile, offset: TextSize) -> Option { +fn on_arrow_typed(file: &SourceFile, offset: TextSize) -> Option { let file_text = file.syntax().text(); assert_eq!(file_text.char_at(offset), Some('>')); let after_arrow = offset + TextSize::of('>'); @@ -125,10 +115,7 @@ fn on_arrow_typed(file: &SourceFile, offset: TextSize) -> Option Option, pub file_system_edits: Vec, pub is_snippet: bool, @@ -18,63 +16,22 @@ pub struct SourceChange { impl SourceChange { /// Creates a new SourceChange with the given label /// from the edits. - pub fn from_edits>( - label: L, + pub fn from_edits( source_file_edits: Vec, file_system_edits: Vec, ) -> Self { - SourceChange { - label: label.into(), - source_file_edits, - file_system_edits, - is_snippet: false, - } + SourceChange { source_file_edits, file_system_edits, is_snippet: false } } /// Creates a new SourceChange with the given label, /// containing only the given `SourceFileEdits`. - pub fn source_file_edits>(label: L, edits: Vec) -> Self { - let label = label.into(); - assert!(label.starts_with(char::is_uppercase)); - SourceChange { - label: label, - source_file_edits: edits, - file_system_edits: vec![], - is_snippet: false, - } - } - - /// Creates a new SourceChange with the given label, - /// containing only the given `FileSystemEdits`. - pub(crate) fn file_system_edits>(label: L, edits: Vec) -> Self { - SourceChange { - label: label.into(), - source_file_edits: vec![], - file_system_edits: edits, - is_snippet: false, - } - } - - /// Creates a new SourceChange with the given label, - /// containing only a single `SourceFileEdit`. - pub fn source_file_edit>(label: L, edit: SourceFileEdit) -> Self { - SourceChange::source_file_edits(label, vec![edit]) - } - - /// Creates a new SourceChange with the given label - /// from the given `FileId` and `TextEdit` - pub fn source_file_edit_from>( - label: L, - file_id: FileId, - edit: TextEdit, - ) -> Self { - SourceChange::source_file_edit(label, SourceFileEdit { file_id, edit }) + pub fn source_file_edits(edits: Vec) -> Self { + SourceChange { source_file_edits: edits, file_system_edits: vec![], is_snippet: false } } - /// Creates a new SourceChange with the given label /// from the given `FileId` and `TextEdit` - pub fn file_system_edit>(label: L, edit: FileSystemEdit) -> Self { - SourceChange::file_system_edits(label, vec![edit]) + pub fn source_file_edit_from(file_id: FileId, edit: TextEdit) -> Self { + SourceFileEdit { file_id, edit }.into() } } @@ -84,23 +41,27 @@ pub struct SourceFileEdit { pub edit: TextEdit, } +impl From for SourceChange { + fn from(edit: SourceFileEdit) -> SourceChange { + SourceChange { + source_file_edits: vec![edit], + file_system_edits: Vec::new(), + is_snippet: false, + } + } +} + #[derive(Debug, Clone)] pub enum FileSystemEdit { CreateFile { source_root: SourceRootId, path: RelativePathBuf }, MoveFile { src: FileId, dst_source_root: SourceRootId, dst_path: RelativePathBuf }, } -pub struct SingleFileChange { - pub label: String, - pub edit: TextEdit, -} - -impl SingleFileChange { - pub fn into_source_change(self, file_id: FileId) -> SourceChange { +impl From for SourceChange { + fn from(edit: FileSystemEdit) -> SourceChange { SourceChange { - label: self.label, - source_file_edits: vec![SourceFileEdit { file_id, edit: self.edit }], - file_system_edits: Vec::new(), + source_file_edits: Vec::new(), + file_system_edits: vec![edit], is_snippet: false, } } diff --git a/crates/rust-analyzer/src/main_loop/handlers.rs b/crates/rust-analyzer/src/main_loop/handlers.rs index 89144f743..a9703e1d6 100644 --- a/crates/rust-analyzer/src/main_loop/handlers.rs +++ b/crates/rust-analyzer/src/main_loop/handlers.rs @@ -731,9 +731,9 @@ pub fn handle_code_action( .filter(|(diag_range, _fix)| diag_range.intersect(range).is_some()) .map(|(_range, fix)| fix); - for source_edit in fixes_from_diagnostics { - let title = source_edit.label.clone(); - let edit = to_proto::snippet_workspace_edit(&world, source_edit)?; + for fix in fixes_from_diagnostics { + let title = fix.label; + let edit = to_proto::snippet_workspace_edit(&world, fix.source_change)?; let action = lsp_ext::CodeAction { title, group: None, kind: None, edit: Some(edit), command: None }; res.push(action); -- cgit v1.2.3