Skip to content

Commit

Permalink
Use clearer multipart suggestions for unnecessary_map_or lint
Browse files Browse the repository at this point in the history 
A multipart suggestion will be used whenever the method call can be
replaced by another one with the first argument removed. It helps place
the new method call in context, especially when it is part of a larger
expression.
  • Loading branch information
@samueltardieu
samueltardieu committed Jan 14, 2025
1 parent 6ab6c3c commit 0ff495c
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 37 deletions.
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5015,7 +5015,7 @@ impl Methods {
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);
unnecessary_map_or::check(cx, expr, recv, def, map, &self.msrv);
unnecessary_map_or::check(cx, expr, recv, def, map, span, &self.msrv);
},
("map_or_else", [def, map]) => {
result_map_or_else_none::check(cx, expr, recv, def, map);
Expand Down
35 changes: 14 additions & 21 deletions clippy_lints/src/methods/unnecessary_map_or.rs
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::borrow::Cow;

use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
use clippy_utils::msrvs::{self, Msrv};
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;
Expand All @@ -12,7 +11,7 @@ use rustc_ast::LitKind::Bool;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
use rustc_lint::LateContext;
use rustc_span::sym;
use rustc_span::{Span, sym};

use super::UNNECESSARY_MAP_OR;

Expand Down Expand Up @@ -42,6 +41,7 @@ pub(super) fn check<'a>(
recv: &Expr<'_>,
def: &Expr<'_>,
map: &Expr<'_>,
method_span: Span,
msrv: &Msrv,
) {
let ExprKind::Lit(def_kind) = def.kind else {
Expand All @@ -60,6 +60,8 @@ pub(super) fn check<'a>(
Some(_) | None => return,
};

let ext_def_span = def.span.with_hi(map.span.lo());

let (sugg, method, applicability) = if let ExprKind::Closure(map_closure) = map.kind
&& let closure_body = cx.tcx.hir().body(map_closure.body)
&& let closure_body_value = closure_body.value.peel_blocks()
Expand Down Expand Up @@ -114,26 +116,17 @@ pub(super) fn check<'a>(
}
.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())
&& let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
{
(vec![(expr.span, sugg)], "a standard comparison", app)
} else if !def_bool && msrv.meets(msrvs::OPTION_RESULT_IS_VARIANT_AND) {
let suggested_name = variant.method_name();
(
format!("{recv_callsite}.{suggested_name}({span_callsite})",),
vec![(method_span, suggested_name.into()), (ext_def_span, String::default())],
suggested_name,
Applicability::MachineApplicable,
)
} else if def_bool
&& matches!(variant, Variant::Some)
&& msrv.meets(msrvs::IS_NONE_OR)
&& let Some(recv_callsite) = snippet_opt(cx, recv.span.source_callsite())
&& let Some(span_callsite) = snippet_opt(cx, map.span.source_callsite())
{
} else if def_bool && matches!(variant, Variant::Some) && msrv.meets(msrvs::IS_NONE_OR) {
(
format!("{recv_callsite}.is_none_or({span_callsite})"),
vec![(method_span, "is_none_or".into()), (ext_def_span, String::default())],
"is_none_or",
Applicability::MachineApplicable,
)
Expand All @@ -145,13 +138,13 @@ pub(super) fn check<'a>(
return;
}

span_lint_and_sugg(
span_lint_and_then(
cx,
UNNECESSARY_MAP_OR,
expr.span,
"this `map_or` can be simplified",
format!("use {method} instead"),
sugg,
applicability,
|diag| {
diag.multipart_suggestion(format!("use {method} instead"), sugg, applicability);
},
);
}
94 changes: 79 additions & 15 deletions tests/ui/unnecessary_map_or.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,41 +35,69 @@ LL | | });
|
help: use is_some_and instead
|
LL ~ let _ = Some(5).is_some_and(|n| {
LL + let _ = n;
LL + 6 >= 5
LL ~ });
LL - let _ = Some(5).map_or(false, |n| {
LL + let _ = Some(5).is_some_and(|n| {
|

error: this `map_or` can be simplified
--> 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])`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
|
LL - let _ = Some(vec![5]).map_or(false, |n| n == [5]);
LL + let _ = Some(vec![5]).is_some_and(|n| n == [5]);
|

error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
|
LL - let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
LL + let _ = Some(vec![1]).is_some_and(|n| vec![2] == n);
|

error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
|
LL - let _ = Some(5).map_or(false, |n| n == n);
LL + let _ = Some(5).is_some_and(|n| n == n);
|

error: this `map_or` can be simplified
--> 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 })`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
|
LL - let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
LL + let _ = 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: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])`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_ok_and instead
|
LL - let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
LL + let _ = 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:28:13
Expand All @@ -87,13 +115,25 @@ error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
|
LL - let _ = Some(5).map_or(true, |n| n == 5);
LL + let _ = Some(5).is_none_or(|n| n == 5);
|

error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
|
LL - let _ = Some(5).map_or(true, |n| 5 == n);
LL + let _ = Some(5).is_none_or(|n| 5 == n);
|

error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:32:14
Expand All @@ -117,25 +157,49 @@ 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_ok_and instead
|
LL - let _ = r.map_or(false, |x| x == 7);
LL + let _ = r.is_ok_and(|x| x == 7);
|

error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^
|
help: use is_ok_and instead
|
LL - let _ = r.map_or(false, func);
LL + let _ = r.is_ok_and(func);
|

error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_some_and instead
|
LL - let _ = Some(5).map_or(false, func);
LL + let _ = Some(5).is_some_and(func);
|

error: this `map_or` can be simplified
--> 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)`
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: use is_none_or instead
|
LL - let _ = Some(5).map_or(true, func);
LL + let _ = Some(5).is_none_or(func);
|

error: this `map_or` can be simplified
--> tests/ui/unnecessary_map_or.rs:70:13
Expand Down

0 comments on commit 0ff495c

Please sign in to comment.