Skip to content

Commit 8f83502

Browse files
pksunkarapksunkara
committed
Recognize unwrap_or methods
1 parent 1d159e7 commit 8f83502

File tree

5 files changed

+120
-44
lines changed

5 files changed

+120
-44
lines changed

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -3664,9 +3664,7 @@ impl Methods {
36643664
Some(("err", recv, [], err_span, _)) => err_expect::check(cx, expr, recv, span, err_span, &self.msrv),
36653665
_ => expect_used::check(cx, expr, recv, false, self.allow_expect_in_tests),
36663666
}
3667-
if let ExprKind::Call(recv, [arg]) = recv.kind {
3668-
unnecessary_literal_unwrap::check(cx, expr, recv, arg, name);
3669-
}
3667+
unnecessary_literal_unwrap::check(cx, expr, recv, name);
36703668
},
36713669
("expect_err", [_]) => expect_used::check(cx, expr, recv, true, self.allow_expect_in_tests),
36723670
("extend", [arg]) => {
@@ -3873,24 +3871,28 @@ impl Methods {
38733871
},
38743872
_ => {},
38753873
}
3876-
if let ExprKind::Call(recv, [arg]) = recv.kind {
3877-
unnecessary_literal_unwrap::check(cx, expr, recv, arg, name);
3878-
}
3874+
unnecessary_literal_unwrap::check(cx, expr, recv, name);
38793875
unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests);
38803876
},
38813877
("unwrap_err", []) => unwrap_used::check(cx, expr, recv, true, self.allow_unwrap_in_tests),
3882-
("unwrap_or", [u_arg]) => match method_call(recv) {
3883-
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _, _)) => {
3884-
manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
3885-
},
3886-
Some(("map", m_recv, [m_arg], span, _)) => {
3887-
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
3888-
},
3889-
Some(("then_some", t_recv, [t_arg], _, _)) => {
3890-
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
3891-
},
3892-
_ => {},
3878+
("unwrap_or", [u_arg]) => {
3879+
match method_call(recv) {
3880+
Some((arith @ ("checked_add" | "checked_sub" | "checked_mul"), lhs, [rhs], _, _)) => {
3881+
manual_saturating_arithmetic::check(cx, expr, lhs, rhs, u_arg, &arith["checked_".len()..]);
3882+
},
3883+
Some(("map", m_recv, [m_arg], span, _)) => {
3884+
option_map_unwrap_or::check(cx, expr, m_recv, m_arg, recv, u_arg, span);
3885+
},
3886+
Some(("then_some", t_recv, [t_arg], _, _)) => {
3887+
obfuscated_if_else::check(cx, expr, t_recv, t_arg, u_arg);
3888+
},
3889+
_ => {},
3890+
}
3891+
unnecessary_literal_unwrap::check(cx, expr, recv, name);
38933892
},
3893+
("unwrap_or_default", []) => {
3894+
unnecessary_literal_unwrap::check(cx, expr, recv, name);
3895+
}
38943896
("unwrap_or_else", [u_arg]) => match method_call(recv) {
38953897
Some(("map", recv, [map_arg], _, _))
38963898
if map_unwrap_or::check(cx, expr, recv, map_arg, u_arg, &self.msrv) => {},

‎clippy_lints/src/methods/unnecessary_literal_unwrap.rs‎

Lines changed: 28 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -5,33 +5,35 @@ use rustc_lint::LateContext;
55

66
use super::UNNECESSARY_LITERAL_UNWRAP;
77

8-
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) {
9-
let mess = if is_res_lang_ctor(cx, path_res(cx, recv), hir::LangItem::OptionSome) {
10-
Some("Some")
11-
} else if is_res_lang_ctor(cx, path_res(cx, recv), hir::LangItem::ResultOk) {
12-
Some("Ok")
13-
} else {
14-
None
15-
};
8+
pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, name: &str) {
9+
if let hir::ExprKind::Call(call, [arg]) = recv.kind {
10+
let mess = if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::OptionSome) {
11+
Some("Some")
12+
} else if is_res_lang_ctor(cx, path_res(cx, call), hir::LangItem::ResultOk) {
13+
Some("Ok")
14+
} else {
15+
None
16+
};
1617

17-
if let Some(constructor) = mess {
18-
span_lint_and_then(
19-
cx,
20-
UNNECESSARY_LITERAL_UNWRAP,
21-
expr.span,
22-
&format!("used `{name}()` on `{constructor}` value"),
23-
|diag| {
24-
let suggestions = vec![
25-
(recv.span.with_hi(arg.span.lo()), String::new()),
26-
(expr.span.with_lo(arg.span.hi()), String::new()),
27-
];
18+
if let Some(constructor) = mess {
19+
span_lint_and_then(
20+
cx,
21+
UNNECESSARY_LITERAL_UNWRAP,
22+
expr.span,
23+
&format!("used `{name}()` on `{constructor}` value"),
24+
|diag| {
25+
let suggestions = vec![
26+
(call.span.with_hi(arg.span.lo()), String::new()),
27+
(expr.span.with_lo(arg.span.hi()), String::new()),
28+
];
2829

29-
diag.multipart_suggestion(
30-
format!("remove the `{constructor}` and `{name}()`"),
31-
suggestions,
32-
Applicability::MachineApplicable,
33-
);
34-
},
35-
);
30+
diag.multipart_suggestion(
31+
format!("remove the `{constructor}` and `{name}()`"),
32+
suggestions,
33+
Applicability::MachineApplicable,
34+
);
35+
},
36+
);
37+
}
3638
}
3739
}

‎tests/ui/unnecessary_literal_unwrap.fixed‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,19 @@ fn unwrap_result() {
1212
// let val = Err(1).unwrap_err();
1313
}
1414

15+
fn unwrap_methods_option() {
16+
let _val = 1;
17+
let _val = 1;
18+
}
19+
20+
fn unwrap_methods_result() {
21+
let _val = 1;
22+
let _val = 1;
23+
}
24+
1525
fn main() {
1626
unwrap_option();
1727
unwrap_result();
28+
unwrap_methods_option();
29+
unwrap_methods_result();
1830
}

‎tests/ui/unnecessary_literal_unwrap.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,19 @@ fn unwrap_result() {
1212
// let val = Err(1).unwrap_err();
1313
}
1414

15+
fn unwrap_methods_option() {
16+
let _val = Some(1).unwrap_or(2);
17+
let _val = Some(1).unwrap_or_default();
18+
}
19+
20+
fn unwrap_methods_result() {
21+
let _val = Ok::<usize, ()>(1).unwrap_or(2);
22+
let _val = Ok::<usize, ()>(1).unwrap_or_default();
23+
}
24+
1525
fn main() {
1626
unwrap_option();
1727
unwrap_result();
28+
unwrap_methods_option();
29+
unwrap_methods_result();
1830
}

‎tests/ui/unnecessary_literal_unwrap.stderr‎

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,5 +47,53 @@ LL - let _val = Ok::<usize, ()>(1).expect("this never happens");
4747
LL + let _val = 1;
4848
|
4949

50-
error: aborting due to 4 previous errors
50+
error: used `unwrap_or()` on `Some` value
51+
--> $DIR/unnecessary_literal_unwrap.rs:16:16
52+
|
53+
LL | let _val = Some(1).unwrap_or(2);
54+
| ^^^^^^^^^^^^^^^^^^^^
55+
|
56+
help: remove the `Some` and `unwrap_or()`
57+
|
58+
LL - let _val = Some(1).unwrap_or(2);
59+
LL + let _val = 1;
60+
|
61+
62+
error: used `unwrap_or_default()` on `Some` value
63+
--> $DIR/unnecessary_literal_unwrap.rs:17:16
64+
|
65+
LL | let _val = Some(1).unwrap_or_default();
66+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
67+
|
68+
help: remove the `Some` and `unwrap_or_default()`
69+
|
70+
LL - let _val = Some(1).unwrap_or_default();
71+
LL + let _val = 1;
72+
|
73+
74+
error: used `unwrap_or()` on `Ok` value
75+
--> $DIR/unnecessary_literal_unwrap.rs:21:16
76+
|
77+
LL | let _val = Ok::<usize, ()>(1).unwrap_or(2);
78+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
79+
|
80+
help: remove the `Ok` and `unwrap_or()`
81+
|
82+
LL - let _val = Ok::<usize, ()>(1).unwrap_or(2);
83+
LL + let _val = 1;
84+
|
85+
86+
error: used `unwrap_or_default()` on `Ok` value
87+
--> $DIR/unnecessary_literal_unwrap.rs:22:16
88+
|
89+
LL | let _val = Ok::<usize, ()>(1).unwrap_or_default();
90+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
91+
|
92+
help: remove the `Ok` and `unwrap_or_default()`
93+
|
94+
LL - let _val = Ok::<usize, ()>(1).unwrap_or_default();
95+
LL + let _val = 1;
96+
|
97+
98+
error: aborting due to 8 previous errors
5199

0 commit comments

Comments
 (0)