From 610db04222445e2e25c911dda9a0ae7e30df3cd9 Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Fri, 25 Mar 2022 20:39:03 +0000 Subject: [PATCH] Provide suggestion context in map_unit_fn --- clippy_lints/src/map_unit_fn.rs | 23 ++++++++++------------ tests/ui/option_map_unit_fn_fixable.fixed | 5 ++++- tests/ui/option_map_unit_fn_fixable.rs | 5 ++++- tests/ui/option_map_unit_fn_fixable.stderr | 12 +++++++++-- tests/ui/result_map_unit_fn_fixable.fixed | 2 ++ tests/ui/result_map_unit_fn_fixable.rs | 2 ++ tests/ui/result_map_unit_fn_fixable.stderr | 10 +++++++++- 7 files changed, 41 insertions(+), 18 deletions(-) diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index 0f6ac478432..f552d5c1afa 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet; +use clippy_utils::source::{snippet, snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{iter_input_pats, method_chain_args}; use if_chain::if_chain; @@ -217,36 +217,33 @@ fn lint_map_unit_fn(cx: &LateContext<'_>, stmt: &hir::Stmt<'_>, expr: &hir::Expr let fn_arg = &map_args[1]; if is_unit_function(cx, fn_arg) { + let mut applicability = Applicability::MachineApplicable; let msg = suggestion_msg("function", map_type); let suggestion = format!( "if let {0}({binding}) = {1} {{ {2}({binding}) }}", variant, - snippet(cx, var_arg.span, "_"), - snippet(cx, fn_arg.span, "_"), + snippet_with_applicability(cx, var_arg.span, "_", &mut applicability), + snippet_with_applicability(cx, fn_arg.span, "_", &mut applicability), binding = let_binding_name(cx, var_arg) ); span_lint_and_then(cx, lint, expr.span, &msg, |diag| { - diag.span_suggestion(stmt.span, "try this", suggestion, Applicability::MachineApplicable); + diag.span_suggestion(stmt.span, "try this", suggestion, applicability); }); } else if let Some((binding, closure_expr)) = unit_closure(cx, fn_arg) { let msg = suggestion_msg("closure", map_type); span_lint_and_then(cx, lint, expr.span, &msg, |diag| { if let Some(reduced_expr_span) = reduce_unit_expression(cx, closure_expr) { + let mut applicability = Applicability::MachineApplicable; let suggestion = format!( "if let {0}({1}) = {2} {{ {3} }}", variant, - snippet(cx, binding.pat.span, "_"), - snippet(cx, var_arg.span, "_"), - snippet(cx, reduced_expr_span, "_") - ); - diag.span_suggestion( - stmt.span, - "try this", - suggestion, - Applicability::MachineApplicable, // snippet + snippet_with_applicability(cx, binding.pat.span, "_", &mut applicability), + snippet_with_applicability(cx, var_arg.span, "_", &mut applicability), + snippet_with_context(cx, reduced_expr_span, var_arg.span.ctxt(), "_", &mut applicability).0, ); + diag.span_suggestion(stmt.span, "try this", suggestion, applicability); } else { let suggestion = format!( "if let {0}({1}) = {2} {{ ... }}", diff --git a/tests/ui/option_map_unit_fn_fixable.fixed b/tests/ui/option_map_unit_fn_fixable.fixed index 7d29445e66c..1290bd8efeb 100644 --- a/tests/ui/option_map_unit_fn_fixable.fixed +++ b/tests/ui/option_map_unit_fn_fixable.fixed @@ -80,6 +80,9 @@ fn option_map_unit_fn() { if let Some(ref value) = x.field { do_nothing(value + captured) } - if let Some(a) = option() { do_nothing(a) }} + if let Some(a) = option() { do_nothing(a) } + + if let Some(value) = option() { println!("{:?}", value) } +} fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.rs b/tests/ui/option_map_unit_fn_fixable.rs index b6f834f686f..f3e5b62c65b 100644 --- a/tests/ui/option_map_unit_fn_fixable.rs +++ b/tests/ui/option_map_unit_fn_fixable.rs @@ -80,6 +80,9 @@ fn option_map_unit_fn() { x.field.map(|ref value| { do_nothing(value + captured) }); - option().map(do_nothing);} + option().map(do_nothing); + + option().map(|value| println!("{:?}", value)); +} fn main() {} diff --git a/tests/ui/option_map_unit_fn_fixable.stderr b/tests/ui/option_map_unit_fn_fixable.stderr index 8abdbcafb6e..ab2a294a060 100644 --- a/tests/ui/option_map_unit_fn_fixable.stderr +++ b/tests/ui/option_map_unit_fn_fixable.stderr @@ -139,10 +139,18 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); error: called `map(f)` on an `Option` value where `f` is a function that returns the unit type `()` --> $DIR/option_map_unit_fn_fixable.rs:83:5 | -LL | option().map(do_nothing);} +LL | option().map(do_nothing); | ^^^^^^^^^^^^^^^^^^^^^^^^- | | | help: try this: `if let Some(a) = option() { do_nothing(a) }` -error: aborting due to 18 previous errors +error: called `map(f)` on an `Option` value where `f` is a closure that returns the unit type `()` + --> $DIR/option_map_unit_fn_fixable.rs:85:5 + | +LL | option().map(|value| println!("{:?}", value)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- + | | + | help: try this: `if let Some(value) = option() { println!("{:?}", value) }` + +error: aborting due to 19 previous errors diff --git a/tests/ui/result_map_unit_fn_fixable.fixed b/tests/ui/result_map_unit_fn_fixable.fixed index 631042c616b..14c331f67e7 100644 --- a/tests/ui/result_map_unit_fn_fixable.fixed +++ b/tests/ui/result_map_unit_fn_fixable.fixed @@ -75,6 +75,8 @@ fn result_map_unit_fn() { if let Ok(ref value) = x.field { do_nothing(value + captured) } + + if let Ok(value) = x.field { println!("{:?}", value) } } fn main() {} diff --git a/tests/ui/result_map_unit_fn_fixable.rs b/tests/ui/result_map_unit_fn_fixable.rs index 679eb232626..8b0fca9ece1 100644 --- a/tests/ui/result_map_unit_fn_fixable.rs +++ b/tests/ui/result_map_unit_fn_fixable.rs @@ -75,6 +75,8 @@ fn result_map_unit_fn() { x.field.map(|ref value| { do_nothing(value + captured) }); + + x.field.map(|value| println!("{:?}", value)); } fn main() {} diff --git a/tests/ui/result_map_unit_fn_fixable.stderr b/tests/ui/result_map_unit_fn_fixable.stderr index 4f3a8c6b792..782febd5264 100644 --- a/tests/ui/result_map_unit_fn_fixable.stderr +++ b/tests/ui/result_map_unit_fn_fixable.stderr @@ -136,5 +136,13 @@ LL | x.field.map(|ref value| { do_nothing(value + captured) }); | | | help: try this: `if let Ok(ref value) = x.field { do_nothing(value + captured) }` -error: aborting due to 17 previous errors +error: called `map(f)` on an `Result` value where `f` is a closure that returns the unit type `()` + --> $DIR/result_map_unit_fn_fixable.rs:79:5 + | +LL | x.field.map(|value| println!("{:?}", value)); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^- + | | + | help: try this: `if let Ok(value) = x.field { println!("{:?}", value) }` + +error: aborting due to 18 previous errors