From 3016cfa33f7ade5e6e6beeba2fd369d9e86b6d11 Mon Sep 17 00:00:00 2001 From: ivanlomeli Date: Mon, 17 Aug 2026 03:45:56 -0700 Subject: [PATCH] Add new lint `map_or_same_constant` (closes #17544) --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + .../src/methods/map_or_same_constant.rs | 68 ++++++++++++++ clippy_lints/src/methods/mod.rs | 30 ++++++ tests/ui/map_or_same_constant.rs | 46 ++++++++++ tests/ui/map_or_same_constant.stderr | 92 +++++++++++++++++++ 6 files changed, 238 insertions(+) create mode 100644 clippy_lints/src/methods/map_or_same_constant.rs create mode 100644 tests/ui/map_or_same_constant.rs create mode 100644 tests/ui/map_or_same_constant.stderr diff --git a/CHANGELOG.md b/CHANGELOG.md index 62c8a63baed2..900e76304be7 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 5fd28aae0bee..9b5190aa1584 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/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, diff --git a/clippy_lints/src/methods/map_or_same_constant.rs b/clippy_lints/src/methods/map_or_same_constant.rs new file mode 100644 index 000000000000..e1374259d823 --- /dev/null +++ b/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"), + ); + } +} diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 3c26f07cd51d..6ea903aa767f 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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 = Ok(1); + /// let _ = res.map_or(false, |_| false); + /// ``` + /// Use instead: + /// ```no_run + /// let res: Result = 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) { diff --git a/tests/ui/map_or_same_constant.rs b/tests/ui/map_or_same_constant.rs new file mode 100644 index 000000000000..0a4d3bff1d5a --- /dev/null +++ b/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 = Some(42); + let res: Result = 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); +} diff --git a/tests/ui/map_or_same_constant.stderr b/tests/ui/map_or_same_constant.stderr new file mode 100644 index 000000000000..dbac5a03a963 --- /dev/null +++ b/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 +