diff options
| -rw-r--r-- | clippy_lints/src/manual_map.rs | 40 | ||||
| -rw-r--r-- | tests/ui/manual_map_option.fixed | 13 | ||||
| -rw-r--r-- | tests/ui/manual_map_option.rs | 11 | ||||
| -rw-r--r-- | tests/ui/manual_map_option.stderr | 26 |
4 files changed, 60 insertions, 30 deletions
diff --git a/clippy_lints/src/manual_map.rs b/clippy_lints/src/manual_map.rs index e399c8fda55..a50a3943bab 100644 --- a/clippy_lints/src/manual_map.rs +++ b/clippy_lints/src/manual_map.rs @@ -1,10 +1,14 @@ -use crate::utils::{ - is_type_diagnostic_item, match_def_path, paths, peel_hir_expr_refs, peel_mid_ty_refs_is_mutable, - snippet_with_applicability, span_lint_and_sugg, +use crate::{ + map_unit_fn::OPTION_MAP_UNIT_FN, + matches::MATCH_AS_REF, + utils::{ + is_allowed, is_type_diagnostic_item, match_def_path, match_var, paths, peel_hir_expr_refs, + peel_mid_ty_refs_is_mutable, snippet_with_applicability, span_lint_and_sugg, + }, }; use rustc_ast::util::parser::PREC_POSTFIX; use rustc_errors::Applicability; -use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, Path, QPath}; +use rustc_hir::{Arm, BindingAnnotation, Block, Expr, ExprKind, Mutability, Pat, PatKind, QPath}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -37,6 +41,7 @@ declare_clippy_lint! { declare_lint_pass!(ManualMap => [MANUAL_MAP]); impl LateLintPass<'_> for ManualMap { + #[allow(clippy::too_many_lines)] fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { if in_external_macro(cx.sess(), expr.span) { return; @@ -88,14 +93,17 @@ impl LateLintPass<'_> for ManualMap { None => return, }; + if cx.typeck_results().expr_ty(some_expr) == cx.tcx.types.unit + && !is_allowed(cx, OPTION_MAP_UNIT_FN, expr.hir_id) + { + return; + } + // Determine which binding mode to use. let explicit_ref = some_pat.contains_explicit_ref_binding(); - let binding_mutability = explicit_ref.or(if ty_ref_count != pat_ref_count { - Some(ty_mutability) - } else { - None - }); - let as_ref_str = match binding_mutability { + let binding_ref = explicit_ref.or_else(|| (ty_ref_count != pat_ref_count).then(|| ty_mutability)); + + let as_ref_str = match binding_ref { Some(Mutability::Mut) => ".as_mut()", Some(Mutability::Not) => ".as_ref()", None => "", @@ -118,6 +126,13 @@ impl LateLintPass<'_> for ManualMap { if let Some(func) = can_pass_as_func(cx, some_binding, some_expr) { snippet_with_applicability(cx, func.span, "..", &mut app).into_owned() } else { + if match_var(some_expr, some_binding.name) + && !is_allowed(cx, MATCH_AS_REF, expr.hir_id) + && binding_ref.is_some() + { + return; + } + // `ref` and `ref mut` annotations were handled earlier. let annotation = if matches!(annotation, BindingAnnotation::Mutable) { "mut " @@ -161,10 +176,7 @@ impl LateLintPass<'_> for ManualMap { fn can_pass_as_func(cx: &LateContext<'tcx>, binding: Ident, expr: &'tcx Expr<'_>) -> Option<&'tcx Expr<'tcx>> { match expr.kind { ExprKind::Call(func, [arg]) - if matches!(arg.kind, - ExprKind::Path(QPath::Resolved(None, Path { segments: [path], ..})) - if path.ident == binding - ) && cx.typeck_results().expr_adjustments(arg).is_empty() => + if match_var(arg, binding.name) && cx.typeck_results().expr_adjustments(arg).is_empty() => { Some(func) }, diff --git a/tests/ui/manual_map_option.fixed b/tests/ui/manual_map_option.fixed index 6cb9a37b230..19350906758 100644 --- a/tests/ui/manual_map_option.fixed +++ b/tests/ui/manual_map_option.fixed @@ -32,9 +32,18 @@ fn main() { Some(0).map(|x| x + x); - Some(String::new()).as_mut().map(|x| x.push_str("")); + #[warn(clippy::option_map_unit_fn)] + match &mut Some(String::new()) { + Some(x) => Some(x.push_str("")), + None => None, + }; - Some(String::new()).as_ref().map(|x| &**x); + #[allow(clippy::option_map_unit_fn)] + { + Some(String::new()).as_mut().map(|x| x.push_str("")); + } + + Some(String::new()).as_ref().map(|x| x.len()); Some(String::new()).as_ref().map(|x| x.is_empty()); diff --git a/tests/ui/manual_map_option.rs b/tests/ui/manual_map_option.rs index b9753060b99..8b8187db0a9 100644 --- a/tests/ui/manual_map_option.rs +++ b/tests/ui/manual_map_option.rs @@ -66,13 +66,22 @@ fn main() { &&_ => None, }; + #[warn(clippy::option_map_unit_fn)] match &mut Some(String::new()) { Some(x) => Some(x.push_str("")), None => None, }; + #[allow(clippy::option_map_unit_fn)] + { + match &mut Some(String::new()) { + Some(x) => Some(x.push_str("")), + None => None, + }; + } + match &mut Some(String::new()) { - Some(ref x) => Some(&**x), + Some(ref x) => Some(x.len()), None => None, }; diff --git a/tests/ui/manual_map_option.stderr b/tests/ui/manual_map_option.stderr index f8e1bda83b2..210a30d05d4 100644 --- a/tests/ui/manual_map_option.stderr +++ b/tests/ui/manual_map_option.stderr @@ -101,25 +101,25 @@ LL | | }; | |_____^ help: try this: `Some(0).map(|x| x + x)` error: manual implementation of `Option::map` - --> $DIR/manual_map_option.rs:69:5 + --> $DIR/manual_map_option.rs:77:9 | -LL | / match &mut Some(String::new()) { -LL | | Some(x) => Some(x.push_str("")), -LL | | None => None, -LL | | }; - | |_____^ help: try this: `Some(String::new()).as_mut().map(|x| x.push_str(""))` +LL | / match &mut Some(String::new()) { +LL | | Some(x) => Some(x.push_str("")), +LL | | None => None, +LL | | }; + | |_________^ help: try this: `Some(String::new()).as_mut().map(|x| x.push_str(""))` error: manual implementation of `Option::map` - --> $DIR/manual_map_option.rs:74:5 + --> $DIR/manual_map_option.rs:83:5 | LL | / match &mut Some(String::new()) { -LL | | Some(ref x) => Some(&**x), +LL | | Some(ref x) => Some(x.len()), LL | | None => None, LL | | }; - | |_____^ help: try this: `Some(String::new()).as_ref().map(|x| &**x)` + | |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.len())` error: manual implementation of `Option::map` - --> $DIR/manual_map_option.rs:79:5 + --> $DIR/manual_map_option.rs:88:5 | LL | / match &mut &Some(String::new()) { LL | | Some(x) => Some(x.is_empty()), @@ -128,7 +128,7 @@ LL | | }; | |_____^ help: try this: `Some(String::new()).as_ref().map(|x| x.is_empty())` error: manual implementation of `Option::map` - --> $DIR/manual_map_option.rs:84:5 + --> $DIR/manual_map_option.rs:93:5 | LL | / match Some((0, 1, 2)) { LL | | Some((x, y, z)) => Some(x + y + z), @@ -137,7 +137,7 @@ LL | | }; | |_____^ help: try this: `Some((0, 1, 2)).map(|(x, y, z)| x + y + z)` error: manual implementation of `Option::map` - --> $DIR/manual_map_option.rs:89:5 + --> $DIR/manual_map_option.rs:98:5 | LL | / match Some([1, 2, 3]) { LL | | Some([first, ..]) => Some(first), @@ -146,7 +146,7 @@ LL | | }; | |_____^ help: try this: `Some([1, 2, 3]).map(|[first, ..]| first)` error: manual implementation of `Option::map` - --> $DIR/manual_map_option.rs:94:5 + --> $DIR/manual_map_option.rs:103:5 | LL | / match &Some((String::new(), "test")) { LL | | Some((x, y)) => Some((y, x)), |