diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3710342d841..461adb729f9 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1134,6 +1134,7 @@ Released 2018-09-13
[`transmuting_null`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmuting_null
[`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
[`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
+[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
[`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
[`unicode_not_nfc`]: https://rust-lang.github.io/rust-clippy/master/index.html#unicode_not_nfc
[`unimplemented`]: https://rust-lang.github.io/rust-clippy/master/index.html#unimplemented
diff --git a/README.md b/README.md
index 51f618da4b7..c1a25aa1300 100644
--- a/README.md
+++ b/README.md
@@ -6,7 +6,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
-[There are 305 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 306 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index b56413fc2c4..d239f14339f 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -263,6 +263,7 @@ pub mod temporary_assignment;
pub mod transmute;
pub mod transmuting_null;
pub mod trivially_copy_pass_by_ref;
+pub mod try_err;
pub mod types;
pub mod unicode;
pub mod unsafe_removed_from_name;
@@ -546,6 +547,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
reg.register_early_lint_pass(box literal_representation::DecimalLiteralRepresentation::new(
conf.literal_representation_threshold
));
+ reg.register_late_lint_pass(box try_err::TryErr);
reg.register_late_lint_pass(box use_self::UseSelf);
reg.register_late_lint_pass(box bytecount::ByteCount);
reg.register_late_lint_pass(box infinite_iter::InfiniteIter);
@@ -861,6 +863,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
transmute::WRONG_TRANSMUTE,
transmuting_null::TRANSMUTING_NULL,
trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF,
+ try_err::TRY_ERR,
types::ABSURD_EXTREME_COMPARISONS,
types::BORROWED_BOX,
types::BOX_VEC,
@@ -963,6 +966,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
returns::NEEDLESS_RETURN,
returns::UNUSED_UNIT,
strings::STRING_LIT_AS_BYTES,
+ try_err::TRY_ERR,
types::FN_TO_NUMERIC_CAST,
types::FN_TO_NUMERIC_CAST_WITH_TRUNCATION,
types::IMPLICIT_HASHER,
diff --git a/clippy_lints/src/try_err.rs b/clippy_lints/src/try_err.rs
new file mode 100644
index 00000000000..ec0b96174da
--- /dev/null
+++ b/clippy_lints/src/try_err.rs
@@ -0,0 +1,120 @@
+use crate::utils::{match_qpath, paths, snippet, span_lint_and_then};
+use if_chain::if_chain;
+use rustc::hir::*;
+use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::ty::Ty;
+use rustc::{declare_lint_pass, declare_tool_lint};
+use rustc_errors::Applicability;
+
+declare_clippy_lint! {
+ /// **What it does:** Checks for usages of `Err(x)?`.
+ ///
+ /// **Why is this bad?** The `?` operator is designed to allow calls that
+ /// can fail to be easily chained. For example, `foo()?.bar()` or
+ /// `foo(bar()?)`. Because `Err(x)?` can't be used that way (it will
+ /// always return), it is more clear to write `return Err(x)`.
+ ///
+ /// **Known problems:** None.
+ ///
+ /// **Example:**
+ ///
+ /// ```rust,ignore
+ /// // Bad
+ /// fn foo(fail: bool) -> Result<i32, String> {
+ /// if fail {
+ /// Err("failed")?;
+ /// }
+ /// Ok(0)
+ /// }
+ ///
+ /// // Good
+ /// fn foo(fail: bool) -> Result<i32, String> {
+ /// if fail {
+ /// return Err("failed".into());
+ /// }
+ /// Ok(0)
+ /// }
+ /// ```
+ pub TRY_ERR,
+ style,
+ "return errors explicitly rather than hiding them behind a `?`"
+}
+
+declare_lint_pass!(TryErr => [TRY_ERR]);
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for TryErr {
+ fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
+ // Looks for a structure like this:
+ // match ::std::ops::Try::into_result(Err(5)) {
+ // ::std::result::Result::Err(err) =>
+ // #[allow(unreachable_code)]
+ // return ::std::ops::Try::from_error(::std::convert::From::from(err)),
+ // ::std::result::Result::Ok(val) =>
+ // #[allow(unreachable_code)]
+ // val,
+ // };
+ if_chain! {
+ if let ExprKind::Match(ref match_arg, _, MatchSource::TryDesugar) = expr.node;
+ if let ExprKind::Call(ref match_fun, ref try_args) = match_arg.node;
+ if let ExprKind::Path(ref match_fun_path) = match_fun.node;
+ if match_qpath(match_fun_path, &["std", "ops", "Try", "into_result"]);
+ if let Some(ref try_arg) = try_args.get(0);
+ if let ExprKind::Call(ref err_fun, ref err_args) = try_arg.node;
+ if let Some(ref err_arg) = err_args.get(0);
+ if let ExprKind::Path(ref err_fun_path) = err_fun.node;
+ if match_qpath(err_fun_path, &paths::RESULT_ERR);
+ if let Some(return_type) = find_err_return_type(cx, &expr.node);
+
+ then {
+ let err_type = cx.tables.expr_ty(err_arg);
+ let suggestion = if err_type == return_type {
+ format!("return Err({})", snippet(cx, err_arg.span, "_"))
+ } else {
+ format!("return Err({}.into())", snippet(cx, err_arg.span, "_"))
+ };
+
+ span_lint_and_then(
+ cx,
+ TRY_ERR,
+ expr.span,
+ &format!("confusing error return, consider using `{}`", suggestion),
+ |db| {
+ db.span_suggestion(
+ expr.span,
+ "try this",
+ suggestion,
+ Applicability::MaybeIncorrect
+ );
+ },
+ );
+ }
+ }
+ }
+}
+
+// In order to determine whether to suggest `.into()` or not, we need to find the error type the
+// function returns. To do that, we look for the From::from call (see tree above), and capture
+// its output type.
+fn find_err_return_type<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, expr: &'tcx ExprKind) -> Option<Ty<'tcx>> {
+ if let ExprKind::Match(_, ref arms, MatchSource::TryDesugar) = expr {
+ arms.iter().filter_map(|ty| find_err_return_type_arm(cx, ty)).nth(0)
+ } else {
+ None
+ }
+}
+
+// Check for From::from in one of the match arms.
+fn find_err_return_type_arm<'a, 'tcx>(cx: &LateContext<'a, 'tcx>, arm: &'tcx Arm) -> Option<Ty<'tcx>> {
+ if_chain! {
+ if let ExprKind::Ret(Some(ref err_ret)) = arm.body.node;
+ if let ExprKind::Call(ref from_error_path, ref from_error_args) = err_ret.node;
+ if let ExprKind::Path(ref from_error_fn) = from_error_path.node;
+ if match_qpath(from_error_fn, &["std", "ops", "Try", "from_error"]);
+ if let Some(from_error_arg) = from_error_args.get(0);
+ then {
+ Some(cx.tables.expr_ty(from_error_arg))
+ } else {
+ None
+ }
+ }
+}
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index 663b6a5e709..e8abb198e78 100644
--- a/src/lintlist/mod.rs
+++ b/src/lintlist/mod.rs
@@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 305] = [
+pub const ALL_LINTS: [Lint; 306] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
@@ -1820,6 +1820,13 @@ pub const ALL_LINTS: [Lint; 305] = [
deprecation: None,
module: "trivially_copy_pass_by_ref",
},
+ Lint {
+ name: "try_err",
+ group: "style",
+ desc: "TODO",
+ deprecation: None,
+ module: "try_err",
+ },
Lint {
name: "type_complexity",
group: "complexity",
diff --git a/tests/ui/try_err.rs b/tests/ui/try_err.rs
new file mode 100644
index 00000000000..d8decbbac66
--- /dev/null
+++ b/tests/ui/try_err.rs
@@ -0,0 +1,65 @@
+#![deny(clippy::try_err)]
+
+// Tests that a simple case works
+// Should flag `Err(err)?`
+pub fn basic_test() -> Result<i32, i32> {
+ let err: i32 = 1;
+ Err(err)?;
+ Ok(0)
+}
+
+// Tests that `.into()` is added when appropriate
+pub fn into_test() -> Result<i32, i32> {
+ let err: u8 = 1;
+ Err(err)?;
+ Ok(0)
+}
+
+// Tests that tries in general don't trigger the error
+pub fn negative_test() -> Result<i32, i32> {
+ Ok(nested_error()? + 1)
+}
+
+
+// Tests that `.into()` isn't added when the error type
+// matches the surrounding closure's return type, even
+// when it doesn't match the surrounding function's.
+pub fn closure_matches_test() -> Result<i32, i32> {
+ let res: Result<i32, i8> = Some(1).into_iter()
+ .map(|i| {
+ let err: i8 = 1;
+ Err(err)?;
+ Ok(i)
+ })
+ .next()
+ .unwrap();
+
+ Ok(res?)
+}
+
+// Tests that `.into()` isn't added when the error type
+// doesn't match the surrounding closure's return type.
+pub fn closure_into_test() -> Result<i32, i32> {
+ let res: Result<i32, i16> = Some(1).into_iter()
+ .map(|i| {
+ let err: i8 = 1;
+ Err(err)?;
+ Ok(i)
+ })
+ .next()
+ .unwrap();
+
+ Ok(res?)
+}
+
+fn nested_error() -> Result<i32, i32> {
+ Ok(1)
+}
+
+fn main() {
+ basic_test().unwrap();
+ into_test().unwrap();
+ negative_test().unwrap();
+ closure_matches_test().unwrap();
+ closure_into_test().unwrap();
+}
diff --git a/tests/ui/try_err.stderr b/tests/ui/try_err.stderr
new file mode 100644
index 00000000000..4e6f273052f
--- /dev/null
+++ b/tests/ui/try_err.stderr
@@ -0,0 +1,32 @@
+error: confusing error return, consider using `return Err(err)`
+ --> $DIR/try_err.rs:7:5
+ |
+LL | Err(err)?;
+ | ^^^^^^^^^ help: try this: `return Err(err)`
+ |
+note: lint level defined here
+ --> $DIR/try_err.rs:1:9
+ |
+LL | #![deny(clippy::try_err)]
+ | ^^^^^^^^^^^^^^^
+
+error: confusing error return, consider using `return Err(err.into())`
+ --> $DIR/try_err.rs:14:5
+ |
+LL | Err(err)?;
+ | ^^^^^^^^^ help: try this: `return Err(err.into())`
+
+error: confusing error return, consider using `return Err(err)`
+ --> $DIR/try_err.rs:31:13
+ |
+LL | Err(err)?;
+ | ^^^^^^^^^ help: try this: `return Err(err)`
+
+error: confusing error return, consider using `return Err(err.into())`
+ --> $DIR/try_err.rs:46:13
+ |
+LL | Err(err)?;
+ | ^^^^^^^^^ help: try this: `return Err(err.into())`
+
+error: aborting due to 4 previous errors
+