Skip to content

Commit 844c06a

Browse files
committed
Auto merge of rust-lang#8964 - tamaroning:read_zero_byte_vec, r=dswij
Warn about read into zero-length `Vec` Closes rust-lang#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
2 parents 32a86c0 + 14478bb commit 844c06a

8 files changed

+299
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -3678,6 +3678,7 @@ Released 2018-09-13
36783678
[`rc_buffer`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_buffer
36793679
[`rc_clone_in_vec_init`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_clone_in_vec_init
36803680
[`rc_mutex`]: https://rust-lang.github.io/rust-clippy/master/index.html#rc_mutex
3681+
[`read_zero_byte_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#read_zero_byte_vec
36813682
[`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl
36823683
[`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation
36833684
[`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone

clippy_lints/src/lib.register_all.rs

+1
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
271271
LintId::of(ranges::RANGE_ZIP_WITH_LEN),
272272
LintId::of(ranges::REVERSED_EMPTY_RANGES),
273273
LintId::of(rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT),
274+
LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC),
274275
LintId::of(redundant_clone::REDUNDANT_CLONE),
275276
LintId::of(redundant_closure_call::REDUNDANT_CLOSURE_CALL),
276277
LintId::of(redundant_field_names::REDUNDANT_FIELD_NAMES),

clippy_lints/src/lib.register_correctness.rs

+1
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
5555
LintId::of(ptr::INVALID_NULL_PTR_USAGE),
5656
LintId::of(ptr::MUT_FROM_REF),
5757
LintId::of(ranges::REVERSED_EMPTY_RANGES),
58+
LintId::of(read_zero_byte_vec::READ_ZERO_BYTE_VEC),
5859
LintId::of(regex::INVALID_REGEX),
5960
LintId::of(self_assignment::SELF_ASSIGNMENT),
6061
LintId::of(serde_api::SERDE_API_MISUSE),

clippy_lints/src/lib.register_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@ store.register_lints(&[
459459
ranges::RANGE_ZIP_WITH_LEN,
460460
ranges::REVERSED_EMPTY_RANGES,
461461
rc_clone_in_vec_init::RC_CLONE_IN_VEC_INIT,
462+
read_zero_byte_vec::READ_ZERO_BYTE_VEC,
462463
redundant_clone::REDUNDANT_CLONE,
463464
redundant_closure_call::REDUNDANT_CLOSURE_CALL,
464465
redundant_else::REDUNDANT_ELSE,

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,7 @@ mod pub_use;
348348
mod question_mark;
349349
mod ranges;
350350
mod rc_clone_in_vec_init;
351+
mod read_zero_byte_vec;
351352
mod redundant_clone;
352353
mod redundant_closure_call;
353354
mod redundant_else;
@@ -908,6 +909,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
908909
store.register_late_pass(|| Box::new(swap_ptr_to_ref::SwapPtrToRef));
909910
store.register_late_pass(|| Box::new(mismatching_type_param_order::TypeParamMismatch));
910911
store.register_late_pass(|| Box::new(as_underscore::AsUnderscore));
912+
store.register_late_pass(|| Box::new(read_zero_byte_vec::ReadZeroByteVec));
911913
// add lints here, do not remove this comment, it's used in `new_lint`
912914
}
913915

+142
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
use clippy_utils::{
2+
diagnostics::{span_lint, span_lint_and_sugg},
3+
higher::{get_vec_init_kind, VecInitKind},
4+
source::snippet,
5+
visitors::expr_visitor_no_bodies,
6+
};
7+
use hir::{intravisit::Visitor, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind};
8+
use rustc_errors::Applicability;
9+
use rustc_hir as hir;
10+
use rustc_lint::{LateContext, LateLintPass};
11+
use rustc_session::{declare_lint_pass, declare_tool_lint};
12+
13+
declare_clippy_lint! {
14+
/// ### What it does
15+
/// This lint catches reads into a zero-length `Vec`.
16+
/// Especially in the case of a call to `with_capacity`, this lint warns that read
17+
/// gets the number of bytes from the `Vec`'s length, not its capacity.
18+
///
19+
/// ### Why is this bad?
20+
/// Reading zero bytes is almost certainly not the intended behavior.
21+
///
22+
/// ### Known problems
23+
/// In theory, a very unusual read implementation could assign some semantic meaning
24+
/// to zero-byte reads. But it seems exceptionally unlikely that code intending to do
25+
/// a zero-byte read would allocate a `Vec` for it.
26+
///
27+
/// ### Example
28+
/// ```rust
29+
/// use std::io;
30+
/// fn foo<F: io::Read>(mut f: F) {
31+
/// let mut data = Vec::with_capacity(100);
32+
/// f.read(&mut data).unwrap();
33+
/// }
34+
/// ```
35+
/// Use instead:
36+
/// ```rust
37+
/// use std::io;
38+
/// fn foo<F: io::Read>(mut f: F) {
39+
/// let mut data = Vec::with_capacity(100);
40+
/// data.resize(100, 0);
41+
/// f.read(&mut data).unwrap();
42+
/// }
43+
/// ```
44+
#[clippy::version = "1.63.0"]
45+
pub READ_ZERO_BYTE_VEC,
46+
correctness,
47+
"checks for reads into a zero-length `Vec`"
48+
}
49+
declare_lint_pass!(ReadZeroByteVec => [READ_ZERO_BYTE_VEC]);
50+
51+
impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec {
52+
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &hir::Block<'tcx>) {
53+
for (idx, stmt) in block.stmts.iter().enumerate() {
54+
if !stmt.span.from_expansion()
55+
// matches `let v = Vec::new();`
56+
&& let StmtKind::Local(local) = stmt.kind
57+
&& let Local { pat, init: Some(init), .. } = local
58+
&& let PatKind::Binding(_, _, ident, _) = pat.kind
59+
&& let Some(vec_init_kind) = get_vec_init_kind(cx, init)
60+
{
61+
// finds use of `_.read(&mut v)`
62+
let mut read_found = false;
63+
let mut visitor = expr_visitor_no_bodies(|expr| {
64+
if let ExprKind::MethodCall(path, [_self, arg], _) = expr.kind
65+
&& let PathSegment { ident: read_or_read_exact, .. } = *path
66+
&& matches!(read_or_read_exact.as_str(), "read" | "read_exact")
67+
&& let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind
68+
&& let ExprKind::Path(QPath::Resolved(None, inner_path)) = inner.kind
69+
&& let [inner_seg] = inner_path.segments
70+
&& ident.name == inner_seg.ident.name
71+
{
72+
read_found = true;
73+
}
74+
!read_found
75+
});
76+
77+
let next_stmt_span;
78+
if idx == block.stmts.len() - 1 {
79+
// case { .. stmt; expr }
80+
if let Some(e) = block.expr {
81+
visitor.visit_expr(e);
82+
next_stmt_span = e.span;
83+
} else {
84+
return;
85+
}
86+
} else {
87+
// case { .. stmt; stmt; .. }
88+
let next_stmt = &block.stmts[idx + 1];
89+
visitor.visit_stmt(next_stmt);
90+
next_stmt_span = next_stmt.span;
91+
}
92+
drop(visitor);
93+
94+
if read_found && !next_stmt_span.from_expansion() {
95+
let applicability = Applicability::MaybeIncorrect;
96+
match vec_init_kind {
97+
VecInitKind::WithConstCapacity(len) => {
98+
span_lint_and_sugg(
99+
cx,
100+
READ_ZERO_BYTE_VEC,
101+
next_stmt_span,
102+
"reading zero byte data to `Vec`",
103+
"try",
104+
format!("{}.resize({}, 0); {}",
105+
ident.as_str(),
106+
len,
107+
snippet(cx, next_stmt_span, "..")
108+
),
109+
applicability,
110+
);
111+
}
112+
VecInitKind::WithExprCapacity(hir_id) => {
113+
let e = cx.tcx.hir().expect_expr(hir_id);
114+
span_lint_and_sugg(
115+
cx,
116+
READ_ZERO_BYTE_VEC,
117+
next_stmt_span,
118+
"reading zero byte data to `Vec`",
119+
"try",
120+
format!("{}.resize({}, 0); {}",
121+
ident.as_str(),
122+
snippet(cx, e.span, ".."),
123+
snippet(cx, next_stmt_span, "..")
124+
),
125+
applicability,
126+
);
127+
}
128+
_ => {
129+
span_lint(
130+
cx,
131+
READ_ZERO_BYTE_VEC,
132+
next_stmt_span,
133+
"reading zero byte data to `Vec`",
134+
);
135+
136+
}
137+
}
138+
}
139+
}
140+
}
141+
}
142+
}

tests/ui/read_zero_byte_vec.rs

+87
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
#![warn(clippy::read_zero_byte_vec)]
2+
#![allow(clippy::unused_io_amount)]
3+
use std::fs::File;
4+
use std::io;
5+
use std::io::prelude::*;
6+
7+
extern crate futures;
8+
use futures::io::{AsyncRead, AsyncReadExt};
9+
use tokio::io::{AsyncRead as TokioAsyncRead, AsyncReadExt as _, AsyncWrite as TokioAsyncWrite, AsyncWriteExt as _};
10+
11+
fn test() -> io::Result<()> {
12+
let cap = 1000;
13+
let mut f = File::open("foo.txt").unwrap();
14+
15+
// should lint
16+
let mut data = Vec::with_capacity(20);
17+
f.read_exact(&mut data).unwrap();
18+
19+
// should lint
20+
let mut data2 = Vec::with_capacity(cap);
21+
f.read_exact(&mut data2)?;
22+
23+
// should lint
24+
let mut data3 = Vec::new();
25+
f.read_exact(&mut data3)?;
26+
27+
// should lint
28+
let mut data4 = vec![];
29+
let _ = f.read(&mut data4)?;
30+
31+
// should lint
32+
let _ = {
33+
let mut data5 = Vec::new();
34+
f.read(&mut data5)
35+
};
36+
37+
// should lint
38+
let _ = {
39+
let mut data6: Vec<u8> = Default::default();
40+
f.read(&mut data6)
41+
};
42+
43+
// should not lint
44+
let mut buf = [0u8; 100];
45+
f.read(&mut buf)?;
46+
47+
// should not lint
48+
let mut empty = vec![];
49+
let mut data7 = vec![];
50+
f.read(&mut empty);
51+
52+
// should not lint
53+
f.read(&mut data7);
54+
55+
// should not lint
56+
let mut data8 = Vec::new();
57+
data8.resize(100, 0);
58+
f.read_exact(&mut data8)?;
59+
60+
// should not lint
61+
let mut data9 = vec![1, 2, 3];
62+
f.read_exact(&mut data9)?;
63+
64+
Ok(())
65+
}
66+
67+
async fn test_futures<R: AsyncRead + Unpin>(r: &mut R) {
68+
// should lint
69+
let mut data = Vec::new();
70+
r.read(&mut data).await.unwrap();
71+
72+
// should lint
73+
let mut data2 = Vec::new();
74+
r.read_exact(&mut data2).await.unwrap();
75+
}
76+
77+
async fn test_tokio<R: TokioAsyncRead + Unpin>(r: &mut R) {
78+
// should lint
79+
let mut data = Vec::new();
80+
r.read(&mut data).await.unwrap();
81+
82+
// should lint
83+
let mut data2 = Vec::new();
84+
r.read_exact(&mut data2).await.unwrap();
85+
}
86+
87+
fn main() {}

tests/ui/read_zero_byte_vec.stderr

+64
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
error: reading zero byte data to `Vec`
2+
--> $DIR/read_zero_byte_vec.rs:17:5
3+
|
4+
LL | f.read_exact(&mut data).unwrap();
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data).unwrap();`
6+
|
7+
= note: `-D clippy::read-zero-byte-vec` implied by `-D warnings`
8+
9+
error: reading zero byte data to `Vec`
10+
--> $DIR/read_zero_byte_vec.rs:21:5
11+
|
12+
LL | f.read_exact(&mut data2)?;
13+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); f.read_exact(&mut data2)?;`
14+
15+
error: reading zero byte data to `Vec`
16+
--> $DIR/read_zero_byte_vec.rs:25:5
17+
|
18+
LL | f.read_exact(&mut data3)?;
19+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
21+
error: reading zero byte data to `Vec`
22+
--> $DIR/read_zero_byte_vec.rs:29:5
23+
|
24+
LL | let _ = f.read(&mut data4)?;
25+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26+
27+
error: reading zero byte data to `Vec`
28+
--> $DIR/read_zero_byte_vec.rs:34:9
29+
|
30+
LL | f.read(&mut data5)
31+
| ^^^^^^^^^^^^^^^^^^
32+
33+
error: reading zero byte data to `Vec`
34+
--> $DIR/read_zero_byte_vec.rs:40:9
35+
|
36+
LL | f.read(&mut data6)
37+
| ^^^^^^^^^^^^^^^^^^
38+
39+
error: reading zero byte data to `Vec`
40+
--> $DIR/read_zero_byte_vec.rs:70:5
41+
|
42+
LL | r.read(&mut data).await.unwrap();
43+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
44+
45+
error: reading zero byte data to `Vec`
46+
--> $DIR/read_zero_byte_vec.rs:74:5
47+
|
48+
LL | r.read_exact(&mut data2).await.unwrap();
49+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
50+
51+
error: reading zero byte data to `Vec`
52+
--> $DIR/read_zero_byte_vec.rs:80:5
53+
|
54+
LL | r.read(&mut data).await.unwrap();
55+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
56+
57+
error: reading zero byte data to `Vec`
58+
--> $DIR/read_zero_byte_vec.rs:84:5
59+
|
60+
LL | r.read_exact(&mut data2).await.unwrap();
61+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
62+
63+
error: aborting due to 10 previous errors
64+

0 commit comments

Comments
 (0)