nordic-dev.net/rust
nordic-dev.net/rust
2
0
Fork 0
mirror of https://github.com/rust-lang/rust.git synced 2025-01-31 09:04:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Extend [unused_io_amount] to cover AsyncRead and AsyncWrite.

Browse Source 
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.

changelog: [`unused_io_amount`] now supports AsyncReadExt and AsyncWriteExt.
This commit is contained in:
Nick Mathewson 2021-12-27 11:17:17 -05:00
parent 0eff589afc
commit 65d1f83d2c
6 changed files with 181 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
Cargo.toml
View File

@ -47,7 +47,9 @@ itertools = "0.10"
quote = "1.0"
serde = { version = "1.0", features = ["derive"] }
syn = { version = "1.0", features = ["full"] }
futures = "0.3"
parking_lot = "0.11.2"
tokio = { version = "1", features = ["io-util"] }
[build-dependencies]
rustc_tools_util = { version = "0.2", path = "rustc_tools_util" }

75
clippy_lints/src/unused_io_amount.rs
View File

@ -17,10 +17,17 @@ declare_clippy_lint! {
/// partial-write/read, use
/// `write_all`/`read_exact` instead.
///
/// When working with asynchronous code (either with the `futures`
/// crate or with `tokio`), a similar issue exists for
/// `AsyncWriteExt::write()` and `AsyncReadExt::read()` : these
/// functions are also not guaranteed to process the entire
/// buffer. Your code should either handle partial-writes/reads, or
/// call the `write_all`/`read_exact` methods on those traits instead.
///
/// ### Known problems
/// Detects only common patterns.
///
/// ### Example
/// ### Examples
/// ```rust,ignore
/// use std::io;
/// fn foo<W: io::Write>(w: &mut W) -> io::Result<()> {
@ -68,6 +75,23 @@ impl<'tcx> LateLintPass<'tcx> for UnusedIoAmount {
}
}
/// If `expr` is an (e).await, return the inner expression "e" that's being
/// waited on. Otherwise return None.
fn try_remove_await<'a>(expr: &'a hir::Expr<'a>) -> Option<&hir::Expr<'a>> {
if let hir::ExprKind::Match(expr, _, hir::MatchSource::AwaitDesugar) = expr.kind {
if let hir::ExprKind::Call(func, [ref arg_0, ..]) = expr.kind {
if matches!(
func.kind,
hir::ExprKind::Path(hir::QPath::LangItem(hir::LangItem::IntoFutureIntoFuture, ..))
) {
return Some(arg_0);
}
}
}
None
}
fn check_map_error(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>) {
let mut call = call;
while let hir::ExprKind::MethodCall(path, _, args, _) = call.kind {
@ -77,30 +101,61 @@ fn check_map_error(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<
break;
}
}
check_method_call(cx, call, expr);
if let Some(call) = try_remove_await(call) {
check_method_call(cx, call, expr, true);
} else {
check_method_call(cx, call, expr, false);
}
}
fn check_method_call(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>) {
fn check_method_call(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>, is_await: bool) {
if let hir::ExprKind::MethodCall(path, _, _, _) = call.kind {
let symbol = path.ident.as_str();
let read_trait = match_trait_method(cx, call, &paths::IO_READ);
let write_trait = match_trait_method(cx, call, &paths::IO_WRITE);
let read_trait = if is_await {
match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCREADEXT)
|| match_trait_method(cx, call, &paths::TOKIO_IO_ASYNCREADEXT)
} else {
match_trait_method(cx, call, &paths::IO_READ)
};
let write_trait = if is_await {
match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCWRITEEXT)
|| match_trait_method(cx, call, &paths::TOKIO_IO_ASYNCWRITEEXT)
} else {
match_trait_method(cx, call, &paths::IO_WRITE)
};
match (read_trait, write_trait, symbol) {
(true, _, "read") => span_lint(
match (read_trait, write_trait, symbol, is_await) {
(true, _, "read", false) => span_lint(
cx,
UNUSED_IO_AMOUNT,
expr.span,
"read amount is not handled. Use `Read::read_exact` instead",
),
(true, _, "read_vectored") => span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "read amount is not handled"),
(_, true, "write") => span_lint(
(true, _, "read", true) => span_lint(
cx,
UNUSED_IO_AMOUNT,
expr.span,
"read amount is not handled. Use `AsyncReadExt::read_exact` instead",
),
(true, _, "read_vectored", _) => {
span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "read amount is not handled");
},
(_, true, "write", false) => span_lint(
cx,
UNUSED_IO_AMOUNT,
expr.span,
"written amount is not handled. Use `Write::write_all` instead",
),
(_, true, "write_vectored") => span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "written amount is not handled"),
(_, true, "write", true) => span_lint(
cx,
UNUSED_IO_AMOUNT,
expr.span,
"written amount is not handled. Use `AsyncWriteExt::write_all` instead",
),
(_, true, "write_vectored", _) => {
span_lint(cx, UNUSED_IO_AMOUNT, expr.span, "written amount is not handled");
},
_ => (),
}
}

8
clippy_utils/src/paths.rs
View File

@ -64,6 +64,10 @@ pub const FROM_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "From
pub const FROM_ITERATOR_METHOD: [&str; 6] = ["core", "iter", "traits", "collect", "FromIterator", "from_iter"];
pub const FROM_STR_METHOD: [&str; 5] = ["core", "str", "traits", "FromStr", "from_str"];
pub const FUTURE_FROM_GENERATOR: [&str; 3] = ["core", "future", "from_generator"];
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
pub const FUTURES_IO_ASYNCREADEXT: [&str; 3] = ["futures_util", "io", "AsyncReadExt"];
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
pub const FUTURES_IO_ASYNCWRITEEXT: [&str; 3] = ["futures_util", "io", "AsyncWriteExt"];
pub const HASH: [&str; 3] = ["core", "hash", "Hash"];
pub const HASHMAP_CONTAINS_KEY: [&str; 6] = ["std", "collections", "hash", "map", "HashMap", "contains_key"];
pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entry"];
@ -194,6 +198,10 @@ pub const SYM_MODULE: [&str; 3] = ["rustc_span", "symbol", "sym"];
pub const SYNTAX_CONTEXT: [&str; 3] = ["rustc_span", "hygiene", "SyntaxContext"];
pub const TO_OWNED_METHOD: [&str; 4] = ["alloc", "borrow", "ToOwned", "to_owned"];
pub const TO_STRING_METHOD: [&str; 4] = ["alloc", "string", "ToString", "to_string"];
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
pub const TOKIO_IO_ASYNCREADEXT: [&str; 5] = ["tokio", "io", "util", "async_read_ext", "AsyncReadExt"];
#[allow(clippy::invalid_paths)] // internal lints do not know about all external crates
pub const TOKIO_IO_ASYNCWRITEEXT: [&str; 5] = ["tokio", "io", "util", "async_write_ext", "AsyncWriteExt"];
pub const TRY_FROM: [&str; 4] = ["core", "convert", "TryFrom", "try_from"];
pub const VEC_AS_MUT_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_mut_slice"];
pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];

6
tests/compile-test.rs
View File

@ -21,6 +21,7 @@ const RUN_INTERNAL_TESTS: bool = cfg!(feature = "internal-lints");
static TEST_DEPENDENCIES: &[&str] = &[
"clippy_utils",
"derive_new",
"futures",
"if_chain",
"itertools",
"quote",
@ -28,6 +29,7 @@ static TEST_DEPENDENCIES: &[&str] = &[
"serde",
"serde_derive",
"syn",
"tokio",
"parking_lot",
];
@ -38,6 +40,8 @@ extern crate clippy_utils;
#[allow(unused_extern_crates)]
extern crate derive_new;
#[allow(unused_extern_crates)]
extern crate futures;
#[allow(unused_extern_crates)]
extern crate if_chain;
#[allow(unused_extern_crates)]
extern crate itertools;
@ -47,6 +51,8 @@ extern crate parking_lot;
extern crate quote;
#[allow(unused_extern_crates)]
extern crate syn;
#[allow(unused_extern_crates)]
extern crate tokio;
/// Produces a string with an `--extern` flag for all UI test crate
/// dependencies.

53
tests/ui/unused_io_amount.rs
View File

@ -1,6 +1,8 @@
#![allow(dead_code)]
#![warn(clippy::unused_io_amount)]
extern crate futures;
use futures::io::{AsyncRead, AsyncReadExt, AsyncWrite, AsyncWriteExt};
use std::io::{self, Read};
fn question_mark<T: io::Read + io::Write>(s: &mut T) -> io::Result<()> {
@ -61,4 +63,55 @@ fn combine_or(file: &str) -> Result<(), Error> {
Ok(())
}
async fn bad_async_write<W: AsyncWrite + Unpin>(w: &mut W) {
w.write(b"hello world").await.unwrap();
}
async fn bad_async_read<R: AsyncRead + Unpin>(r: &mut R) {
let mut buf = [0u8; 0];
r.read(&mut buf[..]).await.unwrap();
}
async fn io_not_ignored_async_write<W: AsyncWrite + Unpin>(mut w: W) {
// Here we're forgetting to await the future, so we should get a
// warning about _that_ (or we would, if it were enabled), but we
// won't get one about ignoring the return value.
w.write(b"hello world");
}
fn bad_async_write_closure<W: AsyncWrite + Unpin + 'static>(w: W) -> impl futures::Future<Output = io::Result<()>> {
let mut w = w;
async move {
w.write(b"hello world").await?;
Ok(())
}
}
async fn async_read_nested_or<R: AsyncRead + Unpin>(r: &mut R, do_it: bool) -> Result<[u8; 1], Error> {
let mut buf = [0u8; 1];
if do_it {
r.read(&mut buf[..]).await.or(Err(Error::Kind))?;
}
Ok(buf)
}
use tokio::io::{AsyncRead as TokioAsyncRead, AsyncReadExt as _, AsyncWrite as TokioAsyncWrite, AsyncWriteExt as _};
async fn bad_async_write_tokio<W: TokioAsyncWrite + Unpin>(w: &mut W) {
w.write(b"hello world").await.unwrap();
}
async fn bad_async_read_tokio<R: TokioAsyncRead + Unpin>(r: &mut R) {
let mut buf = [0u8; 0];
r.read(&mut buf[..]).await.unwrap();
}
async fn undetected_bad_async_write<W: AsyncWrite + Unpin>(w: &mut W) {
// It would be good to detect this case some day, but the current lint
// doesn't handle it. (The documentation says that this lint "detects
// only common patterns".)
let future = w.write(b"Hello world");
future.await.unwrap();
}
fn main() {}

58
tests/ui/unused_io_amount.stderr
View File

@ -1,5 +1,5 @@
error: written amount is not handled. Use `Write::write_all` instead
--> $DIR/unused_io_amount.rs:7:5
--> $DIR/unused_io_amount.rs:9:5
|
LL | s.write(b"test")?;
| ^^^^^^^^^^^^^^^^^
@ -7,55 +7,55 @@ LL | s.write(b"test")?;
= note: `-D clippy::unused-io-amount` implied by `-D warnings`
error: read amount is not handled. Use `Read::read_exact` instead
--> $DIR/unused_io_amount.rs:9:5
--> $DIR/unused_io_amount.rs:11:5
|
LL | s.read(&mut buf)?;
| ^^^^^^^^^^^^^^^^^
error: written amount is not handled. Use `Write::write_all` instead
--> $DIR/unused_io_amount.rs:14:5
--> $DIR/unused_io_amount.rs:16:5
|
LL | s.write(b"test").unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `Read::read_exact` instead
--> $DIR/unused_io_amount.rs:16:5
--> $DIR/unused_io_amount.rs:18:5
|
LL | s.read(&mut buf).unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled
--> $DIR/unused_io_amount.rs:20:5
--> $DIR/unused_io_amount.rs:22:5
|
LL | s.read_vectored(&mut [io::IoSliceMut::new(&mut [])])?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: written amount is not handled
--> $DIR/unused_io_amount.rs:21:5
--> $DIR/unused_io_amount.rs:23:5
|
LL | s.write_vectored(&[io::IoSlice::new(&[])])?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `Read::read_exact` instead
--> $DIR/unused_io_amount.rs:28:5
--> $DIR/unused_io_amount.rs:30:5
|
LL | reader.read(&mut result).ok()?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `Read::read_exact` instead
--> $DIR/unused_io_amount.rs:37:5
--> $DIR/unused_io_amount.rs:39:5
|
LL | reader.read(&mut result).or_else(|err| Err(err))?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `Read::read_exact` instead
--> $DIR/unused_io_amount.rs:49:5
--> $DIR/unused_io_amount.rs:51:5
|
LL | reader.read(&mut result).or(Err(Error::Kind))?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `Read::read_exact` instead
--> $DIR/unused_io_amount.rs:56:5
--> $DIR/unused_io_amount.rs:58:5
|
LL | / reader
LL | | .read(&mut result)
@ -64,5 +64,41 @@ LL | | .or(Err(Error::Kind))
LL | | .expect("error");
| |________________________^
error: aborting due to 10 previous errors
error: written amount is not handled. Use `AsyncWriteExt::write_all` instead
--> $DIR/unused_io_amount.rs:67:5
|
LL | w.write(b"hello world").await.unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `AsyncReadExt::read_exact` instead
--> $DIR/unused_io_amount.rs:72:5
|
LL | r.read(&mut buf[..]).await.unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: written amount is not handled. Use `AsyncWriteExt::write_all` instead
--> $DIR/unused_io_amount.rs:85:9
|
LL | w.write(b"hello world").await?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `AsyncReadExt::read_exact` instead
--> $DIR/unused_io_amount.rs:93:9
|
LL | r.read(&mut buf[..]).await.or(Err(Error::Kind))?;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: written amount is not handled. Use `AsyncWriteExt::write_all` instead
--> $DIR/unused_io_amount.rs:101:5
|
LL | w.write(b"hello world").await.unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: read amount is not handled. Use `AsyncReadExt::read_exact` instead
--> $DIR/unused_io_amount.rs:106:5
|
LL | r.read(&mut buf[..]).await.unwrap();
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 16 previous errors