Separate diagnostics and diagnostics fix ranges

This commit is contained in:
Kirill Bulatov 2020-07-27 15:53:57 +03:00
parent e0de247520
commit 26e102a567
3 changed files with 59 additions and 47 deletions

View file

@ -60,14 +60,16 @@ pub(crate) fn diagnostics(
FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() } FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() }
.into(), .into(),
); );
let range = sema.diagnostics_range(d).range;
res.borrow_mut().push(Diagnostic { res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range, range,
message: d.message(), message: d.message(),
severity: Severity::Error, severity: Severity::Error,
fix: Some(fix), fix: Some((fix, range)),
}) })
}) })
.on::<hir::diagnostics::MissingFields, _>(|d| { .on::<hir::diagnostics::MissingFields, _>(|d| {
let range = sema.diagnostics_range(d).range;
// Note that although we could add a diagnostics to // Note that although we could add a diagnostics to
// fill the missing tuple field, e.g : // fill the missing tuple field, e.g :
// `struct A(usize);` // `struct A(usize);`
@ -91,11 +93,15 @@ pub(crate) fn diagnostics(
.into_text_edit(&mut builder); .into_text_edit(&mut builder);
builder.finish() builder.finish()
}; };
Some(Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into())) Some((
Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()),
range,
))
}; };
res.borrow_mut().push(Diagnostic { res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range, // TODO kb use a smaller range here
range,
message: d.message(), message: d.message(),
severity: Severity::Error, severity: Severity::Error,
fix, fix,
@ -106,20 +112,21 @@ pub(crate) fn diagnostics(
let replacement = format!("Ok({})", node.syntax()); let replacement = format!("Ok({})", node.syntax());
let edit = TextEdit::replace(node.syntax().text_range(), replacement); let edit = TextEdit::replace(node.syntax().text_range(), replacement);
let source_change = SourceFileEdit { file_id, edit }.into(); let source_change = SourceFileEdit { file_id, edit }.into();
let fix = Fix::new("Wrap with ok", source_change); let range = sema.diagnostics_range(d).range;
res.borrow_mut().push(Diagnostic { res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range, range,
message: d.message(), message: d.message(),
severity: Severity::Error, severity: Severity::Error,
fix: Some(fix), fix: Some((Fix::new("Wrap with ok", source_change), range)),
}) })
}) })
.on::<hir::diagnostics::NoSuchField, _>(|d| { .on::<hir::diagnostics::NoSuchField, _>(|d| {
let range = sema.diagnostics_range(d).range;
res.borrow_mut().push(Diagnostic { res.borrow_mut().push(Diagnostic {
range: sema.diagnostics_range(d).range, range,
message: d.message(), message: d.message(),
severity: Severity::Error, severity: Severity::Error,
fix: missing_struct_field_fix(&sema, file_id, d), fix: missing_struct_field_fix(&sema, file_id, d).map(|fix| (fix, range)),
}) })
}) })
// Only collect experimental diagnostics when they're enabled. // Only collect experimental diagnostics when they're enabled.
@ -222,24 +229,24 @@ fn check_unnecessary_braces_in_use_statement(
) -> Option<()> { ) -> Option<()> {
let use_tree_list = ast::UseTreeList::cast(node.clone())?; let use_tree_list = ast::UseTreeList::cast(node.clone())?;
if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() { if let Some((single_use_tree,)) = use_tree_list.use_trees().collect_tuple() {
let range = use_tree_list.syntax().text_range(); let use_range = use_tree_list.syntax().text_range();
let edit = let edit =
text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree) text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree)
.unwrap_or_else(|| { .unwrap_or_else(|| {
let to_replace = single_use_tree.syntax().text().to_string(); let to_replace = single_use_tree.syntax().text().to_string();
let mut edit_builder = TextEditBuilder::default(); let mut edit_builder = TextEditBuilder::default();
edit_builder.delete(range); edit_builder.delete(use_range);
edit_builder.insert(range.start(), to_replace); edit_builder.insert(use_range.start(), to_replace);
edit_builder.finish() edit_builder.finish()
}); });
acc.push(Diagnostic { acc.push(Diagnostic {
range, range: use_range,
message: "Unnecessary braces in use statement".to_string(), message: "Unnecessary braces in use statement".to_string(),
severity: Severity::WeakWarning, severity: Severity::WeakWarning,
fix: Some(Fix::new( fix: Some((
"Remove unnecessary braces", Fix::new("Remove unnecessary braces", SourceFileEdit { file_id, edit }.into()),
SourceFileEdit { file_id, edit }.into(), use_range,
)), )),
}); });
} }
@ -254,8 +261,7 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
if single_use_tree.path()?.segment()?.syntax().first_child_or_token()?.kind() == T![self] { if single_use_tree.path()?.segment()?.syntax().first_child_or_token()?.kind() == T![self] {
let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start();
let end = use_tree_list_node.text_range().end(); let end = use_tree_list_node.text_range().end();
let range = TextRange::new(start, end); return Some(TextEdit::delete(TextRange::new(start, end)));
return Some(TextEdit::delete(range));
} }
None None
} }
@ -278,13 +284,17 @@ fn check_struct_shorthand_initialization(
edit_builder.insert(record_field.syntax().text_range().start(), field_name); edit_builder.insert(record_field.syntax().text_range().start(), field_name);
let edit = edit_builder.finish(); let edit = edit_builder.finish();
let field_range = record_field.syntax().text_range();
acc.push(Diagnostic { acc.push(Diagnostic {
range: record_field.syntax().text_range(), range: field_range,
message: "Shorthand struct initialization".to_string(), message: "Shorthand struct initialization".to_string(),
severity: Severity::WeakWarning, severity: Severity::WeakWarning,
fix: Some(Fix::new( fix: Some((
"Use struct shorthand initialization", Fix::new(
SourceFileEdit { file_id, edit }.into(), "Use struct shorthand initialization",
SourceFileEdit { file_id, edit }.into(),
),
field_range,
)), )),
}); });
} }
@ -304,14 +314,14 @@ mod tests {
/// Takes a multi-file input fixture with annotated cursor positions, /// Takes a multi-file input fixture with annotated cursor positions,
/// and checks that: /// and checks that:
/// * a diagnostic is produced /// * a diagnostic is produced
/// * this diagnostic touches the input cursor position /// * this diagnostic fix touches the input cursor position
/// * that the contents of the file containing the cursor match `after` after the diagnostic fix is applied /// * that the contents of the file containing the cursor match `after` after the diagnostic fix is applied
fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) {
let after = trim_indent(ra_fixture_after); let after = trim_indent(ra_fixture_after);
let (analysis, file_position) = analysis_and_position(ra_fixture_before); let (analysis, file_position) = analysis_and_position(ra_fixture_before);
let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap(); let diagnostic = analysis.diagnostics(file_position.file_id, true).unwrap().pop().unwrap();
let mut fix = diagnostic.fix.unwrap(); let (mut fix, fix_range) = diagnostic.fix.unwrap();
let edit = fix.source_change.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 target_file_contents = analysis.file_text(file_position.file_id).unwrap();
let actual = { let actual = {
@ -322,10 +332,9 @@ mod tests {
assert_eq_text!(&after, &actual); assert_eq_text!(&after, &actual);
assert!( assert!(
diagnostic.range.start() <= file_position.offset fix_range.start() <= file_position.offset && fix_range.end() >= file_position.offset,
&& diagnostic.range.end() >= file_position.offset, "diagnostic fix range {:?} does not touch cursor position {:?}",
"diagnostic range {:?} does not touch cursor position {:?}", fix_range,
diagnostic.range,
file_position.offset file_position.offset
); );
} }
@ -337,7 +346,7 @@ mod tests {
let (analysis, file_pos) = analysis_and_position(ra_fixture_before); let (analysis, file_pos) = analysis_and_position(ra_fixture_before);
let current_file_id = file_pos.file_id; let current_file_id = file_pos.file_id;
let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap(); let diagnostic = analysis.diagnostics(current_file_id, true).unwrap().pop().unwrap();
let mut fix = diagnostic.fix.unwrap(); let mut fix = diagnostic.fix.unwrap().0;
let edit = fix.source_change.source_file_edits.pop().unwrap(); let edit = fix.source_change.source_file_edits.pop().unwrap();
let changed_file_id = edit.file_id; let changed_file_id = edit.file_id;
let before = analysis.file_text(changed_file_id).unwrap(); let before = analysis.file_text(changed_file_id).unwrap();
@ -628,21 +637,24 @@ fn test_fn() {
range: 0..8, range: 0..8,
severity: Error, severity: Error,
fix: Some( fix: Some(
Fix { (
label: "Create module", Fix {
source_change: SourceChange { label: "Create module",
source_file_edits: [], source_change: SourceChange {
file_system_edits: [ source_file_edits: [],
CreateFile { file_system_edits: [
anchor: FileId( CreateFile {
1, anchor: FileId(
), 1,
dst: "foo.rs", ),
}, dst: "foo.rs",
], },
is_snippet: false, ],
is_snippet: false,
},
}, },
}, 0..8,
),
), ),
}, },
] ]

View file

@ -105,7 +105,7 @@ pub struct Diagnostic {
pub message: String, pub message: String,
pub range: TextRange, pub range: TextRange,
pub severity: Severity, pub severity: Severity,
pub fix: Option<Fix>, pub fix: Option<(Fix, TextRange)>,
} }
#[derive(Debug)] #[derive(Debug)]

View file

@ -775,9 +775,9 @@ fn handle_fixes(
let fixes_from_diagnostics = diagnostics let fixes_from_diagnostics = diagnostics
.into_iter() .into_iter()
.filter_map(|d| Some((d.range, d.fix?))) .filter_map(|d| d.fix)
.filter(|(diag_range, _fix)| diag_range.intersect(range).is_some()) .filter(|(_fix, fix_range)| fix_range.intersect(range).is_some())
.map(|(_range, fix)| fix); .map(|(fix, _range)| fix);
for fix in fixes_from_diagnostics { for fix in fixes_from_diagnostics {
let title = fix.label; let title = fix.label;
let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?; let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?;