diff --git a/CHANGELOG.md b/CHANGELOG.md index 462e56892..6aaf12ed9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3678,6 +3678,7 @@ Released 2018-09-13 [`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer [`rc_clone_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_clone_in_vec_init [`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex +[`read_zero_byte_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_zero_byte_vec [`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs index 4691f957c..8a2cfbff9 100644 --- a/clippy_lints/src/lib.register_all.rs +++ b/clippy_lints/src/lib.register_all.rs @@ -271,6 +271,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![ LintId::of(ranges::RANGE_ZIP_WITH_LEN), LintId::of(ranges::REVERSED_EMPTY_RANGES), LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT), + LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC), LintId::of(redundant_clone::REDUNDANT_CLONE), LintId::of(redundant_closure_call::REDUNDANT_CLOSURE_CALL), LintId::of(redundant_field_names::REDUNDANT_FIELD_NAMES), diff --git a/clippy_lints/src/lib.register_correctness.rs b/clippy_lints/src/lib.register_correctness.rs index 50cdd0af9..92a3a0aab 100644 --- a/clippy_lints/src/lib.register_correctness.rs +++ b/clippy_lints/src/lib.register_correctness.rs @@ -55,6 +55,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve LintId::of(ptr::INVALID_NULL_PTR_USAGE), LintId::of(ptr::MUT_FROM_REF), LintId::of(ranges::REVERSED_EMPTY_RANGES), + LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC), LintId::of(regex::INVALID_REGEX), LintId::of(self_assignment::SELF_ASSIGNMENT), LintId::of(serde_api::SERDE_API_MISUSE), diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index efb073af6..8ad984c68 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -459,6 +459,7 @@ store.register_lints(&[ ranges::RANGE_ZIP_WITH_LEN, ranges::REVERSED_EMPTY_RANGES, rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT, + read_zero_byte_vec::READ_ZERO_BYTE_VEC, redundant_clone::REDUNDANT_CLONE, redundant_closure_call::REDUNDANT_CLOSURE_CALL, redundant_else::REDUNDANT_ELSE, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 02499f6e3..84898eae0 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -348,6 +348,7 @@ mod pub_use; mod question_mark; mod ranges; mod rc_clone_in_vec_init; +mod read_zero_byte_vec; mod redundant_clone; mod redundant_closure_call; mod redundant_else; @@ -908,6 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(swap_ptr_to_ref::SwapPtrToRef)); store.register_late_pass(|| Box::new(mismatching_type_param_order::TypeParamMismatch)); store.register_late_pass(|| Box::new(as_underscore::AsUnderscore)); + store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/read_zero_byte_vec.rs b/clippy_lints/src/read_zero_byte_vec.rs new file mode 100644 index 000000000..9538a8104 --- /dev/null +++ b/clippy_lints/src/read_zero_byte_vec.rs @@ -0,0 +1,142 @@ +use clippy_utils::{ + diagnostics::{span_lint, span_lint_and_sugg}, + higher::{get_vec_init_kind, VecInitKind}, + source::snippet, + visitors::expr_visitor_no_bodies, +}; +use hir::{intravisit::Visitor, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind}; +use rustc_errors::Applicability; +use rustc_hir as hir; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// This lint catches reads into a zero-length `Vec`. + /// Especially in the case of a call to `with_capacity`, this lint warns that read + /// gets the number of bytes from the `Vec`'s length, not its capacity. + /// + /// ### Why is this bad? + /// Reading zero bytes is almost certainly not the intended behavior. + /// + /// ### Known problems + /// In theory, a very unusual read implementation could assign some semantic meaning + /// to zero-byte reads. But it seems exceptionally unlikely that code intending to do + /// a zero-byte read would allocate a `Vec` for it. + /// + /// ### Example + /// ```rust + /// use std::io; + /// fn foo(mut f: F) { + /// let mut data = Vec::with_capacity(100); + /// f.read(&mut data).unwrap(); + /// } + /// ``` + /// Use instead: + /// ```rust + /// use std::io; + /// fn foo(mut f: F) { + /// let mut data = Vec::with_capacity(100); + /// data.resize(100, 0); + /// f.read(&mut data).unwrap(); + /// } + /// ``` + #[clippy::version = "1.63.0"] + pub READ_ZERO_BYTE_VEC, + correctness, + "checks for reads into a zero-length `Vec`" +} +declare_lint_pass!(ReadZeroByteVec => [READ_ZERO_BYTE_VEC]); + +impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { + fn check_block(&mut self, cx: &LateContext<'tcx>, block: &hir::Block<'tcx>) { + for (idx, stmt) in block.stmts.iter().enumerate() { + if !stmt.span.from_expansion() + // matches `let v = Vec::new();` + && let StmtKind::Local(local) = stmt.kind + && let Local { pat, init: Some(init), .. } = local + && let PatKind::Binding(_, _, ident, _) = pat.kind + && let Some(vec_init_kind) = get_vec_init_kind(cx, init) + { + // finds use of `_.read(&mut v)` + let mut read_found = false; + let mut visitor = expr_visitor_no_bodies(|expr| { + if let ExprKind::MethodCall(path, [_self, arg], _) = expr.kind + && let PathSegment { ident: read_or_read_exact, .. } = *path + && matches!(read_or_read_exact.as_str(), "read" | "read_exact") + && let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind + && let ExprKind::Path(QPath::Resolved(None, inner_path)) = inner.kind + && let [inner_seg] = inner_path.segments + && ident.name == inner_seg.ident.name + { + read_found = true; + } + !read_found + }); + + let next_stmt_span; + if idx == block.stmts.len() - 1 { + // case { .. stmt; expr } + if let Some(e) = block.expr { + visitor.visit_expr(e); + next_stmt_span = e.span; + } else { + return; + } + } else { + // case { .. stmt; stmt; .. } + let next_stmt = &block.stmts[idx + 1]; + visitor.visit_stmt(next_stmt); + next_stmt_span = next_stmt.span; + } + drop(visitor); + + if read_found && !next_stmt_span.from_expansion() { + let applicability = Applicability::MaybeIncorrect; + match vec_init_kind { + VecInitKind::WithConstCapacity(len) => { + span_lint_and_sugg( + cx, + READ_ZERO_BYTE_VEC, + next_stmt_span, + "reading zero byte data to `Vec`", + "try", + format!("{}.resize({}, 0); {}", + ident.as_str(), + len, + snippet(cx, next_stmt_span, "..") + ), + applicability, + ); + } + VecInitKind::WithExprCapacity(hir_id) => { + let e = cx.tcx.hir().expect_expr(hir_id); + span_lint_and_sugg( + cx, + READ_ZERO_BYTE_VEC, + next_stmt_span, + "reading zero byte data to `Vec`", + "try", + format!("{}.resize({}, 0); {}", + ident.as_str(), + snippet(cx, e.span, ".."), + snippet(cx, next_stmt_span, "..") + ), + applicability, + ); + } + _ => { + span_lint( + cx, + READ_ZERO_BYTE_VEC, + next_stmt_span, + "reading zero byte data to `Vec`", + ); + + } + } + } + } + } + } +} diff --git a/tests/ui/read_zero_byte_vec.rs b/tests/ui/read_zero_byte_vec.rs new file mode 100644 index 000000000..30807e0f8 --- /dev/null +++ b/tests/ui/read_zero_byte_vec.rs @@ -0,0 +1,87 @@ +#![warn(clippy::read_zero_byte_vec)] +#![allow(clippy::unused_io_amount)] +use std::fs::File; +use std::io; +use std::io::prelude::*; + +extern crate futures; +use futures::io::{AsyncRead, AsyncReadExt}; +use tokio::io::{AsyncRead as TokioAsyncRead, AsyncReadExt as _, AsyncWrite as TokioAsyncWrite, AsyncWriteExt as _}; + +fn test() -> io::Result<()> { + let cap = 1000; + let mut f = File::open("foo.txt").unwrap(); + + // should lint + let mut data = Vec::with_capacity(20); + f.read_exact(&mut data).unwrap(); + + // should lint + let mut data2 = Vec::with_capacity(cap); + f.read_exact(&mut data2)?; + + // should lint + let mut data3 = Vec::new(); + f.read_exact(&mut data3)?; + + // should lint + let mut data4 = vec![]; + let _ = f.read(&mut data4)?; + + // should lint + let _ = { + let mut data5 = Vec::new(); + f.read(&mut data5) + }; + + // should lint + let _ = { + let mut data6: Vec = Default::default(); + f.read(&mut data6) + }; + + // should not lint + let mut buf = [0u8; 100]; + f.read(&mut buf)?; + + // should not lint + let mut empty = vec![]; + let mut data7 = vec![]; + f.read(&mut empty); + + // should not lint + f.read(&mut data7); + + // should not lint + let mut data8 = Vec::new(); + data8.resize(100, 0); + f.read_exact(&mut data8)?; + + // should not lint + let mut data9 = vec![1, 2, 3]; + f.read_exact(&mut data9)?; + + Ok(()) +} + +async fn test_futures(r: &mut R) { + // should lint + let mut data = Vec::new(); + r.read(&mut data).await.unwrap(); + + // should lint + let mut data2 = Vec::new(); + r.read_exact(&mut data2).await.unwrap(); +} + +async fn test_tokio(r: &mut R) { + // should lint + let mut data = Vec::new(); + r.read(&mut data).await.unwrap(); + + // should lint + let mut data2 = Vec::new(); + r.read_exact(&mut data2).await.unwrap(); +} + +fn main() {} diff --git a/tests/ui/read_zero_byte_vec.stderr b/tests/ui/read_zero_byte_vec.stderr new file mode 100644 index 000000000..08ba9753d --- /dev/null +++ b/tests/ui/read_zero_byte_vec.stderr @@ -0,0 +1,64 @@ +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:17:5 + | +LL | f.read_exact(&mut data).unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data).unwrap();` + | + = note: `-D clippy::read-zero-byte-vec` implied by `-D warnings` + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:21:5 + | +LL | f.read_exact(&mut data2)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); f.read_exact(&mut data2)?;` + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:25:5 + | +LL | f.read_exact(&mut data3)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:29:5 + | +LL | let _ = f.read(&mut data4)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:34:9 + | +LL | f.read(&mut data5) + | ^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:40:9 + | +LL | f.read(&mut data6) + | ^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:70:5 + | +LL | r.read(&mut data).await.unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:74:5 + | +LL | r.read_exact(&mut data2).await.unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:80:5 + | +LL | r.read(&mut data).await.unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:84:5 + | +LL | r.read_exact(&mut data2).await.unwrap(); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 10 previous errors +