Skip to content

Commit bbffe82

Browse files
togami2864togami2864
committed
add reduce_unit_expression
1 parent 4f71ff3 commit bbffe82

File tree

3 files changed

+96
-58
lines changed

3 files changed

+96
-58
lines changed

‎clippy_lints/src/methods/option_map_or_none.rs

Lines changed: 89 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::is_lang_ctor;
32
use clippy_utils::source::snippet;
43
use clippy_utils::ty::is_type_diagnostic_item;
4+
use clippy_utils::{is_lang_ctor, single_segment_path};
55
use rustc_errors::Applicability;
66
use rustc_hir as hir;
77
use rustc_hir::LangItem::{OptionNone, OptionSome};
@@ -11,6 +11,26 @@ use rustc_span::symbol::sym;
1111
use super::OPTION_MAP_OR_NONE;
1212
use super::RESULT_MAP_OR_INTO_OPTION;
1313

14+
fn reduce_unit_expression<'a>(
15+
cx: &LateContext<'_>,
16+
expr: &'a hir::Expr<'_>,
17+
) -> Option<(&'a hir::Expr<'a>, &'a [hir::Expr<'a>])> {
18+
match expr.kind {
19+
hir::ExprKind::Call(func, arg_char) => Some((func, arg_char)),
20+
hir::ExprKind::Block(block, _) => {
21+
match block.expr {
22+
Some(inner_expr) => {
23+
// If block only contains an expression,
24+
// reduce `{ X }` to `X`
25+
reduce_unit_expression(cx, inner_expr)
26+
},
27+
_ => None,
28+
}
29+
},
30+
_ => None,
31+
}
32+
}
33+
1434
/// lint use of `_.map_or(None, _)` for `Option`s and `Result`s
1535
pub(super) fn check<'tcx>(
1636
cx: &LateContext<'tcx>,
@@ -31,58 +51,78 @@ pub(super) fn check<'tcx>(
3151
return;
3252
}
3353

34-
let (lint_name, msg, instead, hint) = {
35-
let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind {
36-
is_lang_ctor(cx, qpath, OptionNone)
37-
} else {
38-
return;
39-
};
54+
// let (lint_name, msg, instead, hint) = {
55+
let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind {
56+
is_lang_ctor(cx, qpath, OptionNone)
57+
} else {
58+
return;
59+
};
4060

41-
if !default_arg_is_none {
42-
// nothing to lint!
43-
return;
44-
}
61+
if !default_arg_is_none {
62+
// nothing to lint!
63+
return;
64+
}
4565

46-
let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind {
47-
is_lang_ctor(cx, qpath, OptionSome)
48-
} else {
49-
false
50-
};
66+
let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind {
67+
is_lang_ctor(cx, qpath, OptionSome)
68+
} else {
69+
false
70+
};
5171

52-
if is_option {
53-
let self_snippet = snippet(cx, recv.span, "..");
54-
let func_snippet = snippet(cx, map_arg.span, "..");
55-
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
72+
if is_option {
73+
let self_snippet = snippet(cx, recv.span, "..");
74+
if let hir::ExprKind::Closure(_, _, id, _, _) = map_arg.kind {
75+
if_chain! {
76+
let body = cx.tcx.hir().body(id);
77+
if let Some((func, arg_char)) = reduce_unit_expression(cx, &body.value);
78+
if arg_char.len() == 1;
79+
if let hir::ExprKind::Path(ref qpath) = func.kind;
80+
if let Some(segment) = single_segment_path(qpath);
81+
if segment.ident.name == sym::Some;
82+
then {
83+
let func_snippet = snippet(cx, arg_char[0].span, "..");
84+
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
5685
`map(..)` instead";
57-
(
58-
OPTION_MAP_OR_NONE,
59-
msg,
60-
"try using `map` instead",
61-
format!("{0}.map({1})", self_snippet, func_snippet),
62-
)
63-
} else if f_arg_is_some {
64-
let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \
65-
`ok()` instead";
66-
let self_snippet = snippet(cx, recv.span, "..");
67-
(
68-
RESULT_MAP_OR_INTO_OPTION,
69-
msg,
70-
"try using `ok` instead",
71-
format!("{0}.ok()", self_snippet),
72-
)
73-
} else {
74-
// nothing to lint!
75-
return;
86+
return span_lint_and_sugg(
87+
cx,
88+
OPTION_MAP_OR_NONE,
89+
expr.span,
90+
msg,
91+
"try using `map` instead",
92+
format!("{0}.map({1})", self_snippet, func_snippet),
93+
Applicability::MachineApplicable,
94+
);
95+
}
96+
}
7697
}
77-
};
7898

79-
span_lint_and_sugg(
80-
cx,
81-
lint_name,
82-
expr.span,
83-
msg,
84-
instead,
85-
hint,
86-
Applicability::MachineApplicable,
87-
);
99+
let func_snippet = snippet(cx, map_arg.span, "..");
100+
let msg = "called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling \
101+
`and_then(..)` instead";
102+
span_lint_and_sugg(
103+
cx,
104+
OPTION_MAP_OR_NONE,
105+
expr.span,
106+
msg,
107+
"try using `and_then` instead",
108+
format!("{0}.and_then({1})", self_snippet, func_snippet),
109+
Applicability::MachineApplicable,
110+
)
111+
} else if f_arg_is_some {
112+
let msg = "called `map_or(None, Some)` on a `Result` value. This can be done more directly by calling \
113+
`ok()` instead";
114+
let self_snippet = snippet(cx, recv.span, "..");
115+
span_lint_and_sugg(
116+
cx,
117+
RESULT_MAP_OR_INTO_OPTION,
118+
expr.span,
119+
msg,
120+
"try using `ok` instead",
121+
format!("{0}.ok()", self_snippet),
122+
Applicability::MachineApplicable,
123+
)
124+
} else {
125+
// nothing to lint!
126+
return;
127+
}
88128
}

‎tests/ui/option_map_or_none.fixed

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,15 @@
55
fn main() {
66
let opt = Some(1);
77
let bar = |_| {
8-
Some(1)
8+
Some(1)
99
};
1010

1111
// Check `OPTION_MAP_OR_NONE`.
1212
// Single line case.
1313
let _ :Option<i32> = opt.map(|x| x + 1);
1414
// Multi-line case.
1515
#[rustfmt::skip]
16-
let _ :Option<i32> = opt.map(|x| {
17-
x + 1
18-
});
16+
let _ :Option<i32> = opt.map(|x| x + 1);
1917
// function returning `Option`
2018
let _ :Option<i32> = opt.and_then(bar);
2119
}

‎tests/ui/option_map_or_none.stderr

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
2-
--> $DIR/option_map_or_none.rs:10:13
2+
--> $DIR/option_map_or_none.rs:13:26
33
|
44
LL | let _ :Option<i32> = opt.map_or(None, |x| Some(x + 1));
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `map` instead: `opt.map(|x| x + 1)`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `map` instead: `opt.map(|x| x + 1)`
66
|
77
= note: `-D clippy::option-map-or-none` implied by `-D warnings`
88

99
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
10-
--> $DIR/option_map_or_none.rs:13:13
10+
--> $DIR/option_map_or_none.rs:16:26
1111
|
1212
LL | let _ :Option<i32> = opt.map_or(None, |x| {
13-
| _____________^
13+
| __________________________^
1414
LL | | Some(x + 1)
1515
LL | | });
1616
| |_________________________^
@@ -22,7 +22,7 @@ LL + x + 1
2222
LL ~ });
2323
|
2424

25-
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `map(..)` instead
25+
error: called `map_or(None, ..)` on an `Option` value. This can be done more directly by calling `and_then(..)` instead
2626
--> $DIR/option_map_or_none.rs:20:26
2727
|
2828
LL | let _ :Option<i32> = opt.map_or(None, bar);

0 commit comments

Comments
 (0)