From 74f3cca85ab870614f314c6180e2fbb883ad4fe3 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 20:13:15 +0300 Subject: internal: refactor remove this semicolon diagnostics --- crates/ide/src/diagnostics.rs | 17 +----- crates/ide/src/diagnostics/fixes.rs | 2 - crates/ide/src/diagnostics/fixes/create_field.rs | 1 - .../ide/src/diagnostics/fixes/remove_semicolon.rs | 41 -------------- crates/ide/src/diagnostics/no_such_field.rs | 2 +- .../ide/src/diagnostics/remove_this_semicolon.rs | 64 ++++++++++++++++++++++ 6 files changed, 68 insertions(+), 59 deletions(-) delete mode 100644 crates/ide/src/diagnostics/fixes/create_field.rs delete mode 100644 crates/ide/src/diagnostics/fixes/remove_semicolon.rs create mode 100644 crates/ide/src/diagnostics/remove_this_semicolon.rs (limited to 'crates/ide') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 4c92d0cf4..3ced08f30 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -11,6 +11,7 @@ mod mismatched_arg_count; mod missing_fields; mod missing_unsafe; mod no_such_field; +mod remove_this_semicolon; mod unimplemented_builtin_macro; mod unresolved_extern_crate; mod unresolved_import; @@ -165,9 +166,6 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); }) - .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); - }) .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) @@ -223,10 +221,11 @@ pub(crate) fn diagnostics( let d = match diag { AnyDiagnostic::BreakOutsideOfLoop(d) => break_outside_of_loop::break_outside_of_loop(&ctx, &d), AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), + AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), - AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), + AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), @@ -838,16 +837,6 @@ fn x(a: S) { ) } - #[test] - fn missing_semicolon() { - check_diagnostics( - r#" - fn test() -> i32 { 123; } - //^^^ Remove this semicolon - "#, - ); - } - #[test] fn import_extern_crate_clash_with_inner_item() { // This is more of a resolver test, but doesn't really work with the hir_def testsuite. diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index a2e792b3b..70f17881e 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -1,8 +1,6 @@ //! Provides a way to attach fixes to the diagnostics. //! The same module also has all curret custom fixes for the diagnostics implemented. mod change_case; -mod create_field; -mod remove_semicolon; mod replace_with_find_map; mod wrap_tail_expr; diff --git a/crates/ide/src/diagnostics/fixes/create_field.rs b/crates/ide/src/diagnostics/fixes/create_field.rs deleted file mode 100644 index 8b1378917..000000000 --- a/crates/ide/src/diagnostics/fixes/create_field.rs +++ /dev/null @@ -1 +0,0 @@ - diff --git a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs b/crates/ide/src/diagnostics/fixes/remove_semicolon.rs deleted file mode 100644 index f1724d479..000000000 --- a/crates/ide/src/diagnostics/fixes/remove_semicolon.rs +++ /dev/null @@ -1,41 +0,0 @@ -use hir::{db::AstDatabase, diagnostics::RemoveThisSemicolon, Semantics}; -use ide_assists::{Assist, AssistResolveStrategy}; -use ide_db::{source_change::SourceChange, RootDatabase}; -use syntax::{ast, AstNode}; -use text_edit::TextEdit; - -use crate::diagnostics::{fix, DiagnosticWithFixes}; - -impl DiagnosticWithFixes for RemoveThisSemicolon { - fn fixes( - &self, - sema: &Semantics, - _resolve: &AssistResolveStrategy, - ) -> Option> { - let root = sema.db.parse_or_expand(self.file)?; - - let semicolon = self - .expr - .to_node(&root) - .syntax() - .parent() - .and_then(ast::ExprStmt::cast) - .and_then(|expr| expr.semicolon_token())? - .text_range(); - - let edit = TextEdit::delete(semicolon); - let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); - - Some(vec![fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)]) - } -} - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::check_fix; - - #[test] - fn remove_semicolon() { - check_fix(r#"fn f() -> i32 { 92$0; }"#, r#"fn f() -> i32 { 92 }"#); - } -} diff --git a/crates/ide/src/diagnostics/no_such_field.rs b/crates/ide/src/diagnostics/no_such_field.rs index 61962de28..edc63c246 100644 --- a/crates/ide/src/diagnostics/no_such_field.rs +++ b/crates/ide/src/diagnostics/no_such_field.rs @@ -17,7 +17,7 @@ use crate::{ pub(super) fn no_such_field(ctx: &DiagnosticsContext<'_>, d: &hir::NoSuchField) -> Diagnostic { Diagnostic::new( "no-such-field", - "no such field".to_string(), + "no such field", ctx.sema.diagnostics_display_range(d.field.clone().map(|it| it.into())).range, ) .with_fixes(fixes(ctx, d)) diff --git a/crates/ide/src/diagnostics/remove_this_semicolon.rs b/crates/ide/src/diagnostics/remove_this_semicolon.rs new file mode 100644 index 000000000..814cb0f8c --- /dev/null +++ b/crates/ide/src/diagnostics/remove_this_semicolon.rs @@ -0,0 +1,64 @@ +use hir::db::AstDatabase; +use ide_db::source_change::SourceChange; +use syntax::{ast, AstNode}; +use text_edit::TextEdit; + +use crate::{ + diagnostics::{fix, Diagnostic, DiagnosticsContext}, + Assist, +}; + +// Diagnostic: remove-this-semicolon +// +// This diagnostic is triggered when there's an erroneous `;` at the end of the block. +pub(super) fn remove_this_semicolon( + ctx: &DiagnosticsContext<'_>, + d: &hir::RemoveThisSemicolon, +) -> Diagnostic { + Diagnostic::new( + "remove-this-semicolon", + "remove this semicolon", + ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, + ) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::RemoveThisSemicolon) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; + + let semicolon = d + .expr + .value + .to_node(&root) + .syntax() + .parent() + .and_then(ast::ExprStmt::cast) + .and_then(|expr| expr.semicolon_token())? + .text_range(); + + let edit = TextEdit::delete(semicolon); + let source_change = + SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + + Some(vec![fix("remove_semicolon", "Remove this semicolon", source_change, semicolon)]) +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::{check_diagnostics, check_fix}; + + #[test] + fn missing_semicolon() { + check_diagnostics( + r#" +fn test() -> i32 { 123; } + //^^^ remove this semicolon +"#, + ); + } + + #[test] + fn remove_semicolon() { + check_fix(r#"fn f() -> i32 { 92$0; }"#, r#"fn f() -> i32 { 92 }"#); + } +} -- cgit v1.2.3 From 949a6ec469507db5e79578da94e17cb63cb54d19 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 20:19:11 +0300 Subject: internal: refactor missing or or some diagnostic --- crates/ide/src/diagnostics.rs | 5 +- crates/ide/src/diagnostics/fixes.rs | 1 - crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs | 211 ------------------- .../diagnostics/missing_ok_or_some_in_tail_expr.rs | 230 +++++++++++++++++++++ 4 files changed, 232 insertions(+), 215 deletions(-) delete mode 100644 crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs create mode 100644 crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs (limited to 'crates/ide') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index 3ced08f30..af282db0c 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -9,6 +9,7 @@ mod inactive_code; mod macro_error; mod mismatched_arg_count; mod missing_fields; +mod missing_ok_or_some_in_tail_expr; mod missing_unsafe; mod no_such_field; mod remove_this_semicolon; @@ -163,9 +164,6 @@ pub(crate) fn diagnostics( } let res = RefCell::new(res); let sink_builder = DiagnosticSinkBuilder::new() - .on::(|d| { - res.borrow_mut().push(diagnostic_with_fix(d, &sema, resolve)); - }) .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) @@ -223,6 +221,7 @@ pub(crate) fn diagnostics( AnyDiagnostic::MacroError(d) => macro_error::macro_error(&ctx, &d), AnyDiagnostic::MismatchedArgCount(d) => mismatched_arg_count::mismatched_arg_count(&ctx, &d), AnyDiagnostic::MissingFields(d) => missing_fields::missing_fields(&ctx, &d), + AnyDiagnostic::MissingOkOrSomeInTailExpr(d) => missing_ok_or_some_in_tail_expr::missing_ok_or_some_in_tail_expr(&ctx, &d), AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 70f17881e..350575b3a 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -2,7 +2,6 @@ //! The same module also has all curret custom fixes for the diagnostics implemented. mod change_case; mod replace_with_find_map; -mod wrap_tail_expr; use hir::{diagnostics::Diagnostic, Semantics}; use ide_assists::AssistResolveStrategy; diff --git a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs b/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs deleted file mode 100644 index dcb21e037..000000000 --- a/crates/ide/src/diagnostics/fixes/wrap_tail_expr.rs +++ /dev/null @@ -1,211 +0,0 @@ -use hir::{db::AstDatabase, diagnostics::MissingOkOrSomeInTailExpr, Semantics}; -use ide_assists::{Assist, AssistResolveStrategy}; -use ide_db::{source_change::SourceChange, RootDatabase}; -use syntax::AstNode; -use text_edit::TextEdit; - -use crate::diagnostics::{fix, DiagnosticWithFixes}; - -impl DiagnosticWithFixes for MissingOkOrSomeInTailExpr { - fn fixes( - &self, - sema: &Semantics, - _resolve: &AssistResolveStrategy, - ) -> 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 replacement = format!("{}({})", self.required, tail_expr.syntax()); - 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(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)]) - } -} - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::{check_diagnostics, check_fix}; - - #[test] - fn test_wrap_return_type_option() { - check_fix( - r#" -//- /main.rs crate:main deps:core -use core::option::Option::{self, Some, None}; - -fn div(x: i32, y: i32) -> Option { - if y == 0 { - return None; - } - x / y$0 -} -//- /core/lib.rs crate:core -pub mod result { - pub enum Result { Ok(T), Err(E) } -} -pub mod option { - pub enum Option { Some(T), None } -} -"#, - r#" -use core::option::Option::{self, Some, None}; - -fn div(x: i32, y: i32) -> Option { - if y == 0 { - return None; - } - Some(x / y) -} -"#, - ); - } - - #[test] - fn test_wrap_return_type() { - check_fix( - r#" -//- /main.rs crate:main deps:core -use core::result::Result::{self, Ok, Err}; - -fn div(x: i32, y: i32) -> Result { - if y == 0 { - return Err(()); - } - x / y$0 -} -//- /core/lib.rs crate:core -pub mod result { - pub enum Result { Ok(T), Err(E) } -} -pub mod option { - pub enum Option { Some(T), None } -} -"#, - r#" -use core::result::Result::{self, Ok, Err}; - -fn div(x: i32, y: i32) -> Result { - if y == 0 { - return Err(()); - } - Ok(x / y) -} -"#, - ); - } - - #[test] - fn test_wrap_return_type_handles_generic_functions() { - check_fix( - r#" -//- /main.rs crate:main deps:core -use core::result::Result::{self, Ok, Err}; - -fn div(x: T) -> Result { - if x == 0 { - return Err(7); - } - $0x -} -//- /core/lib.rs crate:core -pub mod result { - pub enum Result { Ok(T), Err(E) } -} -pub mod option { - pub enum Option { Some(T), None } -} -"#, - r#" -use core::result::Result::{self, Ok, Err}; - -fn div(x: T) -> Result { - if x == 0 { - return Err(7); - } - Ok(x) -} -"#, - ); - } - - #[test] - fn test_wrap_return_type_handles_type_aliases() { - check_fix( - r#" -//- /main.rs crate:main deps:core -use core::result::Result::{self, Ok, Err}; - -type MyResult = Result; - -fn div(x: i32, y: i32) -> MyResult { - if y == 0 { - return Err(()); - } - x $0/ y -} -//- /core/lib.rs crate:core -pub mod result { - pub enum Result { Ok(T), Err(E) } -} -pub mod option { - pub enum Option { Some(T), None } -} -"#, - r#" -use core::result::Result::{self, Ok, Err}; - -type MyResult = Result; - -fn div(x: i32, y: i32) -> MyResult { - if y == 0 { - return Err(()); - } - Ok(x / y) -} -"#, - ); - } - - #[test] - fn test_wrap_return_type_not_applicable_when_expr_type_does_not_match_ok_type() { - check_diagnostics( - r#" -//- /main.rs crate:main deps:core -use core::result::Result::{self, Ok, Err}; - -fn foo() -> Result<(), i32> { 0 } - -//- /core/lib.rs crate:core -pub mod result { - pub enum Result { Ok(T), Err(E) } -} -pub mod option { - pub enum Option { Some(T), None } -} -"#, - ); - } - - #[test] - fn test_wrap_return_type_not_applicable_when_return_type_is_not_result_or_option() { - check_diagnostics( - r#" -//- /main.rs crate:main deps:core -use core::result::Result::{self, Ok, Err}; - -enum SomeOtherEnum { Ok(i32), Err(String) } - -fn foo() -> SomeOtherEnum { 0 } - -//- /core/lib.rs crate:core -pub mod result { - pub enum Result { Ok(T), Err(E) } -} -pub mod option { - pub enum Option { Some(T), None } -} -"#, - ); - } -} diff --git a/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs new file mode 100644 index 000000000..e27b54e66 --- /dev/null +++ b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs @@ -0,0 +1,230 @@ +use hir::{db::AstDatabase, Semantics}; +use ide_assists::Assist; +use ide_db::source_change::SourceChange; +use syntax::AstNode; +use text_edit::TextEdit; + +use crate::diagnostics::{fix, Diagnostic, DiagnosticsContext}; + +// Diagnostic: missing-ok-or-some-in-tail-expr +// +// This diagnostic is triggered if a block that should return `Result` returns a value not wrapped in `Ok`, +// or if a block that should return `Option` returns a value not wrapped in `Some`. +// +// Example: +// +// ```rust +// fn foo() -> Result { +// 10 +// } +// ``` +pub(super) fn missing_ok_or_some_in_tail_expr( + ctx: &DiagnosticsContext<'_>, + d: &hir::MissingOkOrSomeInTailExpr, +) -> Diagnostic { + Diagnostic::new( + "missing-ok-or-some-in-tail-expr", + format!("wrap return expression in {}", d.required), + ctx.sema.diagnostics_display_range(d.expr.clone().map(|it| it.into())).range, + ) + .with_fixes(fixes(ctx, d)) +} + +fn fixes(ctx: &DiagnosticsContext<'_>, d: &hir::MissingOkOrSomeInTailExpr) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.expr.file_id)?; + let tail_expr = d.expr.value.to_node(&root); + let tail_expr_range = tail_expr.syntax().text_range(); + let replacement = format!("{}({})", d.required, tail_expr.syntax()); + let edit = TextEdit::replace(tail_expr_range, replacement); + let source_change = + SourceChange::from_text_edit(d.expr.file_id.original_file(ctx.sema.db), edit); + let name = if d.required == "Ok" { "Wrap with Ok" } else { "Wrap with Some" }; + Some(vec![fix("wrap_tail_expr", name, source_change, tail_expr_range)]) +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::{check_diagnostics, check_fix}; + + #[test] + fn test_wrap_return_type_option() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::option::Option::{self, Some, None}; + +fn div(x: i32, y: i32) -> Option { + if y == 0 { + return None; + } + x / y$0 +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::option::Option::{self, Some, None}; + +fn div(x: i32, y: i32) -> Option { + if y == 0 { + return None; + } + Some(x / y) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +fn div(x: i32, y: i32) -> Result { + if y == 0 { + return Err(()); + } + x / y$0 +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::result::Result::{self, Ok, Err}; + +fn div(x: i32, y: i32) -> Result { + if y == 0 { + return Err(()); + } + Ok(x / y) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_handles_generic_functions() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +fn div(x: T) -> Result { + if x == 0 { + return Err(7); + } + $0x +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::result::Result::{self, Ok, Err}; + +fn div(x: T) -> Result { + if x == 0 { + return Err(7); + } + Ok(x) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_handles_type_aliases() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +type MyResult = Result; + +fn div(x: i32, y: i32) -> MyResult { + if y == 0 { + return Err(()); + } + x $0/ y +} +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + r#" +use core::result::Result::{self, Ok, Err}; + +type MyResult = Result; + +fn div(x: i32, y: i32) -> MyResult { + if y == 0 { + return Err(()); + } + Ok(x / y) +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_not_applicable_when_expr_type_does_not_match_ok_type() { + check_diagnostics( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +fn foo() -> Result<(), i32> { 0 } + +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + ); + } + + #[test] + fn test_wrap_return_type_not_applicable_when_return_type_is_not_result_or_option() { + check_diagnostics( + r#" +//- /main.rs crate:main deps:core +use core::result::Result::{self, Ok, Err}; + +enum SomeOtherEnum { Ok(i32), Err(String) } + +fn foo() -> SomeOtherEnum { 0 } + +//- /core/lib.rs crate:core +pub mod result { + pub enum Result { Ok(T), Err(E) } +} +pub mod option { + pub enum Option { Some(T), None } +} +"#, + ); + } +} -- cgit v1.2.3 From 24262f9ff6ae9ea326fa35d238704d18e99d52a1 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 20:20:58 +0300 Subject: minor --- crates/ide/src/diagnostics.rs | 43 ---------------------- crates/ide/src/diagnostics/missing_fields.rs | 33 +++++++++++++++++ .../diagnostics/missing_ok_or_some_in_tail_expr.rs | 2 +- 3 files changed, 34 insertions(+), 44 deletions(-) (limited to 'crates/ide') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index af282db0c..e3974e1ae 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -251,16 +251,6 @@ pub(crate) fn diagnostics( res } -fn diagnostic_with_fix( - d: &D, - sema: &Semantics, - resolve: &AssistResolveStrategy, -) -> Diagnostic { - Diagnostic::error(sema.diagnostics_display_range(d.display_source()).range, d.message()) - .with_fixes(d.fixes(sema, resolve)) - .with_code(Some(d.code())) -} - fn warning_with_fix( d: &D, sema: &Semantics, @@ -446,39 +436,6 @@ mod tests { } } - #[test] - fn range_mapping_out_of_macros() { - // FIXME: this is very wrong, but somewhat tricky to fix. - check_fix( - r#" -fn some() {} -fn items() {} -fn here() {} - -macro_rules! id { ($($tt:tt)*) => { $($tt)*}; } - -fn main() { - let _x = id![Foo { a: $042 }]; -} - -pub struct Foo { pub a: i32, pub b: i32 } -"#, - r#" -fn some(, b: () ) {} -fn items() {} -fn here() {} - -macro_rules! id { ($($tt:tt)*) => { $($tt)*}; } - -fn main() { - let _x = id![Foo { a: 42 }]; -} - -pub struct Foo { pub a: i32, pub b: i32 } -"#, - ); - } - #[test] fn test_check_unnecessary_braces_in_use_statement() { check_diagnostics( diff --git a/crates/ide/src/diagnostics/missing_fields.rs b/crates/ide/src/diagnostics/missing_fields.rs index 95cd64956..c4b6a3679 100644 --- a/crates/ide/src/diagnostics/missing_fields.rs +++ b/crates/ide/src/diagnostics/missing_fields.rs @@ -93,6 +93,39 @@ fn baz(s: S) { ); } + #[test] + fn range_mapping_out_of_macros() { + // FIXME: this is very wrong, but somewhat tricky to fix. + check_fix( + r#" +fn some() {} +fn items() {} +fn here() {} + +macro_rules! id { ($($tt:tt)*) => { $($tt)*}; } + +fn main() { + let _x = id![Foo { a: $042 }]; +} + +pub struct Foo { pub a: i32, pub b: i32 } +"#, + r#" +fn some(, b: () ) {} +fn items() {} +fn here() {} + +macro_rules! id { ($($tt:tt)*) => { $($tt)*}; } + +fn main() { + let _x = id![Foo { a: 42 }]; +} + +pub struct Foo { pub a: i32, pub b: i32 } +"#, + ); + } + #[test] fn test_fill_struct_fields_empty() { check_fix( diff --git a/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs index e27b54e66..06005d156 100644 --- a/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs +++ b/crates/ide/src/diagnostics/missing_ok_or_some_in_tail_expr.rs @@ -1,4 +1,4 @@ -use hir::{db::AstDatabase, Semantics}; +use hir::db::AstDatabase; use ide_assists::Assist; use ide_db::source_change::SourceChange; use syntax::AstNode; -- cgit v1.2.3 From de1fc70ccd3bf7a0850e036a12cf866a80d46458 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 20:32:54 +0300 Subject: internal: refactor find_map diagnostic --- crates/ide/src/diagnostics.rs | 88 +--------- crates/ide/src/diagnostics/fixes.rs | 1 - .../src/diagnostics/fixes/replace_with_find_map.rs | 84 ---------- .../replace_filter_map_next_with_find_map.rs | 182 +++++++++++++++++++++ 4 files changed, 184 insertions(+), 171 deletions(-) delete mode 100644 crates/ide/src/diagnostics/fixes/replace_with_find_map.rs create mode 100644 crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs (limited to 'crates/ide') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index e3974e1ae..ec5318594 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -13,6 +13,7 @@ mod missing_ok_or_some_in_tail_expr; mod missing_unsafe; mod no_such_field; mod remove_this_semicolon; +mod replace_filter_map_next_with_find_map; mod unimplemented_builtin_macro; mod unresolved_extern_crate; mod unresolved_import; @@ -167,9 +168,6 @@ pub(crate) fn diagnostics( .on::(|d| { res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); }) - .on::(|d| { - res.borrow_mut().push(warning_with_fix(d, &sema, resolve)); - }) .on::(|d| { // Limit diagnostic to the first few characters in the file. This matches how VS Code // renders it with the full span, but on other editors, and is less invasive. @@ -225,6 +223,7 @@ pub(crate) fn diagnostics( AnyDiagnostic::MissingUnsafe(d) => missing_unsafe::missing_unsafe(&ctx, &d), AnyDiagnostic::NoSuchField(d) => no_such_field::no_such_field(&ctx, &d), AnyDiagnostic::RemoveThisSemicolon(d) => remove_this_semicolon::remove_this_semicolon(&ctx, &d), + AnyDiagnostic::ReplaceFilterMapNextWithFindMap(d) => replace_filter_map_next_with_find_map::replace_filter_map_next_with_find_map(&ctx, &d), AnyDiagnostic::UnimplementedBuiltinMacro(d) => unimplemented_builtin_macro::unimplemented_builtin_macro(&ctx, &d), AnyDiagnostic::UnresolvedExternCrate(d) => unresolved_extern_crate::unresolved_extern_crate(&ctx, &d), AnyDiagnostic::UnresolvedImport(d) => unresolved_import::unresolved_import(&ctx, &d), @@ -672,89 +671,6 @@ mod foo; ); } - // Register the required standard library types to make the tests work - fn add_filter_map_with_find_next_boilerplate(body: &str) -> String { - let prefix = r#" - //- /main.rs crate:main deps:core - use core::iter::Iterator; - use core::option::Option::{self, Some, None}; - "#; - let suffix = r#" - //- /core/lib.rs crate:core - pub mod option { - pub enum Option { Some(T), None } - } - pub mod iter { - pub trait Iterator { - type Item; - fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } - fn next(&mut self) -> Option; - } - pub struct FilterMap {} - impl Iterator for FilterMap { - type Item = i32; - fn next(&mut self) -> i32 { 7 } - } - } - "#; - format!("{}{}{}", prefix, body, suffix) - } - - #[test] - fn replace_filter_map_next_with_find_map2() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); - //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }) - .len(); - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_with_intervening_methods() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }) - .map(|x| x + 2) - .len(); - } - "#, - )); - } - - #[test] - fn replace_filter_map_next_with_find_map_no_diagnostic_if_not_in_chain() { - check_diagnostics(&add_filter_map_with_find_next_boilerplate( - r#" - fn foo() { - let m = [1, 2, 3] - .iter() - .filter_map(|x| if *x == 2 { Some (4) } else { None }); - let n = m.next(); - } - "#, - )); - } - #[test] fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { check_diagnostics( diff --git a/crates/ide/src/diagnostics/fixes.rs b/crates/ide/src/diagnostics/fixes.rs index 350575b3a..e4bd90c3f 100644 --- a/crates/ide/src/diagnostics/fixes.rs +++ b/crates/ide/src/diagnostics/fixes.rs @@ -1,7 +1,6 @@ //! Provides a way to attach fixes to the diagnostics. //! The same module also has all curret custom fixes for the diagnostics implemented. mod change_case; -mod replace_with_find_map; use hir::{diagnostics::Diagnostic, Semantics}; use ide_assists::AssistResolveStrategy; diff --git a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs b/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs deleted file mode 100644 index 444bf563b..000000000 --- a/crates/ide/src/diagnostics/fixes/replace_with_find_map.rs +++ /dev/null @@ -1,84 +0,0 @@ -use hir::{db::AstDatabase, diagnostics::ReplaceFilterMapNextWithFindMap, Semantics}; -use ide_assists::{Assist, AssistResolveStrategy}; -use ide_db::{source_change::SourceChange, RootDatabase}; -use syntax::{ - ast::{self, ArgListOwner}, - AstNode, TextRange, -}; -use text_edit::TextEdit; - -use crate::diagnostics::{fix, DiagnosticWithFixes}; - -impl DiagnosticWithFixes for ReplaceFilterMapNextWithFindMap { - fn fixes( - &self, - sema: &Semantics, - _resolve: &AssistResolveStrategy, - ) -> 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())?; - - let filter_map_call = ast::MethodCallExpr::cast(next_call.receiver()?.syntax().clone())?; - let filter_map_name_range = filter_map_call.name_ref()?.ident_token()?.text_range(); - let filter_map_args = filter_map_call.arg_list()?; - - let range_to_replace = - TextRange::new(filter_map_name_range.start(), next_expr.syntax().text_range().end()); - let replacement = format!("find_map{}", filter_map_args.syntax().text()); - let trigger_range = next_expr.syntax().text_range(); - - let edit = TextEdit::replace(range_to_replace, replacement); - - let source_change = SourceChange::from_text_edit(self.file.original_file(sema.db), edit); - - Some(vec![fix( - "replace_with_find_map", - "Replace filter_map(..).next() with find_map()", - source_change, - trigger_range, - )]) - } -} - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::check_fix; - - #[test] - fn replace_with_wind_map() { - check_fix( - r#" -//- /main.rs crate:main deps:core -use core::iter::Iterator; -use core::option::Option::{self, Some, None}; -fn foo() { - let m = [1, 2, 3].iter().$0filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); -} -//- /core/lib.rs crate:core -pub mod option { - pub enum Option { Some(T), None } -} -pub mod iter { - pub trait Iterator { - type Item; - fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } - fn next(&mut self) -> Option; - } - pub struct FilterMap {} - impl Iterator for FilterMap { - type Item = i32; - fn next(&mut self) -> i32 { 7 } - } -} -"#, - r#" -use core::iter::Iterator; -use core::option::Option::{self, Some, None}; -fn foo() { - let m = [1, 2, 3].iter().find_map(|x| if *x == 2 { Some (4) } else { None }); -} -"#, - ) - } -} diff --git a/crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs b/crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs new file mode 100644 index 000000000..f3b011495 --- /dev/null +++ b/crates/ide/src/diagnostics/replace_filter_map_next_with_find_map.rs @@ -0,0 +1,182 @@ +use hir::{db::AstDatabase, InFile}; +use ide_db::source_change::SourceChange; +use syntax::{ + ast::{self, ArgListOwner}, + AstNode, TextRange, +}; +use text_edit::TextEdit; + +use crate::{ + diagnostics::{fix, Diagnostic, DiagnosticsContext}, + Assist, Severity, +}; + +// Diagnostic: replace-filter-map-next-with-find-map +// +// This diagnostic is triggered when `.filter_map(..).next()` is used, rather than the more concise `.find_map(..)`. +pub(super) fn replace_filter_map_next_with_find_map( + ctx: &DiagnosticsContext<'_>, + d: &hir::ReplaceFilterMapNextWithFindMap, +) -> Diagnostic { + Diagnostic::new( + "replace-filter-map-next-with-find-map", + "replace filter_map(..).next() with find_map(..)", + ctx.sema.diagnostics_display_range(InFile::new(d.file, d.next_expr.clone().into())).range, + ) + .severity(Severity::WeakWarning) + .with_fixes(fixes(ctx, d)) +} + +fn fixes( + ctx: &DiagnosticsContext<'_>, + d: &hir::ReplaceFilterMapNextWithFindMap, +) -> Option> { + let root = ctx.sema.db.parse_or_expand(d.file)?; + let next_expr = d.next_expr.to_node(&root); + let next_call = ast::MethodCallExpr::cast(next_expr.syntax().clone())?; + + let filter_map_call = ast::MethodCallExpr::cast(next_call.receiver()?.syntax().clone())?; + let filter_map_name_range = filter_map_call.name_ref()?.ident_token()?.text_range(); + let filter_map_args = filter_map_call.arg_list()?; + + let range_to_replace = + TextRange::new(filter_map_name_range.start(), next_expr.syntax().text_range().end()); + let replacement = format!("find_map{}", filter_map_args.syntax().text()); + let trigger_range = next_expr.syntax().text_range(); + + let edit = TextEdit::replace(range_to_replace, replacement); + + let source_change = SourceChange::from_text_edit(d.file.original_file(ctx.sema.db), edit); + + Some(vec![fix( + "replace_with_find_map", + "Replace filter_map(..).next() with find_map()", + source_change, + trigger_range, + )]) +} + +#[cfg(test)] +mod tests { + use crate::diagnostics::tests::check_fix; + + // Register the required standard library types to make the tests work + #[track_caller] + fn check_diagnostics(ra_fixture: &str) { + let prefix = r#" +//- /main.rs crate:main deps:core +use core::iter::Iterator; +use core::option::Option::{self, Some, None}; +"#; + let suffix = r#" +//- /core/lib.rs crate:core +pub mod option { + pub enum Option { Some(T), None } +} +pub mod iter { + pub trait Iterator { + type Item; + fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } + fn next(&mut self) -> Option; + } + pub struct FilterMap {} + impl Iterator for FilterMap { + type Item = i32; + fn next(&mut self) -> i32 { 7 } + } +} +"#; + crate::diagnostics::tests::check_diagnostics(&format!("{}{}{}", prefix, ra_fixture, suffix)) + } + + #[test] + fn replace_filter_map_next_with_find_map2() { + check_diagnostics( + r#" + fn foo() { + let m = [1, 2, 3].iter().filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); + } //^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ replace filter_map(..).next() with find_map(..) +"#, + ); + } + + #[test] + fn replace_filter_map_next_with_find_map_no_diagnostic_without_next() { + check_diagnostics( + r#" +fn foo() { + let m = [1, 2, 3] + .iter() + .filter_map(|x| if *x == 2 { Some (4) } else { None }) + .len(); +} +"#, + ); + } + + #[test] + fn replace_filter_map_next_with_find_map_no_diagnostic_with_intervening_methods() { + check_diagnostics( + r#" +fn foo() { + let m = [1, 2, 3] + .iter() + .filter_map(|x| if *x == 2 { Some (4) } else { None }) + .map(|x| x + 2) + .len(); +} +"#, + ); + } + + #[test] + fn replace_filter_map_next_with_find_map_no_diagnostic_if_not_in_chain() { + check_diagnostics( + r#" +fn foo() { + let m = [1, 2, 3] + .iter() + .filter_map(|x| if *x == 2 { Some (4) } else { None }); + let n = m.next(); +} +"#, + ); + } + + #[test] + fn replace_with_wind_map() { + check_fix( + r#" +//- /main.rs crate:main deps:core +use core::iter::Iterator; +use core::option::Option::{self, Some, None}; +fn foo() { + let m = [1, 2, 3].iter().$0filter_map(|x| if *x == 2 { Some (4) } else { None }).next(); +} +//- /core/lib.rs crate:core +pub mod option { + pub enum Option { Some(T), None } +} +pub mod iter { + pub trait Iterator { + type Item; + fn filter_map(self, f: F) -> FilterMap where F: FnMut(Self::Item) -> Option { FilterMap } + fn next(&mut self) -> Option; + } + pub struct FilterMap {} + impl Iterator for FilterMap { + type Item = i32; + fn next(&mut self) -> i32 { 7 } + } +} +"#, + r#" +use core::iter::Iterator; +use core::option::Option::{self, Some, None}; +fn foo() { + let m = [1, 2, 3].iter().find_map(|x| if *x == 2 { Some (4) } else { None }); +} +"#, + ) + } +} -- cgit v1.2.3 From b66f4bb8d1748b83a6f4c5edc3c77a46b213e1c2 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sun, 13 Jun 2021 20:33:59 +0300 Subject: minor --- crates/ide/src/diagnostics.rs | 38 ---------------------------- crates/ide/src/diagnostics/missing_fields.rs | 38 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 38 deletions(-) (limited to 'crates/ide') diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index ec5318594..814e64ae4 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -671,44 +671,6 @@ mod foo; ); } - #[test] - fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { - check_diagnostics( - r" -struct S { foo: i32, bar: () } -fn baz(s: S) -> i32 { - match s { - S { foo, .. } => foo, - } -} -", - ) - } - - #[test] - fn missing_record_pat_field_box() { - check_diagnostics( - r" -struct S { s: Box } -fn x(a: S) { - let S { box s } = a; -} -", - ) - } - - #[test] - fn missing_record_pat_field_ref() { - check_diagnostics( - r" -struct S { s: u32 } -fn x(a: S) { - let S { ref s } = a; -} -", - ) - } - #[test] fn import_extern_crate_clash_with_inner_item() { // This is more of a resolver test, but doesn't really work with the hir_def testsuite. diff --git a/crates/ide/src/diagnostics/missing_fields.rs b/crates/ide/src/diagnostics/missing_fields.rs index c4b6a3679..d01f05041 100644 --- a/crates/ide/src/diagnostics/missing_fields.rs +++ b/crates/ide/src/diagnostics/missing_fields.rs @@ -93,6 +93,44 @@ fn baz(s: S) { ); } + #[test] + fn missing_record_pat_field_no_diagnostic_if_not_exhaustive() { + check_diagnostics( + r" +struct S { foo: i32, bar: () } +fn baz(s: S) -> i32 { + match s { + S { foo, .. } => foo, + } +} +", + ) + } + + #[test] + fn missing_record_pat_field_box() { + check_diagnostics( + r" +struct S { s: Box } +fn x(a: S) { + let S { box s } = a; +} +", + ) + } + + #[test] + fn missing_record_pat_field_ref() { + check_diagnostics( + r" +struct S { s: u32 } +fn x(a: S) { + let S { ref s } = a; +} +", + ) + } + #[test] fn range_mapping_out_of_macros() { // FIXME: this is very wrong, but somewhat tricky to fix. -- cgit v1.2.3