internal: prepare for lazy diagnostics

This commit is contained in:
Aleksey Kladov 2021-04-12 17:58:01 +03:00
parent cae920a1bb
commit 426d098bd6
6 changed files with 96 additions and 77 deletions

View file

@ -25,7 +25,7 @@ use syntax::{
use text_edit::TextEdit; use text_edit::TextEdit;
use unlinked_file::UnlinkedFile; use unlinked_file::UnlinkedFile;
use crate::{FileId, Label, SourceChange}; use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange};
use self::fixes::DiagnosticWithFix; use self::fixes::DiagnosticWithFix;
@ -35,7 +35,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<Assist>,
pub unused: bool, pub unused: bool,
pub code: Option<DiagnosticCode>, pub code: Option<DiagnosticCode>,
} }
@ -56,7 +56,7 @@ impl Diagnostic {
} }
} }
fn with_fix(self, fix: Option<Fix>) -> Self { fn with_fix(self, fix: Option<Assist>) -> Self {
Self { fix, ..self } Self { fix, ..self }
} }
@ -69,21 +69,6 @@ impl Diagnostic {
} }
} }
#[derive(Debug)]
pub struct Fix {
pub label: Label,
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 {
fn new(label: &str, source_change: SourceChange, fix_trigger_range: TextRange) -> Self {
let label = Label::new(label);
Self { label, source_change, fix_trigger_range }
}
}
#[derive(Debug, Copy, Clone)] #[derive(Debug, Copy, Clone)]
pub enum Severity { pub enum Severity {
Error, Error,
@ -261,7 +246,8 @@ fn check_unnecessary_braces_in_use_statement(
acc.push( acc.push(
Diagnostic::hint(use_range, "Unnecessary braces in use statement".to_string()) Diagnostic::hint(use_range, "Unnecessary braces in use statement".to_string())
.with_fix(Some(Fix::new( .with_fix(Some(fix(
"remove_braces",
"Remove unnecessary braces", "Remove unnecessary braces",
SourceChange::from_text_edit(file_id, edit), SourceChange::from_text_edit(file_id, edit),
use_range, use_range,
@ -284,6 +270,17 @@ fn text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(
None None
} }
fn fix(id: &'static str, label: &str, source_change: SourceChange, target: TextRange) -> Assist {
assert!(!id.contains(' '));
Assist {
id: AssistId(id, AssistKind::QuickFix),
label: Label::new(label),
group: None,
target,
source_change: Some(source_change),
}
}
#[cfg(test)] #[cfg(test)]
mod tests { mod tests {
use expect_test::{expect, Expect}; use expect_test::{expect, Expect};
@ -308,10 +305,11 @@ mod tests {
.unwrap(); .unwrap();
let fix = diagnostic.fix.unwrap(); let fix = diagnostic.fix.unwrap();
let actual = { let actual = {
let file_id = *fix.source_change.source_file_edits.keys().next().unwrap(); let source_change = fix.source_change.unwrap();
let file_id = *source_change.source_file_edits.keys().next().unwrap();
let mut actual = analysis.file_text(file_id).unwrap().to_string(); let mut actual = analysis.file_text(file_id).unwrap().to_string();
for edit in fix.source_change.source_file_edits.values() { for edit in source_change.source_file_edits.values() {
edit.apply(&mut actual); edit.apply(&mut actual);
} }
actual actual
@ -319,9 +317,9 @@ mod tests {
assert_eq_text!(&after, &actual); assert_eq_text!(&after, &actual);
assert!( assert!(
fix.fix_trigger_range.contains_inclusive(file_position.offset), fix.target.contains_inclusive(file_position.offset),
"diagnostic fix range {:?} does not touch cursor position {:?}", "diagnostic fix range {:?} does not touch cursor position {:?}",
fix.fix_trigger_range, fix.target,
file_position.offset file_position.offset
); );
} }
@ -665,9 +663,16 @@ fn test_fn() {
range: 0..8, range: 0..8,
severity: Error, severity: Error,
fix: Some( fix: Some(
Fix { Assist {
id: AssistId(
"create_module",
QuickFix,
),
label: "Create module", label: "Create module",
source_change: SourceChange { group: None,
target: 0..8,
source_change: Some(
SourceChange {
source_file_edits: {}, source_file_edits: {},
file_system_edits: [ file_system_edits: [
CreateFile { CreateFile {
@ -682,7 +687,7 @@ fn test_fn() {
], ],
is_snippet: false, is_snippet: false,
}, },
fix_trigger_range: 0..8, ),
}, },
), ),
unused: false, unused: false,

View file

@ -5,7 +5,7 @@ use ide_db::{base_db::FileId, source_change::SourceChange};
use syntax::{ast, match_ast, AstNode, SyntaxNode}; use syntax::{ast, match_ast, AstNode, SyntaxNode};
use text_edit::TextEdit; use text_edit::TextEdit;
use crate::{Diagnostic, Fix}; use crate::{diagnostics::fix, Diagnostic};
pub(super) fn check(acc: &mut Vec<Diagnostic>, file_id: FileId, node: &SyntaxNode) { pub(super) fn check(acc: &mut Vec<Diagnostic>, file_id: FileId, node: &SyntaxNode) {
match_ast! { match_ast! {
@ -47,7 +47,8 @@ fn check_expr_field_shorthand(
let field_range = record_field.syntax().text_range(); let field_range = record_field.syntax().text_range();
acc.push( acc.push(
Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()).with_fix( Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()).with_fix(
Some(Fix::new( Some(fix(
"use_expr_field_shorthand",
"Use struct shorthand initialization", "Use struct shorthand initialization",
SourceChange::from_text_edit(file_id, edit), SourceChange::from_text_edit(file_id, edit),
field_range, field_range,
@ -86,7 +87,8 @@ fn check_pat_field_shorthand(
let field_range = record_pat_field.syntax().text_range(); let field_range = record_pat_field.syntax().text_range();
acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fix( acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fix(
Some(Fix::new( Some(fix(
"use_pat_field_shorthand",
"Use struct field shorthand", "Use struct field shorthand",
SourceChange::from_text_edit(file_id, edit), SourceChange::from_text_edit(file_id, edit),
field_range, field_range,

View file

@ -20,20 +20,21 @@ use syntax::{
}; };
use text_edit::TextEdit; use text_edit::TextEdit;
use crate::{diagnostics::Fix, references::rename::rename_with_semantics, FilePosition}; use crate::{diagnostics::fix, references::rename::rename_with_semantics, Assist, FilePosition};
/// A [Diagnostic] that potentially has a fix available. /// A [Diagnostic] that potentially has a fix available.
/// ///
/// [Diagnostic]: hir::diagnostics::Diagnostic /// [Diagnostic]: hir::diagnostics::Diagnostic
pub(crate) trait DiagnosticWithFix: Diagnostic { pub(crate) trait DiagnosticWithFix: Diagnostic {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix>; fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist>;
} }
impl DiagnosticWithFix for UnresolvedModule { impl DiagnosticWithFix for UnresolvedModule {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
let unresolved_module = self.decl.to_node(&root); let unresolved_module = self.decl.to_node(&root);
Some(Fix::new( Some(fix(
"create_module",
"Create module", "Create module",
FileSystemEdit::CreateFile { FileSystemEdit::CreateFile {
dst: AnchoredPathBuf { dst: AnchoredPathBuf {
@ -49,7 +50,7 @@ impl DiagnosticWithFix for UnresolvedModule {
} }
impl DiagnosticWithFix for NoSuchField { impl DiagnosticWithFix for NoSuchField {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
missing_record_expr_field_fix( missing_record_expr_field_fix(
&sema, &sema,
@ -60,7 +61,7 @@ impl DiagnosticWithFix for NoSuchField {
} }
impl DiagnosticWithFix for MissingFields { impl DiagnosticWithFix for MissingFields {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
// 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);`
@ -86,7 +87,8 @@ impl DiagnosticWithFix for MissingFields {
.into_text_edit(&mut builder); .into_text_edit(&mut builder);
builder.finish() builder.finish()
}; };
Some(Fix::new( Some(fix(
"fill_missing_fields",
"Fill struct fields", "Fill struct fields",
SourceChange::from_text_edit(self.file.original_file(sema.db), edit), SourceChange::from_text_edit(self.file.original_file(sema.db), edit),
sema.original_range(&field_list_parent.syntax()).range, sema.original_range(&field_list_parent.syntax()).range,
@ -95,7 +97,7 @@ impl DiagnosticWithFix for MissingFields {
} }
impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
let tail_expr = self.expr.to_node(&root); let tail_expr = self.expr.to_node(&root);
let tail_expr_range = tail_expr.syntax().text_range(); let tail_expr_range = tail_expr.syntax().text_range();
@ -103,12 +105,12 @@ impl DiagnosticWithFix for MissingOkOrSomeInTailExpr {
let edit = TextEdit::replace(tail_expr_range, replacement); let edit = TextEdit::replace(tail_expr_range, replacement);
let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" }; let name = if self.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" };
Some(Fix::new(name, source_change, tail_expr_range)) Some(fix("wrap_tail_expr", name, source_change, tail_expr_range))
} }
} }
impl DiagnosticWithFix for RemoveThisSemicolon { impl DiagnosticWithFix for RemoveThisSemicolon {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
let semicolon = self let semicolon = self
@ -123,12 +125,12 @@ impl DiagnosticWithFix for RemoveThisSemicolon {
let edit = TextEdit::delete(semicolon); let edit = TextEdit::delete(semicolon);
let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
Some(Fix::new("Remove this semicolon", source_change, semicolon)) Some(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon))
} }
} }
impl DiagnosticWithFix for IncorrectCase { impl DiagnosticWithFix for IncorrectCase {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
let name_node = self.ident.to_node(&root); let name_node = self.ident.to_node(&root);
@ -140,12 +142,12 @@ impl DiagnosticWithFix for IncorrectCase {
rename_with_semantics(sema, file_position, &self.suggested_text).ok()?; rename_with_semantics(sema, file_position, &self.suggested_text).ok()?;
let label = format!("Rename to {}", self.suggested_text); let label = format!("Rename to {}", self.suggested_text);
Some(Fix::new(&label, rename_changes, frange.range)) Some(fix("change_case", &label, rename_changes, frange.range))
} }
} }
impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap { impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &Semantics<RootDatabase>) -> Option<Assist> {
let root = sema.db.parse_or_expand(self.file)?; let root = sema.db.parse_or_expand(self.file)?;
let next_expr = self.next_expr.to_node(&root); let next_expr = self.next_expr.to_node(&root);
let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?;
@ -163,7 +165,8 @@ impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap {
let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit);
Some(Fix::new( Some(fix(
"replace_with_find_map",
"Replace filter_map(..).next() with find_map()", "Replace filter_map(..).next() with find_map()",
source_change, source_change,
trigger_range, trigger_range,
@ -175,7 +178,7 @@ fn missing_record_expr_field_fix(
sema: &Semantics<RootDatabase>, sema: &Semantics<RootDatabase>,
usage_file_id: FileId, usage_file_id: FileId,
record_expr_field: &ast::RecordExprField, record_expr_field: &ast::RecordExprField,
) -> Option<Fix> { ) -> Option<Assist> {
let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?;
let def_id = sema.resolve_variant(record_lit)?; let def_id = sema.resolve_variant(record_lit)?;
let module; let module;
@ -233,7 +236,12 @@ fn missing_record_expr_field_fix(
def_file_id, def_file_id,
TextEdit::insert(last_field_syntax.text_range().end(), new_field), TextEdit::insert(last_field_syntax.text_range().end(), new_field),
); );
return Some(Fix::new("Create field", source_change, record_expr_field.syntax().text_range())); return Some(fix(
"create_field",
"Create field",
source_change,
record_expr_field.syntax().text_range(),
));
fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> { fn record_field_list(field_def_list: ast::FieldList) -> Option<ast::RecordFieldList> {
match field_def_list { match field_def_list {

View file

@ -16,9 +16,10 @@ use syntax::{
}; };
use text_edit::TextEdit; use text_edit::TextEdit;
use crate::Fix; use crate::{
diagnostics::{fix, fixes::DiagnosticWithFix},
use super::fixes::DiagnosticWithFix; Assist,
};
// Diagnostic: unlinked-file // Diagnostic: unlinked-file
// //
@ -49,7 +50,7 @@ impl Diagnostic for UnlinkedFile {
} }
impl DiagnosticWithFix for UnlinkedFile { impl DiagnosticWithFix for UnlinkedFile {
fn fix(&self, sema: &hir::Semantics<RootDatabase>) -> Option<Fix> { fn fix(&self, sema: &hir::Semantics<RootDatabase>) -> Option<Assist> {
// If there's an existing module that could add a `mod` item to include the unlinked file, // If there's an existing module that could add a `mod` item to include the unlinked file,
// suggest that as a fix. // suggest that as a fix.
@ -100,7 +101,7 @@ fn make_fix(
parent_file_id: FileId, parent_file_id: FileId,
new_mod_name: &str, new_mod_name: &str,
added_file_id: FileId, added_file_id: FileId,
) -> Option<Fix> { ) -> Option<Assist> {
fn is_outline_mod(item: &ast::Item) -> bool { fn is_outline_mod(item: &ast::Item) -> bool {
matches!(item, ast::Item::Module(m) if m.item_list().is_none()) matches!(item, ast::Item::Module(m) if m.item_list().is_none())
} }
@ -152,7 +153,8 @@ fn make_fix(
let edit = builder.finish(); let edit = builder.finish();
let trigger_range = db.parse(added_file_id).tree().syntax().text_range(); let trigger_range = db.parse(added_file_id).tree().syntax().text_range();
Some(Fix::new( Some(fix(
"add_mod_declaration",
&format!("Insert `{}`", mod_decl), &format!("Insert `{}`", mod_decl),
SourceChange::from_text_edit(parent_file_id, edit), SourceChange::from_text_edit(parent_file_id, edit),
trigger_range, trigger_range,

View file

@ -69,7 +69,7 @@ use crate::display::ToNav;
pub use crate::{ pub use crate::{
annotations::{Annotation, AnnotationConfig, AnnotationKind}, annotations::{Annotation, AnnotationConfig, AnnotationKind},
call_hierarchy::CallItem, call_hierarchy::CallItem,
diagnostics::{Diagnostic, DiagnosticsConfig, Fix, Severity}, diagnostics::{Diagnostic, DiagnosticsConfig, Severity},
display::navigation_target::NavigationTarget, display::navigation_target::NavigationTarget,
expand_macro::ExpandedMacro, expand_macro::ExpandedMacro,
file_structure::{StructureNode, StructureNodeKind}, file_structure::{StructureNode, StructureNodeKind},

View file

@ -1039,9 +1039,10 @@ fn add_quick_fixes(
for fix in diagnostics for fix in diagnostics
.into_iter() .into_iter()
.filter_map(|d| d.fix) .filter_map(|d| d.fix)
.filter(|fix| fix.fix_trigger_range.intersect(frange.range).is_some()) .filter(|fix| fix.target.intersect(frange.range).is_some())
{ {
let edit = to_proto::snippet_workspace_edit(&snap, fix.source_change)?; if let Some(source_change) = fix.source_change {
let edit = to_proto::snippet_workspace_edit(&snap, source_change)?;
let action = lsp_ext::CodeAction { let action = lsp_ext::CodeAction {
title: fix.label.to_string(), title: fix.label.to_string(),
group: None, group: None,
@ -1052,6 +1053,7 @@ fn add_quick_fixes(
}; };
acc.push(action); acc.push(action);
} }
}
for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() { for fix in snap.check_fixes.get(&frange.file_id).into_iter().flatten() {
let fix_range = from_proto::text_range(&line_index, fix.range); let fix_range = from_proto::text_range(&line_index, fix.range);