diff options
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/methods/mod.rs | 28 | ||||
| -rw-r--r-- | clippy_lints/src/methods/option_map_or_err_ok.rs | 41 |
3 files changed, 70 insertions, 0 deletions
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 7d4ab9fde1f..5a109fcc2cc 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -405,6 +405,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::methods::OK_EXPECT_INFO, crate::methods::OPTION_AS_REF_DEREF_INFO, crate::methods::OPTION_FILTER_MAP_INFO, + crate::methods::OPTION_MAP_OR_ERR_OK_INFO, crate::methods::OPTION_MAP_OR_NONE_INFO, crate::methods::OR_FUN_CALL_INFO, crate::methods::OR_THEN_UNWRAP_INFO, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 71de77acfc1..7ce14242cae 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/clippy_lints/src/methods/mod.rs @@ -70,6 +70,7 @@ mod obfuscated_if_else; mod ok_expect; mod open_options; mod option_as_ref_deref; +mod option_map_or_err_ok; mod option_map_or_none; mod option_map_unwrap_or; mod or_fun_call; @@ -3726,6 +3727,31 @@ declare_clippy_lint! { "calls to `Path::join` which will overwrite the original path" } +declare_clippy_lint! { + /// ### What it does + /// Checks for usage of `_.map_or(Err(_), Ok)`. + /// + /// ### Why is this bad? + /// Readability, this can be written more concisely as + /// `_.ok_or(_)`. + /// + /// ### Example + /// ```no_run + /// # let opt = Some(1); + /// opt.map_or(Err("error"), Ok); + /// ``` + /// + /// Use instead: + /// ```no_run + /// # let opt = Some(1); + /// opt.ok_or("error"); + /// ``` + #[clippy::version = "1.76.0"] + pub OPTION_MAP_OR_ERR_OK, + style, + "using `Option.map_or(Err(_), Ok)`, which is more succinctly expressed as `Option.ok_or(_)`" +} + pub struct Methods { avoid_breaking_exported_api: bool, msrv: Msrv, @@ -3876,6 +3902,7 @@ impl_lint_pass!(Methods => [ WAKER_CLONE_WAKE, UNNECESSARY_FALLIBLE_CONVERSIONS, JOIN_ABSOLUTE_PATHS, + OPTION_MAP_OR_ERR_OK, ]); /// Extracts a method call name, args, and `Span` of the method name. @@ -4335,6 +4362,7 @@ impl Methods { ("map_or", [def, map]) => { option_map_or_none::check(cx, expr, recv, def, map); manual_ok_or::check(cx, expr, recv, def, map); + option_map_or_err_ok::check(cx, expr, recv, def, map); }, ("map_or_else", [def, map]) => { result_map_or_else_none::check(cx, expr, recv, def, map); diff --git a/clippy_lints/src/methods/option_map_or_err_ok.rs b/clippy_lints/src/methods/option_map_or_err_ok.rs new file mode 100644 index 00000000000..91e39d5a1cd --- /dev/null +++ b/clippy_lints/src/methods/option_map_or_err_ok.rs @@ -0,0 +1,41 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::{is_res_lang_ctor, path_res}; +use rustc_errors::Applicability; +use rustc_hir::LangItem::{ResultErr, ResultOk}; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::LateContext; +use rustc_span::symbol::sym; + +use super::OPTION_MAP_OR_ERR_OK; + +pub(super) fn check<'tcx>( + cx: &LateContext<'tcx>, + expr: &'tcx Expr<'tcx>, + recv: &'tcx Expr<'_>, + or_expr: &'tcx Expr<'_>, + map_expr: &'tcx Expr<'_>, +) { + // We check that it's called on an `Option` type. + if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Option) + // We check that first we pass an `Err`. + && let ExprKind::Call(call, &[arg]) = or_expr.kind + && is_res_lang_ctor(cx, path_res(cx, call), ResultErr) + // And finally we check that it is mapped as `Ok`. + && is_res_lang_ctor(cx, path_res(cx, map_expr), ResultOk) + { + let msg = "called `map_or(Err(_), Ok)` on an `Option` value"; + let self_snippet = snippet(cx, recv.span, ".."); + let err_snippet = snippet(cx, arg.span, ".."); + span_lint_and_sugg( + cx, + OPTION_MAP_OR_ERR_OK, + expr.span, + msg, + "try using `ok_or` instead", + format!("{self_snippet}.ok_or({err_snippet})"), + Applicability::MachineApplicable, + ); + } +} |