Skip to content

Commit e27ebf2

Browse files
borsbors
committed
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
2 parents 5f3a060 + 8c79f78 commit e27ebf2

File tree

3 files changed

+112
-69
lines changed

3 files changed

+112
-69
lines changed

‎clippy_lints/src/read_zero_byte_vec.rs‎

Lines changed: 71 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
2+
use clippy_utils::get_enclosing_block;
23
use clippy_utils::higher::{get_vec_init_kind, VecInitKind};
34
use clippy_utils::source::snippet;
4-
use clippy_utils::visitors::for_each_expr;
5-
use core::ops::ControlFlow;
6-
use hir::{Expr, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind};
5+
6+
use hir::{Expr, ExprKind, HirId, Local, PatKind, PathSegment, QPath, StmtKind};
77
use rustc_errors::Applicability;
88
use rustc_hir as hir;
9+
use rustc_hir::def::Res;
10+
use rustc_hir::intravisit::{walk_expr, Visitor};
911
use rustc_lint::{LateContext, LateLintPass};
1012
use rustc_session::declare_lint_pass;
1113

@@ -49,57 +51,40 @@ declare_lint_pass!(ReadZeroByteVec => [READ_ZERO_BYTE_VEC]);
4951

5052
impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec {
5153
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &hir::Block<'tcx>) {
52-
for (idx, stmt) in block.stmts.iter().enumerate() {
53-
if !stmt.span.from_expansion()
54-
// matches `let v = Vec::new();`
55-
&& let StmtKind::Local(local) = stmt.kind
56-
&& let Local { pat, init: Some(init), .. } = local
57-
&& let PatKind::Binding(_, _, ident, _) = pat.kind
54+
for stmt in block.stmts {
55+
if stmt.span.from_expansion() {
56+
return;
57+
}
58+
59+
if let StmtKind::Local(local) = stmt.kind
60+
&& let Local {
61+
pat, init: Some(init), ..
62+
} = local
63+
&& let PatKind::Binding(_, id, ident, _) = pat.kind
5864
&& let Some(vec_init_kind) = get_vec_init_kind(cx, init)
5965
{
60-
let visitor = |expr: &Expr<'_>| {
61-
if let ExprKind::MethodCall(path, _, [arg], _) = expr.kind
62-
&& let PathSegment {
63-
ident: read_or_read_exact,
64-
..
65-
} = *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-
ControlFlow::Break(())
73-
} else {
74-
ControlFlow::Continue(())
75-
}
66+
let mut visitor = ReadVecVisitor {
67+
local_id: id,
68+
read_zero_expr: None,
69+
has_resize: false,
7670
};
7771

78-
let (read_found, next_stmt_span) = if let Some(next_stmt) = block.stmts.get(idx + 1) {
79-
// case { .. stmt; stmt; .. }
80-
(for_each_expr(next_stmt, visitor).is_some(), next_stmt.span)
81-
} else if let Some(e) = block.expr {
82-
// case { .. stmt; expr }
83-
(for_each_expr(e, visitor).is_some(), e.span)
84-
} else {
72+
let Some(enclosing_block) = get_enclosing_block(cx, id) else {
8573
return;
8674
};
75+
visitor.visit_block(enclosing_block);
8776

88-
if read_found && !next_stmt_span.from_expansion() {
77+
if let Some(expr) = visitor.read_zero_expr {
8978
let applicability = Applicability::MaybeIncorrect;
9079
match vec_init_kind {
9180
VecInitKind::WithConstCapacity(len) => {
9281
span_lint_and_sugg(
9382
cx,
9483
READ_ZERO_BYTE_VEC,
95-
next_stmt_span,
84+
expr.span,
9685
"reading zero byte data to `Vec`",
9786
"try",
98-
format!(
99-
"{}.resize({len}, 0); {}",
100-
ident.as_str(),
101-
snippet(cx, next_stmt_span, "..")
102-
),
87+
format!("{}.resize({len}, 0); {}", ident.as_str(), snippet(cx, expr.span, "..")),
10388
applicability,
10489
);
10590
},
@@ -108,29 +93,68 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec {
10893
span_lint_and_sugg(
10994
cx,
11095
READ_ZERO_BYTE_VEC,
111-
next_stmt_span,
96+
expr.span,
11297
"reading zero byte data to `Vec`",
11398
"try",
11499
format!(
115100
"{}.resize({}, 0); {}",
116101
ident.as_str(),
117102
snippet(cx, e.span, ".."),
118-
snippet(cx, next_stmt_span, "..")
103+
snippet(cx, expr.span, "..")
119104
),
120105
applicability,
121106
);
122107
},
123108
_ => {
124-
span_lint(
125-
cx,
126-
READ_ZERO_BYTE_VEC,
127-
next_stmt_span,
128-
"reading zero byte data to `Vec`",
129-
);
109+
span_lint(cx, READ_ZERO_BYTE_VEC, expr.span, "reading zero byte data to `Vec`");
130110
},
131111
}
132112
}
133113
}
134114
}
135115
}
136116
}
117+
118+
struct ReadVecVisitor<'tcx> {
119+
local_id: HirId,
120+
read_zero_expr: Option<&'tcx Expr<'tcx>>,
121+
has_resize: bool,
122+
}
123+
124+
impl<'tcx> Visitor<'tcx> for ReadVecVisitor<'tcx> {
125+
fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) {
126+
if let ExprKind::MethodCall(path, receiver, args, _) = e.kind {
127+
let PathSegment { ident, .. } = *path;
128+
129+
match ident.as_str() {
130+
"read" | "read_exact" => {
131+
let [arg] = args else { return };
132+
if let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind
133+
&& let ExprKind::Path(QPath::Resolved(None, inner_path)) = inner.kind
134+
&& let [inner_seg] = inner_path.segments
135+
&& let Res::Local(res_id) = inner_seg.res
136+
&& self.local_id == res_id
137+
{
138+
self.read_zero_expr = Some(e);
139+
return;
140+
}
141+
},
142+
"resize" => {
143+
// If the Vec is resized, then it's a valid read
144+
if let ExprKind::Path(QPath::Resolved(_, inner_path)) = receiver.kind
145+
&& let Res::Local(res_id) = inner_path.res
146+
&& self.local_id == res_id
147+
{
148+
self.has_resize = true;
149+
return;
150+
}
151+
},
152+
_ => {},
153+
}
154+
}
155+
156+
if !self.has_resize && self.read_zero_expr.is_none() {
157+
walk_expr(self, e);
158+
}
159+
}
160+
}

‎tests/ui/read_zero_byte_vec.rs‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -55,14 +55,6 @@ fn test() -> io::Result<()> {
5555
let mut buf = [0u8; 100];
5656
f.read(&mut buf)?;
5757

58-
// should not lint
59-
let mut empty = vec![];
60-
let mut data7 = vec![];
61-
f.read(&mut empty);
62-
63-
// should not lint
64-
f.read(&mut data7);
65-
6658
// should not lint
6759
let mut data8 = Vec::new();
6860
data8.resize(100, 0);
@@ -75,6 +67,27 @@ fn test() -> io::Result<()> {
7567
Ok(())
7668
}
7769

70+
fn test_nested() -> io::Result<()> {
71+
let cap = 1000;
72+
let mut f = File::open("foo.txt").unwrap();
73+
74+
// Issue #9274
75+
// Should not lint
76+
let mut v = Vec::new();
77+
{
78+
v.resize(10, 0);
79+
f.read(&mut v)?;
80+
}
81+
82+
let mut v = Vec::new();
83+
{
84+
f.read(&mut v)?;
85+
//~^ ERROR: reading zero byte data to `Vec`
86+
}
87+
88+
Ok(())
89+
}
90+
7891
async fn test_futures<R: AsyncRead + Unpin>(r: &mut R) {
7992
// should lint
8093
let mut data = Vec::new();

‎tests/ui/read_zero_byte_vec.stderr‎

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: reading zero byte data to `Vec`
22
--> $DIR/read_zero_byte_vec.rs:21:5
33
|
44
LL | f.read_exact(&mut data).unwrap();
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data).unwrap();`
5+
| ^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data.resize(20, 0); f.read_exact(&mut data)`
66
|
77
= note: `-D clippy::read-zero-byte-vec` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::read_zero_byte_vec)]`
@@ -11,19 +11,19 @@ error: reading zero byte data to `Vec`
1111
--> $DIR/read_zero_byte_vec.rs:27:5
1212
|
1313
LL | f.read_exact(&mut data2)?;
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); f.read_exact(&mut data2)?;`
14+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `data2.resize(cap, 0); f.read_exact(&mut data2)`
1515

1616
error: reading zero byte data to `Vec`
1717
--> $DIR/read_zero_byte_vec.rs:32:5
1818
|
1919
LL | f.read_exact(&mut data3)?;
20-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
20+
| ^^^^^^^^^^^^^^^^^^^^^^^^
2121

2222
error: reading zero byte data to `Vec`
23-
--> $DIR/read_zero_byte_vec.rs:37:5
23+
--> $DIR/read_zero_byte_vec.rs:37:13
2424
|
2525
LL | let _ = f.read(&mut data4)?;
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
26+
| ^^^^^^^^^^^^^^^^^^
2727

2828
error: reading zero byte data to `Vec`
2929
--> $DIR/read_zero_byte_vec.rs:43:9
@@ -38,28 +38,34 @@ LL | f.read(&mut data6)
3838
| ^^^^^^^^^^^^^^^^^^
3939

4040
error: reading zero byte data to `Vec`
41-
--> $DIR/read_zero_byte_vec.rs:81:5
41+
--> $DIR/read_zero_byte_vec.rs:84:9
42+
|
43+
LL | f.read(&mut v)?;
44+
| ^^^^^^^^^^^^^^
45+
46+
error: reading zero byte data to `Vec`
47+
--> $DIR/read_zero_byte_vec.rs:94:5
4248
|
4349
LL | r.read(&mut data).await.unwrap();
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
50+
| ^^^^^^^^^^^^^^^^^
4551

4652
error: reading zero byte data to `Vec`
47-
--> $DIR/read_zero_byte_vec.rs:86:5
53+
--> $DIR/read_zero_byte_vec.rs:99:5
4854
|
4955
LL | r.read_exact(&mut data2).await.unwrap();
50-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
56+
| ^^^^^^^^^^^^^^^^^^^^^^^^
5157

5258
error: reading zero byte data to `Vec`
53-
--> $DIR/read_zero_byte_vec.rs:93:5
59+
--> $DIR/read_zero_byte_vec.rs:106:5
5460
|
5561
LL | r.read(&mut data).await.unwrap();
56-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
62+
| ^^^^^^^^^^^^^^^^^
5763

5864
error: reading zero byte data to `Vec`
59-
--> $DIR/read_zero_byte_vec.rs:98:5
65+
--> $DIR/read_zero_byte_vec.rs:111:5
6066
|
6167
LL | r.read_exact(&mut data2).await.unwrap();
62-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^
6369

64-
error: aborting due to 10 previous errors
70+
error: aborting due to 11 previous errors
6571

0 commit comments

Comments
 (0)