From c4a80f2e3e8375ff54826840e2e2a5c53a20e2ce Mon Sep 17 00:00:00 2001 From: Yuxiang Qiu Date: Fri, 24 Nov 2023 20:56:57 -0500 Subject: feat: add `manual_is_variant_and` lint --- clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lines_filter_map_ok.rs | 3 +- clippy_lints/src/methods/manual_is_variant_and.rs | 59 +++++++++++++++++++++++ clippy_lints/src/methods/mod.rs | 36 +++++++++++++- clippy_lints/src/methods/option_map_unwrap_or.rs | 2 +- clippy_lints/src/methods/path_ends_with_ext.rs | 2 +- 6 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 clippy_lints/src/methods/manual_is_variant_and.rs (limited to 'clippy_lints/src') diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index a00e2d3e044..1db94ad3045 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -385,6 +385,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::JOIN_ABSOLUTE_PATHS_INFO, crate::methods::MANUAL_FILTER_MAP_INFO, crate::methods::MANUAL_FIND_MAP_INFO, + crate::methods::MANUAL_IS_VARIANT_AND_INFO, crate::methods::MANUAL_NEXT_BACK_INFO, crate::methods::MANUAL_OK_OR_INFO, crate::methods::MANUAL_SATURATING_ARITHMETIC_INFO, diff --git a/clippy_lints/src/lines_filter_map_ok.rs b/clippy_lints/src/lines_filter_map_ok.rs index 8a0955147bb..29957e423b0 100644 --- a/clippy_lints/src/lines_filter_map_ok.rs +++ b/clippy_lints/src/lines_filter_map_ok.rs @@ -96,8 +96,7 @@ fn should_lint(cx: &LateContext<'_>, args: &[Expr<'_>], method_str: &str) -> boo ExprKind::Path(qpath) => cx .qpath_res(qpath, fm_arg.hir_id) .opt_def_id() - .map(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)) - .unwrap_or_default(), + .is_some_and(|did| match_def_path(cx, did, &paths::CORE_RESULT_OK_METHOD)), // Detect `|x| x.ok()` ExprKind::Closure(Closure { body, .. }) => { if let Body { diff --git a/clippy_lints/src/methods/manual_is_variant_and.rs b/clippy_lints/src/methods/manual_is_variant_and.rs new file mode 100644 index 00000000000..d29acd4622a --- /dev/null +++ b/clippy_lints/src/methods/manual_is_variant_and.rs @@ -0,0 +1,59 @@ +use clippy_config::msrvs::{Msrv, OPTION_RESULT_IS_VARIANT_AND}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use clippy_utils::ty::is_type_diagnostic_item; +use rustc_errors::Applicability; +use rustc_lint::LateContext; +use rustc_span::{sym, Span}; + +use super::MANUAL_IS_VARIANT_AND; + +pub(super) fn check<'tcx>( + cx: &LateContext<'_>, + expr: &'tcx rustc_hir::Expr<'_>, + map_recv: &'tcx rustc_hir::Expr<'_>, + map_arg: &'tcx rustc_hir::Expr<'_>, + map_span: Span, + msrv: &Msrv, +) { + // Don't lint if: + + // 1. the `expr` is generated by a macro + if expr.span.from_expansion() { + return; + } + + // 2. the caller of `map()` is neither `Option` nor `Result` + let is_option = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(map_recv), sym::Option); + let is_result = is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(map_recv), sym::Result); + if !is_option && !is_result { + return; + } + + // 3. the caller of `unwrap_or_default` is neither `Option` nor `Result` + if !cx.typeck_results().expr_ty(expr).is_bool() { + return; + } + + // 4. msrv doesn't meet `OPTION_RESULT_IS_VARIANT_AND` + if !msrv.meets(OPTION_RESULT_IS_VARIANT_AND) { + return; + } + + let lint_msg = if is_option { + "called `map().unwrap_or_default()` on an `Option` value" + } else { + "called `map().unwrap_or_default()` on a `Result` value" + }; + let suggestion = if is_option { "is_some_and" } else { "is_ok_and" }; + + span_lint_and_sugg( + cx, + MANUAL_IS_VARIANT_AND, + expr.span.with_lo(map_span.lo()), + lint_msg, + "use", + format!("{}({})", suggestion, snippet(cx, map_arg.span, "..")), + Applicability::MachineApplicable, + ); +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 25b1ea526e2..0cde17ef5ad 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -51,6 +51,7 @@ mod iter_skip_zero; mod iter_with_drain; mod iterator_step_by_zero; mod join_absolute_paths; +mod manual_is_variant_and; mod manual_next_back; mod manual_ok_or; mod manual_saturating_arithmetic; @@ -3829,6 +3830,32 @@ declare_clippy_lint! { "filtering an iterator over `Result`s for `Ok` can be achieved with `flatten`" } +declare_clippy_lint! { + /// Checks for usage of `option.map(f).unwrap_or_default()` and `result.map(f).unwrap_or_default()` where f is a function or closure that returns the `bool` type. + /// + /// ### Why is this bad? + /// Readability. These can be written more concisely as `option.is_some_and(f)` and `result.is_ok_and(f)`. + /// + /// ### Example + /// ```no_run + /// # let option = Some(1); + /// # let result: Result = Ok(1); + /// option.map(|a| a > 10).unwrap_or_default(); + /// result.map(|a| a > 10).unwrap_or_default(); + /// ``` + /// Use instead: + /// ```no_run + /// # let option = Some(1); + /// # let result: Result = Ok(1); + /// option.is_some_and(|a| a > 10); + /// result.is_ok_and(|a| a > 10); + /// ``` + #[clippy::version = "1.76.0"] + pub MANUAL_IS_VARIANT_AND, + pedantic, + "using `.map(f).unwrap_or_default()`, which is more succinctly expressed as `is_some_and(f)` or `is_ok_and(f)`" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3983,6 +4010,7 @@ impl_lint_pass!(Methods => [ RESULT_FILTER_MAP, ITER_FILTER_IS_SOME, ITER_FILTER_IS_OK, + MANUAL_IS_VARIANT_AND, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4664,7 +4692,13 @@ impl Methods { } unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, - ("unwrap_or_default" | "unwrap_unchecked" | "unwrap_err_unchecked", []) => { + ("unwrap_or_default", []) => { + if let Some(("map", m_recv, [arg], span, _)) = method_call(recv) { + manual_is_variant_and::check(cx, expr, m_recv, arg, span, &self.msrv); + } + unnecessary_literal_unwrap::check(cx, expr, recv, name, args); + }, + ("unwrap_unchecked" | "unwrap_err_unchecked", []) => { unnecessary_literal_unwrap::check(cx, expr, recv, name, args); }, ("unwrap_or_else", [u_arg]) => { diff --git a/clippy_lints/src/methods/option_map_unwrap_or.rs b/clippy_lints/src/methods/option_map_unwrap_or.rs index 63e64a5b35d..35deabe49b6 100644 --- a/clippy_lints/src/methods/option_map_unwrap_or.rs +++ b/clippy_lints/src/methods/option_map_unwrap_or.rs @@ -72,7 +72,7 @@ pub(super) fn check<'tcx>( } // is_some_and is stabilised && `unwrap_or` argument is false; suggest `is_some_and` instead - let suggest_is_some_and = msrv.meets(msrvs::OPTION_IS_SOME_AND) + let suggest_is_some_and = msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND) && matches!(&unwrap_arg.kind, ExprKind::Lit(lit) if matches!(lit.node, rustc_ast::LitKind::Bool(false))); diff --git a/clippy_lints/src/methods/path_ends_with_ext.rs b/clippy_lints/src/methods/path_ends_with_ext.rs index 094ead9f4ad..29f44ec2a4d 100644 --- a/clippy_lints/src/methods/path_ends_with_ext.rs +++ b/clippy_lints/src/methods/path_ends_with_ext.rs @@ -33,7 +33,7 @@ pub(super) fn check( && path.chars().all(char::is_alphanumeric) { let mut sugg = snippet(cx, recv.span, "..").into_owned(); - if msrv.meets(msrvs::OPTION_IS_SOME_AND) { + if msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND) { let _ = write!(sugg, r#".extension().is_some_and(|ext| ext == "{path}")"#); } else { let _ = write!(sugg, r#".extension().map_or(false, |ext| ext == "{path}")"#); -- cgit 1.4.1-3-g733a5