diff options
| -rw-r--r-- | clippy_lints/src/methods/or_then_unwrap.rs | 30 | ||||
| -rw-r--r-- | tests/ui/or_then_unwrap.fixed | 48 | ||||
| -rw-r--r-- | tests/ui/or_then_unwrap.rs | 2 | ||||
| -rw-r--r-- | tests/ui/or_then_unwrap.stderr | 8 |
4 files changed, 75 insertions, 13 deletions
diff --git a/clippy_lints/src/methods/or_then_unwrap.rs b/clippy_lints/src/methods/or_then_unwrap.rs index 578f898da78..4e2dcf67231 100644 --- a/clippy_lints/src/methods/or_then_unwrap.rs +++ b/clippy_lints/src/methods/or_then_unwrap.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_errors::Applicability; @@ -17,15 +18,20 @@ pub(super) fn check<'tcx>( ) { let ty = cx.typeck_results().expr_ty(recv); // get type of x (we later check if it's Option or Result) let title; + let or_arg_content: Span; if is_type_diagnostic_item(cx, ty, sym::Option) { title = ".or(Some(…)).unwrap() found"; - if !is(or_arg, "Some") { + if let Some(content) = get_content_if_is(or_arg, "Some") { + or_arg_content = content; + } else { return; } } else if is_type_diagnostic_item(cx, ty, sym::Result) { title = ".or(Ok(…)).unwrap() found"; - if !is(or_arg, "Ok") { + if let Some(content) = get_content_if_is(or_arg, "Ok") { + or_arg_content = content; + } else { return; } } else { @@ -41,30 +47,36 @@ pub(super) fn check<'tcx>( unwrap_expr.span }; + let mut applicability = Applicability::MachineApplicable; + let suggestion = format!( + "unwrap_or({})", + snippet_with_applicability(cx, or_arg_content, "..", &mut applicability) + ); + span_lint_and_sugg( cx, OR_THEN_UNWRAP, or_span.to(unwrap_span), title, "try this", - "unwrap_or(...)".into(), - Applicability::HasPlaceholders, + suggestion, + applicability, ); } -/// is expr a Call to name? +/// is expr a Call to name? if so, return what it's wrapping /// name might be "Some", "Ok", "Err", etc. -fn is<'a>(expr: &Expr<'a>, name: &str) -> bool { +fn get_content_if_is<'a>(expr: &Expr<'a>, name: &str) -> Option<Span> { if_chain! { - if let ExprKind::Call(some_expr, _some_args) = expr.kind; + if let ExprKind::Call(some_expr, [arg]) = expr.kind; if let ExprKind::Path(QPath::Resolved(_, path)) = &some_expr.kind; if let Some(path_segment) = path.segments.first(); if path_segment.ident.name.as_str() == name; then { - true + Some(arg.span) } else { - false + None } } } diff --git a/tests/ui/or_then_unwrap.fixed b/tests/ui/or_then_unwrap.fixed new file mode 100644 index 00000000000..b1e69ce2d26 --- /dev/null +++ b/tests/ui/or_then_unwrap.fixed @@ -0,0 +1,48 @@ +// run-rustfix + +#![warn(clippy::or_then_unwrap)] +#![allow(clippy::map_identity)] + +struct SomeStruct {} +impl SomeStruct { + fn or(self, _: Option<Self>) -> Self { + self + } + fn unwrap(&self) {} +} + +struct SomeOtherStruct {} +impl SomeOtherStruct { + fn or(self) -> Self { + self + } + fn unwrap(&self) {} +} + +fn main() { + let option: Option<&str> = None; + let _ = option.unwrap_or("fallback"); // should trigger lint + + let result: Result<&str, &str> = Err("Error"); + let _ = result.unwrap_or("fallback"); // should trigger lint + + // Not Option/Result + let instance = SomeStruct {}; + let _ = instance.or(Some(SomeStruct {})).unwrap(); // should not trigger lint + + // or takes no argument + let instance = SomeOtherStruct {}; + let _ = instance.or().unwrap(); // should not trigger lint and should not panic + + // None in or + let option: Option<&str> = None; + let _ = option.or(None).unwrap(); // should not trigger lint + + // Not Err in or + let result: Result<&str, &str> = Err("Error"); + let _ = result.or::<&str>(Err("Other Error")).unwrap(); // should not trigger lint + + // other function between + let option: Option<&str> = None; + let _ = option.or(Some("fallback")).map(|v| v).unwrap(); // should not trigger lint +} diff --git a/tests/ui/or_then_unwrap.rs b/tests/ui/or_then_unwrap.rs index 8d7e50ad405..dc66e86be6f 100644 --- a/tests/ui/or_then_unwrap.rs +++ b/tests/ui/or_then_unwrap.rs @@ -1,3 +1,5 @@ +// run-rustfix + #![warn(clippy::or_then_unwrap)] #![allow(clippy::map_identity)] diff --git a/tests/ui/or_then_unwrap.stderr b/tests/ui/or_then_unwrap.stderr index 7eade84bd65..301a54530cf 100644 --- a/tests/ui/or_then_unwrap.stderr +++ b/tests/ui/or_then_unwrap.stderr @@ -1,16 +1,16 @@ error: .or(Some(…)).unwrap() found - --> $DIR/or_then_unwrap.rs:22:20 + --> $DIR/or_then_unwrap.rs:24:20 | LL | let _ = option.or(Some("fallback")).unwrap(); // should trigger lint - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or(...)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or("fallback")` | = note: `-D clippy::or-then-unwrap` implied by `-D warnings` error: .or(Ok(…)).unwrap() found - --> $DIR/or_then_unwrap.rs:25:20 + --> $DIR/or_then_unwrap.rs:27:20 | LL | let _ = result.or::<&str>(Ok("fallback")).unwrap(); // should trigger lint - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or(...)` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `unwrap_or("fallback")` error: aborting due to 2 previous errors |
