Auto merge of #11766 - dswij:issue-9274, r=blyxyas

`read_zero_byte_vec` refactor for better heuristics

Fixes #9274

Previously, the implementation of `read_zero_byte_vec` only checks for the next statement after the vec init. This fails when there is a block with statements that are expanded and walked by the old visitor.

This PR refactors so that:

1. It checks if there is a `resize`	on the vec
2. It works on blocks properly

e.g. This should properly lint now:

```
    let mut v = Vec::new();
    {
        f.read(&mut v)?;
        //~^ ERROR: reading zero byte data to `Vec`
    }
```

changelog: [`read_zero_byte_vec`] Refactored for better heuristics

Author: bors

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,29 +93,68 @@ 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`");
                         },
                     }
                 }
             }
         }
     }
 }
+
+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);
+        }
+    }
+}

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: AsyncRead + Unpin>(r: &mut R) {
     // should lint
     let mut data = Vec::new();

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