user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Remove unneeded parentheses in unnecessary_map_or lint output

Browse source 
When the expression is transformed into an equality, parentheses are
needed only if the resulting equality is used:

- as a receiver in a method call
- as part of a binary or unary expression
- as part of a cast

In other cases, which will be the majority, no parentheses are required.
This makes the lint suggestions cleaner.
This commit is contained in:
Samuel Tardieu 2025-01-02 23:53:24 +01:00
parent 631d9a2c5c
commit 9c46e1173f
4 changed files with 74 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

24
clippy_lints/src/methods/unnecessary_map_or.rs
View file

@ -7,7 +7,7 @@ use clippy_utils::source::snippet_opt;
use clippy_utils::sugg::{Sugg, make_binop};
use clippy_utils::ty::{get_type_diagnostic_name, implements_trait};
use clippy_utils::visitors::is_local_used;
use clippy_utils::{is_from_proc_macro, path_to_local_id};
use clippy_utils::{get_parent_expr, is_from_proc_macro, path_to_local_id};
use rustc_ast::LitKind::Bool;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
@ -96,11 +96,25 @@ pub(super) fn check<'a>(
Sugg::hir(cx, non_binding_location, "")
)));
let binop = make_binop(op.node, &Sugg::hir(cx, recv, ".."), &inner_non_binding)
.maybe_par()
.into_string();
let mut app = Applicability::MachineApplicable;
let binop = make_binop(
op.node,
&Sugg::hir_with_applicability(cx, recv, "..", &mut app),
&inner_non_binding,
);
(binop, "a standard comparison", Applicability::MaybeIncorrect)
let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) {
match parent_expr.kind {
ExprKind::Binary(..) | ExprKind::Unary(..) | ExprKind::Cast(..) => binop.maybe_par(),
ExprKind::MethodCall(_, receiver, _, _) if receiver.hir_id == expr.hir_id => binop.maybe_par(),
_ => binop,
}
} else {
binop
}
.into_string();
(sugg, "a standard comparison", app)
} else if !def_bool
&& msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND)
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())

14
tests/ui/unnecessary_map_or.fixed
View file

@ -3,15 +3,16 @@
#![allow(clippy::no_effect)]
#![allow(clippy::eq_op)]
#![allow(clippy::unnecessary_lazy_evaluations)]
#![allow(clippy::nonminimal_bool)]
#[clippy::msrv = "1.70.0"]
#[macro_use]
extern crate proc_macros;
fn main() {
// should trigger
let _ = (Some(5) == Some(5));
let _ = (Some(5) != Some(5));
let _ = (Some(5) == Some(5));
let _ = Some(5) == Some(5);
let _ = Some(5) != Some(5);
let _ = Some(5) == Some(5);
let _ = Some(5).is_some_and(|n| {
let _ = n;
6 >= 5
@ -21,10 +22,13 @@ fn main() {
let _ = Some(5).is_some_and(|n| n == n);
let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
let _ = (Ok::<i32, i32>(5) == Ok(5));
let _ = Ok::<i32, i32>(5) == Ok(5);
let _ = (Some(5) == Some(5)).then(|| 1);
let _ = Some(5).is_none_or(|n| n == 5);
let _ = Some(5).is_none_or(|n| 5 == n);
let _ = !(Some(5) == Some(5));
let _ = (Some(5) == Some(5)) || false;
let _ = (Some(5) == Some(5)) as usize;
macro_rules! x {
() => {
@ -60,7 +64,7 @@ fn main() {
#[derive(PartialEq)]
struct S2;
let r: Result<i32, S2> = Ok(4);
let _ = (r == Ok(8));
let _ = r == Ok(8);
// do not lint `Result::map_or(true, …)`
let r: Result<i32, S2> = Ok(4);

4
tests/ui/unnecessary_map_or.rs
View file

@ -3,6 +3,7 @@
#![allow(clippy::no_effect)]
#![allow(clippy::eq_op)]
#![allow(clippy::unnecessary_lazy_evaluations)]
#![allow(clippy::nonminimal_bool)]
#[clippy::msrv = "1.70.0"]
#[macro_use]
extern crate proc_macros;
@ -28,6 +29,9 @@ fn main() {
let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
let _ = Some(5).map_or(true, |n| n == 5);
let _ = Some(5).map_or(true, |n| 5 == n);
let _ = !Some(5).map_or(false, |n| n == 5);
let _ = Some(5).map_or(false, |n| n == 5) || false;
let _ = Some(5).map_or(false, |n| n == 5) as usize;
macro_rules! x {
() => {

66
tests/ui/unnecessary_map_or.stderr
View file

@ -1,30 +1,30 @@
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:12:13
--> tests/ui/unnecessary_map_or.rs:13:13
|
LL | let _ = Some(5).map_or(false, |n| n == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) == Some(5)`
|
= note: `-D clippy::unnecessary-map-or` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_map_or)]`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:13:13
--> tests/ui/unnecessary_map_or.rs:14:13
|
LL | let _ = Some(5).map_or(true, |n| n != 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) != Some(5))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Some(5) != Some(5)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:14:13
--> tests/ui/unnecessary_map_or.rs:15:13
|
LL | let _ = Some(5).map_or(false, |n| {
| _____________^
LL | | let _ = 1;
LL | | n == 5
LL | | });
| |______^ help: use a standard comparison instead: `(Some(5) == Some(5))`
| |______^ help: use a standard comparison instead: `Some(5) == Some(5)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:18:13
--> tests/ui/unnecessary_map_or.rs:19:13
|
LL | let _ = Some(5).map_or(false, |n| {
| _____________^
@ -42,88 +42,106 @@ LL ~ });
|
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:22:13
--> tests/ui/unnecessary_map_or.rs:23:13
|
LL | let _ = Some(vec![5]).map_or(false, |n| n == [5]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![5]).is_some_and(|n| n == [5])`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:23:13
--> tests/ui/unnecessary_map_or.rs:24:13
|
LL | let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(vec![1]).is_some_and(|n| vec![2] == n)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:24:13
--> tests/ui/unnecessary_map_or.rs:25:13
|
LL | let _ = Some(5).map_or(false, |n| n == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == n)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:25:13
--> tests/ui/unnecessary_map_or.rs:26:13
|
LL | let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 })`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:26:13
--> tests/ui/unnecessary_map_or.rs:27:13
|
LL | let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5])`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:27:13
--> tests/ui/unnecessary_map_or.rs:28:13
|
LL | let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Ok::<i32, i32>(5) == Ok(5))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `Ok::<i32, i32>(5) == Ok(5)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:28:13
--> tests/ui/unnecessary_map_or.rs:29:13
|
LL | let _ = Some(5).map_or(false, |n| n == 5).then(|| 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:29:13
--> tests/ui/unnecessary_map_or.rs:30:13
|
LL | let _ = Some(5).map_or(true, |n| n == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| n == 5)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:30:13
--> tests/ui/unnecessary_map_or.rs:31:13
|
LL | let _ = Some(5).map_or(true, |n| 5 == n);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(|n| 5 == n)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:54:13
--> tests/ui/unnecessary_map_or.rs:32:14
|
LL | let _ = !Some(5).map_or(false, |n| n == 5);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:33:13
|
LL | let _ = Some(5).map_or(false, |n| n == 5) || false;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:34:13
|
LL | let _ = Some(5).map_or(false, |n| n == 5) as usize;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(Some(5) == Some(5))`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:58:13
|
LL | let _ = r.map_or(false, |x| x == 7);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(|x| x == 7)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:59:13
--> tests/ui/unnecessary_map_or.rs:63:13
|
LL | let _ = r.map_or(false, func);
| ^^^^^^^^^^^^^^^^^^^^^ help: use is_ok_and instead: `r.is_ok_and(func)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:60:13
--> tests/ui/unnecessary_map_or.rs:64:13
|
LL | let _ = Some(5).map_or(false, func);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_some_and instead: `Some(5).is_some_and(func)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:61:13
--> tests/ui/unnecessary_map_or.rs:65:13
|
LL | let _ = Some(5).map_or(true, func);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use is_none_or instead: `Some(5).is_none_or(func)`
error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:66:13
--> tests/ui/unnecessary_map_or.rs:70:13
|
LL | let _ = r.map_or(false, |x| x == 8);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `(r == Ok(8))`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use a standard comparison instead: `r == Ok(8)`
error: aborting due to 18 previous errors
error: aborting due to 21 previous errors