implemented basic lint logic for [unnecessary_blocking_ops]

CHANGELOG.md

@@ -5664,6 +5664,7 @@ Released 2018-09-13
 [`unit_hash`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_hash
 [`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord
 [`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
+[`unnecessary_blocking_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_blocking_ops
 [`unnecessary_box_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns
 [`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
 [`unnecessary_fallible_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions

clippy_config/src/conf.rs

@@ -552,6 +552,26 @@ define_Conf! {
     /// Lint "public" fields in a struct that are prefixed with an underscore based on their
     /// exported visibility, or whether they are marked as "pub".
     (pub_underscore_fields_behavior: PubUnderscoreFieldsBehaviour = PubUnderscoreFieldsBehaviour::PublicallyExported),
+    /// Lint: UNNECESSARY_BLOCKING_OPS.
+    ///
+    /// List of additional blocking function paths to check.
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// blocking-ops = ["my_crate::some_blocking_fn"]
+    /// ```
+    (blocking_ops: Vec<String> = <_>::default()),
+    /// Lint: UNNECESSARY_BLOCKING_OPS.
+    ///
+    /// List of additional blocking function paths to check, with replacement suggestion function paths.
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// blocking-ops-with-suggestions = [["my_crate::some_blocking_fn" , "my_crate::use_this_instead"]]
+    /// ```
+    (blocking_ops_with_suggestions: Vec<[String; 2]> = <_>::default()),
 }
 
 /// Search for the configuration file.

clippy_lints/src/declared_lints.rs

@@ -700,6 +700,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::unit_types::UNIT_ARG_INFO,
     crate::unit_types::UNIT_CMP_INFO,
     crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
+    crate::unnecessary_blocking_ops::UNNECESSARY_BLOCKING_OPS_INFO,
     crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
     crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
     crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,

clippy_lints/src/lib.rs

@@ -338,6 +338,7 @@ mod uninit_vec;
 mod unit_return_expecting_ord;
 mod unit_types;
 mod unnamed_address;
+mod unnecessary_blocking_ops;
 mod unnecessary_box_returns;
 mod unnecessary_map_on_constructor;
 mod unnecessary_owned_empty_strings;
@@ -527,6 +528,8 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         array_size_threshold,
         avoid_breaking_exported_api,
         ref await_holding_invalid_types,
+        ref blocking_ops,
+        ref blocking_ops_with_suggestions,
         cargo_ignore_publish,
         cognitive_complexity_threshold,
         ref disallowed_macros,
@@ -1092,6 +1095,12 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(move |_| {
         Box::new(thread_local_initializer_can_be_made_const::ThreadLocalInitializerCanBeMadeConst::new(msrv()))
     });
+    store.register_late_pass(move |_| {
+        Box::new(unnecessary_blocking_ops::UnnecessaryBlockingOps::new(
+            blocking_ops.clone(),
+            blocking_ops_with_suggestions.clone(),
+        ))
+    });
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/unnecessary_blocking_ops.rs

@@ -0,0 +1,162 @@
+use std::ops::ControlFlow;
+
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::visitors::for_each_expr_with_closures;
+use clippy_utils::{def_path_def_ids, fn_def_id, is_lint_allowed};
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::Applicability;
+use rustc_hir::def_id::DefId;
+use rustc_hir::hir_id::CRATE_HIR_ID;
+use rustc_hir::{Body, ExprKind, GeneratorKind, HirIdSet};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for async function or async closure with blocking operations that
+    /// could be replaced with their async counterpart.
+    ///
+    /// ### Why is this bad?
+    /// Blocking a thread prevents tasks being swapped, causing other tasks to stop running
+    /// until the thread is no longer blocked, which might not be as expected in an async context.
+    ///
+    /// ### Known problems
+    /// Not all blocking operations can be detected, as for now, this lint only detects a small
+    /// set of functions in standard library by default. And some of the suggestions might need
+    /// additional features to work properly.
+    ///
+    /// ### Example
+    /// ```rust
+    /// use std::time::Duration;
+    /// pub async fn foo() {
+    ///     std::thread::sleep(Duration::from_secs(5));
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// use std::time::Duration;
+    /// pub async fn foo() {
+    ///     tokio::time::sleep(Duration::from_secs(5));
+    /// }
+    /// ```
+    #[clippy::version = "1.74.0"]
+    pub UNNECESSARY_BLOCKING_OPS,
+    nursery,
+    "blocking operations in an async context"
+}
+
+pub(crate) struct UnnecessaryBlockingOps {
+    blocking_ops: Vec<String>,
+    blocking_ops_with_suggs: Vec<[String; 2]>,
+    /// Map of resolved funtion def_id with suggestion string after checking crate
+    id_with_suggs: FxHashMap<DefId, String>,
+    /// Keep track of visited block ids to skip checking the same bodies in `check_body` calls
+    visited_block: HirIdSet,
+}
+
+impl UnnecessaryBlockingOps {
+    pub(crate) fn new(blocking_ops: Vec<String>, blocking_ops_with_suggs: Vec<[String; 2]>) -> Self {
+        Self {
+            blocking_ops,
+            blocking_ops_with_suggs,
+            id_with_suggs: FxHashMap::default(),
+            visited_block: HirIdSet::default(),
+        }
+    }
+}
+
+impl_lint_pass!(UnnecessaryBlockingOps => [UNNECESSARY_BLOCKING_OPS]);
+
+// TODO: Should we throw away all suggestions and and give full control to user configurations?
+// this feels like a free ad for tokio :P
+static HARD_CODED_BLOCKING_OPS_WITH_SUGG: [[&str; 2]; 26] = [
+    // Sleep
+    ["std::thread::sleep", "tokio::time::sleep"],
+    // IO functions
+    ["std::io::copy", "tokio::io::copy"],
+    ["std::io::empty", "tokio::io::empty"],
+    ["std::io::repeat", "tokio::io::repeat"],
+    ["std::io::sink", "tokio::io::sink"],
+    ["std::io::stderr", "tokio::io::stderr"],
+    ["std::io::stdin", "tokio::io::stdin"],
+    ["std::io::stdout", "tokio::io::stdout"],
+    // Filesystem functions
+    ["std::fs::try_exists", "tokio::fs::try_exists"],
+    ["std::fs::canonicalize", "tokio::fs::canonicalize"],
+    ["std::fs::copy", "tokio::fs::copy"],
+    ["std::fs::create_dir", "tokio::fs::create_dir"],
+    ["std::fs::create_dir_all", "tokio::fs::create_dir_all"],
+    ["std::fs::hard_link", "tokio::fs::hard_link"],
+    ["std::fs::metadata", "tokio::fs::metadata"],
+    ["std::fs::read", "tokio::fs::read"],
+    ["std::fs::read_dir", "tokio::fs::read_dir"],
+    ["std::fs::read_to_string", "tokio::fs::read_to_string"],
+    ["std::fs::remove_dir", "tokio::fs::remove_dir"],
+    ["std::fs::remove_dir_all", "tokio::fs::remove_dir_all"],
+    ["std::fs::remove_file", "tokio::fs::remove_file"],
+    ["std::fs::rename", "tokio::fs::rename"],
+    ["std::fs::set_permissions", "tokio::fs::set_permissions"],
+    ["std::fs::soft_link", "tokio::fs::soft_link"],
+    ["std::fs::symlink_metadata", "tokio::fs::symlink_metadata"],
+    ["std::fs::write", "tokio::fs::write"],
+];
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps {
+    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
+        // Avoids processing and storing a long list of paths if this lint was allowed entirely
+        if is_lint_allowed(cx, UNNECESSARY_BLOCKING_OPS, CRATE_HIR_ID) {
+            return;
+        }
+
+        let full_fn_list = HARD_CODED_BLOCKING_OPS_WITH_SUGG
+            .into_iter()
+            // Chain configured functions without suggestions
+            .chain(self.blocking_ops.iter().map(|p| [p, ""]))
+            // Chain configured functions with suggestions
+            .chain(
+                self.blocking_ops_with_suggs
+                    .iter()
+                    .map(|[p, s]| [p.as_str(), s.as_str()]),
+            );
+
+        for [path_str, sugg_path_str] in full_fn_list {
+            let path = path_str.split("::").collect::<Vec<_>>();
+            for did in def_path_def_ids(cx, &path) {
+                self.id_with_suggs.insert(did, sugg_path_str.to_string());
+            }
+        }
+    }
+
+    fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'tcx>) {
+        if self.visited_block.contains(&body.value.hir_id) {
+            return;
+        }
+        if let Some(GeneratorKind::Async(_)) = body.generator_kind() {
+            for_each_expr_with_closures(cx, body, |ex| {
+                if let ExprKind::Block(block, _) = ex.kind {
+                    self.visited_block.insert(block.hir_id);
+                } else if let Some(call_did) = fn_def_id(cx, ex) &&
+                    let Some(replace_sugg) = self.id_with_suggs.get(&call_did)
+                {
+                    span_lint_and_then(
+                        cx,
+                        UNNECESSARY_BLOCKING_OPS,
+                        ex.span,
+                        "blocking function call detected in an async body",
+                        |diag| {
+                            if !replace_sugg.is_empty() {
+                                diag.span_suggestion(
+                                    ex.span,
+                                    "try using an async counterpart such as",
+                                    replace_sugg,
+                                    Applicability::Unspecified,
+                                );
+                            }
+                        }
+                    );
+                }
+                ControlFlow::<()>::Continue(())
+            });
+        }
+    }
+}

tests/ui/unnecessary_blocking_ops.rs

@@ -0,0 +1,66 @@
+//@no-rustfix
+#![feature(async_fn_in_trait)]
+#![feature(async_closure)]
+#![allow(incomplete_features)]
+#![warn(clippy::unnecessary_blocking_ops)]
+use std::{fs, io};
+use std::thread::sleep;
+use std::time::Duration;
+use tokio::io as tokio_io;
+
+mod totally_thread_safe {
+    pub async fn sleep(_dur: std::time::Duration) {}
+}
+
+pub async fn async_fn() {
+    sleep(Duration::from_secs(1));
+    fs::remove_dir("").unwrap();
+    fs::copy("", "_").unwrap();
+    let _ = fs::canonicalize("");
+
+    {
+        fs::write("", "").unwrap();
+        let _ = io::stdin();
+    }
+    let _stdout = io::stdout();
+    let mut r: &[u8] = b"hello";
+    let mut w: Vec<u8> = vec![];
+    io::copy(&mut r, &mut w).unwrap();
+}
+
+pub async fn non_blocking() {
+    totally_thread_safe::sleep(Duration::from_secs(1)).await; // don't lint, not blocking
+
+
+    let mut r: &[u8] = b"hello";
+    let mut w: Vec<u8> = vec![];
+    tokio_io::copy(&mut r, &mut w).await; // don't lint, not blocking
+}
+
+trait AsyncTrait {
+    async fn foo(&self);
+}
+
+struct SomeType(u8);
+impl AsyncTrait for SomeType {
+    async fn foo(&self) {
+        sleep(Duration::from_secs(self.0 as _));
+    }
+}
+
+fn do_something() {}
+
+fn closures() {
+    let _ = async || sleep(Duration::from_secs(1));
+    let async_closure = async move |_a: i32| {
+        let _ = 1;
+        do_something();
+        sleep(Duration::from_secs(1));
+    };
+    let non_async_closure = |_a: i32| {
+        sleep(Duration::from_secs(1)); // don't lint, not async
+        do_something();
+    };
+}
+
+fn main() {}

tests/ui/unnecessary_blocking_ops.stderr

@@ -0,0 +1,71 @@
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:16:5
+   |
+LL |     sleep(Duration::from_secs(1));
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
+   |
+   = note: `-D clippy::unnecessary-blocking-ops` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::unnecessary_blocking_ops)]`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:17:5
+   |
+LL |     fs::remove_dir("").unwrap();
+   |     ^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::remove_dir`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:18:5
+   |
+LL |     fs::copy("", "_").unwrap();
+   |     ^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::copy`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:19:13
+   |
+LL |     let _ = fs::canonicalize("");
+   |             ^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::canonicalize`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:22:9
+   |
+LL |         fs::write("", "").unwrap();
+   |         ^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::write`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:23:17
+   |
+LL |         let _ = io::stdin();
+   |                 ^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::io::stdin`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:25:19
+   |
+LL |     let _stdout = io::stdout();
+   |                   ^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::io::stdout`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:28:5
+   |
+LL |     io::copy(&mut r, &mut w).unwrap();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::io::copy`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:47:9
+   |
+LL |         sleep(Duration::from_secs(self.0 as _));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:54:22
+   |
+LL |     let _ = async || sleep(Duration::from_secs(1));
+   |                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
+
+error: blocking function call detected in an async body
+  --> $DIR/unnecessary_blocking_ops.rs:58:9
+   |
+LL |         sleep(Duration::from_secs(1));
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
+
+error: aborting due to 11 previous errors
+