From 26e102a567aadcf86f2e5b575cb6b915991ba088 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 15:53:57 +0300 Subject: Separate diagnostics and diagnostics fix ranges --- crates/ra_ide/src/diagnostics.rs | 98 ++++++++++++++++++++++------------------ crates/ra_ide/src/lib.rs | 2 +- 2 files changed, 56 insertions(+), 44 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 73c0b8275..5c8ea46ab 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -60,14 +60,16 @@ pub(crate) fn diagnostics( FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() } .into(), ); + let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range, message: d.message(), severity: Severity::Error, - fix: Some(fix), + fix: Some((fix, range)), }) }) .on::(|d| { + let range = sema.diagnostics_range(d).range; // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -91,11 +93,15 @@ pub(crate) fn diagnostics( .into_text_edit(&mut builder); 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 { - range: sema.diagnostics_range(d).range, + // TODO kb use a smaller range here + range, message: d.message(), severity: Severity::Error, fix, @@ -106,20 +112,21 @@ pub(crate) fn diagnostics( let replacement = format!("Ok({})", node.syntax()); let edit = TextEdit::replace(node.syntax().text_range(), replacement); 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 { - range: sema.diagnostics_range(d).range, + range, message: d.message(), severity: Severity::Error, - fix: Some(fix), + fix: Some((Fix::new("Wrap with ok", source_change), range)), }) }) .on::(|d| { + let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range, message: d.message(), 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. @@ -222,24 +229,24 @@ fn check_unnecessary_braces_in_use_statement( ) -> Option<()> { let use_tree_list = ast::UseTreeList::cast(node.clone())?; 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 = text_edit_for_remove_unnecessary_braces_with_self_in_use_statement(&single_use_tree) .unwrap_or_else(|| { let to_replace = single_use_tree.syntax().text().to_string(); let mut edit_builder = TextEditBuilder::default(); - edit_builder.delete(range); - edit_builder.insert(range.start(), to_replace); + edit_builder.delete(use_range); + edit_builder.insert(use_range.start(), to_replace); edit_builder.finish() }); acc.push(Diagnostic { - range, + 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, )), }); } @@ -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] { let start = use_tree_list_node.prev_sibling_or_token()?.text_range().start(); let end = use_tree_list_node.text_range().end(); - let range = TextRange::new(start, end); - return Some(TextEdit::delete(range)); + return Some(TextEdit::delete(TextRange::new(start, end))); } None } @@ -278,13 +284,17 @@ fn check_struct_shorthand_initialization( edit_builder.insert(record_field.syntax().text_range().start(), field_name); let edit = edit_builder.finish(); + let field_range = record_field.syntax().text_range(); acc.push(Diagnostic { - range: record_field.syntax().text_range(), + 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, )), }); } @@ -304,14 +314,14 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: /// * 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 fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { let after = trim_indent(ra_fixture_after); 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 = diagnostic.fix.unwrap(); + let (mut fix, fix_range) = 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 = { @@ -322,10 +332,9 @@ mod tests { assert_eq_text!(&after, &actual); assert!( - diagnostic.range.start() <= file_position.offset - && diagnostic.range.end() >= file_position.offset, - "diagnostic range {:?} does not touch cursor position {:?}", - diagnostic.range, + fix_range.start() <= file_position.offset && fix_range.end() >= file_position.offset, + "diagnostic fix range {:?} does not touch cursor position {:?}", + fix_range, file_position.offset ); } @@ -337,7 +346,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(); + let mut fix = diagnostic.fix.unwrap().0; 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(); @@ -628,21 +637,24 @@ 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, + ), ), }, ] diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 0fede0d87..45a4b2421 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -105,7 +105,7 @@ pub struct Diagnostic { pub message: String, pub range: TextRange, pub severity: Severity, - pub fix: Option, + pub fix: Option<(Fix, TextRange)>, } #[derive(Debug)] -- cgit v1.2.3 From 21e5224484b9214648826e1b15aa9150c79a407c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 18:45:08 +0300 Subject: Custom ranges for missing fields --- crates/ra_ide/src/diagnostics.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 5c8ea46ab..7ae4bda0b 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -100,8 +100,10 @@ pub(crate) fn diagnostics( }; res.borrow_mut().push(Diagnostic { - // TODO kb use a smaller range here - range, + range: d + .list_parent_ast(db) + .map(|path| path.syntax().text_range()) + .unwrap_or(range), message: d.message(), severity: Severity::Error, fix, -- cgit v1.2.3 From a61f2445cba2a48bb7ea6c8477e3198b297f3c67 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 22:30:55 +0300 Subject: Less stubs --- crates/ra_ide/src/diagnostics.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 7ae4bda0b..e847df6ea 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -69,7 +69,6 @@ pub(crate) fn diagnostics( }) }) .on::(|d| { - let range = sema.diagnostics_range(d).range; // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -95,15 +94,12 @@ pub(crate) fn diagnostics( }; Some(( Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - range, + sema.diagnostics_range(d).range, )) }; res.borrow_mut().push(Diagnostic { - range: d - .list_parent_ast(db) - .map(|path| path.syntax().text_range()) - .unwrap_or(range), + range: d.highlighting_source().file_syntax(db).text_range(), message: d.message(), severity: Severity::Error, fix, -- cgit v1.2.3 From ee1586c1ed058ff0f090b552d52fe6bbe2dd7f7f Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 27 Jul 2020 22:46:25 +0300 Subject: Better naming --- crates/ra_ide/src/diagnostics.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index e847df6ea..0d2ff17e1 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -54,18 +54,19 @@ pub(crate) fn diagnostics( let res = RefCell::new(res); let mut sink = DiagnosticSinkBuilder::new() .on::(|d| { - let original_file = d.source().file_id.original_file(db); let fix = Fix::new( "Create module", - FileSystemEdit::CreateFile { anchor: original_file, dst: d.candidate.clone() } - .into(), + FileSystemEdit::CreateFile { + anchor: d.file.original_file(db), + dst: d.candidate.clone(), + } + .into(), ); - let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range, + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((fix, range)), + fix: Some((fix, sema.diagnostics_fix_range(d).range)), }) }) .on::(|d| { @@ -94,12 +95,12 @@ pub(crate) fn diagnostics( }; Some(( Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - sema.diagnostics_range(d).range, + sema.diagnostics_fix_range(d).range, )) }; res.borrow_mut().push(Diagnostic { - range: d.highlighting_source().file_syntax(db).text_range(), + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, fix, @@ -110,21 +111,23 @@ pub(crate) fn diagnostics( let replacement = format!("Ok({})", node.syntax()); let edit = TextEdit::replace(node.syntax().text_range(), replacement); let source_change = SourceFileEdit { file_id, edit }.into(); - let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range, + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((Fix::new("Wrap with ok", source_change), range)), + fix: Some(( + Fix::new("Wrap with ok", source_change), + sema.diagnostics_fix_range(d).range, + )), }) }) .on::(|d| { - let range = sema.diagnostics_range(d).range; res.borrow_mut().push(Diagnostic { - range, + range: sema.diagnostics_range(d).range, message: d.message(), severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d).map(|fix| (fix, range)), + fix: missing_struct_field_fix(&sema, file_id, d) + .map(|fix| (fix, sema.diagnostics_fix_range(d).range)), }) }) // Only collect experimental diagnostics when they're enabled. -- cgit v1.2.3 From 9963f43d51071ea02f8f6d490b9c49882034b42c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Sun, 9 Aug 2020 01:59:26 +0300 Subject: Refactor the diagnostics --- crates/ra_ide/src/diagnostics.rs | 76 +++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 37 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 0d2ff17e1..55593a8cb 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -7,7 +7,7 @@ use std::cell::RefCell; use hir::{ - diagnostics::{AstDiagnostic, Diagnostic as _, DiagnosticSinkBuilder}, + diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, HasSource, HirDisplay, Semantics, VariantDef, }; use itertools::Itertools; @@ -63,10 +63,10 @@ pub(crate) fn diagnostics( .into(), ); res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((fix, sema.diagnostics_fix_range(d).range)), + fix: Some((fix, sema.diagnostic_fix_source(d).syntax().text_range())), }) }) .on::(|d| { @@ -78,56 +78,58 @@ pub(crate) fn diagnostics( let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { None } else { - let mut field_list = d.ast(db); - for f in d.missed_fields.iter() { - let field = make::record_expr_field( - make::name_ref(&f.to_string()), - Some(make::expr_unit()), - ); - field_list = field_list.append_field(&field); + let record_expr = sema.diagnostic_fix_source(d); + if let Some(old_field_list) = record_expr.record_expr_field_list() { + let mut new_field_list = old_field_list.clone(); + for f in d.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, edit }.into()), + sema.original_range(&old_field_list.syntax()).range, + )) + } else { + None } - - 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()), - sema.diagnostics_fix_range(d).range, - )) }; res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, fix, }) }) .on::(|d| { - let node = d.ast(db); - let replacement = format!("Ok({})", node.syntax()); - let edit = TextEdit::replace(node.syntax().text_range(), replacement); + let tail_expr = sema.diagnostic_fix_source(d); + 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, edit }.into(); res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some(( - Fix::new("Wrap with ok", source_change), - sema.diagnostics_fix_range(d).range, - )), + fix: Some((Fix::new("Wrap with ok", source_change), tail_expr_range)), }) }) .on::(|d| { res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, fix: missing_struct_field_fix(&sema, file_id, d) - .map(|fix| (fix, sema.diagnostics_fix_range(d).range)), + .map(|fix| (fix, sema.diagnostic_fix_source(d).syntax().text_range())), }) }) // Only collect experimental diagnostics when they're enabled. @@ -136,7 +138,7 @@ pub(crate) fn diagnostics( .build(|d| { res.borrow_mut().push(Diagnostic { message: d.message(), - range: sema.diagnostics_range(d).range, + range: sema.diagnostics_presentation_range(d).range, severity: Severity::Error, fix: None, }) @@ -154,9 +156,9 @@ fn missing_struct_field_fix( usage_file_id: FileId, d: &hir::diagnostics::NoSuchField, ) -> Option { - let record_expr = sema.ast(d); + let record_expr_field = sema.diagnostic_fix_source(d); - let record_lit = ast::RecordExpr::cast(record_expr.syntax().parent()?.parent()?)?; + let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; let module; let def_file_id; @@ -184,12 +186,12 @@ fn missing_struct_field_fix( }; let def_file_id = def_file_id.original_file(sema.db); - let new_field_type = sema.type_of_expr(&record_expr.expr()?)?; + let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?; if new_field_type.is_unknown() { return None; } let new_field = make::record_field( - record_expr.field_name()?, + record_expr_field.field_name()?, make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), ); -- cgit v1.2.3 From 936861993935d5b2c78b953e2f4b719e1992bd73 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Mon, 10 Aug 2020 22:53:10 +0300 Subject: Make the fix AST source Optional --- crates/ra_ide/src/diagnostics.rs | 87 +++++++++++++++++++++++----------------- 1 file changed, 50 insertions(+), 37 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 55593a8cb..043ce357b 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -62,12 +62,18 @@ pub(crate) fn diagnostics( } .into(), ); + let fix = sema + .diagnostic_fix_source(d) + .map(|unresolved_module| unresolved_module.syntax().text_range()) + .map(|fix_range| (fix, fix_range)); + res.borrow_mut().push(Diagnostic { range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((fix, sema.diagnostic_fix_source(d).syntax().text_range())), - }) + fix, + }); + Some(()) }) .on::(|d| { // Note that although we could add a diagnostics to @@ -78,30 +84,29 @@ pub(crate) fn diagnostics( let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { None } else { - let record_expr = sema.diagnostic_fix_source(d); - if let Some(old_field_list) = record_expr.record_expr_field_list() { - let mut new_field_list = old_field_list.clone(); - for f in d.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, edit }.into()), - sema.original_range(&old_field_list.syntax()).range, - )) - } else { - None - } + sema.diagnostic_fix_source(d) + .and_then(|record_expr| record_expr.record_expr_field_list()) + .map(|old_field_list| { + let mut new_field_list = old_field_list.clone(); + for f in d.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() + }; + ( + Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), + sema.original_range(&old_field_list.syntax()).range, + ) + }) }; res.borrow_mut().push(Diagnostic { @@ -109,28 +114,36 @@ pub(crate) fn diagnostics( message: d.message(), severity: Severity::Error, fix, - }) + }); + Some(()) }) .on::(|d| { - let tail_expr = sema.diagnostic_fix_source(d); - 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, edit }.into(); + let fix = sema.diagnostic_fix_source(d).map(|tail_expr| { + 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, edit }.into(); + (Fix::new("Wrap with ok", source_change), tail_expr_range) + }); + res.borrow_mut().push(Diagnostic { range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: Some((Fix::new("Wrap with ok", source_change), tail_expr_range)), - }) + fix, + }); + Some(()) }) .on::(|d| { res.borrow_mut().push(Diagnostic { range: sema.diagnostics_presentation_range(d).range, message: d.message(), severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d) - .map(|fix| (fix, sema.diagnostic_fix_source(d).syntax().text_range())), - }) + fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| { + Some((fix, sema.diagnostic_fix_source(d)?.syntax().text_range())) + }), + }); + Some(()) }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental) @@ -156,7 +169,7 @@ fn missing_struct_field_fix( usage_file_id: FileId, d: &hir::diagnostics::NoSuchField, ) -> Option { - let record_expr_field = sema.diagnostic_fix_source(d); + let record_expr_field = sema.diagnostic_fix_source(d)?; let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; -- cgit v1.2.3 From 29fbc8e02180aac1f4d7819a9626206aa64028a0 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 00:37:23 +0300 Subject: Move the DiagnosticsWithFix trait on the ide level --- crates/ra_ide/src/diagnostics.rs | 28 +++++++++---- .../ra_ide/src/diagnostics/diagnostics_with_fix.rs | 46 ++++++++++++++++++++++ 2 files changed, 67 insertions(+), 7 deletions(-) create mode 100644 crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 043ce357b..ca1a7c1aa 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -7,11 +7,12 @@ use std::cell::RefCell; use hir::{ + db::AstDatabase, diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, HasSource, HirDisplay, Semantics, VariantDef, }; use itertools::Itertools; -use ra_db::SourceDatabase; +use ra_db::{SourceDatabase, Upcast}; use ra_ide_db::RootDatabase; use ra_prof::profile; use ra_syntax::{ @@ -23,6 +24,9 @@ use ra_text_edit::{TextEdit, TextEditBuilder}; use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; +mod diagnostics_with_fix; +use diagnostics_with_fix::DiagnosticWithFix; + #[derive(Debug, Copy, Clone)] pub enum Severity { Error, @@ -62,8 +66,7 @@ pub(crate) fn diagnostics( } .into(), ); - let fix = sema - .diagnostic_fix_source(d) + let fix = diagnostic_fix_source(&sema, d) .map(|unresolved_module| unresolved_module.syntax().text_range()) .map(|fix_range| (fix, fix_range)); @@ -84,7 +87,7 @@ pub(crate) fn diagnostics( let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { None } else { - sema.diagnostic_fix_source(d) + diagnostic_fix_source(&sema, d) .and_then(|record_expr| record_expr.record_expr_field_list()) .map(|old_field_list| { let mut new_field_list = old_field_list.clone(); @@ -105,6 +108,7 @@ pub(crate) fn diagnostics( ( Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), sema.original_range(&old_field_list.syntax()).range, + // old_field_list.syntax().text_range(), ) }) }; @@ -118,7 +122,7 @@ pub(crate) fn diagnostics( Some(()) }) .on::(|d| { - let fix = sema.diagnostic_fix_source(d).map(|tail_expr| { + let fix = diagnostic_fix_source(&sema, d).map(|tail_expr| { let tail_expr_range = tail_expr.syntax().text_range(); let edit = TextEdit::replace(tail_expr_range, format!("Ok({})", tail_expr.syntax())); @@ -140,7 +144,7 @@ pub(crate) fn diagnostics( message: d.message(), severity: Severity::Error, fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| { - Some((fix, sema.diagnostic_fix_source(d)?.syntax().text_range())) + Some((fix, diagnostic_fix_source(&sema, d)?.syntax().text_range())) }), }); Some(()) @@ -164,12 +168,22 @@ pub(crate) fn diagnostics( res.into_inner() } +fn diagnostic_fix_source( + sema: &Semantics, + d: &T, +) -> Option<::AST> { + let file_id = d.presentation().file_id; + let root = sema.db.parse_or_expand(file_id)?; + sema.cache(root, file_id); + d.fix_source(sema.db.upcast()) +} + fn missing_struct_field_fix( sema: &Semantics, usage_file_id: FileId, d: &hir::diagnostics::NoSuchField, ) -> Option { - let record_expr_field = sema.diagnostic_fix_source(d)?; + let record_expr_field = diagnostic_fix_source(&sema, d)?; let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs new file mode 100644 index 000000000..8578a90ec --- /dev/null +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -0,0 +1,46 @@ +use hir::{ + db::AstDatabase, + diagnostics::{MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, +}; +use ra_syntax::ast; + +// TODO kb +pub trait DiagnosticWithFix { + type AST; + fn fix_source(&self, db: &dyn AstDatabase) -> Option; +} + +impl DiagnosticWithFix for UnresolvedModule { + type AST = ast::Module; + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.decl.to_node(&root)) + } +} + +impl DiagnosticWithFix for NoSuchField { + type AST = ast::RecordExprField; + + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.field.to_node(&root)) + } +} + +impl DiagnosticWithFix for MissingFields { + type AST = ast::RecordExpr; + + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.field_list_parent.to_node(&root)) + } +} + +impl DiagnosticWithFix for MissingOkInTailExpr { + type AST = ast::Expr; + + fn fix_source(&self, db: &dyn AstDatabase) -> Option { + let root = db.parse_or_expand(self.file)?; + Some(self.expr.to_node(&root)) + } +} -- cgit v1.2.3 From c8cad76d25f7fab856c9646b70122e0f9f7d7218 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 00:55:57 +0300 Subject: Improve the ide diagnostics trait API --- crates/ra_ide/src/diagnostics.rs | 188 ++------------------- .../ra_ide/src/diagnostics/diagnostics_with_fix.rs | 163 +++++++++++++++--- 2 files changed, 160 insertions(+), 191 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index ca1a7c1aa..165ff5249 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -7,22 +7,20 @@ use std::cell::RefCell; use hir::{ - db::AstDatabase, - diagnostics::{Diagnostic as _, DiagnosticSinkBuilder}, - HasSource, HirDisplay, Semantics, VariantDef, + diagnostics::{Diagnostic as HirDiagnostics, DiagnosticSinkBuilder}, + Semantics, }; use itertools::Itertools; -use ra_db::{SourceDatabase, Upcast}; +use ra_db::SourceDatabase; use ra_ide_db::RootDatabase; use ra_prof::profile; use ra_syntax::{ - algo, - ast::{self, edit::IndentLevel, make, AstNode}, + ast::{self, AstNode}, SyntaxNode, TextRange, T, }; use ra_text_edit::{TextEdit, TextEditBuilder}; -use crate::{Diagnostic, FileId, FileSystemEdit, Fix, SourceFileEdit}; +use crate::{Diagnostic, FileId, Fix, SourceFileEdit}; mod diagnostics_with_fix; use diagnostics_with_fix::DiagnosticWithFix; @@ -58,96 +56,16 @@ pub(crate) fn diagnostics( let res = RefCell::new(res); let mut sink = DiagnosticSinkBuilder::new() .on::(|d| { - let fix = Fix::new( - "Create module", - FileSystemEdit::CreateFile { - anchor: d.file.original_file(db), - dst: d.candidate.clone(), - } - .into(), - ); - let fix = diagnostic_fix_source(&sema, d) - .map(|unresolved_module| unresolved_module.syntax().text_range()) - .map(|fix_range| (fix, fix_range)); - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - // 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. - let fix = if d.missed_fields.iter().any(|it| it.as_tuple_index().is_some()) { - None - } else { - diagnostic_fix_source(&sema, d) - .and_then(|record_expr| record_expr.record_expr_field_list()) - .map(|old_field_list| { - let mut new_field_list = old_field_list.clone(); - for f in d.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() - }; - ( - Fix::new("Fill struct fields", SourceFileEdit { file_id, edit }.into()), - sema.original_range(&old_field_list.syntax()).range, - // old_field_list.syntax().text_range(), - ) - }) - }; - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - let fix = diagnostic_fix_source(&sema, d).map(|tail_expr| { - 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, edit }.into(); - (Fix::new("Wrap with ok", source_change), tail_expr_range) - }); - - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix, - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) .on::(|d| { - res.borrow_mut().push(Diagnostic { - range: sema.diagnostics_presentation_range(d).range, - message: d.message(), - severity: Severity::Error, - fix: missing_struct_field_fix(&sema, file_id, d).and_then(|fix| { - Some((fix, diagnostic_fix_source(&sema, d)?.syntax().text_range())) - }), - }); - Some(()) + res.borrow_mut().push(diagnostic_with_fix(d, &sema)); }) // Only collect experimental diagnostics when they're enabled. .filter(|diag| !diag.is_experimental() || enable_experimental) @@ -168,87 +86,15 @@ pub(crate) fn diagnostics( res.into_inner() } -fn diagnostic_fix_source( +fn diagnostic_with_fix( + d: &D, sema: &Semantics, - d: &T, -) -> Option<::AST> { - let file_id = d.presentation().file_id; - let root = sema.db.parse_or_expand(file_id)?; - sema.cache(root, file_id); - d.fix_source(sema.db.upcast()) -} - -fn missing_struct_field_fix( - sema: &Semantics, - usage_file_id: FileId, - d: &hir::diagnostics::NoSuchField, -) -> Option { - let record_expr_field = diagnostic_fix_source(&sema, d)?; - - let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; - let def_id = sema.resolve_variant(record_lit)?; - let module; - let def_file_id; - let record_fields = match VariantDef::from(def_id) { - VariantDef::Struct(s) => { - module = s.module(sema.db); - let source = s.source(sema.db); - def_file_id = source.file_id; - let fields = source.value.field_list()?; - record_field_list(fields)? - } - VariantDef::Union(u) => { - module = u.module(sema.db); - let source = u.source(sema.db); - def_file_id = source.file_id; - source.value.record_field_list()? - } - VariantDef::EnumVariant(e) => { - module = e.module(sema.db); - let source = e.source(sema.db); - def_file_id = source.file_id; - let fields = source.value.field_list()?; - record_field_list(fields)? - } - }; - let def_file_id = def_file_id.original_file(sema.db); - - let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?; - if new_field_type.is_unknown() { - return None; - } - let new_field = make::record_field( - record_expr_field.field_name()?, - make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), - ); - - let last_field = record_fields.fields().last()?; - let last_field_syntax = last_field.syntax(); - let indent = IndentLevel::from_node(last_field_syntax); - - let mut new_field = new_field.to_string(); - if usage_file_id != def_file_id { - new_field = format!("pub(crate) {}", new_field); - } - new_field = format!("\n{}{}", indent, new_field); - - let needs_comma = !last_field_syntax.to_string().ends_with(','); - if needs_comma { - new_field = format!(",{}", new_field); - } - - let source_change = SourceFileEdit { - 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); - - fn record_field_list(field_def_list: ast::FieldList) -> Option { - match field_def_list { - ast::FieldList::RecordFieldList(it) => Some(it), - ast::FieldList::TupleFieldList(_) => None, - } +) -> Diagnostic { + Diagnostic { + range: sema.diagnostics_presentation_range(d).range, + message: d.message(), + severity: Severity::Error, + fix: d.fix(&sema), } } diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 8578a90ec..56d454ac6 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,46 +1,169 @@ +use crate::Fix; +use ast::{edit::IndentLevel, make}; use hir::{ db::AstDatabase, diagnostics::{MissingFields, MissingOkInTailExpr, NoSuchField, UnresolvedModule}, + HasSource, HirDisplay, Semantics, VariantDef, }; -use ra_syntax::ast; +use ra_db::FileId; +use ra_ide_db::{ + source_change::{FileSystemEdit, SourceFileEdit}, + RootDatabase, +}; +use ra_syntax::{algo, ast, AstNode, TextRange}; +use ra_text_edit::{TextEdit, TextEditBuilder}; // TODO kb pub trait DiagnosticWithFix { - type AST; - fn fix_source(&self, db: &dyn AstDatabase) -> Option; + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)>; } impl DiagnosticWithFix for UnresolvedModule { - type AST = ast::Module; - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.decl.to_node(&root)) + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + let fix = 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())) } } impl DiagnosticWithFix for NoSuchField { - type AST = ast::RecordExprField; - - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.field.to_node(&root)) + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + 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())) } } impl DiagnosticWithFix for MissingFields { - type AST = ast::RecordExpr; + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + // 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); + } - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.field_list_parent.to_node(&root)) + 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(), + )) + } } } impl DiagnosticWithFix for MissingOkInTailExpr { - type AST = ast::Expr; + fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { + 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)) + } +} + +fn missing_struct_field_fix( + sema: &Semantics, + usage_file_id: FileId, + record_expr_field: &ast::RecordExprField, +) -> Option { + let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; + let def_id = sema.resolve_variant(record_lit)?; + let module; + let def_file_id; + let record_fields = match VariantDef::from(def_id) { + VariantDef::Struct(s) => { + module = s.module(sema.db); + let source = s.source(sema.db); + def_file_id = source.file_id; + let fields = source.value.field_list()?; + record_field_list(fields)? + } + VariantDef::Union(u) => { + module = u.module(sema.db); + let source = u.source(sema.db); + def_file_id = source.file_id; + source.value.record_field_list()? + } + VariantDef::EnumVariant(e) => { + module = e.module(sema.db); + let source = e.source(sema.db); + def_file_id = source.file_id; + let fields = source.value.field_list()?; + record_field_list(fields)? + } + }; + let def_file_id = def_file_id.original_file(sema.db); + + let new_field_type = sema.type_of_expr(&record_expr_field.expr()?)?; + if new_field_type.is_unknown() { + return None; + } + let new_field = make::record_field( + record_expr_field.field_name()?, + make::ty(&new_field_type.display_source_code(sema.db, module.into()).ok()?), + ); + + let last_field = record_fields.fields().last()?; + let last_field_syntax = last_field.syntax(); + let indent = IndentLevel::from_node(last_field_syntax); + + let mut new_field = new_field.to_string(); + if usage_file_id != def_file_id { + new_field = format!("pub(crate) {}", new_field); + } + new_field = format!("\n{}{}", indent, new_field); + + let needs_comma = !last_field_syntax.to_string().ends_with(','); + if needs_comma { + new_field = format!(",{}", new_field); + } + + let source_change = SourceFileEdit { + 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); - fn fix_source(&self, db: &dyn AstDatabase) -> Option { - let root = db.parse_or_expand(self.file)?; - Some(self.expr.to_node(&root)) + fn record_field_list(field_def_list: ast::FieldList) -> Option { + match field_def_list { + ast::FieldList::RecordFieldList(it) => Some(it), + ast::FieldList::TupleFieldList(_) => None, + } } } -- cgit v1.2.3 From 37aa68f050fae0079db7b6ebd81bacea4441fb7e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 15:08:55 +0300 Subject: Add rustdocs --- crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 56d454ac6..1955e1521 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,3 +1,4 @@ +//! Provides a way to derive fixes based on the diagnostic data. use crate::Fix; use ast::{edit::IndentLevel, make}; use hir::{ @@ -13,8 +14,9 @@ use ra_ide_db::{ use ra_syntax::{algo, ast, AstNode, TextRange}; use ra_text_edit::{TextEdit, TextEditBuilder}; -// TODO kb +/// 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) -> Option<(Fix, TextRange)>; } -- cgit v1.2.3 From 188ec3459e795732ad097758f7bf6b6b95bdbf5e Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 17:13:40 +0300 Subject: Simplify fix structure --- crates/ra_ide/src/diagnostics.rs | 68 ++++++-------- .../ra_ide/src/diagnostics/diagnostics_with_fix.rs | 101 ++++++++++----------- crates/ra_ide/src/lib.rs | 12 ++- 3 files changed, 89 insertions(+), 92 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 165ff5249..757b76fd4 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -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: &D, - sema: &Semantics, -) -> Diagnostic { +fn diagnostic_with_fix(d: &D, sema: &Semantics) -> 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, + }, ), }, ] diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 1955e1521..57b54a61e 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -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) -> Option<(Fix, TextRange)>; +/// A [Diagnostic] that potentially has a fix available. +/// +/// [Diagnostic]: hir::diagnostics::Diagnostic +pub trait DiagnosticWithFix: Diagnostic { + fn fix(&self, sema: &Semantics) -> Option; } impl DiagnosticWithFix for UnresolvedModule { - fn fix(&self, sema: &Semantics) -> Option<(Fix, TextRange)> { - let fix = Fix::new( + fn fix(&self, sema: &Semantics) -> Option { + 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) -> Option<(Fix, TextRange)> { + fn fix(&self, sema: &Semantics) -> Option { 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) -> Option<(Fix, TextRange)> { + fn fix(&self, sema: &Semantics) -> Option { // 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); - } + return None; + } - 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(), - )) + 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) -> Option<(Fix, TextRange)> { + fn fix(&self, sema: &Semantics) -> Option { 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, 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 { match field_def_list { diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 45a4b2421..89fcb6f17 100644 --- a/crates/ra_ide/src/lib.rs +++ b/crates/ra_ide/src/lib.rs @@ -105,20 +105,26 @@ pub struct Diagnostic { pub message: String, pub range: TextRange, pub severity: Severity, - pub fix: Option<(Fix, TextRange)>, + pub fix: Option, } #[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, source_change: SourceChange) -> Self { + pub fn new( + label: impl Into, + 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 } } } -- cgit v1.2.3 From db12ccee96bf37367b39ad99638d06da7123c088 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Tue, 11 Aug 2020 17:15:11 +0300 Subject: Better naming and docs --- crates/ra_ide/src/diagnostics.rs | 2 +- crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'crates/ra_ide') diff --git a/crates/ra_ide/src/diagnostics.rs b/crates/ra_ide/src/diagnostics.rs index 757b76fd4..1046d7ab3 100644 --- a/crates/ra_ide/src/diagnostics.rs +++ b/crates/ra_ide/src/diagnostics.rs @@ -183,7 +183,7 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: /// * a diagnostic is produced - /// * this diagnostic fix touches the input cursor position + /// * this diagnostic fix trigger range touches the input cursor position /// * 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) { let after = trim_indent(ra_fixture_after); diff --git a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs index 57b54a61e..f7c73773f 100644 --- a/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs +++ b/crates/ra_ide/src/diagnostics/diagnostics_with_fix.rs @@ -1,4 +1,5 @@ -//! Provides a way to attach fix actions to the +//! Provides a way to attach fixes to the diagnostics. +//! The same module also has all curret custom fixes for the diagnostics implemented. use crate::Fix; use ast::{edit::IndentLevel, make}; use hir::{ -- cgit v1.2.3