Merge pull request #1424 from sinkuu/unused_io_amount

Implement `unused_io_amount` lint

CHANGELOG.md

@@ -414,6 +414,7 @@ All notable changes to this project will be documented in this file.
 [`unstable_as_mut_slice`]: https://github.com/Manishearth/rust-clippy/wiki#unstable_as_mut_slice
 [`unstable_as_slice`]: https://github.com/Manishearth/rust-clippy/wiki#unstable_as_slice
 [`unused_collect`]: https://github.com/Manishearth/rust-clippy/wiki#unused_collect
+[`unused_io_amount`]: https://github.com/Manishearth/rust-clippy/wiki#unused_io_amount
 [`unused_label`]: https://github.com/Manishearth/rust-clippy/wiki#unused_label
 [`unused_lifetimes`]: https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes
 [`use_debug`]: https://github.com/Manishearth/rust-clippy/wiki#use_debug

README.md

@@ -180,7 +180,7 @@ transparently:
 
 ## Lints
 
-There are 183 lints included in this crate:
+There are 184 lints included in this crate:
 
 name                                                                                                                   | default | triggers on
 -----------------------------------------------------------------------------------------------------------------------|---------|----------------------------------------------------------------------------------------------------------------------------------
@@ -350,6 +350,7 @@ name
 [unsafe_removed_from_name](https://github.com/Manishearth/rust-clippy/wiki#unsafe_removed_from_name)                   | warn    | `unsafe` removed from API names on import
 [unseparated_literal_suffix](https://github.com/Manishearth/rust-clippy/wiki#unseparated_literal_suffix)               | allow   | literals whose suffix is not separated by an underscore
 [unused_collect](https://github.com/Manishearth/rust-clippy/wiki#unused_collect)                                       | warn    | `collect()`ing an iterator without using the result; this is usually better written as a for loop
+[unused_io_amount](https://github.com/Manishearth/rust-clippy/wiki#unused_io_amount)                                   | deny    | unused written/read amount
 [unused_label](https://github.com/Manishearth/rust-clippy/wiki#unused_label)                                           | warn    | unused labels
 [unused_lifetimes](https://github.com/Manishearth/rust-clippy/wiki#unused_lifetimes)                                   | warn    | unused lifetimes in function definitions
 [use_debug](https://github.com/Manishearth/rust-clippy/wiki#use_debug)                                                 | allow   | use of `Debug`-based formatting

clippy_lints/src/lib.rs

@@ -131,6 +131,7 @@ pub mod transmute;
 pub mod types;
 pub mod unicode;
 pub mod unsafe_removed_from_name;
+pub mod unused_io_amount;
 pub mod unused_label;
 pub mod vec;
 pub mod zero_div_zero;
@@ -287,6 +288,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box partialeq_ne_impl::Pass);
     reg.register_early_lint_pass(box reference::Pass);
     reg.register_early_lint_pass(box double_parens::DoubleParens);
+    reg.register_late_lint_pass(box unused_io_amount::UnusedIoAmount);
 
     reg.register_lint_group("clippy_restrictions", vec![
         arithmetic::FLOAT_ARITHMETIC,
@@ -480,6 +482,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         types::UNIT_CMP,
         unicode::ZERO_WIDTH_SPACE,
         unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
+        unused_io_amount::UNUSED_IO_AMOUNT,
         unused_label::UNUSED_LABEL,
         vec::USELESS_VEC,
         zero_div_zero::ZERO_DIVIDED_BY_ZERO,

clippy_lints/src/unused_io_amount.rs

@@ -0,0 +1,88 @@
+use rustc::lint::*;
+use rustc::hir;
+use utils::{span_lint, match_path, match_trait_method, is_try, paths};
+
+/// **What it does:** Checks for unused written/read amount.
+///
+/// **Why is this bad?** `io::Write::write` and `io::Read::read` are not guaranteed to
+/// process the entire buffer. They return how many bytes were processed, which might be smaller
+/// than a given buffer's length. If you don't need to deal with partial-write/read, use
+/// `write_all`/`read_exact` instead.
+///
+/// **Known problems:** Detects only common patterns.
+///
+/// **Example:**
+/// ```rust,ignore
+/// use std::io;
+/// fn foo<W: io::Write>(w: &mut W) -> io::Result<()> {
+///     // must be `w.write_all(b"foo")?;`
+///     w.write(b"foo")?;
+///     Ok(())
+/// }
+/// ```
+declare_lint! {
+    pub UNUSED_IO_AMOUNT,
+    Deny,
+    "unused written/read amount"
+}
+
+pub struct UnusedIoAmount;
+
+impl LintPass for UnusedIoAmount {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(UNUSED_IO_AMOUNT)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnusedIoAmount {
+    fn check_stmt(&mut self, cx: &LateContext, s: &hir::Stmt) {
+        let expr = match s.node {
+            hir::StmtSemi(ref expr, _) |
+            hir::StmtExpr(ref expr, _) => &**expr,
+            _ => return,
+        };
+
+        match 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, res, expr);
+                }
+            },
+
+            hir::ExprMethodCall(ref symbol, _, ref args) => {
+                let symbol = &*symbol.node.as_str();
+                match symbol {
+                    "expect" | "unwrap" | "unwrap_or" | "unwrap_or_else" => {
+                        check_method_call(cx, &args[0], expr);
+                    },
+                    _ => (),
+                }
+            },
+
+            _ => (),
+        }
+    }
+}
+
+fn check_method_call(cx: &LateContext, call: &hir::Expr, expr: &hir::Expr) {
+    if let hir::ExprMethodCall(ref symbol, _, _) = call.node {
+        let symbol = &*symbol.node.as_str();
+        if match_trait_method(cx, call, &paths::IO_READ) && symbol == "read" {
+            span_lint(cx,
+                      UNUSED_IO_AMOUNT,
+                      expr.span,
+                      "handle read amount returned or use `Read::read_exact` instead");
+        } else if match_trait_method(cx, call, &paths::IO_WRITE) && symbol == "write" {
+            span_lint(cx,
+                      UNUSED_IO_AMOUNT,
+                      expr.span,
+                      "handle written amount returned or use `Write::write_all` instead");
+        }
+    }
+}

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
+}

clippy_lints/src/utils/paths.rs

@@ -7,6 +7,7 @@ pub const BOX_NEW: [&'static str; 4] = ["std", "boxed", "Box", "new"];
 pub const BTREEMAP: [&'static str; 4] = ["collections", "btree", "map", "BTreeMap"];
 pub const BTREEMAP_ENTRY: [&'static str; 4] = ["collections", "btree", "map", "Entry"];
 pub const BTREESET: [&'static str; 4] = ["collections", "btree", "set", "BTreeSet"];
+pub const CARRIER_TRANSLATE: [&'static str; 4] = ["std", "ops", "Carrier", "translate"];
 pub const CLONE: [&'static str; 4] = ["core", "clone", "Clone", "clone"];
 pub const CLONE_TRAIT: [&'static str; 3] = ["core", "clone", "Clone"];
 pub const CMP_MAX: [&'static str; 3] = ["core", "cmp", "max"];
@@ -25,6 +26,8 @@ pub const HASHMAP_ENTRY: [&'static str; 5] = ["std", "collections", "hash", "map
 pub const HASHSET: [&'static str; 5] = ["std", "collections", "hash", "set", "HashSet"];
 pub const INTO_ITERATOR: [&'static str; 4] = ["core", "iter", "traits", "IntoIterator"];
 pub const IO_PRINT: [&'static str; 4] = ["std", "io", "stdio", "_print"];
+pub const IO_READ: [&'static str; 3] = ["std", "io", "Read"];
+pub const IO_WRITE: [&'static str; 3] = ["std", "io", "Write"];
 pub const ITERATOR: [&'static str; 4] = ["core", "iter", "iterator", "Iterator"];
 pub const LINKED_LIST: [&'static str; 3] = ["collections", "linked_list", "LinkedList"];
 pub const LINT: [&'static str; 3] = ["rustc", "lint", "Lint"];

tests/compile-fail/unused_io_amount.rs

@@ -0,0 +1,34 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![allow(dead_code)]
+
+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"));
+    let mut buf = [0u8; 4];
+    try!(s.read(&mut buf));
+    Ok(())
+}
+
+fn question_mark<T: io::Read + io::Write>(s: &mut T) -> io::Result<()> {
+    s.write(b"test")?;
+    //~^ ERROR handle written amount returned
+    let mut buf = [0u8; 4];
+    s.read(&mut buf)?;
+    //~^ ERROR handle read amount returned
+    Ok(())
+}
+
+fn unwrap<T: io::Read + io::Write>(s: &mut T) {
+    s.write(b"test").unwrap();
+    //~^ ERROR handle written amount returned
+    let mut buf = [0u8; 4];
+    s.read(&mut buf).unwrap();
+    //~^ ERROR handle read amount returned
+}
+
+fn main() {
+}