-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Auto merge of #8964 - tamaroning:read_zero_byte_vec, r=dswij
Warn about read into zero-length `Vec` Closes #8886 - \[x] Followed [lint naming conventions][lint_naming] - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - \[x] Added lint documentation - \[x] Run `cargo dev fmt` changelog: none
- Loading branch information
Showing
8 changed files
with
299 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<F: io::Read>(mut f: F) { | ||
| /// let mut data = Vec::with_capacity(100); | ||
| /// f.read(&mut data).unwrap(); | ||
| /// } | ||
| /// ``` | ||
| /// Use instead: | ||
| /// ```rust | ||
| /// use std::io; | ||
| /// fn foo<F: io::Read>(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`", | ||
| ); | ||
|
|
||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<u8> = 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: AsyncRead + Unpin>(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: TokioAsyncRead + Unpin>(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() {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
|