Skip to content

Commit 26fa413

Browse files
nmathewsonnmathewson
committed
Extend [unused_io_amount] to cover AsyncRead and AsyncWrite.
Clippy helpfully warns about code like this, telling you that you probably meant "write_all": fn say_hi<W:Write>(w: &mut W) { w.write(b"hello").unwrap(); } This patch attempts to extend the lint so it also covers this case: async fn say_hi<W:AsyncWrite>(w: &mut W) { w.write(b"hello").await.unwrap(); } (I've run into this second case several times in my own programming, and so have my coworkers, so unless we're especially accident-prone in this area, it's probably worth addressing?) This patch covers the Async{Read,Write}Ext traits in futures-rs, and in tokio, since both are quite widely used.
1 parent 0eff589 commit 26fa413

File tree

6 files changed

+130
-20
lines changed

6 files changed

+130
-20
lines changed

‎Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ itertools = "0.10"
4747
quote = "1.0"
4848
serde = { version = "1.0", features = ["derive"] }
4949
syn = { version = "1.0", features = ["full"] }
50+
futures = "0.3"
5051
parking_lot = "0.11.2"
5152

5253
[build-dependencies]

‎clippy_lints/src/unused_io_amount.rs‎

Lines changed: 53 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,23 @@ impl<'tcx> LateLintPass<'tcx> for UnusedIoAmount {
6868
}
6969
}
7070

71+
/// If `expr` is an (e).await, return the inner expression "e" that's being
72+
/// waited on. Otherwise return None.
73+
fn try_remove_await<'a>(expr: &'a hir::Expr<'a>) -> Option<&hir::Expr<'a>> {
74+
if let hir::ExprKind::Match(expr, _, hir::MatchSource::AwaitDesugar) = expr.kind {
75+
if let hir::ExprKind::Call(func, [ref arg_0, ..]) = expr.kind {
76+
if matches!(
77+
func.kind,
78+
hir::ExprKind::Path(hir::QPath::LangItem(hir::LangItem::IntoFutureIntoFuture, ..))
79+
) {
80+
return Some(arg_0);
81+
}
82+
}
83+
}
84+
85+
None
86+
}
87+
7188
fn check_map_error(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>) {
7289
let mut call = call;
7390
while let hir::ExprKind::MethodCall(path, _, args, _) = call.kind {
@@ -77,30 +94,57 @@ fn check_map_error(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<
7794
break;
7895
}
7996
}
80-
check_method_call(cx, call, expr);
97+
98+
if let Some(call) = try_remove_await(call) {
99+
check_method_call(cx, call, expr, true);
100+
} else {
101+
check_method_call(cx, call, expr, false);
102+
}
81103
}
82104

83-
fn check_method_call(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>) {
105+
fn check_method_call(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>, is_await: bool) {
84106
if let hir::ExprKind::MethodCall(path, _, _, _) = call.kind {
85107
let symbol = path.ident.as_str();
86-
let read_trait = match_trait_method(cx, call, &paths::IO_READ);
87-
let write_trait = match_trait_method(cx, call, &paths::IO_WRITE);
108+
let read_trait = if is_await {
109+
match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCREADEXT)
110+
} else {
111+
match_trait_method(cx, call, &paths::IO_READ)
112+
};
113+
let write_trait = if is_await {
114+
match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCWRITEEXT)
115+
} else {
116+
match_trait_method(cx, call, &paths::IO_WRITE)
117+
};
88118

89-
match (read_trait, write_trait, symbol) {
90-
(true, _, "read") => span_lint(
119+
match (read_trait, write_trait, symbol, is_await) {
120+
(true, _, "read", false) => span_lint(
91121
cx,
92122
UNUSED_IO_AMOUNT,
93123
expr.span,
94124
"read amount is not handled. Use `Read::read_exact` instead",
95125
),
96-
(true, _, "read_vectored") => span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "read amount is not handled"),
97-
(_, true, "write") => span_lint(
126+
(true, _, "read", true) => span_lint(
127+
cx,
128+
UNUSED_IO_AMOUNT,
129+
expr.span,
130+
"read amount is not handled. Use `AsyncReadExt::read_exact` instead",
131+
),
132+
(true, _, "read_vectored", _) => span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "read amount is not handled"),
133+
(_, true, "write", false) => span_lint(
98134
cx,
99135
UNUSED_IO_AMOUNT,
100136
expr.span,
101137
"written amount is not handled. Use `Write::write_all` instead",
102138
),
103-
(_, true, "write_vectored") => span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "written amount is not handled"),
139+
(_, true, "write", true) => span_lint(
140+
cx,
141+
UNUSED_IO_AMOUNT,
142+
expr.span,
143+
"written amount is not handled. Use `AsyncWriteExt::write_all` instead",
144+
),
145+
(_, true, "write_vectored", _) => {
146+
span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "written amount is not handled")
147+
},
104148
_ => (),
105149
}
106150
}

‎clippy_utils/src/paths.rs‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ pub const FROM_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "From
6464
pub const FROM_ITERATOR_METHOD: [&str; 6] = ["core", "iter", "traits", "collect", "FromIterator", "from_iter"];
6565
pub const FROM_STR_METHOD: [&str; 5] = ["core", "str", "traits", "FromStr", "from_str"];
6666
pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"];
67+
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
68+
pub const FUTURES_IO_ASYNCREADEXT: [&str; 3] = ["futures_util", "io", "AsyncReadExt"];
69+
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
70+
pub const FUTURES_IO_ASYNCWRITEEXT: [&str; 3] = ["futures_util", "io", "AsyncWriteExt"];
6771
pub const HASH: [&str; 3] = ["core", "hash", "Hash"];
6872
pub const HASHMAP_CONTAINS_KEY: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "contains_key"];
6973
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];

‎tests/compile-test.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ const RUN_INTERNAL_TESTS: bool = cfg!(feature = "internal-lints");
2121
static TEST_DEPENDENCIES: &[&str] = &[
2222
"clippy_utils",
2323
"derive_new",
24+
"futures",
2425
"if_chain",
2526
"itertools",
2627
"quote",
@@ -38,6 +39,8 @@ extern crate clippy_utils;
3839
#[allow(unused_extern_crates)]
3940
extern crate derive_new;
4041
#[allow(unused_extern_crates)]
42+
extern crate futures;
43+
#[allow(unused_extern_crates)]
4144
extern crate if_chain;
4245
#[allow(unused_extern_crates)]
4346
extern crate itertools;

‎tests/ui/unused_io_amount.rs‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#![allow(dead_code)]
22
#![warn(clippy::unused_io_amount)]
33

4+
extern crate futures;
5+
use futures::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
46
use std::io::{self, Read};
57

68
fn question_mark<T: io::Read + io::Write>(s: &mut T) -> io::Result<()> {
@@ -61,4 +63,36 @@ fn combine_or(file: &str) -> Result<(), Error> {
6163
Ok(())
6264
}
6365

66+
async fn bad_async_write<W: AsyncWrite + Unpin>(w: &mut W) {
67+
w.write(b"hello world").await.unwrap();
68+
}
69+
70+
async fn bad_async_read<R: AsyncRead + Unpin>(r: &mut R) {
71+
let mut buf = [0u8; 0];
72+
r.read(&mut buf[..]).await.unwrap();
73+
}
74+
75+
async fn io_not_ignored_async_write<W: AsyncWrite + Unpin>(mut w: W) {
76+
// Here we're forgetting to await the future, so we should get a
77+
// warning about _that_ (or we would, if it were enabled), but we
78+
// won't get one about ignoring the return value.
79+
w.write(b"hello world");
80+
}
81+
82+
fn bad_async_write_closure<W: AsyncWrite + Unpin + 'static>(w: W) -> impl futures::Future<Output = io::Result<()>> {
83+
let mut w = w;
84+
async move {
85+
w.write(b"hello world").await?;
86+
Ok(())
87+
}
88+
}
89+
90+
async fn async_read_nested_or<R: AsyncRead + Unpin>(r: &mut R, do_it: bool) -> Result<[u8; 1], Error> {
91+
let mut buf = [0u8; 1];
92+
if do_it {
93+
r.read(&mut buf[..]).await.or(Err(Error::Kind))?;
94+
}
95+
Ok(buf)
96+
}
97+
6498
fn main() {}

‎tests/ui/unused_io_amount.stderr‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,61 +1,61 @@
11
error: written amount is not handled. Use `Write::write_all` instead
2-
--> $DIR/unused_io_amount.rs:7:5
2+
--> $DIR/unused_io_amount.rs:9:5
33
|
44
LL | s.write(b"test")?;
55
| ^^^^^^^^^^^^^^^^^
66
|
77
= note: `-D clippy::unused-io-amount` implied by `-D warnings`
88

99
error: read amount is not handled. Use `Read::read_exact` instead
10-
--> $DIR/unused_io_amount.rs:9:5
10+
--> $DIR/unused_io_amount.rs:11:5
1111
|
1212
LL | s.read(&mut buf)?;
1313
| ^^^^^^^^^^^^^^^^^
1414

1515
error: written amount is not handled. Use `Write::write_all` instead
16-
--> $DIR/unused_io_amount.rs:14:5
16+
--> $DIR/unused_io_amount.rs:16:5
1717
|
1818
LL | s.write(b"test").unwrap();
1919
| ^^^^^^^^^^^^^^^^^^^^^^^^^
2020

2121
error: read amount is not handled. Use `Read::read_exact` instead
22-
--> $DIR/unused_io_amount.rs:16:5
22+
--> $DIR/unused_io_amount.rs:18:5
2323
|
2424
LL | s.read(&mut buf).unwrap();
2525
| ^^^^^^^^^^^^^^^^^^^^^^^^^
2626

2727
error: read amount is not handled
28-
--> $DIR/unused_io_amount.rs:20:5
28+
--> $DIR/unused_io_amount.rs:22:5
2929
|
3030
LL | s.read_vectored(&mut [io::IoSliceMut::new(&mut [])])?;
3131
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3232

3333
error: written amount is not handled
34-
--> $DIR/unused_io_amount.rs:21:5
34+
--> $DIR/unused_io_amount.rs:23:5
3535
|
3636
LL | s.write_vectored(&[io::IoSlice::new(&[])])?;
3737
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3838

3939
error: read amount is not handled. Use `Read::read_exact` instead
40-
--> $DIR/unused_io_amount.rs:28:5
40+
--> $DIR/unused_io_amount.rs:30:5
4141
|
4242
LL | reader.read(&mut result).ok()?;
4343
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4444

4545
error: read amount is not handled. Use `Read::read_exact` instead
46-
--> $DIR/unused_io_amount.rs:37:5
46+
--> $DIR/unused_io_amount.rs:39:5
4747
|
4848
LL | reader.read(&mut result).or_else(|err| Err(err))?;
4949
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5050

5151
error: read amount is not handled. Use `Read::read_exact` instead
52-
--> $DIR/unused_io_amount.rs:49:5
52+
--> $DIR/unused_io_amount.rs:51:5
5353
|
5454
LL | reader.read(&mut result).or(Err(Error::Kind))?;
5555
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5656

5757
error: read amount is not handled. Use `Read::read_exact` instead
58-
--> $DIR/unused_io_amount.rs:56:5
58+
--> $DIR/unused_io_amount.rs:58:5
5959
|
6060
LL | / reader
6161
LL | | .read(&mut result)
@@ -64,5 +64,29 @@ LL | | .or(Err(Error::Kind))
6464
LL | | .expect("error");
6565
| |________________________^
6666

67-
error: aborting due to 10 previous errors
67+
error: written amount is not handled. Use `AsyncWriteExt::write_all` instead
68+
--> $DIR/unused_io_amount.rs:67:5
69+
|
70+
LL | w.write(b"hello world").await.unwrap();
71+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
72+
73+
error: read amount is not handled. Use `AsyncReadExt::read_exact` instead
74+
--> $DIR/unused_io_amount.rs:72:5
75+
|
76+
LL | r.read(&mut buf[..]).await.unwrap();
77+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
78+
79+
error: written amount is not handled. Use `AsyncWriteExt::write_all` instead
80+
--> $DIR/unused_io_amount.rs:86:9
81+
|
82+
LL | w.write(b"hello world").await?;
83+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
84+
85+
error: read amount is not handled. Use `AsyncReadExt::read_exact` instead
86+
--> $DIR/unused_io_amount.rs:94:9
87+
|
88+
LL | r.read(&mut buf[..]).await.or(Err(Error::Kind))?;
89+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
90+
91+
error: aborting due to 14 previous errors
6892

0 commit comments

Comments
 (0)