diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 4172f6caedc..27d347dbd92 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -28,7 +28,7 @@ use unlinked_file::UnlinkedFile; use crate::{Assist, AssistId, AssistKind, FileId, Label, SourceChange}; -use self::fixes::DiagnosticWithFix; +use self::fixes::DiagnosticWithFixes; #[derive(Debug)] pub struct Diagnostic { @@ -36,14 +36,14 @@ pub struct Diagnostic { pub message: String, pub range: TextRange, pub severity: Severity, - pub fix: Option, + pub fixes: Option>, pub unused: bool, pub code: Option, } impl Diagnostic { fn error(range: TextRange, message: String) -> Self { - Self { message, range, severity: Severity::Error, fix: None, unused: false, code: None } + Self { message, range, severity: Severity::Error, fixes: None, unused: false, code: None } } fn hint(range: TextRange, message: String) -> Self { @@ -51,14 +51,14 @@ impl Diagnostic { message, range, severity: Severity::WeakWarning, - fix: None, + fixes: None, unused: false, code: None, } } - fn with_fix(self, fix: Option) -> Self { - Self { fix, ..self } + fn with_fixes(self, fixes: Option>) -> Self { + Self { fixes, ..self } } fn with_unused(self, unused: bool) -> Self { @@ -154,7 +154,7 @@ pub(crate) fn diagnostics( // Override severity and mark as unused. res.borrow_mut().push( Diagnostic::hint(range, d.message()) - .with_fix(d.fix(&sema, resolve)) + .with_fixes(d.fixes(&sema, resolve)) .with_code(Some(d.code())), ); }) @@ -210,23 +210,23 @@ pub(crate) fn diagnostics( res.into_inner() } -fn diagnostic_with_fix( +fn diagnostic_with_fix( d: &D, sema: &Semantics, resolve: &AssistResolveStrategy, ) -> Diagnostic { Diagnostic::error(sema.diagnostics_display_range(d.display_source()).range, d.message()) - .with_fix(d.fix(&sema, resolve)) + .with_fixes(d.fixes(&sema, resolve)) .with_code(Some(d.code())) } -fn warning_with_fix( +fn warning_with_fix( d: &D, sema: &Semantics, resolve: &AssistResolveStrategy, ) -> Diagnostic { Diagnostic::hint(sema.diagnostics_display_range(d.display_source()).range, d.message()) - .with_fix(d.fix(&sema, resolve)) + .with_fixes(d.fixes(&sema, resolve)) .with_code(Some(d.code())) } @@ -256,12 +256,12 @@ fn check_unnecessary_braces_in_use_statement( acc.push( Diagnostic::hint(use_range, "Unnecessary braces in use statement".to_string()) - .with_fix(Some(fix( + .with_fixes(Some(vec![fix( "remove_braces", "Remove unnecessary braces", SourceChange::from_text_edit(file_id, edit), use_range, - ))), + )])), ); } @@ -309,9 +309,23 @@ mod tests { /// Takes a multi-file input fixture with annotated cursor positions, /// and checks that: /// * a diagnostic is produced - /// * this diagnostic fix trigger range touches the input cursor position + /// * the first 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 pub(crate) fn check_fix(ra_fixture_before: &str, ra_fixture_after: &str) { + check_nth_fix(0, ra_fixture_before, ra_fixture_after); + } + /// Takes a multi-file input fixture with annotated cursor positions, + /// and checks that: + /// * a diagnostic is produced + /// * every diagnostic fixes trigger range touches the input cursor position + /// * that the contents of the file containing the cursor match `after` after each diagnostic fix is applied + pub(crate) fn check_fixes(ra_fixture_before: &str, ra_fixtures_after: Vec<&str>) { + for (i, ra_fixture_after) in ra_fixtures_after.iter().enumerate() { + check_nth_fix(i, ra_fixture_before, ra_fixture_after) + } + } + + fn check_nth_fix(nth: usize, ra_fixture_before: &str, ra_fixture_after: &str) { let after = trim_indent(ra_fixture_after); let (analysis, file_position) = fixture::position(ra_fixture_before); @@ -324,9 +338,9 @@ mod tests { .unwrap() .pop() .unwrap(); - let fix = diagnostic.fix.unwrap(); + let fix = &diagnostic.fixes.unwrap()[nth]; let actual = { - let source_change = fix.source_change.unwrap(); + let source_change = fix.source_change.as_ref().unwrap(); let file_id = *source_change.source_file_edits.keys().next().unwrap(); let mut actual = analysis.file_text(file_id).unwrap().to_string(); @@ -344,7 +358,6 @@ mod tests { file_position.offset ); } - /// Checks that there's a diagnostic *without* fix at `$0`. fn check_no_fix(ra_fixture: &str) { let (analysis, file_position) = fixture::position(ra_fixture); @@ -357,7 +370,7 @@ mod tests { .unwrap() .pop() .unwrap(); - assert!(diagnostic.fix.is_none(), "got a fix when none was expected: {:?}", diagnostic); + assert!(diagnostic.fixes.is_none(), "got a fix when none was expected: {:?}", diagnostic); } /// Takes a multi-file input fixture with annotated cursor position and checks that no diagnostics @@ -393,7 +406,7 @@ mod tests { message: "unresolved macro `foo::bar!`", range: 5..8, severity: Error, - fix: None, + fixes: None, unused: false, code: Some( DiagnosticCode( @@ -542,18 +555,26 @@ mod a { #[test] fn unlinked_file_prepend_first_item() { cov_mark::check!(unlinked_file_prepend_before_first_item); - check_fix( + // Only tests the first one for `pub mod` since the rest are the same + check_fixes( r#" //- /main.rs fn f() {} //- /foo.rs $0 "#, - r#" + vec![ + r#" mod foo; fn f() {} "#, + r#" +pub mod foo; + +fn f() {} +"#, + ], ); } diff --git a/crates/ide/src/diagnostics/field_shorthand.rs b/crates/ide/src/diagnostics/field_shorthand.rs index 2b1787f9b15..01bd2dba675 100644 --- a/crates/ide/src/diagnostics/field_shorthand.rs +++ b/crates/ide/src/diagnostics/field_shorthand.rs @@ -46,14 +46,13 @@ fn check_expr_field_shorthand( let field_range = record_field.syntax().text_range(); acc.push( - Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()).with_fix( - Some(fix( + Diagnostic::hint(field_range, "Shorthand struct initialization".to_string()) + .with_fixes(Some(vec![fix( "use_expr_field_shorthand", "Use struct shorthand initialization", SourceChange::from_text_edit(file_id, edit), field_range, - )), - ), + )])), ); } } @@ -86,13 +85,13 @@ fn check_pat_field_shorthand( let edit = edit_builder.finish(); let field_range = record_pat_field.syntax().text_range(); - acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fix( - Some(fix( + acc.push(Diagnostic::hint(field_range, "Shorthand struct pattern".to_string()).with_fixes( + Some(vec![fix( "use_pat_field_shorthand", "Use struct field shorthand", SourceChange::from_text_edit(file_id, edit), field_range, - )), + )]), )); } } diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 92b3f5a2d81..258ac697447 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -14,18 +14,18 @@ use ide_db::RootDatabase; use crate::Assist; -/// A [Diagnostic] that potentially has a fix available. +/// A [Diagnostic] that potentially has some fixes available. /// /// [Diagnostic]: hir::diagnostics::Diagnostic -pub(crate) trait DiagnosticWithFix: Diagnostic { +pub(crate) trait DiagnosticWithFixes: Diagnostic { /// `resolve` determines if the diagnostic should fill in the `edit` field /// of the assist. /// /// If `resolve` is false, the edit will be computed later, on demand, and /// can be omitted. - fn fix( + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option; + ) -> Option>; } diff --git a/crates/ide/src/diagnostics/fixes/change_case.rs b/crates/ide/src/diagnostics/fixes/change_case.rs index 80aca58a1bb..42be3375f35 100644 --- a/crates/ide/src/diagnostics/fixes/change_case.rs +++ b/crates/ide/src/diagnostics/fixes/change_case.rs @@ -4,16 +4,16 @@ use ide_db::{base_db::FilePosition, RootDatabase}; use syntax::AstNode; use crate::{ - diagnostics::{unresolved_fix, DiagnosticWithFix}, + diagnostics::{unresolved_fix, DiagnosticWithFixes}, references::rename::rename_with_semantics, }; -impl DiagnosticWithFix for IncorrectCase { - fn fix( +impl DiagnosticWithFixes for IncorrectCase { + fn fixes( &self, sema: &Semantics, resolve: &AssistResolveStrategy, - ) -> Option { + ) -> Option> { let root = sema.db.parse_or_expand(self.file)?; let name_node = self.ident.to_node(&root); @@ -28,7 +28,7 @@ impl DiagnosticWithFix for IncorrectCase { res.source_change = Some(source_change.ok().unwrap_or_default()); } - Some(res) + Some(vec![res]) } } diff --git a/crates/ide/src/diagnostics/fixes/create_field.rs b/crates/ide/src/diagnostics/fixes/create_field.rs index 24e0fda5204..a5f457dcea5 100644 --- a/crates/ide/src/diagnostics/fixes/create_field.rs +++ b/crates/ide/src/diagnostics/fixes/create_field.rs @@ -7,18 +7,17 @@ use syntax::{ use text_edit::TextEdit; use crate::{ - diagnostics::{fix, DiagnosticWithFix}, + diagnostics::{fix, DiagnosticWithFixes}, Assist, AssistResolveStrategy, }; - -impl DiagnosticWithFix for NoSuchField { - fn fix( +impl DiagnosticWithFixes for NoSuchField { + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { + ) -> Option> { let root = sema.db.parse_or_expand(self.file)?; - missing_record_expr_field_fix( + missing_record_expr_field_fixes( &sema, self.file.original_file(sema.db), &self.field.to_node(&root), @@ -26,11 +25,11 @@ impl DiagnosticWithFix for NoSuchField { } } -fn missing_record_expr_field_fix( +fn missing_record_expr_field_fixes( sema: &Semantics, usage_file_id: FileId, record_expr_field: &ast::RecordExprField, -) -> Option { +) -> Option> { let record_lit = ast::RecordExpr::cast(record_expr_field.syntax().parent()?.parent()?)?; let def_id = sema.resolve_variant(record_lit)?; let module; @@ -89,12 +88,12 @@ fn missing_record_expr_field_fix( TextEdit::insert(last_field_syntax.text_range().end(), new_field), ); - return Some(fix( + return Some(vec![fix( "create_field", "Create field", source_change, record_expr_field.syntax().text_range(), - )); + )]); fn record_field_list(field_def_list: ast::FieldList) -> Option { match field_def_list { diff --git a/crates/ide/src/diagnostics/fixes/fill_missing_fields.rs b/crates/ide/src/diagnostics/fixes/fill_missing_fields.rs index 37a0e37a9f6..b5dd64c08c3 100644 --- a/crates/ide/src/diagnostics/fixes/fill_missing_fields.rs +++ b/crates/ide/src/diagnostics/fixes/fill_missing_fields.rs @@ -5,16 +5,16 @@ use syntax::{algo, ast::make, AstNode}; use text_edit::TextEdit; use crate::{ - diagnostics::{fix, fixes::DiagnosticWithFix}, + diagnostics::{fix, fixes::DiagnosticWithFixes}, Assist, }; -impl DiagnosticWithFix for MissingFields { - fn fix( +impl DiagnosticWithFixes for MissingFields { + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { + ) -> Option> { // Note that although we could add a diagnostics to // fill the missing tuple field, e.g : // `struct A(usize);` @@ -41,12 +41,12 @@ impl DiagnosticWithFix for MissingFields { .into_text_edit(&mut builder); builder.finish() }; - Some(fix( + Some(vec![fix( "fill_missing_fields", "Fill struct fields", SourceChange::from_text_edit(self.file.original_file(sema.db), edit), sema.original_range(&field_list_parent.syntax()).range, - )) + )]) } } diff --git a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs b/crates/ide/src/diagnostics/fixes/remove_semicolon.rs index 45471da4105..f1724d47918 100644 --- a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs +++ b/crates/ide/src/diagnostics/fixes/remove_semicolon.rs @@ -4,14 +4,14 @@ use ide_db::{source_change::SourceChange, RootDatabase}; use syntax::{ast, AstNode}; use text_edit::TextEdit; -use crate::diagnostics::{fix, DiagnosticWithFix}; +use crate::diagnostics::{fix, DiagnosticWithFixes}; -impl DiagnosticWithFix for RemoveThisSemicolon { - fn fix( +impl DiagnosticWithFixes for RemoveThisSemicolon { + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { + ) -> Option> { let root = sema.db.parse_or_expand(self.file)?; let semicolon = self @@ -26,7 +26,7 @@ impl DiagnosticWithFix for RemoveThisSemicolon { let edit = TextEdit::delete(semicolon); let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); - Some(fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)) + Some(vec![fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)]) } } diff --git a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs b/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs index b0ef7b44adc..444bf563b55 100644 --- a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs +++ b/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs @@ -7,14 +7,14 @@ use syntax::{ }; use text_edit::TextEdit; -use crate::diagnostics::{fix, DiagnosticWithFix}; +use crate::diagnostics::{fix, DiagnosticWithFixes}; -impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap { - fn fix( +impl DiagnosticWithFixes for ReplaceFilterMapNextWithFindMap { + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { + ) -> Option> { let root = sema.db.parse_or_expand(self.file)?; let next_expr = self.next_expr.to_node(&root); let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; @@ -32,12 +32,12 @@ impl DiagnosticWithFix for ReplaceFilterMapNextWithFindMap { let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); - Some(fix( + Some(vec![fix( "replace_with_find_map", "Replace filter_map(..).next() with find_map()", source_change, trigger_range, - )) + )]) } } diff --git a/crates/ide/src/diagnostics/fixes/unresolved_module.rs b/crates/ide/src/diagnostics/fixes/unresolved_module.rs index 81244b29386..b3d0283bbfa 100644 --- a/crates/ide/src/diagnostics/fixes/unresolved_module.rs +++ b/crates/ide/src/diagnostics/fixes/unresolved_module.rs @@ -3,17 +3,17 @@ use ide_assists::{Assist, AssistResolveStrategy}; use ide_db::{base_db::AnchoredPathBuf, source_change::FileSystemEdit, RootDatabase}; use syntax::AstNode; -use crate::diagnostics::{fix, DiagnosticWithFix}; +use crate::diagnostics::{fix, DiagnosticWithFixes}; -impl DiagnosticWithFix for UnresolvedModule { - fn fix( +impl DiagnosticWithFixes for UnresolvedModule { + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { + ) -> Option> { let root = sema.db.parse_or_expand(self.file)?; let unresolved_module = self.decl.to_node(&root); - Some(fix( + Some(vec![fix( "create_module", "Create module", FileSystemEdit::CreateFile { @@ -25,7 +25,7 @@ impl DiagnosticWithFix for UnresolvedModule { } .into(), unresolved_module.syntax().text_range(), - )) + )]) } } @@ -45,33 +45,35 @@ mod tests { message: "unresolved module", range: 0..8, severity: Error, - fix: Some( - Assist { - id: AssistId( - "create_module", - QuickFix, - ), - label: "Create module", - group: None, - target: 0..8, - source_change: Some( - SourceChange { - source_file_edits: {}, - file_system_edits: [ - CreateFile { - dst: AnchoredPathBuf { - anchor: FileId( - 0, - ), - path: "foo.rs", + fixes: Some( + [ + Assist { + id: AssistId( + "create_module", + QuickFix, + ), + label: "Create module", + group: None, + target: 0..8, + source_change: Some( + SourceChange { + source_file_edits: {}, + file_system_edits: [ + CreateFile { + dst: AnchoredPathBuf { + anchor: FileId( + 0, + ), + path: "foo.rs", + }, + initial_contents: "", }, - initial_contents: "", - }, - ], - is_snippet: false, - }, - ), - }, + ], + is_snippet: false, + }, + ), + }, + ], ), unused: false, code: Some( diff --git a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs b/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs index 66676064a52..715a403b99c 100644 --- a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs +++ b/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs @@ -4,14 +4,14 @@ use ide_db::{source_change::SourceChange, RootDatabase}; use syntax::AstNode; use text_edit::TextEdit; -use crate::diagnostics::{fix, DiagnosticWithFix}; +use crate::diagnostics::{fix, DiagnosticWithFixes}; -impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { - fn fix( +impl DiagnosticWithFixes for MissingOkOrSomeInTailExpr { + fn fixes( &self, sema: &Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { + ) -> 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(); @@ -19,7 +19,7 @@ impl DiagnosticWithFix for MissingOkOrSomeInTailExpr { let edit = TextEdit::replace(tail_expr_range, replacement); 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" }; - Some(fix("wrap_tail_expr", name, source_change, tail_expr_range)) + Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)]) } } diff --git a/crates/ide/src/diagnostics/unlinked_file.rs b/crates/ide/src/diagnostics/unlinked_file.rs index 93fd25dea47..51fe0f360ed 100644 --- a/crates/ide/src/diagnostics/unlinked_file.rs +++ b/crates/ide/src/diagnostics/unlinked_file.rs @@ -18,7 +18,7 @@ use syntax::{ use text_edit::TextEdit; use crate::{ - diagnostics::{fix, fixes::DiagnosticWithFix}, + diagnostics::{fix, fixes::DiagnosticWithFixes}, Assist, }; @@ -50,13 +50,13 @@ impl Diagnostic for UnlinkedFile { } } -impl DiagnosticWithFix for UnlinkedFile { - fn fix( +impl DiagnosticWithFixes for UnlinkedFile { + fn fixes( &self, sema: &hir::Semantics, _resolve: &AssistResolveStrategy, - ) -> Option { - // If there's an existing module that could add a `mod` item to include the unlinked file, + ) -> Option> { + // If there's an existing module that could add `mod` or `pub mod` items to include the unlinked file, // suggest that as a fix. let source_root = sema.db.source_root(sema.db.file_source_root(self.file_id)); @@ -90,7 +90,7 @@ impl DiagnosticWithFix for UnlinkedFile { } if module.origin.file_id() == Some(*parent_id) { - return make_fix(sema.db, *parent_id, module_name, self.file_id); + return make_fixes(sema.db, *parent_id, module_name, self.file_id); } } } @@ -101,20 +101,23 @@ impl DiagnosticWithFix for UnlinkedFile { } } -fn make_fix( +fn make_fixes( db: &RootDatabase, parent_file_id: FileId, new_mod_name: &str, added_file_id: FileId, -) -> Option { +) -> Option> { fn is_outline_mod(item: &ast::Item) -> bool { matches!(item, ast::Item::Module(m) if m.item_list().is_none()) } let mod_decl = format!("mod {};", new_mod_name); + let pub_mod_decl = format!("pub mod {};", new_mod_name); + let ast: ast::SourceFile = db.parse(parent_file_id).tree(); - let mut builder = TextEdit::builder(); + let mut mod_decl_builder = TextEdit::builder(); + let mut pub_mod_decl_builder = TextEdit::builder(); // If there's an existing `mod m;` statement matching the new one, don't emit a fix (it's // probably `#[cfg]`d out). @@ -138,30 +141,43 @@ fn make_fix( { Some(last) => { cov_mark::hit!(unlinked_file_append_to_existing_mods); - builder.insert(last.syntax().text_range().end(), format!("\n{}", mod_decl)); + let offset = last.syntax().text_range().end(); + mod_decl_builder.insert(offset, format!("\n{}", mod_decl)); + pub_mod_decl_builder.insert(offset, format!("\n{}", pub_mod_decl)); } None => { // Prepend before the first item in the file. match ast.items().next() { Some(item) => { cov_mark::hit!(unlinked_file_prepend_before_first_item); - builder.insert(item.syntax().text_range().start(), format!("{}\n\n", mod_decl)); + let offset = item.syntax().text_range().start(); + mod_decl_builder.insert(offset, format!("{}\n\n", mod_decl)); + pub_mod_decl_builder.insert(offset, format!("{}\n\n", pub_mod_decl)); } None => { // No items in the file, so just append at the end. cov_mark::hit!(unlinked_file_empty_file); - builder.insert(ast.syntax().text_range().end(), format!("{}\n", mod_decl)); + let offset = ast.syntax().text_range().end(); + mod_decl_builder.insert(offset, format!("{}\n", mod_decl)); + pub_mod_decl_builder.insert(offset, format!("{}\n", pub_mod_decl)); } } } } - let edit = builder.finish(); let trigger_range = db.parse(added_file_id).tree().syntax().text_range(); - Some(fix( - "add_mod_declaration", - &format!("Insert `{}`", mod_decl), - SourceChange::from_text_edit(parent_file_id, edit), - trigger_range, - )) + Some(vec![ + fix( + "add_mod_declaration", + &format!("Insert `{}`", mod_decl), + SourceChange::from_text_edit(parent_file_id, mod_decl_builder.finish()), + trigger_range, + ), + fix( + "add_pub_mod_declaration", + &format!("Insert `{}`", pub_mod_decl), + SourceChange::from_text_edit(parent_file_id, pub_mod_decl_builder.finish()), + trigger_range, + ), + ]) } diff --git a/crates/ide/src/lib.rs b/crates/ide/src/lib.rs index db08547d1dc..f4b90db3a02 100644 --- a/crates/ide/src/lib.rs +++ b/crates/ide/src/lib.rs @@ -565,7 +565,7 @@ impl Analysis { let diagnostic_assists = if include_fixes { diagnostics::diagnostics(db, diagnostics_config, &resolve, frange.file_id) .into_iter() - .filter_map(|it| it.fix) + .flat_map(|it| it.fixes.unwrap_or_default()) .filter(|it| it.target.intersect(frange.range).is_some()) .collect() } else {