RFC: extend unused_io_amount to cover async io.

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?)

Since this is my first attempt at a clippy patch, I've probably
made all kinds of mistakes: please help me fix them?  I'd like
to learn more here.

Open questions I have:

  * Should this be a separate lint from unused_io_amount?  Maybe
    unused_async_io_amount?  If so, how should I structure their
    shared code?
  * Should this cover tokio's AsyncWrite too?
  * Is it okay to write lints for stuff that isn't part of
    the standard library?  I see that "regex" also has lints,
    and I figure that "futures" is probably okay too, since it's
    an official rust-lang repository.
  * What other tests are needed?
  * How should I improve the code?

Thanks for your time!

Author: nmathewson

Cargo.toml

@@ -47,6 +47,7 @@ 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"
 
 [build-dependencies]

clippy_lints/src/unused_io_amount.rs

@@ -70,11 +70,27 @@ impl<'tcx> LateLintPass<'tcx> for UnusedIoAmount {
 
 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 {
-        if matches!(&*path.ident.as_str(), "or" | "or_else" | "ok") {
-            call = &args[0];
-        } else {
-            break;
+    loop {
+        match call.kind {
+            hir::ExprKind::MethodCall(path, _, args, _) => {
+                if matches!(&*path.ident.as_str(), "or" | "or_else" | "ok") {
+                    call = &args[0];
+                } else {
+                    break;
+                }
+            },
+            hir::ExprKind::Match(expr, _, hir::MatchSource::AwaitDesugar) => {
+                if let hir::ExprKind::Call(func, [ref arg_0, ..]) = expr.kind {
+                    if matches!(
+                        func.kind,
+                        hir::ExprKind::Path(hir::QPath::LangItem(hir::LangItem::IntoFutureIntoFuture, ..))
+                    ) {
+                        call = arg_0;
+                    }
+                }
+                break;
+            },
+            _ => break,
         }
     }
     check_method_call(cx, call, expr);
@@ -83,8 +99,10 @@ fn check_map_error(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<
 fn check_method_call(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Expr<'_>) {
     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 = match_trait_method(cx, call, &paths::IO_READ)
+            || match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCREADEXT);
+        let write_trait = match_trait_method(cx, call, &paths::IO_WRITE)
+            || match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCWRITEEXT);
 
         match (read_trait, write_trait, symbol) {
             (true, _, "read") => span_lint(

clippy_utils/src/paths.rs

@@ -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"];

tests/compile-test.rs

@@ -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",
@@ -38,6 +39,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;

tests/ui/unused_io_amount.rs

@@ -1,6 +1,8 @@
 #![allow(dead_code)]
 #![warn(clippy::unused_io_amount)]
 
+extern crate futures;
+use futures::io::{AsyncWrite, AsyncWriteExt};
 use std::io::{self, Read};
 
 fn question_mark<T: io::Read + io::Write>(s: &mut T) -> io::Result<()> {
@@ -61,4 +63,8 @@ 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();
+}
+
 fn main() {}

tests/ui/unused_io_amount.stderr

@@ -1,61 +1,61 @@
 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")?;
    |     ^^^^^^^^^^^^^^^^^
    |
    = 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,11 @@ LL | |         .or(Err(Error::Kind))
 LL | |         .expect("error");
    | |________________________^
 
-error: aborting due to 10 previous errors
+error: written amount is not handled. Use `Write::write_all` instead
+  --> $DIR/unused_io_amount.rs:67:5
+   |
+LL |     w.write(b"hello world").await.unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 11 previous errors