Skip to content

Commit 9f53cf3

Browse files
JacherrJacherr
committed
finalise key features of lint
1 parent 58b6f48 commit 9f53cf3

12 files changed

+268
-151
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5536,6 +5536,7 @@ Released 2018-09-13
55365536
[`unnecessary_lazy_evaluations`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_lazy_evaluations
55375537
[`unnecessary_literal_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_literal_unwrap
55385538
[`unnecessary_map_on_constructor`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_on_constructor
5539+
[`unnecessary_map_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_map_or
55395540
[`unnecessary_mut_passed`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_mut_passed
55405541
[`unnecessary_operation`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_operation
55415542
[`unnecessary_owned_empty_strings`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_owned_empty_strings

‎clippy_lints/src/declared_lints.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
438438
crate::methods::UNNECESSARY_JOIN_INFO,
439439
crate::methods::UNNECESSARY_LAZY_EVALUATIONS_INFO,
440440
crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
441-
crate::methods::UNNECESSARY_PATTERN_MATCHING_INFO,
441+
crate::methods::UNNECESSARY_MAP_OR_INFO,
442442
crate::methods::UNNECESSARY_SORT_BY_INFO,
443443
crate::methods::UNNECESSARY_TO_OWNED_INFO,
444444
crate::methods::UNWRAP_OR_DEFAULT_INFO,

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ mod unnecessary_iter_cloned;
106106
mod unnecessary_join;
107107
mod unnecessary_lazy_eval;
108108
mod unnecessary_literal_unwrap;
109-
mod unnecessary_pattern_matching;
109+
mod unnecessary_map_or;
110110
mod unnecessary_sort_by;
111111
mod unnecessary_to_owned;
112112
mod unwrap_expect_used;
@@ -3706,7 +3706,7 @@ declare_clippy_lint! {
37063706
/// }
37073707
/// ```
37083708
#[clippy::version = "1.75.0"]
3709-
pub UNNECESSARY_PATTERN_MATCHING,
3709+
pub UNNECESSARY_MAP_OR,
37103710
style,
37113711
"reduce unnecessary pattern matching for constructs that implement `PartialEq`"
37123712
}
@@ -3858,7 +3858,7 @@ impl_lint_pass!(Methods => [
38583858
REDUNDANT_AS_STR,
38593859
WAKER_CLONE_WAKE,
38603860
UNNECESSARY_FALLIBLE_CONVERSIONS,
3861-
UNNECESSARY_PATTERN_MATCHING,
3861+
UNNECESSARY_MAP_OR,
38623862
]);
38633863

38643864
/// Extracts a method call name, args, and `Span` of the method name.
@@ -4319,7 +4319,7 @@ impl Methods {
43194319
("map_or", [def, map]) => {
43204320
option_map_or_none::check(cx, expr, recv, def, map);
43214321
manual_ok_or::check(cx, expr, recv, def, map);
4322-
unnecessary_pattern_matching::check(cx, expr, recv, def, map);
4322+
unnecessary_map_or::check(cx, expr, recv, def, map, &self.msrv);
43234323
},
43244324
("next", []) => {
43254325
if let Some((name2, recv2, args2, _, _)) = method_call(recv) {

‎clippy_lints/src/methods/unnecessary_map_or.rs‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
use clippy_config::msrvs::{self, Msrv};
2+
use clippy_utils::diagnostics::span_lint_and_sugg;
3+
use clippy_utils::eager_or_lazy::switch_to_eager_eval;
4+
use clippy_utils::path_to_local_id;
5+
use clippy_utils::source::snippet;
6+
use clippy_utils::ty::is_type_diagnostic_item;
7+
use clippy_utils::visitors::is_local_used;
8+
use rustc_ast::LitKind::Bool;
9+
use rustc_errors::Applicability;
10+
use rustc_hir::{BinOpKind, Expr, ExprKind, PatKind};
11+
use rustc_lint::LateContext;
12+
use rustc_span::sym;
13+
14+
use super::UNNECESSARY_MAP_OR;
15+
16+
// Only checking map_or for now
17+
pub(super) fn check(
18+
cx: &LateContext<'_>,
19+
expr: &Expr<'_>,
20+
recv: &Expr<'_>,
21+
def: &Expr<'_>,
22+
map: &Expr<'_>,
23+
msrv: &Msrv,
24+
) {
25+
if let ExprKind::Lit(def_kind) = def.kind
26+
&& let typeck_results = cx.typeck_results()
27+
&& let recv_ty = typeck_results.expr_ty(recv)
28+
&& (is_type_diagnostic_item(cx, recv_ty, sym::Option) || is_type_diagnostic_item(cx, recv_ty, sym::Result))
29+
&& let Bool(def_bool) = def_kind.node
30+
{
31+
let (sugg, method) = if let ExprKind::Closure(map_closure) = map.kind
32+
&& let closure_body = cx.tcx.hir().body(map_closure.body)
33+
&& let closure_body_value = closure_body.value.peel_blocks()
34+
&& let ExprKind::Binary(op, l, r) = closure_body_value.kind
35+
&& let Some(param) = closure_body.params.first()
36+
&& let PatKind::Binding(_, hir_id, _, _) = param.pat.kind
37+
// checking that map_or is either:
38+
// .map_or(false, |x| x == y) OR .map_or(true, |x| x != y)
39+
&& ((BinOpKind::Eq == op.node && !def_bool) || (BinOpKind::Ne == op.node && def_bool))
40+
&& let non_binding_location = if path_to_local_id(l, hir_id) { r } else { l }
41+
&& switch_to_eager_eval(cx, non_binding_location)
42+
// xor, because if its both then thats a strange edge case and
43+
// we can just ignore it, since by default clippy will error on this
44+
&& (path_to_local_id(l, hir_id) ^ path_to_local_id(r, hir_id))
45+
&& !is_local_used(cx, non_binding_location, hir_id)
46+
&& typeck_results.expr_ty(l) == typeck_results.expr_ty(r)
47+
{
48+
let wrap = if is_type_diagnostic_item(cx, recv_ty, sym::Option) {
49+
"Some"
50+
} else {
51+
"Ok"
52+
};
53+
let comparator = if BinOpKind::Eq == op.node { "==" } else { "!=" };
54+
55+
(
56+
format!(
57+
"{} {} {}({})",
58+
snippet(cx, recv.span, ".."),
59+
comparator,
60+
wrap,
61+
snippet(cx, non_binding_location.span.source_callsite(), "..")
62+
),
63+
"standard comparison",
64+
)
65+
} else if def_bool == false && msrv.meets(msrvs::OPTION_IS_SOME_AND) {
66+
let sugg = if is_type_diagnostic_item(cx, recv_ty, sym::Option) {
67+
"is_some_and"
68+
} else {
69+
"is_ok_and"
70+
};
71+
72+
(
73+
format!(
74+
"{}.{}({})",
75+
snippet(cx, recv.span.source_callsite(), ".."),
76+
sugg,
77+
snippet(cx, map.span.source_callsite(), "..")
78+
),
79+
sugg,
80+
)
81+
} else {
82+
return;
83+
};
84+
85+
span_lint_and_sugg(
86+
cx,
87+
UNNECESSARY_MAP_OR,
88+
expr.span,
89+
&format!("`map_or` is redundant, use {} instead", method),
90+
"try",
91+
sugg,
92+
Applicability::Unspecified,
93+
)
94+
}
95+
}

‎clippy_lints/src/methods/unnecessary_pattern_matching.rs‎

Lines changed: 0 additions & 69 deletions
This file was deleted.

‎tests/ui/rename.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
#![allow(clippy::single_char_add_str)]
2828
#![allow(clippy::module_name_repetitions)]
2929
#![allow(clippy::recursive_format_impl)]
30+
#![allow(clippy::unnecessary_map_or)]
3031
#![allow(clippy::unwrap_or_default)]
3132
#![allow(clippy::invisible_characters)]
3233
#![allow(invalid_reference_casting)]

‎tests/ui/unnecessary_map_or.fixed‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
#![warn(clippy::unnecessary_map_or)]
2+
#![allow(clippy::no_effect)]
3+
#![allow(clippy::eq_op)]
4+
5+
#[clippy::msrv = "1.70.0"]
6+
fn main() {
7+
// should trigger
8+
let _ = Some(5) == Some(5);
9+
let _ = Some(5) != Some(5);
10+
let _ = Some(5) == Some(5);
11+
let _ = Some(5).is_some_and(|n| {
12+
let _ = n;
13+
6 >= 5
14+
});
15+
let _ = Some(vec![5]).is_some_and(|n| n == [5]);
16+
let _ = Some(vec![1]).is_some_and(|n| vec![2] == n);
17+
let _ = Some(5).is_some_and(|n| n == n);
18+
let _ = Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 });
19+
let _ = Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5]);
20+
let _ = Ok::<i32, i32>(5) == Ok(5);
21+
22+
// shouldnt trigger
23+
let _ = Some(5).map_or(true, |n| n == 5);
24+
let _ = Some(5).map_or(true, |n| 5 == n);
25+
macro_rules! x {
26+
() => {
27+
Some(1)
28+
};
29+
}
30+
// methods lints dont fire on macros
31+
let _ = x!().map_or(false, |n| n == 1);
32+
let _ = x!().map_or(false, |n| n == vec![1][0]);
33+
34+
msrv_1_69();
35+
}
36+
37+
#[clippy::msrv = "1.69.0"]
38+
fn msrv_1_69() {
39+
// is_some_and added in 1.70.0
40+
let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
41+
}

‎tests/ui/unnecessary_map_or.rs‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
#![warn(clippy::unnecessary_map_or)]
2+
#![allow(clippy::no_effect)]
3+
#![allow(clippy::eq_op)]
4+
5+
#[clippy::msrv = "1.70.0"]
6+
fn main() {
7+
// should trigger
8+
let _ = Some(5).map_or(false, |n| n == 5);
9+
let _ = Some(5).map_or(true, |n| n != 5);
10+
let _ = Some(5).map_or(false, |n| {
11+
let _ = 1;
12+
n == 5
13+
});
14+
let _ = Some(5).map_or(false, |n| {
15+
let _ = n;
16+
6 >= 5
17+
});
18+
let _ = Some(vec![5]).map_or(false, |n| n == [5]);
19+
let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
20+
let _ = Some(5).map_or(false, |n| n == n);
21+
let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
22+
let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
23+
let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
24+
25+
// shouldnt trigger
26+
let _ = Some(5).map_or(true, |n| n == 5);
27+
let _ = Some(5).map_or(true, |n| 5 == n);
28+
macro_rules! x {
29+
() => {
30+
Some(1)
31+
};
32+
}
33+
// methods lints dont fire on macros
34+
let _ = x!().map_or(false, |n| n == 1);
35+
let _ = x!().map_or(false, |n| n == vec![1][0]);
36+
37+
msrv_1_69();
38+
}
39+
40+
#[clippy::msrv = "1.69.0"]
41+
fn msrv_1_69() {
42+
// is_some_and added in 1.70.0
43+
let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
44+
}

‎tests/ui/unnecessary_map_or.stderr‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
error: `map_or` is redundant, use standard comparison instead
2+
--> $DIR/unnecessary_map_or.rs:8:13
3+
|
4+
LL | let _ = Some(5).map_or(false, |n| n == 5);
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5) == Some(5)`
6+
|
7+
= note: `-D clippy::unnecessary-map-or` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_map_or)]`
9+
10+
error: `map_or` is redundant, use standard comparison instead
11+
--> $DIR/unnecessary_map_or.rs:9:13
12+
|
13+
LL | let _ = Some(5).map_or(true, |n| n != 5);
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5) != Some(5)`
15+
16+
error: `map_or` is redundant, use standard comparison instead
17+
--> $DIR/unnecessary_map_or.rs:10:13
18+
|
19+
LL | let _ = Some(5).map_or(false, |n| {
20+
| _____________^
21+
LL | | let _ = 1;
22+
LL | | n == 5
23+
LL | | });
24+
| |______^ help: try: `Some(5) == Some(5)`
25+
26+
error: `map_or` is redundant, use is_some_and instead
27+
--> $DIR/unnecessary_map_or.rs:14:13
28+
|
29+
LL | let _ = Some(5).map_or(false, |n| {
30+
| _____________^
31+
LL | | let _ = n;
32+
LL | | 6 >= 5
33+
LL | | });
34+
| |______^
35+
|
36+
help: try
37+
|
38+
LL ~ let _ = Some(5).is_some_and(|n| {
39+
LL + let _ = n;
40+
LL + 6 >= 5
41+
LL ~ });
42+
|
43+
44+
error: `map_or` is redundant, use is_some_and instead
45+
--> $DIR/unnecessary_map_or.rs:18:13
46+
|
47+
LL | let _ = Some(vec![5]).map_or(false, |n| n == [5]);
48+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(vec![5]).is_some_and(|n| n == [5])`
49+
50+
error: `map_or` is redundant, use is_some_and instead
51+
--> $DIR/unnecessary_map_or.rs:19:13
52+
|
53+
LL | let _ = Some(vec![1]).map_or(false, |n| vec![2] == n);
54+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(vec![1]).is_some_and(|n| vec![2] == n)`
55+
56+
error: `map_or` is redundant, use is_some_and instead
57+
--> $DIR/unnecessary_map_or.rs:20:13
58+
|
59+
LL | let _ = Some(5).map_or(false, |n| n == n);
60+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5).is_some_and(|n| n == n)`
61+
62+
error: `map_or` is redundant, use is_some_and instead
63+
--> $DIR/unnecessary_map_or.rs:21:13
64+
|
65+
LL | let _ = Some(5).map_or(false, |n| n == if 2 > 1 { n } else { 0 });
66+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Some(5).is_some_and(|n| n == if 2 > 1 { n } else { 0 })`
67+
68+
error: `map_or` is redundant, use is_ok_and instead
69+
--> $DIR/unnecessary_map_or.rs:22:13
70+
|
71+
LL | let _ = Ok::<Vec<i32>, i32>(vec![5]).map_or(false, |n| n == [5]);
72+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ok::<Vec<i32>, i32>(vec![5]).is_ok_and(|n| n == [5])`
73+
74+
error: `map_or` is redundant, use standard comparison instead
75+
--> $DIR/unnecessary_map_or.rs:23:13
76+
|
77+
LL | let _ = Ok::<i32, i32>(5).map_or(false, |n| n == 5);
78+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `Ok::<i32, i32>(5) == Ok(5)`
79+
80+
error: aborting due to 10 previous errors
81+

0 commit comments

Comments
 (0)