diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index b1da36a0d40..46af1d8fb09 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -136,7 +136,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(manual_map::MANUAL_MAP), LintId::of(manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE), LintId::of(manual_strip::MANUAL_STRIP), - LintId::of(manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(map_clone::MAP_CLONE), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), @@ -144,6 +143,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(match_str_case_mismatch::MATCH_STR_CASE_MISMATCH), LintId::of(matches::COLLAPSIBLE_MATCH), LintId::of(matches::INFALLIBLE_DESTRUCTURING_MATCH), + LintId::of(matches::MANUAL_UNWRAP_OR), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_LIKE_MATCHES_MACRO), LintId::of(matches::MATCH_OVERLAPPING_ARM), diff --git a/clippy_lints/src/lib.register_complexity.rs b/clippy_lints/src/lib.register_complexity.rs index 29b38a01b7e..4f1c3673f85 100644 --- a/clippy_lints/src/lib.register_complexity.rs +++ b/clippy_lints/src/lib.register_complexity.rs @@ -25,9 +25,9 @@ store.register_group(true, "clippy::complexity", Some("clippy_complexity"), vec! LintId::of(loops::SINGLE_ELEMENT_LOOP), LintId::of(loops::WHILE_LET_LOOP), LintId::of(manual_strip::MANUAL_STRIP), - LintId::of(manual_unwrap_or::MANUAL_UNWRAP_OR), LintId::of(map_unit_fn::OPTION_MAP_UNIT_FN), LintId::of(map_unit_fn::RESULT_MAP_UNIT_FN), + LintId::of(matches::MANUAL_UNWRAP_OR), LintId::of(matches::MATCH_AS_REF), LintId::of(matches::MATCH_SINGLE_BINDING), LintId::of(matches::NEEDLESS_MATCH), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 58e18b710ea..cb52000fa55 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -254,7 +254,6 @@ store.register_lints(&[ manual_non_exhaustive::MANUAL_NON_EXHAUSTIVE, manual_ok_or::MANUAL_OK_OR, manual_strip::MANUAL_STRIP, - manual_unwrap_or::MANUAL_UNWRAP_OR, map_clone::MAP_CLONE, map_err_ignore::MAP_ERR_IGNORE, map_unit_fn::OPTION_MAP_UNIT_FN, @@ -264,6 +263,7 @@ store.register_lints(&[ match_str_case_mismatch::MATCH_STR_CASE_MISMATCH, matches::COLLAPSIBLE_MATCH, matches::INFALLIBLE_DESTRUCTURING_MATCH, + matches::MANUAL_UNWRAP_OR, matches::MATCH_AS_REF, matches::MATCH_BOOL, matches::MATCH_LIKE_MATCHES_MACRO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6e16d470ac8..3fdd0db8187 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -283,7 +283,6 @@ mod manual_map; mod manual_non_exhaustive; mod manual_ok_or; mod manual_strip; -mod manual_unwrap_or; mod map_clone; mod map_err_ignore; mod map_unit_fn; @@ -834,7 +833,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(repeat_once::RepeatOnce)); store.register_late_pass(|| Box::new(unwrap_in_result::UnwrapInResult)); store.register_late_pass(|| Box::new(self_assignment::SelfAssignment)); - store.register_late_pass(|| Box::new(manual_unwrap_or::ManualUnwrapOr)); store.register_late_pass(|| Box::new(manual_ok_or::ManualOkOr)); store.register_late_pass(|| Box::new(float_equality_without_abs::FloatEqualityWithoutAbs)); store.register_late_pass(|| Box::new(semicolon_if_nothing_returned::SemicolonIfNothingReturned)); diff --git a/clippy_lints/src/manual_unwrap_or.rs b/clippy_lints/src/manual_unwrap_or.rs deleted file mode 100644 index b3a91d9f18f..00000000000 --- a/clippy_lints/src/manual_unwrap_or.rs +++ /dev/null @@ -1,123 +0,0 @@ -use clippy_utils::consts::constant_simple; -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; -use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::usage::contains_return_break_continue_macro; -use clippy_utils::{in_constant, is_lang_ctor, path_to_local_id, sugg}; -use if_chain::if_chain; -use rustc_errors::Applicability; -use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; -use rustc_hir::{Arm, Expr, ExprKind, PatKind}; -use rustc_lint::LintContext; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::lint::in_external_macro; -use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; - -declare_clippy_lint! { - /// ### What it does - /// Finds patterns that reimplement `Option::unwrap_or` or `Result::unwrap_or`. - /// - /// ### Why is this bad? - /// Concise code helps focusing on behavior instead of boilerplate. - /// - /// ### Example - /// ```rust - /// let foo: Option = None; - /// match foo { - /// Some(v) => v, - /// None => 1, - /// }; - /// ``` - /// - /// Use instead: - /// ```rust - /// let foo: Option = None; - /// foo.unwrap_or(1); - /// ``` - #[clippy::version = "1.49.0"] - pub MANUAL_UNWRAP_OR, - complexity, - "finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`" -} - -declare_lint_pass!(ManualUnwrapOr => [MANUAL_UNWRAP_OR]); - -impl<'tcx> LateLintPass<'tcx> for ManualUnwrapOr { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if in_external_macro(cx.sess(), expr.span) || in_constant(cx, expr.hir_id) { - return; - } - lint_manual_unwrap_or(cx, expr); - } -} - -fn lint_manual_unwrap_or<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { - if_chain! { - if arms.len() == 2; - if arms.iter().all(|arm| arm.guard.is_none()); - if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| { - match arm.pat.kind { - PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), - PatKind::TupleStruct(ref qpath, [pat], _) => - matches!(pat.kind, PatKind::Wild) && is_lang_ctor(cx, qpath, ResultErr), - _ => false, - } - }); - let unwrap_arm = &arms[1 - idx]; - if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind; - if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); - if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; - if path_to_local_id(unwrap_arm.body, binding_hir_id); - if cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty(); - if !contains_return_break_continue_macro(or_arm.body); - then { - Some(or_arm) - } else { - None - } - } - } - - if_chain! { - if let ExprKind::Match(scrutinee, match_arms, _) = expr.kind; - let ty = cx.typeck_results().expr_ty(scrutinee); - if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::Option) { - Some("Option") - } else if is_type_diagnostic_item(cx, ty, sym::Result) { - Some("Result") - } else { - None - }; - if let Some(or_arm) = applicable_or_arm(cx, match_arms); - if let Some(or_body_snippet) = snippet_opt(cx, or_arm.body.span); - if let Some(indent) = indent_of(cx, expr.span); - if constant_simple(cx, cx.typeck_results(), or_arm.body).is_some(); - then { - let reindented_or_body = - reindent_multiline(or_body_snippet.into(), true, Some(indent)); - - let suggestion = if scrutinee.span.from_expansion() { - // we don't want parentheses around macro, e.g. `(some_macro!()).unwrap_or(0)` - sugg::Sugg::hir_with_macro_callsite(cx, scrutinee, "..") - } - else { - sugg::Sugg::hir(cx, scrutinee, "..").maybe_par() - }; - - span_lint_and_sugg( - cx, - MANUAL_UNWRAP_OR, expr.span, - &format!("this pattern reimplements `{}::unwrap_or`", ty_name), - "replace with", - format!( - "{}.unwrap_or({})", - suggestion, - reindented_or_body, - ), - Applicability::MachineApplicable, - ); - } - } -} diff --git a/clippy_lints/src/matches/manual_unwrap_or.rs b/clippy_lints/src/matches/manual_unwrap_or.rs new file mode 100644 index 00000000000..e1111c80f2f --- /dev/null +++ b/clippy_lints/src/matches/manual_unwrap_or.rs @@ -0,0 +1,83 @@ +use clippy_utils::consts::constant_simple; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::usage::contains_return_break_continue_macro; +use clippy_utils::{is_lang_ctor, path_to_local_id, sugg}; +use if_chain::if_chain; +use rustc_errors::Applicability; +use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; +use rustc_hir::{Arm, Expr, PatKind}; +use rustc_lint::LateContext; +use rustc_span::sym; + +use super::MANUAL_UNWRAP_OR; + +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'tcx>, scrutinee: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>]) { + let ty = cx.typeck_results().expr_ty(scrutinee); + if_chain! { + if let Some(ty_name) = if is_type_diagnostic_item(cx, ty, sym::Option) { + Some("Option") + } else if is_type_diagnostic_item(cx, ty, sym::Result) { + Some("Result") + } else { + None + }; + if let Some(or_arm) = applicable_or_arm(cx, arms); + if let Some(or_body_snippet) = snippet_opt(cx, or_arm.body.span); + if let Some(indent) = indent_of(cx, expr.span); + if constant_simple(cx, cx.typeck_results(), or_arm.body).is_some(); + then { + let reindented_or_body = + reindent_multiline(or_body_snippet.into(), true, Some(indent)); + + let suggestion = if scrutinee.span.from_expansion() { + // we don't want parentheses around macro, e.g. `(some_macro!()).unwrap_or(0)` + sugg::Sugg::hir_with_macro_callsite(cx, scrutinee, "..") + } + else { + sugg::Sugg::hir(cx, scrutinee, "..").maybe_par() + }; + + span_lint_and_sugg( + cx, + MANUAL_UNWRAP_OR, expr.span, + &format!("this pattern reimplements `{}::unwrap_or`", ty_name), + "replace with", + format!( + "{}.unwrap_or({})", + suggestion, + reindented_or_body, + ), + Applicability::MachineApplicable, + ); + } + } +} + +fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&'a Arm<'a>> { + if_chain! { + if arms.len() == 2; + if arms.iter().all(|arm| arm.guard.is_none()); + if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| { + match arm.pat.kind { + PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), + PatKind::TupleStruct(ref qpath, [pat], _) => + matches!(pat.kind, PatKind::Wild) && is_lang_ctor(cx, qpath, ResultErr), + _ => false, + } + }); + let unwrap_arm = &arms[1 - idx]; + if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind; + if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); + if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; + if path_to_local_id(unwrap_arm.body, binding_hir_id); + if cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty(); + if !contains_return_break_continue_macro(or_arm.body); + then { + Some(or_arm) + } else { + None + } + } +} diff --git a/clippy_lints/src/matches/mod.rs b/clippy_lints/src/matches/mod.rs index 4a77fb8c31b..ade47b1ed88 100644 --- a/clippy_lints/src/matches/mod.rs +++ b/clippy_lints/src/matches/mod.rs @@ -1,5 +1,5 @@ use clippy_utils::source::{snippet_opt, span_starts_with, walk_span_to_context}; -use clippy_utils::{higher, meets_msrv, msrvs}; +use clippy_utils::{higher, in_constant, meets_msrv, msrvs}; use rustc_hir::{Arm, Expr, ExprKind, Local, MatchSource, Pat}; use rustc_lexer::{tokenize, TokenKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -10,6 +10,7 @@ use rustc_span::{Span, SpanData, SyntaxContext}; mod collapsible_match; mod infallible_destructuring_match; +mod manual_unwrap_or; mod match_as_ref; mod match_bool; mod match_like_matches; @@ -650,6 +651,33 @@ declare_clippy_lint! { "Nested `match` or `if let` expressions where the patterns may be \"collapsed\" together." } +declare_clippy_lint! { + /// ### What it does + /// Finds patterns that reimplement `Option::unwrap_or` or `Result::unwrap_or`. + /// + /// ### Why is this bad? + /// Concise code helps focusing on behavior instead of boilerplate. + /// + /// ### Example + /// ```rust + /// let foo: Option = None; + /// match foo { + /// Some(v) => v, + /// None => 1, + /// }; + /// ``` + /// + /// Use instead: + /// ```rust + /// let foo: Option = None; + /// foo.unwrap_or(1); + /// ``` + #[clippy::version = "1.49.0"] + pub MANUAL_UNWRAP_OR, + complexity, + "finds patterns that can be encoded more concisely with `Option::unwrap_or` or `Result::unwrap_or`" +} + #[derive(Default)] pub struct Matches { msrv: Option, @@ -685,6 +713,7 @@ impl_lint_pass!(Matches => [ MATCH_SAME_ARMS, NEEDLESS_MATCH, COLLAPSIBLE_MATCH, + MANUAL_UNWRAP_OR, ]); impl<'tcx> LateLintPass<'tcx> for Matches { @@ -722,6 +751,10 @@ impl<'tcx> LateLintPass<'tcx> for Matches { match_as_ref::check(cx, ex, arms, expr); needless_match::check_match(cx, ex, arms, expr); + if !in_constant(cx, expr.hir_id) { + manual_unwrap_or::check(cx, expr, ex, arms); + } + if self.infallible_destructuring_match_linted { self.infallible_destructuring_match_linted = false; } else {