new lint that detects blocking operations in async

CHANGELOG.md

@@ -5910,6 +5910,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_clippy_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_clippy_cfg

clippy_config/src/conf.rs

@@ -1,5 +1,7 @@
 use crate::msrvs::Msrv;
-use crate::types::{DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename};
+use crate::types::{
+    DisallowedPath, MacroMatcher, MatchLintBehaviour, PubUnderscoreFieldsBehaviour, Rename, SuggestedPath,
+};
 use crate::ClippyConfiguration;
 use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
@@ -44,6 +46,31 @@ const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z
 const DEFAULT_ALLOWED_PREFIXES: &[&str] = &["to", "as", "into", "from", "try_into", "try_from"];
 const DEFAULT_ALLOWED_TRAITS_WITH_RENAMED_PARAMS: &[&str] =
     &["core::convert::From", "core::convert::TryFrom", "core::str::FromStr"];
+const DEFAULT_BLOCKING_OP_PATHS: &[&str] = &[
+    "std::thread::sleep",
+    // Filesystem functions
+    "std::fs::try_exists",
+    "std::fs::canonicalize",
+    "std::fs::copy",
+    "std::fs::create_dir",
+    "std::fs::create_dir_all",
+    "std::fs::hard_link",
+    "std::fs::metadata",
+    "std::fs::read",
+    "std::fs::read_dir",
+    "std::fs::read_link",
+    "std::fs::read_to_string",
+    "std::fs::remove_dir",
+    "std::fs::remove_dir_all",
+    "std::fs::remove_file",
+    "std::fs::rename",
+    "std::fs::set_permissions",
+    "std::fs::symlink_metadata",
+    "std::fs::write",
+    // IO functions
+    "std::io::copy",
+    "std::io::read_to_string",
+];
 
 /// Conf with parse errors
 #[derive(Default)]
@@ -642,6 +669,24 @@ define_Conf! {
     ///
     /// Whether to also emit warnings for unsafe blocks with metavariable expansions in **private** macros.
     (warn_unsafe_macro_metavars_in_private_macros: bool = false),
+    /// Lint: UNNECESSARY_BLOCKING_OPS.
+    ///
+    /// List of additional blocking function paths to check, with optional suggestions for each path.
+    ///
+    /// #### Example
+    ///
+    /// ```toml
+    /// blocking-ops = [ "my_crate::blocking_foo" ]
+    /// ```
+    ///
+    /// Or, if you are sure that some functions can be replaced by a suggested one:
+    ///
+    /// ```toml
+    /// blocking-ops = [
+    ///     { path = "my_crate::blocking_foo", suggestion = "my_crate::async::async_foo" }
+    /// ]
+    /// ```
+    (blocking_ops: Vec<SuggestedPath> = DEFAULT_BLOCKING_OP_PATHS.iter().map(SuggestedPath::from_path_str).collect()),
 }
 
 /// Search for the configuration file.

clippy_config/src/types.rs

@@ -32,6 +32,33 @@ impl DisallowedPath {
     }
 }
 
+#[derive(Clone, Debug, Deserialize, PartialEq, Eq)]
+#[serde(untagged)]
+pub enum SuggestedPath {
+    Simple(String),
+    WithSuggestion { path: String, suggestion: Option<String> },
+}
+
+impl SuggestedPath {
+    pub fn path(&self) -> &str {
+        let (Self::Simple(path) | Self::WithSuggestion { path, .. }) = self;
+
+        path
+    }
+
+    pub fn from_path_str<S: ToString>(path: &S) -> Self {
+        Self::Simple(path.to_string())
+    }
+
+    pub fn suggestion(&self) -> Option<&str> {
+        if let Self::WithSuggestion { suggestion, .. } = self {
+            suggestion.as_deref()
+        } else {
+            None
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]
 pub enum MatchLintBehaviour {
     AllTypes,
@@ -125,6 +152,7 @@ unimplemented_serialize! {
     DisallowedPath,
     Rename,
     MacroMatcher,
+    SuggestedPath,
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)]

clippy_lints/src/declared_lints.rs

@@ -731,6 +731,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

@@ -359,6 +359,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;
@@ -556,6 +557,7 @@ 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,
         cargo_ignore_publish,
         cognitive_complexity_threshold,
         ref disallowed_macros,
@@ -1178,6 +1180,11 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     store.register_late_pass(|_| Box::new(set_contains_or_insert::HashsetInsertAfterContains));
     store.register_early_pass(|| Box::new(byte_char_slices::ByteCharSlice));
     store.register_early_pass(|| Box::new(cfg_not_test::CfgNotTest));
+    store.register_late_pass(move |_| {
+        Box::new(unnecessary_blocking_ops::UnnecessaryBlockingOps::new(
+            blocking_ops.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,161 @@
+use clippy_config::types::SuggestedPath;
+use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::{def_path_def_ids, fn_def_id, is_lint_allowed};
+use rustc_data_structures::fx::FxHashMap;
+use rustc_errors::{Applicability, Diag};
+use rustc_hir::def_id::DefId;
+use rustc_hir::{
+    Body, BodyId, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Expr, ExprKind, ImplItem, ImplItemKind,
+    Item, ItemKind, Node, TraitItem, TraitItemKind,
+};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::impl_lint_pass;
+use rustc_span::Span;
+
+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:
+    /// ```ignore
+    /// use std::time::Duration;
+    /// pub async fn foo() {
+    ///     tokio::time::sleep(Duration::from_secs(5));
+    /// }
+    /// ```
+    #[clippy::version = "1.74.0"]
+    pub UNNECESSARY_BLOCKING_OPS,
+    pedantic,
+    "blocking operations in an async context"
+}
+
+pub(crate) struct UnnecessaryBlockingOps {
+    blocking_ops: Vec<SuggestedPath>,
+    /// Map of resolved funtion `def_id` with suggestion string after checking crate
+    id_with_suggs: FxHashMap<DefId, Option<String>>,
+    /// Tracking whether a body is async after entering it.
+    body_asyncness: Vec<bool>,
+}
+
+impl UnnecessaryBlockingOps {
+    pub(crate) fn new(blocking_ops: Vec<SuggestedPath>) -> Self {
+        Self {
+            blocking_ops,
+            id_with_suggs: FxHashMap::default(),
+            body_asyncness: vec![],
+        }
+    }
+}
+
+impl_lint_pass!(UnnecessaryBlockingOps => [UNNECESSARY_BLOCKING_OPS]);
+
+impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps {
+    fn check_crate(&mut self, cx: &LateContext<'tcx>) {
+        let full_fn_list = self.blocking_ops.iter().map(|p| (p.path(), p.suggestion()));
+        for (path_str, maybe_sugg_str) in full_fn_list {
+            let path: Vec<&str> = path_str.split("::").collect();
+            for did in def_path_def_ids(cx, &path) {
+                self.id_with_suggs.insert(did, maybe_sugg_str.map(ToOwned::to_owned));
+            }
+        }
+    }
+
+    fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'tcx>) {
+        if is_lint_allowed(cx, UNNECESSARY_BLOCKING_OPS, body.value.hir_id) {
+            return;
+        }
+        self.body_asyncness.push(in_async_body(cx, body.id()));
+    }
+
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
+        if !in_external_macro(cx.sess(), expr.span)
+            && matches!(self.body_asyncness.last(), Some(true))
+            && let ExprKind::Call(call, _) = &expr.kind
+            && let Some(call_did) = fn_def_id(cx, expr)
+            && let Some(maybe_sugg) = self.id_with_suggs.get(&call_did)
+        {
+            span_lint_and_then(
+                cx,
+                UNNECESSARY_BLOCKING_OPS,
+                call.span,
+                "blocking function call detected in an async body",
+                |diag| {
+                    if let Some(sugg_fn_path) = maybe_sugg {
+                        make_suggestion(diag, cx, expr, call.span, sugg_fn_path);
+                    }
+                },
+            );
+        }
+    }
+
+    fn check_body_post(&mut self, _: &LateContext<'tcx>, _: &'tcx Body<'tcx>) {
+        self.body_asyncness.pop();
+    }
+}
+
+fn make_suggestion(diag: &mut Diag<'_, ()>, cx: &LateContext<'_>, expr: &Expr<'_>, fn_span: Span, sugg_fn_path: &str) {
+    // Suggestion should only be offered when user specified it in the configuration file,
+    // so we only assume it can be fixed here if only the path could be found.
+    let mut applicability = if def_path_def_ids(cx, &sugg_fn_path.split("::").collect::<Vec<_>>())
+        .next()
+        .is_some()
+    {
+        Applicability::MaybeIncorrect
+    } else {
+        Applicability::Unspecified
+    };
+
+    let args_span = expr.span.with_lo(fn_span.hi());
+    let args_snippet = snippet_with_applicability(cx, args_span, "..", &mut applicability);
+    let suggestion = format!("{sugg_fn_path}{args_snippet}.await");
+    diag.span_suggestion(expr.span, "try using its async counterpart", suggestion, applicability);
+}
+
+/// Check whether a body is from an async function/closure.
+fn in_async_body(cx: &LateContext<'_>, body_id: BodyId) -> bool {
+    let parent_node = cx.tcx.parent_hir_node(body_id.hir_id);
+    match parent_node {
+        Node::Expr(expr) => matches!(
+            expr.kind,
+            ExprKind::Closure(Closure {
+                kind: ClosureKind::Coroutine(CoroutineKind::Desugared(
+                    CoroutineDesugaring::Async | CoroutineDesugaring::AsyncGen,
+                    _
+                )),
+                ..
+            })
+        ),
+        Node::Item(Item {
+            kind: ItemKind::Fn(fn_sig, ..),
+            ..
+        })
+        | Node::ImplItem(ImplItem {
+            kind: ImplItemKind::Fn(fn_sig, _),
+            ..
+        })
+        | Node::TraitItem(TraitItem {
+            kind: TraitItemKind::Fn(fn_sig, _),
+            ..
+        }) => fn_sig.header.is_async(),
+        _ => false,
+    }
+}

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -27,6 +27,7 @@ error: error reading Clippy's configuration file: unknown field `foobar`, expect
            avoid-breaking-exported-api
            await-holding-invalid-types
            blacklisted-names
+           blocking-ops
            cargo-ignore-publish
            check-private-items
            cognitive-complexity-threshold
@@ -111,6 +112,7 @@ error: error reading Clippy's configuration file: unknown field `barfoo`, expect
            avoid-breaking-exported-api
            await-holding-invalid-types
            blacklisted-names
+           blocking-ops
            cargo-ignore-publish
            check-private-items
            cognitive-complexity-threshold
@@ -195,6 +197,7 @@ error: error reading Clippy's configuration file: unknown field `allow_mixed_uni
            avoid-breaking-exported-api
            await-holding-invalid-types
            blacklisted-names
+           blocking-ops
            cargo-ignore-publish
            check-private-items
            cognitive-complexity-threshold

tests/ui-toml/unnecessary_blocking_ops/clippy.toml

@@ -0,0 +1,6 @@
+blocking-ops = [
+    { path = "unnecessary_blocking_ops::blocking_mod::sleep", suggestion = "crate::async_mod::sleep" },
+    { path = "std::io::copy", suggestion = "tokio::io::copy" },
+    { path = "std::io::read_to_string", suggestion = "crate::async_mod::read_to_string" },
+    { path = "std::thread::sleep", suggestion = "crate::async_mod::sleep" },
+]

tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.fixed

@@ -0,0 +1,31 @@
+#![warn(clippy::unnecessary_blocking_ops)]
+use std::thread::sleep;
+use std::time::Duration;
+use std::{fs, io};
+
+mod async_mod {
+    pub async fn sleep(_dur: std::time::Duration) {}
+    pub async fn read_to_string(mut reader: std::io::Stdin) -> Result<String, ()> {
+        Ok(String::new())
+    }
+}
+
+mod blocking_mod {
+    pub async fn sleep(_dur: std::time::Duration) {}
+}
+
+pub async fn async_fn() {
+    crate::async_mod::sleep(Duration::from_secs(1)).await;
+    //~^ ERROR: blocking function call detected in an async body
+    crate::async_mod::sleep(Duration::from_secs(1)).await;
+    //~^ ERROR: blocking function call detected in an async body
+    let mut r: &[u8] = b"hello";
+    let mut w: Vec<u8> = vec![];
+    tokio::io::copy(&mut r, &mut w).await.unwrap();
+    //~^ ERROR: blocking function call detected in an async body
+    let _cont = crate::async_mod::read_to_string(io::stdin()).await.unwrap();
+    //~^ ERROR: blocking function call detected in an async body
+    fs::create_dir("").unwrap(); // Don't lint, config overrided
+}
+
+fn main() {}