diff --git a/clippy_lints/src/read_zero_byte_vec.rs b/clippy_lints/src/read_zero_byte_vec.rs index 62f3c09aa..650324d42 100644 --- a/clippy_lints/src/read_zero_byte_vec.rs +++ b/clippy_lints/src/read_zero_byte_vec.rs @@ -1,11 +1,13 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg}; +use clippy_utils::get_enclosing_block; use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; use clippy_utils::source::snippet; -use clippy_utils::visitors::for_each_expr; -use core::ops::ControlFlow; -use hir::{Expr, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind}; + +use hir::{Expr, ExprKind, HirId, Local, PatKind, PathSegment, QPath, StmtKind}; use rustc_errors::Applicability; use rustc_hir as hir; +use rustc_hir::def::Res; +use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; @@ -49,57 +51,40 @@ 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 + for stmt in block.stmts { + if stmt.span.from_expansion() { + return; + } + + if let StmtKind::Local(local) = stmt.kind + && let Local { + pat, init: Some(init), .. + } = local + && let PatKind::Binding(_, id, ident, _) = pat.kind && let Some(vec_init_kind) = get_vec_init_kind(cx, init) { - let visitor = |expr: &Expr<'_>| { - if let ExprKind::MethodCall(path, _, [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 - { - ControlFlow::Break(()) - } else { - ControlFlow::Continue(()) - } + let mut visitor = ReadVecVisitor { + local_id: id, + read_zero_expr: None, + has_resize: false, }; - let (read_found, next_stmt_span) = if let Some(next_stmt) = block.stmts.get(idx + 1) { - // case { .. stmt; stmt; .. } - (for_each_expr(next_stmt, visitor).is_some(), next_stmt.span) - } else if let Some(e) = block.expr { - // case { .. stmt; expr } - (for_each_expr(e, visitor).is_some(), e.span) - } else { + let Some(enclosing_block) = get_enclosing_block(cx, id) else { return; }; + visitor.visit_block(enclosing_block); - if read_found && !next_stmt_span.from_expansion() { + if let Some(expr) = visitor.read_zero_expr { let applicability = Applicability::MaybeIncorrect; match vec_init_kind { VecInitKind::WithConstCapacity(len) => { span_lint_and_sugg( cx, READ_ZERO_BYTE_VEC, - next_stmt_span, + expr.span, "reading zero byte data to `Vec`", "try", - format!( - "{}.resize({len}, 0); {}", - ident.as_str(), - snippet(cx, next_stmt_span, "..") - ), + format!("{}.resize({len}, 0); {}", ident.as_str(), snippet(cx, expr.span, "..")), applicability, ); }, @@ -108,25 +93,20 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { span_lint_and_sugg( cx, READ_ZERO_BYTE_VEC, - next_stmt_span, + expr.span, "reading zero byte data to `Vec`", "try", format!( "{}.resize({}, 0); {}", ident.as_str(), snippet(cx, e.span, ".."), - snippet(cx, next_stmt_span, "..") + snippet(cx, expr.span, "..") ), applicability, ); }, _ => { - span_lint( - cx, - READ_ZERO_BYTE_VEC, - next_stmt_span, - "reading zero byte data to `Vec`", - ); + span_lint(cx, READ_ZERO_BYTE_VEC, expr.span, "reading zero byte data to `Vec`"); }, } } @@ -134,3 +114,47 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { } } } + +struct ReadVecVisitor<'tcx> { + local_id: HirId, + read_zero_expr: Option<&'tcx Expr<'tcx>>, + has_resize: bool, +} + +impl<'tcx> Visitor<'tcx> for ReadVecVisitor<'tcx> { + fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) { + if let ExprKind::MethodCall(path, receiver, args, _) = e.kind { + let PathSegment { ident, .. } = *path; + + match ident.as_str() { + "read" | "read_exact" => { + let [arg] = args else { return }; + if 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 + && let Res::Local(res_id) = inner_seg.res + && self.local_id == res_id + { + self.read_zero_expr = Some(e); + return; + } + }, + "resize" => { + // If the Vec is resized, then it's a valid read + if let ExprKind::Path(QPath::Resolved(_, inner_path)) = receiver.kind + && let Res::Local(res_id) = inner_path.res + && self.local_id == res_id + { + self.has_resize = true; + return; + } + }, + _ => {}, + } + } + + if !self.has_resize && self.read_zero_expr.is_none() { + walk_expr(self, e); + } + } +} diff --git a/tests/ui/read_zero_byte_vec.rs b/tests/ui/read_zero_byte_vec.rs index 76b9b9818..fd5a88a37 100644 --- a/tests/ui/read_zero_byte_vec.rs +++ b/tests/ui/read_zero_byte_vec.rs @@ -55,14 +55,6 @@ fn test() -> io::Result<()> { 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); @@ -75,6 +67,27 @@ fn test() -> io::Result<()> { Ok(()) } +fn test_nested() -> io::Result<()> { + let cap = 1000; + let mut f = File::open("foo.txt").unwrap(); + + // Issue #9274 + // Should not lint + let mut v = Vec::new(); + { + v.resize(10, 0); + f.read(&mut v)?; + } + + let mut v = Vec::new(); + { + f.read(&mut v)?; + //~^ ERROR: reading zero byte data to `Vec` + } + + Ok(()) +} + async fn test_futures(r: &mut R) { // should lint let mut data = Vec::new(); diff --git a/tests/ui/read_zero_byte_vec.stderr b/tests/ui/read_zero_byte_vec.stderr index 523ecb294..e85aa051c 100644 --- a/tests/ui/read_zero_byte_vec.stderr +++ b/tests/ui/read_zero_byte_vec.stderr @@ -2,7 +2,7 @@ error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:21:5 | LL | f.read_exact(&mut data).unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data).unwrap();` + | ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data)` | = note: `-D clippy::read-zero-byte-vec` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::read_zero_byte_vec)]` @@ -11,19 +11,19 @@ error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:27:5 | LL | f.read_exact(&mut data2)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); 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:32:5 | LL | f.read_exact(&mut data3)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:37:5 + --> $DIR/read_zero_byte_vec.rs:37:13 | LL | let _ = f.read(&mut data4)?; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` --> $DIR/read_zero_byte_vec.rs:43:9 @@ -38,28 +38,34 @@ LL | f.read(&mut data6) | ^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:81:5 + --> $DIR/read_zero_byte_vec.rs:84:9 + | +LL | f.read(&mut v)?; + | ^^^^^^^^^^^^^^ + +error: reading zero byte data to `Vec` + --> $DIR/read_zero_byte_vec.rs:94:5 | LL | r.read(&mut data).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:86:5 + --> $DIR/read_zero_byte_vec.rs:99:5 | LL | r.read_exact(&mut data2).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:93:5 + --> $DIR/read_zero_byte_vec.rs:106:5 | LL | r.read(&mut data).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ error: reading zero byte data to `Vec` - --> $DIR/read_zero_byte_vec.rs:98:5 + --> $DIR/read_zero_byte_vec.rs:111:5 | LL | r.read_exact(&mut data2).await.unwrap(); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^^^^^^^^ -error: aborting due to 10 previous errors +error: aborting due to 11 previous errors