Skip to content

Commit

Permalink
Auto merge of rust-lang#7108 - rust-lang:fix-return-try-err, r=Manish…
Browse files Browse the repository at this point in the history
…earth

un-double `return` on try_err

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

---

changelog: none
  • Loading branch information
bors committed Apr 18, 2021
2 parents e441b33 + 243dc46 commit c569d33
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 4 deletions.
11 changes: 8 additions & 3 deletions clippy_lints/src/try_err.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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(
Expand Down
8 changes: 8 additions & 0 deletions tests/ui/try_err.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 8 additions & 0 deletions tests/ui/try_err.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 7 additions & 1 deletion tests/ui/try_err.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit c569d33

Please sign in to comment.