diff options
| -rw-r--r-- | clippy_lints/src/methods/option_map_or_none.rs | 138 | ||||
| -rw-r--r-- | tests/ui/option_map_or_none.fixed | 6 | ||||
| -rw-r--r-- | tests/ui/option_map_or_none.stderr | 10 |
3 files changed, 96 insertions, 58 deletions
diff --git a/clippy_lints/src/methods/option_map_or_none.rs b/clippy_lints/src/methods/option_map_or_none.rs index 8f61bd97b45..00a4dda9701 100644 --- a/clippy_lints/src/methods/option_map_or_none.rs +++ b/clippy_lints/src/methods/option_map_or_none.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_lang_ctor; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_lang_ctor, single_segment_path}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::LangItem::{OptionNone, OptionSome}; @@ -11,6 +11,26 @@ use rustc_span::symbol::sym; use super::OPTION_MAP_OR_NONE; use super::RESULT_MAP_OR_INTO_OPTION; +fn reduce_unit_expression<'a>( + cx: &LateContext<'_>, + expr: &'a hir::Expr<'_>, +) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> { + match expr.kind { + hir::ExprKind::Call(func, arg_char) => Some((func, arg_char)), + hir::ExprKind::Block(block, _) => { + match block.expr { + Some(inner_expr) => { + // If block only contains an expression, + // reduce `{ X }` to `X` + reduce_unit_expression(cx, inner_expr) + }, + _ => None, + } + }, + _ => None, + } +} + /// lint use of `_.map_or(None, _)` for `Option`s and `Result`s pub(super) fn check<'tcx>( cx: &LateContext<'tcx>, @@ -31,58 +51,78 @@ pub(super) fn check<'tcx>( return; } - let (lint_name, msg, instead, hint) = { - let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind { - is_lang_ctor(cx, qpath, OptionNone) - } else { - return; - }; + // let (lint_name, msg, instead, hint) = { + let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind { + is_lang_ctor(cx, qpath, OptionNone) + } else { + return; + }; - if !default_arg_is_none { - // nothing to lint! - return; - } + if !default_arg_is_none { + // nothing to lint! + return; + } - let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind { - is_lang_ctor(cx, qpath, OptionSome) - } else { - false - }; + let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind { + is_lang_ctor(cx, qpath, OptionSome) + } else { + false + }; - if is_option { - let self_snippet = snippet(cx, recv.span, ".."); - let func_snippet = snippet(cx, map_arg.span, ".."); - let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \ + if is_option { + let self_snippet = snippet(cx, recv.span, ".."); + if let hir::ExprKind::Closure(_, _, id, _, _) = map_arg.kind { + if_chain! { + let body = cx.tcx.hir().body(id); + if let Some((func, arg_char)) = reduce_unit_expression(cx, &body.value); + if arg_char.len() == 1; + if let hir::ExprKind::Path(ref qpath) = func.kind; + if let Some(segment) = single_segment_path(qpath); + if segment.ident.name == sym::Some; + then { + let func_snippet = snippet(cx, arg_char[0].span, ".."); + let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \ `map(..)` instead"; - ( - OPTION_MAP_OR_NONE, - msg, - "try using `map` instead", - format!("{0}.map({1})", self_snippet, func_snippet), - ) - } else if f_arg_is_some { - let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \ - `ok()` instead"; - let self_snippet = snippet(cx, recv.span, ".."); - ( - RESULT_MAP_OR_INTO_OPTION, - msg, - "try using `ok` instead", - format!("{0}.ok()", self_snippet), - ) - } else { - // nothing to lint! - return; + return span_lint_and_sugg( + cx, + OPTION_MAP_OR_NONE, + expr.span, + msg, + "try using `map` instead", + format!("{0}.map({1})", self_snippet, func_snippet), + Applicability::MachineApplicable, + ); + } + } } - }; - span_lint_and_sugg( - cx, - lint_name, - expr.span, - msg, - instead, - hint, - Applicability::MachineApplicable, - ); + let func_snippet = snippet(cx, map_arg.span, ".."); + let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \ + `and_then(..)` instead"; + span_lint_and_sugg( + cx, + OPTION_MAP_OR_NONE, + expr.span, + msg, + "try using `and_then` instead", + format!("{0}.and_then({1})", self_snippet, func_snippet), + Applicability::MachineApplicable, + ) + } else if f_arg_is_some { + let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \ + `ok()` instead"; + let self_snippet = snippet(cx, recv.span, ".."); + span_lint_and_sugg( + cx, + RESULT_MAP_OR_INTO_OPTION, + expr.span, + msg, + "try using `ok` instead", + format!("{0}.ok()", self_snippet), + Applicability::MachineApplicable, + ) + } else { + // nothing to lint! + return; + } } diff --git a/tests/ui/option_map_or_none.fixed b/tests/ui/option_map_or_none.fixed index 77ad2d5285a..9333cb89c9a 100644 --- a/tests/ui/option_map_or_none.fixed +++ b/tests/ui/option_map_or_none.fixed @@ -5,7 +5,7 @@ fn main() { let opt = Some(1); let bar = |_| { - Some(1) + Some(1) }; // Check `OPTION_MAP_OR_NONE`. @@ -13,9 +13,7 @@ fn main() { let _ :Option<i32> = opt.map(|x| x + 1); // Multi-line case. #[rustfmt::skip] - let _ :Option<i32> = opt.map(|x| { - x + 1 - }); + let _ :Option<i32> = opt.map(|x| x + 1); // function returning `Option` let _ :Option<i32> = opt.and_then(bar); } diff --git a/tests/ui/option_map_or_none.stderr b/tests/ui/option_map_or_none.stderr index 86ac93b9c80..3d75f2776f6 100644 --- a/tests/ui/option_map_or_none.stderr +++ b/tests/ui/option_map_or_none.stderr @@ -1,16 +1,16 @@ error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead - --> $DIR/option_map_or_none.rs:10:13 + --> $DIR/option_map_or_none.rs:13:26 | LL | let _ :Option<i32> = opt.map_or(None, |x| Some(x + 1)); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `map` instead: `opt.map(|x| x + 1)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `map` instead: `opt.map(|x| x + 1)` | = note: `-D clippy::option-map-or-none` implied by `-D warnings` error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead - --> $DIR/option_map_or_none.rs:13:13 + --> $DIR/option_map_or_none.rs:16:26 | LL | let _ :Option<i32> = opt.map_or(None, |x| { - | _____________^ + | __________________________^ LL | | Some(x + 1) LL | | }); | |_________________________^ @@ -22,7 +22,7 @@ LL + x + 1 LL ~ }); | -error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead +error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead --> $DIR/option_map_or_none.rs:20:26 | LL | let _ :Option<i32> = opt.map_or(None, bar); |