Move `is_try` to util

Removed unnecessary condition

Also changed lint span of `try` from surrounded expression to entire `try`
invocation. It turned out that compiletest misses errors for macro invocations.

diff --git a/clippy_lints/src/unused_io_amount.rs b/clippy_lints/src/unused_io_amount.rs
index eef8751cd0b3ead8eb8093257147fc11a8e9a942..fc50e668c85d4ca8863afc12c74aaa1fdab91dc9 100644 (file)
--- a/clippy_lints/src/unused_io_amount.rs
+++ b/clippy_lints/src/unused_io_amount.rs
@@ -1,6 +1,6 @@
 use rustc::lint::*;
 use rustc::hir;
-use utils::{span_lint, match_path, match_trait_method, paths};
+use utils::{span_lint, match_path, match_trait_method, is_try, paths};
 
 /// **What it does:** Checks for unused written/read amount.
 ///
@@ -42,20 +42,16 @@ fn check_stmt(&mut self, cx: &LateContext, s: &hir::Stmt) {
             _ => return,
         };
 
-        if let hir::ExprRet(..) = expr.node {
-            return;
-        }
-
         match expr.node {
-            hir::ExprMatch(ref expr, ref arms, _) if is_try(arms) => {
-                if let hir::ExprCall(ref func, ref args) = expr.node {
+            hir::ExprMatch(ref res, _, _) if is_try(expr).is_some() => {
+                if let hir::ExprCall(ref func, ref args) = res.node {
                     if let hir::ExprPath(ref path) = func.node {
                         if match_path(path, &paths::CARRIER_TRANSLATE) && args.len() == 1 {
                             check_method_call(cx, &args[0], expr);
                         }
                     }
                 } else {
-                    check_method_call(cx, expr, expr);
+                    check_method_call(cx, res, expr);
                 }
             },
 
@@ -90,49 +86,3 @@ fn check_method_call(cx: &LateContext, call: &hir::Expr, expr: &hir::Expr) {
         }
     }
 }
-
-fn is_try(arms: &[hir::Arm]) -> bool {
-    // `Ok(x) => x` or `Ok(_) => ...`
-    fn is_ok(arm: &hir::Arm) -> bool {
-        if let hir::PatKind::TupleStruct(ref path, ref pat, ref dotdot) = arm.pats[0].node {
-            // cut off `core`
-            if match_path(path, &paths::RESULT_OK[1..]) {
-                if *dotdot == Some(0) {
-                    return true;
-                }
-
-                match pat[0].node {
-                    hir::PatKind::Wild => {
-                        return true;
-                    },
-                    hir::PatKind::Binding(_, defid, _, None) => {
-                        if let hir::ExprPath(hir::QPath::Resolved(None, ref path)) = arm.body.node {
-                            if path.def.def_id() == defid {
-                                return true;
-                            }
-                        }
-                    },
-                    _ => (),
-                }
-            }
-        }
-
-        false
-    }
-
-    /// Detects `_ => ...` or `Err(x) => ...`
-    fn is_err_or_wild(arm: &hir::Arm) -> bool {
-        match arm.pats[0].node {
-            hir::PatKind::Wild => true,
-            hir::PatKind::TupleStruct(ref path, _, _) => match_path(path, &paths::RESULT_ERR[1..]),
-            _ => false,
-        }
-    }
-
-    if arms.len() == 2 && arms[0].pats.len() == 1 && arms[0].guard.is_none() && arms[1].pats.len() == 1 &&
-       arms[1].guard.is_none() {
-        (is_ok(&arms[0]) && is_err_or_wild(&arms[1])) || (is_ok(&arms[1]) && is_err_or_wild(&arms[0]))
-    } else {
-        false
-    }
-}

diff --git a/clippy_lints/src/utils/mod.rs b/clippy_lints/src/utils/mod.rs
index f971779b2edb3e8fe53427b662e8f93f498ba200..866a35427624d50eaade33f05548b3f869510cb4 100644 (file)
--- a/clippy_lints/src/utils/mod.rs
+++ b/clippy_lints/src/utils/mod.rs
@@ -920,3 +920,47 @@ pub fn is_self_ty(slf: &Ty) -> bool {
 pub fn iter_input_pats<'tcx>(decl: &FnDecl, body: &'tcx Body) -> impl Iterator<Item = &'tcx Arg> {
     (0..decl.inputs.len()).map(move |i| &body.arguments[i])
 }
+
+/// Check if a given expression is a match expression
+/// expanded from `?` operator or `try` macro.
+pub fn is_try(expr: &Expr) -> Option<&Expr> {
+    fn is_ok(arm: &Arm) -> bool {
+        if_let_chain! {[
+            let PatKind::TupleStruct(ref path, ref pat, None) = arm.pats[0].node,
+            match_path(path, &paths::RESULT_OK[1..]),
+            let PatKind::Binding(_, defid, _, None) = pat[0].node,
+            let ExprPath(QPath::Resolved(None, ref path)) = arm.body.node,
+            path.def.def_id() == defid,
+        ], {
+            return true;
+        }}
+        false
+    }
+
+    fn is_err(arm: &Arm) -> bool {
+        if let PatKind::TupleStruct(ref path, _, _) = arm.pats[0].node {
+            match_path(path, &paths::RESULT_ERR[1..])
+        } else {
+            false
+        }
+    }
+
+    if let ExprMatch(_, ref arms, ref source) = expr.node {
+        // desugared from a `?` operator
+        if let MatchSource::TryDesugar = *source {
+            return Some(expr);
+        }
+
+        if_let_chain! {[
+            arms.len() == 2,
+            arms[0].pats.len() == 1 && arms[0].guard.is_none(),
+            arms[1].pats.len() == 1 && arms[1].guard.is_none(),
+            (is_ok(&arms[0]) && is_err(&arms[1])) ||
+                (is_ok(&arms[1]) && is_err(&arms[0])),
+        ], {
+            return Some(expr);
+        }}
+    }
+
+    None
+}

diff --git a/tests/compile-fail/unused_io_amount.rs b/tests/compile-fail/unused_io_amount.rs
index 1436605a1ba50065d73b0a8958fff06500f85f0b..2e63705c5107cb5028408f343a9254813ca9325a 100644 (file)
--- a/tests/compile-fail/unused_io_amount.rs
+++ b/tests/compile-fail/unused_io_amount.rs
@@ -5,12 +5,11 @@
 
 use std::io;
 
+// FIXME: compiletest doesn't understand errors from macro invocation span
 fn try_macro<T: io::Read + io::Write>(s: &mut T) -> io::Result<()> {
     try!(s.write(b"test"));
-    //~^ ERROR handle written amount returned
     let mut buf = [0u8; 4];
     try!(s.read(&mut buf));
-    //~^ ERROR handle read amount returned
     Ok(())
 }