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.
This commit is contained in:
Aleksey Kladov 2020-05-22 18:03:08 +02:00
parent 2075e77ee5
commit 2c04aad2d2
8 changed files with 112 additions and 153 deletions

View File

@ -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;
}

View File

@ -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<Diagnostic>
.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<Diagnostic>
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<Diagnostic>
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,
},
]
"###);

View File

@ -97,8 +97,22 @@ pub type Cancelable<T> = Result<T, Canceled>;
pub struct Diagnostic {
pub message: String,
pub range: TextRange,
pub fix: Option<SourceChange>,
pub severity: Severity,
pub fix: Option<Fix>,
}
#[derive(Debug)]
pub struct Fix {
pub label: String,
pub source_change: SourceChange,
}
impl Fix {
pub fn new(label: impl Into<String>, 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<Result<SourceChange, SsrError>> {
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))
})
}

View File

@ -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<RangeInfo<SourceChange>> {
@ -171,7 +171,7 @@ fn rename_to_self(db: &RootDatabase, position: FilePosition) -> Option<RangeInfo
),
});
Some(RangeInfo::new(range, SourceChange::source_file_edits("Rename", edits)))
Some(RangeInfo::new(range, SourceChange::source_file_edits(edits)))
}
fn text_edit_from_self_param(
@ -234,7 +234,7 @@ fn rename_self_to_param(
let range = ast::SelfParam::cast(self_token.parent())
.map_or(self_token.text_range(), |p| p.syntax().text_range());
Some(RangeInfo::new(range, SourceChange::source_file_edits("Rename", edits)))
Some(RangeInfo::new(range, SourceChange::source_file_edits(edits)))
}
fn rename_reference(
@ -253,7 +253,7 @@ fn rename_reference(
return None;
}
Some(RangeInfo::new(range, SourceChange::source_file_edits("Rename", edit)))
Some(RangeInfo::new(range, SourceChange::source_file_edits(edit)))
}
#[cfg(test)]
@ -642,7 +642,6 @@ mod tests {
RangeInfo {
range: 4..7,
info: SourceChange {
label: "Rename",
source_file_edits: [
SourceFileEdit {
file_id: FileId(
@ -694,7 +693,6 @@ mod tests {
RangeInfo {
range: 4..7,
info: SourceChange {
label: "Rename",
source_file_edits: [
SourceFileEdit {
file_id: FileId(
@ -777,7 +775,6 @@ mod tests {
RangeInfo {
range: 8..11,
info: SourceChange {
label: "Rename",
source_file_edits: [
SourceFileEdit {
file_id: FileId(

View File

@ -17,7 +17,7 @@ mod on_enter;
use ra_db::{FilePosition, SourceDatabase};
use ra_fmt::leading_indent;
use ra_ide_db::{source_change::SingleFileChange, RootDatabase};
use ra_ide_db::RootDatabase;
use ra_syntax::{
algo::find_node_at_offset,
ast::{self, AstToken},
@ -40,15 +40,11 @@ pub(crate) fn on_char_typed(
assert!(TRIGGER_CHARS.contains(char_typed));
let file = &db.parse(position.file_id).tree();
assert_eq!(file.syntax().text().char_at(position.offset), Some(char_typed));
let single_file_change = on_char_typed_inner(file, position.offset, char_typed)?;
Some(single_file_change.into_source_change(position.file_id))
let text_edit = on_char_typed_inner(file, position.offset, char_typed)?;
Some(SourceChange::source_file_edit_from(position.file_id, text_edit))
}
fn on_char_typed_inner(
file: &SourceFile,
offset: TextSize,
char_typed: char,
) -> Option<SingleFileChange> {
fn on_char_typed_inner(file: &SourceFile, offset: TextSize, char_typed: char) -> Option<TextEdit> {
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<SingleFileChange> {
fn on_eq_typed(file: &SourceFile, offset: TextSize) -> Option<TextEdit> {
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<SingleFileChange>
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<SingleFileChange> {
fn on_dot_typed(file: &SourceFile, offset: TextSize) -> Option<TextEdit> {
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<SingleFileChange>
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<SingleFileChange> {
fn on_arrow_typed(file: &SourceFile, offset: TextSize) -> Option<TextEdit> {
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<SingleFileChang
return None;
}
Some(SingleFileChange {
label: "add space after return type".to_string(),
edit: TextEdit::insert(after_arrow, " ".to_string()),
})
Some(TextEdit::insert(after_arrow, " ".to_string()))
}
#[cfg(test)]
@ -144,7 +131,7 @@ mod tests {
edit.apply(&mut before);
let parse = SourceFile::parse(&before);
on_char_typed_inner(&parse.tree(), offset, char_typed).map(|it| {
it.edit.apply(&mut before);
it.apply(&mut before);
before.to_string()
})
}

View File

@ -41,10 +41,7 @@ pub(crate) fn on_enter(db: &RootDatabase, position: FilePosition) -> Option<Sour
let inserted = format!("\n{}{} $0", indent, prefix);
let edit = TextEdit::insert(position.offset, inserted);
let mut res = SourceChange::source_file_edit(
"On enter",
SourceFileEdit { edit, file_id: position.file_id },
);
let mut res = SourceChange::from(SourceFileEdit { edit, file_id: position.file_id });
res.is_snippet = true;
Some(res)
}

View File

@ -8,8 +8,6 @@ use ra_text_edit::TextEdit;
#[derive(Debug, Clone)]
pub struct SourceChange {
/// For display in the undo log in the editor
pub label: String,
pub source_file_edits: Vec<SourceFileEdit>,
pub file_system_edits: Vec<FileSystemEdit>,
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<L: Into<String>>(
label: L,
pub fn from_edits(
source_file_edits: Vec<SourceFileEdit>,
file_system_edits: Vec<FileSystemEdit>,
) -> 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<L: Into<String>>(label: L, edits: Vec<SourceFileEdit>) -> 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,
}
pub fn source_file_edits(edits: Vec<SourceFileEdit>) -> Self {
SourceChange { 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<L: Into<String>>(label: L, edits: Vec<FileSystemEdit>) -> 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<L: Into<String>>(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<L: Into<String>>(
label: L,
file_id: FileId,
edit: TextEdit,
) -> Self {
SourceChange::source_file_edit(label, SourceFileEdit { file_id, edit })
}
/// Creates a new SourceChange with the given label
/// from the given `FileId` and `TextEdit`
pub fn file_system_edit<L: Into<String>>(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<SourceFileEdit> 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<FileSystemEdit> 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,
}
}

View File

@ -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);