Simplify fix structure

This commit is contained in:
Kirill Bulatov 2020-08-11 17:13:40 +03:00
parent 37aa68f050
commit 188ec3459e
4 changed files with 93 additions and 97 deletions

View file

@ -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: HirDiagnostics + DiagnosticWithFix>(
d: &D,
sema: &Semantics<RootDatabase>,
) -> Diagnostic {
fn diagnostic_with_fix<D: DiagnosticWithFix>(d: &D, sema: &Semantics<RootDatabase>) -> 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,
},
),
},
]

View file

@ -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<RootDatabase>) -> Option<(Fix, TextRange)>;
/// A [Diagnostic] that potentially has a fix available.
///
/// [Diagnostic]: hir::diagnostics::Diagnostic
pub trait DiagnosticWithFix: Diagnostic {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix>;
}
impl DiagnosticWithFix for UnresolvedModule {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<(Fix, TextRange)> {
let fix = Fix::new(
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
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<RootDatabase>) -> Option<(Fix, TextRange)> {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
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<RootDatabase>) -> Option<(Fix, TextRange)> {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
// 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);
}
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(),
))
return None;
}
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<RootDatabase>) -> Option<(Fix, TextRange)> {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> {
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<RootDatabase>,
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<ast::RecordFieldList> {
match field_def_list {

View file

@ -105,20 +105,26 @@ pub struct Diagnostic {
pub message: String,
pub range: TextRange,
pub severity: Severity,
pub fix: Option<(Fix, TextRange)>,
pub fix: Option<Fix>,
}
#[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<String>, source_change: SourceChange) -> Self {
pub fn new(
label: impl Into<String>,
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 }
}
}

View file

@ -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 {