From ce6e037ac5527c7dd2153a929f2ce8bda2c2bb1a Mon Sep 17 00:00:00 2001 From: sinkuu Date: Sat, 7 Jan 2017 20:35:45 +0900 Subject: [PATCH] Implement `unused_io_amount` lint --- CHANGELOG.md | 1 + README.md | 3 +- clippy_lints/src/lib.rs | 3 + clippy_lints/src/unused_io_amount.rs | 138 +++++++++++++++++++++++++ clippy_lints/src/utils/paths.rs | 3 + tests/compile-fail/unused_io_amount.rs | 35 +++++++ 6 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 clippy_lints/src/unused_io_amount.rs create mode 100644 tests/compile-fail/unused_io_amount.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d848aebe0b4..d5f7c47fb689 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/README.md b/README.md index f0a362295fca..37bd3db944d7 100644 --- a/README.md +++ b/README.md @@ -179,7 +179,7 @@ transparently: ## Lints -There are 183 lints included in this crate: +There are 184 lints included in this crate: name | default | triggers on -----------------------------------------------------------------------------------------------------------------------|---------|---------------------------------------------------------------------------------------------------------------------------------- @@ -349,6 +349,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 diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f362ff12a85c..b88b62040cec 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/clippy_lints/src/unused_io_amount.rs b/clippy_lints/src/unused_io_amount.rs new file mode 100644 index 000000000000..eef8751cd0b3 --- /dev/null +++ b/clippy_lints/src/unused_io_amount.rs @@ -0,0 +1,138 @@ +use rustc::lint::*; +use rustc::hir; +use utils::{span_lint, match_path, match_trait_method, 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: &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, + }; + + 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 { + 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); + } + }, + + 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"); + } + } +} + +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/paths.rs b/clippy_lints/src/utils/paths.rs index 402985613aeb..5edff76d9969 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/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"]; diff --git a/tests/compile-fail/unused_io_amount.rs b/tests/compile-fail/unused_io_amount.rs new file mode 100644 index 000000000000..1436605a1ba5 --- /dev/null +++ b/tests/compile-fail/unused_io_amount.rs @@ -0,0 +1,35 @@ +#![feature(plugin)] +#![plugin(clippy)] + +#![allow(dead_code)] + +use std::io; + +fn try_macro(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(()) +} + +fn question_mark(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(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() { +}