about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2021-04-18 23:47:18 +0000
committerbors <bors@rust-lang.org>2021-04-18 23:47:18 +0000
commitc569d33be52a9be7b1070d1a7c9bc1255e004e87 (patch)
treee20b255ad7dc878d0095046678755c799cbf5eda
parente441b33ba00d897fff4479513210d41f988dfde3 (diff)
parent243dc4625050d791eb8f91b6b4da2f8dc8449a8d (diff)
downloadrust-c569d33be52a9be7b1070d1a7c9bc1255e004e87.tar.gz
rust-c569d33be52a9be7b1070d1a7c9bc1255e004e87.zip
Auto merge of #7108 - rust-lang:fix-return-try-err, r=Manishearth
un-double `return` on try_err

This fixes #7103 by looking at the parent expression and omitting the "return " in the suggestion when its already a `return` expression.

---

changelog: none
-rw-r--r--clippy_lints/src/try_err.rs11
-rw-r--r--tests/ui/try_err.fixed8
-rw-r--r--tests/ui/try_err.rs8
-rw-r--r--tests/ui/try_err.stderr8
4 files changed, 31 insertions, 4 deletions
diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs
index 0b8c03c6865..ebb39ea4877 100644
--- a/clippy_lints/src/try_err.rs
+++ b/clippy_lints/src/try_err.rs
@@ -1,7 +1,7 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::{snippet, snippet_with_macro_callsite};
 use clippy_utils::ty::is_type_diagnostic_item;
-use clippy_utils::{differing_macro_contexts, in_macro, is_lang_ctor, match_def_path, paths};
+use clippy_utils::{differing_macro_contexts, get_parent_expr, in_macro, is_lang_ctor, match_def_path, paths};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::LangItem::ResultErr;
@@ -102,10 +102,15 @@ impl<'tcx> LateLintPass<'tcx> for TryErr {
                 } else {
                     snippet(cx, err_arg.span, "_")
                 };
+                let ret_prefix = if get_parent_expr(cx, expr).map_or(false, |e| matches!(e.kind, ExprKind::Ret(_))) {
+                    "" // already returns
+                } else {
+                    "return "
+                };
                 let suggestion = if err_ty == expr_err_ty {
-                    format!("return {}{}{}", prefix, origin_snippet, suffix)
+                    format!("{}{}{}{}", ret_prefix, prefix, origin_snippet, suffix)
                 } else {
-                    format!("return {}{}.into(){}", prefix, origin_snippet, suffix)
+                    format!("{}{}{}.into(){}", ret_prefix, prefix, origin_snippet, suffix)
                 };
 
                 span_lint_and_sugg(
diff --git a/tests/ui/try_err.fixed b/tests/ui/try_err.fixed
index 5b96bb59c5f..264194419c7 100644
--- a/tests/ui/try_err.fixed
+++ b/tests/ui/try_err.fixed
@@ -160,3 +160,11 @@ pub fn poll_next(ready: bool) -> Poll<Option<io::Result<()>>> {
 
     Poll::Ready(None)
 }
+
+// Tests that `return` is not duplicated
+pub fn try_return(x: bool) -> Result<i32, i32> {
+    if x {
+        return Err(42);
+    }
+    Ok(0)
+}
diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs
index f220d697d2c..bc6979bf457 100644
--- a/tests/ui/try_err.rs
+++ b/tests/ui/try_err.rs
@@ -160,3 +160,11 @@ pub fn poll_next(ready: bool) -> Poll<Option<io::Result<()>>> {
 
     Poll::Ready(None)
 }
+
+// Tests that `return` is not duplicated
+pub fn try_return(x: bool) -> Result<i32, i32> {
+    if x {
+        return Err(42)?;
+    }
+    Ok(0)
+}
diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr
index 2c01d37192e..8f332a9b649 100644
--- a/tests/ui/try_err.stderr
+++ b/tests/ui/try_err.stderr
@@ -74,5 +74,11 @@ error: returning an `Err(_)` with the `?` operator
 LL |         Err(io::ErrorKind::NotFound)?
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `return Poll::Ready(Some(Err(io::ErrorKind::NotFound.into())))`
 
-error: aborting due to 10 previous errors
+error: returning an `Err(_)` with the `?` operator
+  --> $DIR/try_err.rs:167:16
+   |
+LL |         return Err(42)?;
+   |                ^^^^^^^^ help: try this: `Err(42)`
+
+error: aborting due to 11 previous errors