Add new lint map_or_same_constant

CHANGELOG.md

@@ -7163,6 +7163,7 @@ Released 2018-09-13
 [`map_flatten`]: https://rust-lang.github.io/rust-clippy/main/index.html#map_flatten
 [`map_identity`]: https://rust-lang.github.io/rust-clippy/main/index.html#map_identity
 [`map_or_identity`]: https://rust-lang.github.io/rust-clippy/main/index.html#map_or_identity
+[`map_or_same_constant`]: https://rust-lang.github.io/rust-clippy/main/index.html#map_or_same_constant
 [`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/main/index.html#map_unwrap_or
 [`map_with_unused_argument_over_ranges`]: https://rust-lang.github.io/rust-clippy/main/index.html#map_with_unused_argument_over_ranges
 [`match_as_ref`]: https://rust-lang.github.io/rust-clippy/main/index.html#match_as_ref

clippy_lints/src/declared_lints.rs

@@ -443,6 +443,7 @@ pub static LINTS: &[&::declare_clippy_lint::LintInfo] = &[
     crate::methods::MAP_FLATTEN_INFO,
     crate::methods::MAP_IDENTITY_INFO,
     crate::methods::MAP_OR_IDENTITY_INFO,
+    crate::methods::MAP_OR_SAME_CONSTANT_INFO,
     crate::methods::MAP_UNWRAP_OR_INFO,
     crate::methods::MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES_INFO,
     crate::methods::MUT_MUTEX_LOCK_INFO,

clippy_lints/src/methods/map_or_same_constant.rs

@@ -0,0 +1,68 @@
+use clippy_utils::consts::ConstEvalCtxt;
+use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::is_from_proc_macro;
+use clippy_utils::res::MaybeDef as _;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::MAP_OR_SAME_CONSTANT;
+
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx Expr<'tcx>,
+    recv: &'tcx Expr<'tcx>,
+    def: &'tcx Expr<'tcx>,
+    map: &'tcx Expr<'tcx>,
+    is_map_or_else: bool,
+) {
+    if expr.span.from_expansion() || is_from_proc_macro(cx, expr) {
+        return;
+    }
+
+    let recv_ty = cx.typeck_results().expr_ty_adjusted(recv);
+    let Some(type_sym) = recv_ty.opt_diag_name(cx) else {
+        return;
+    };
+    if !matches!(type_sym, sym::Option | sym::Result) {
+        return;
+    }
+
+    let const_eval = ConstEvalCtxt::new(cx);
+
+    let def_const = if is_map_or_else {
+        if let ExprKind::Closure(closure) = def.kind {
+            let body = cx.tcx.hir_body(closure.body);
+            const_eval.eval(body.value.peel_blocks())
+        } else {
+            None
+        }
+    } else {
+        const_eval.eval(def)
+    };
+
+    let Some(def_const) = def_const else {
+        return;
+    };
+
+    let map_const = if let ExprKind::Closure(closure) = map.kind {
+        let body = cx.tcx.hir_body(closure.body);
+        const_eval.eval(body.value.peel_blocks())
+    } else {
+        None
+    };
+
+    if let Some(map_const) = map_const
+        && def_const == map_const
+    {
+        let method_name = if is_map_or_else { "map_or_else" } else { "map_or" };
+        span_lint_and_note(
+            cx,
+            MAP_OR_SAME_CONSTANT,
+            expr.span,
+            format!("both branches of `{method_name}` return the same constant value"),
+            None,
+            format!("this `{method_name}` always evaluates to the same value, regardless of the `{type_sym}` variant"),
+        );
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -77,6 +77,7 @@ mod map_err_ignore;
 mod map_flatten;
 mod map_identity;
 mod map_or_identity;
+mod map_or_same_constant;
 mod map_unwrap_or;
 mod map_unwrap_or_else;
 mod map_with_unused_argument_over_ranges;
@@ -2438,6 +2439,32 @@ declare_clippy_lint! {
     "using an identity function when mapping with `.map_or(|err| ..., |x| x)`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for calls to `Option::map_or`, `Result::map_or`, or their `map_or_else` variants
+    /// where both branches return the same constant value.
+    ///
+    /// ### Why is this bad?
+    /// If both branches return the same constant value, the expression always evaluates to
+    /// that constant regardless of the `Option` or `Result` variant. This is likely a typo
+    /// or copy-paste error.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// let res: Result<u32, ()> = Ok(1);
+    /// let _ = res.map_or(false, |_| false);
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// let res: Result<u32, ()> = Ok(1);
+    /// let _ = res.is_ok();
+    /// ```
+    #[clippy::version = "1.99.0"]
+    pub MAP_OR_SAME_CONSTANT,
+    suspicious,
+    "checks for `map_or` or `map_or_else` where both branches return the same constant value"
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usage of `option.map(_).unwrap_or(_)` or `option.map(_).unwrap_or_else(_)` or
@@ -5000,6 +5027,7 @@ impl_lint_pass!(Methods => [
     MAP_FLATTEN,
     MAP_IDENTITY,
     MAP_OR_IDENTITY,
+    MAP_OR_SAME_CONSTANT,
     MAP_UNWRAP_OR,
     MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
     MUT_MUTEX_LOCK,
@@ -5635,10 +5663,12 @@ impl Methods {
                     option_map_or_none::check(cx, expr, recv, def, map);
                     manual_ok_or::check(cx, expr, recv, def, map);
                     unnecessary_map_or::check(cx, expr, recv, def, map, span, self.msrv);
+                    map_or_same_constant::check(cx, expr, recv, def, map, false);
                 },
                 (sym::map_or_else, [def, map]) => {
                     result_map_or_else_none::check(cx, expr, recv, def, map);
                     unnecessary_map_or_else::check(cx, expr, recv, def, map, call_span);
+                    map_or_same_constant::check(cx, expr, recv, def, map, true);
                 },
                 (sym::next, []) => {
                     if let Some((name2, recv2, args2, _, _)) = method_call(recv) {

tests/ui/map_or_same_constant.rs

@@ -0,0 +1,46 @@
+#![warn(clippy::map_or_same_constant)]
+#![allow(clippy::unnecessary_map_or)]
+
+fn main() {
+    let opt: Option<i32> = Some(42);
+    let res: Result<i32, ()> = Ok(42);
+
+    // Should lint: Option::map_or with identical constants
+    let _ = opt.map_or(false, |_| false);
+    //~^ map_or_same_constant
+    let _ = opt.map_or(true, |_| true);
+    //~^ map_or_same_constant
+    let _ = opt.map_or(0, |_| 0);
+    //~^ map_or_same_constant
+    let _ = opt.map_or("foo", |_| "foo");
+    //~^ map_or_same_constant
+
+    // Should lint: Result::map_or with identical constants
+    let _ = res.map_or(false, |_| false);
+    //~^ map_or_same_constant
+    let _ = res.map_or(true, |_| true);
+    //~^ map_or_same_constant
+    let _ = res.map_or(123, |_| 123);
+    //~^ map_or_same_constant
+
+    // Should lint: Option::map_or_else with identical constants
+    let _ = opt.map_or_else(|| true, |_| true);
+    //~^ map_or_same_constant
+    let _ = opt.map_or_else(|| 0, |_| 0);
+    //~^ map_or_same_constant
+
+    // Should lint: Result::map_or_else with identical constants
+    let _ = res.map_or_else(|_| false, |_| false);
+    //~^ map_or_same_constant
+    let _ = res.map_or_else(|_| "same", |_| "same");
+    //~^ map_or_same_constant
+
+    // Should NOT lint: different constants
+    let _ = opt.map_or(false, |_| true);
+    let _ = res.map_or(0, |_| 1);
+    let _ = opt.map_or_else(|| false, |_| true);
+
+    // Should NOT lint: dynamic values
+    let _ = opt.map_or(0, |x| x + 1);
+    let _ = res.map_or_else(|_| 0, |x| x * 2);
+}

tests/ui/map_or_same_constant.stderr

@@ -0,0 +1,92 @@
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:9:13
+   |
+LL |     let _ = opt.map_or(false, |_| false);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Option` variant
+   = note: `-D clippy::map-or-same-constant` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::map_or_same_constant)]`
+
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:11:13
+   |
+LL |     let _ = opt.map_or(true, |_| true);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Option` variant
+
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:13:13
+   |
+LL |     let _ = opt.map_or(0, |_| 0);
+   |             ^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Option` variant
+
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:15:13
+   |
+LL |     let _ = opt.map_or("foo", |_| "foo");
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Option` variant
+
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:19:13
+   |
+LL |     let _ = res.map_or(false, |_| false);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Result` variant
+
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:21:13
+   |
+LL |     let _ = res.map_or(true, |_| true);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Result` variant
+
+error: both branches of `map_or` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:23:13
+   |
+LL |     let _ = res.map_or(123, |_| 123);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or` always evaluates to the same value, regardless of the `Result` variant
+
+error: both branches of `map_or_else` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:27:13
+   |
+LL |     let _ = opt.map_or_else(|| true, |_| true);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or_else` always evaluates to the same value, regardless of the `Option` variant
+
+error: both branches of `map_or_else` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:29:13
+   |
+LL |     let _ = opt.map_or_else(|| 0, |_| 0);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or_else` always evaluates to the same value, regardless of the `Option` variant
+
+error: both branches of `map_or_else` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:33:13
+   |
+LL |     let _ = res.map_or_else(|_| false, |_| false);
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or_else` always evaluates to the same value, regardless of the `Result` variant
+
+error: both branches of `map_or_else` return the same constant value
+  --> tests/ui/map_or_same_constant.rs:35:13
+   |
+LL |     let _ = res.map_or_else(|_| "same", |_| "same");
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: this `map_or_else` always evaluates to the same value, regardless of the `Result` variant
+
+error: aborting due to 11 previous errors
+