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_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 +- 5 files changed, 84 insertions(+), 86 deletions(-) (limited to 'crates/ra_ide') 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