Skip to content

Commit 895de1f

Browse files
committed
feat: make fixable
1 parent f00e844 commit 895de1f

File tree

4 files changed

+75
-13
lines changed

4 files changed

+75
-13
lines changed

clippy_lints/src/methods/or_then_unwrap.rs

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet_with_applicability;
23
use clippy_utils::ty::is_type_diagnostic_item;
34
use if_chain::if_chain;
45
use rustc_errors::Applicability;
@@ -17,15 +18,20 @@ pub(super) fn check<'tcx>(
1718
) {
1819
let ty = cx.typeck_results().expr_ty(recv); // get type of x (we later check if it's Option or Result)
1920
let title;
21+
let or_arg_content: Span;
2022

2123
if is_type_diagnostic_item(cx, ty, sym::Option) {
2224
title = ".or(Some(…)).unwrap() found";
23-
if !is(or_arg, "Some") {
25+
if let Some(content) = get_content_if_is(or_arg, "Some") {
26+
or_arg_content = content;
27+
} else {
2428
return;
2529
}
2630
} else if is_type_diagnostic_item(cx, ty, sym::Result) {
2731
title = ".or(Ok(…)).unwrap() found";
28-
if !is(or_arg, "Ok") {
32+
if let Some(content) = get_content_if_is(or_arg, "Ok") {
33+
or_arg_content = content;
34+
} else {
2935
return;
3036
}
3137
} else {
@@ -41,30 +47,36 @@ pub(super) fn check<'tcx>(
4147
unwrap_expr.span
4248
};
4349

50+
let mut applicability = Applicability::MachineApplicable;
51+
let suggestion = format!(
52+
"unwrap_or({})",
53+
snippet_with_applicability(cx, or_arg_content, "..", &mut applicability)
54+
);
55+
4456
span_lint_and_sugg(
4557
cx,
4658
OR_THEN_UNWRAP,
4759
or_span.to(unwrap_span),
4860
title,
4961
"try this",
50-
"unwrap_or(...)".into(),
51-
Applicability::HasPlaceholders,
62+
suggestion,
63+
applicability,
5264
);
5365
}
5466

55-
/// is expr a Call to name?
67+
/// is expr a Call to name? if so, return what it's wrapping
5668
/// name might be "Some", "Ok", "Err", etc.
57-
fn is<'a>(expr: &Expr<'a>, name: &str) -> bool {
69+
fn get_content_if_is<'a>(expr: &Expr<'a>, name: &str) -> Option<Span> {
5870
if_chain! {
59-
if let ExprKind::Call(some_expr, _some_args) = expr.kind;
71+
if let ExprKind::Call(some_expr, [arg]) = expr.kind;
6072
if let ExprKind::Path(QPath::Resolved(_, path)) = &some_expr.kind;
6173
if let Some(path_segment) = path.segments.first();
6274
if path_segment.ident.name.as_str() == name;
6375
then {
64-
true
76+
Some(arg.span)
6577
}
6678
else {
67-
false
79+
None
6880
}
6981
}
7082
}

tests/ui/or_then_unwrap.fixed

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// run-rustfix
2+
3+
#![warn(clippy::or_then_unwrap)]
4+
#![allow(clippy::map_identity)]
5+
6+
struct SomeStruct {}
7+
impl SomeStruct {
8+
fn or(self, _: Option<Self>) -> Self {
9+
self
10+
}
11+
fn unwrap(&self) {}
12+
}
13+
14+
struct SomeOtherStruct {}
15+
impl SomeOtherStruct {
16+
fn or(self) -> Self {
17+
self
18+
}
19+
fn unwrap(&self) {}
20+
}
21+
22+
fn main() {
23+
let option: Option<&str> = None;
24+
let _ = option.unwrap_or("fallback"); // should trigger lint
25+
26+
let result: Result<&str, &str> = Err("Error");
27+
let _ = result.unwrap_or("fallback"); // should trigger lint
28+
29+
// Not Option/Result
30+
let instance = SomeStruct {};
31+
let _ = instance.or(Some(SomeStruct {})).unwrap(); // should not trigger lint
32+
33+
// or takes no argument
34+
let instance = SomeOtherStruct {};
35+
let _ = instance.or().unwrap(); // should not trigger lint and should not panic
36+
37+
// None in or
38+
let option: Option<&str> = None;
39+
let _ = option.or(None).unwrap(); // should not trigger lint
40+
41+
// Not Err in or
42+
let result: Result<&str, &str> = Err("Error");
43+
let _ = result.or::<&str>(Err("Other Error")).unwrap(); // should not trigger lint
44+
45+
// other function between
46+
let option: Option<&str> = None;
47+
let _ = option.or(Some("fallback")).map(|v| v).unwrap(); // should not trigger lint
48+
}

tests/ui/or_then_unwrap.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
// run-rustfix
2+
13
#![warn(clippy::or_then_unwrap)]
24
#![allow(clippy::map_identity)]
35

tests/ui/or_then_unwrap.stderr

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
error: .or(Some(…)).unwrap() found
2-
--> $DIR/or_then_unwrap.rs:22:20
2+
--> $DIR/or_then_unwrap.rs:24:20
33
|
44
LL | let _ = option.or(Some("fallback")).unwrap(); // should trigger lint
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or(...)`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or("fallback")`
66
|
77
= note: `-D clippy::or-then-unwrap` implied by `-D warnings`
88

99
error: .or(Ok(…)).unwrap() found
10-
--> $DIR/or_then_unwrap.rs:25:20
10+
--> $DIR/or_then_unwrap.rs:27:20
1111
|
1212
LL | let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint
13-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or(...)`
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or("fallback")`
1414

1515
error: aborting due to 2 previous errors
1616

0 commit comments

Comments
 (0)