diff --git a/clippy_lints/src/unnecessary_blocking_ops.rs b/clippy_lints/src/unnecessary_blocking_ops.rs index 389ccc147f37..b155e7f3ccba 100644 --- a/clippy_lints/src/unnecessary_blocking_ops.rs +++ b/clippy_lints/src/unnecessary_blocking_ops.rs @@ -1,15 +1,17 @@ use std::ops::ControlFlow; use clippy_utils::diagnostics::span_lint_and_then; +use clippy_utils::source::snippet_with_applicability; 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_errors::{Applicability, Diagnostic}; use rustc_hir::def_id::DefId; use rustc_hir::hir_id::CRATE_HIR_ID; -use rustc_hir::{Body, ExprKind, GeneratorKind, HirIdSet}; +use rustc_hir::{Body, Expr, ExprKind, GeneratorKind, HirIdSet}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; +use rustc_span::Span; declare_clippy_lint! { /// ### What it does @@ -33,7 +35,7 @@ declare_clippy_lint! { /// } /// ``` /// Use instead: - /// ```rust + /// ```ignore /// use std::time::Duration; /// pub async fn foo() { /// tokio::time::sleep(Duration::from_secs(5)); @@ -49,7 +51,7 @@ pub(crate) struct UnnecessaryBlockingOps { blocking_ops: Vec, blocking_ops_with_suggs: Vec<[String; 2]>, /// Map of resolved funtion def_id with suggestion string after checking crate - id_with_suggs: FxHashMap, + id_with_suggs: FxHashMap>, /// Keep track of visited block ids to skip checking the same bodies in `check_body` calls visited_block: HirIdSet, } @@ -67,38 +69,30 @@ impl UnnecessaryBlockingOps { 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"], +static HARD_CODED_BLOCKING_OPS: [&[&str]; 21] = [ + &["std", "thread", "sleep"], // 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"], + &["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"], ]; impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps { @@ -108,55 +102,72 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps { return; } - let full_fn_list = HARD_CODED_BLOCKING_OPS_WITH_SUGG + let full_fn_list = HARD_CODED_BLOCKING_OPS .into_iter() + .map(|p| (p.to_vec(), None)) // Chain configured functions without suggestions - .chain(self.blocking_ops.iter().map(|p| [p, ""])) + .chain( + self.blocking_ops + .iter() + .map(|p| (p.split("::").collect::>(), None)), + ) // Chain configured functions with suggestions .chain( self.blocking_ops_with_suggs .iter() - .map(|[p, s]| [p.as_str(), s.as_str()]), + .map(|[p, s]| (p.split("::").collect::>(), Some(s.as_str()))), ); - - for [path_str, sugg_path_str] in full_fn_list { - let path = path_str.split("::").collect::>(); + for (path, maybe_sugg_str) in full_fn_list { for did in def_path_def_ids(cx, &path) { - self.id_with_suggs.insert(did, sugg_path_str.to_string()); + 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 self.visited_block.contains(&body.value.hir_id) { + if is_lint_allowed(cx, UNNECESSARY_BLOCKING_OPS, body.value.hir_id) + || 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, - ); + match ex.kind { + ExprKind::Block(block, _) => { + self.visited_block.insert(block.hir_id); + } + ExprKind::Call(call, _) + if let Some(call_did) = fn_def_id(cx, ex) && + 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, ex, call.span, sugg_fn_path); + } } - } - ); + ); + } + _ => {} } ControlFlow::<()>::Continue(()) }); } } } + +fn make_suggestion(diag: &mut Diagnostic, cx: &LateContext<'_>, expr: &Expr<'_>, fn_span: Span, sugg_fn_path: &str) { + let mut applicability = 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::Unspecified, + ); +} diff --git a/tests/ui-toml/unnecessary_blocking_ops/clippy.toml b/tests/ui-toml/unnecessary_blocking_ops/clippy.toml new file mode 100644 index 000000000000..0ce53ba99ee9 --- /dev/null +++ b/tests/ui-toml/unnecessary_blocking_ops/clippy.toml @@ -0,0 +1,8 @@ +blocking-ops = ["unnecessary_blocking_ops::blocking_mod::sleep"] +blocking-ops-with-suggestions = [ + ["std::fs::remove_dir", "tokio::fs::remove_dir"], + ["std::fs::copy", "tokio::fs::copy"], + ["std::io::copy", "tokio::io::copy"], + ["std::io::read_to_string", "unnecessary_blocking_ops::async_mod::read_to_string"], + ["std::thread::sleep", "unnecessary_blocking_ops::async_mod::sleep"], +] diff --git a/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs new file mode 100644 index 000000000000..4fce92aa70ef --- /dev/null +++ b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs @@ -0,0 +1,35 @@ +//@no-rustfix +#![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 { + Ok(String::new()) + } +} + +mod blocking_mod { + pub async fn sleep(_dur: std::time::Duration) {} +} + +pub async fn async_fn() { + sleep(Duration::from_secs(1)); + //~^ ERROR: blocking function call detected in an async body + fs::remove_dir("").unwrap(); + //~^ ERROR: blocking function call detected in an async body + fs::copy("", "_").unwrap(); + //~^ ERROR: blocking function call detected in an async body + let mut r: &[u8] = b"hello"; + let mut w: Vec = vec![]; + io::copy(&mut r, &mut w).unwrap(); + //~^ ERROR: blocking function call detected in an async body + let _cont = io::read_to_string(io::stdin()).unwrap(); + //~^ ERROR: blocking function call detected in an async body + fs::create_dir("").unwrap(); + //~^ ERROR: blocking function call detected in an async body +} + +fn main() {} diff --git a/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.stderr b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.stderr new file mode 100644 index 000000000000..de2414295939 --- /dev/null +++ b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.stderr @@ -0,0 +1,51 @@ +error: blocking function call detected in an async body + --> $DIR/unnecessary_blocking_ops.rs:19:5 + | +LL | sleep(Duration::from_secs(1)); + | ^^^^^------------------------ + | | + | help: try using its async counterpart: `unnecessary_blocking_ops::async_mod::sleep(Duration::from_secs(1)).await` + | + = 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:21:5 + | +LL | fs::remove_dir("").unwrap(); + | ^^^^^^^^^^^^^^---- + | | + | help: try using its async counterpart: `tokio::fs::remove_dir("").await` + +error: blocking function call detected in an async body + --> $DIR/unnecessary_blocking_ops.rs:23:5 + | +LL | fs::copy("", "_").unwrap(); + | ^^^^^^^^--------- + | | + | help: try using its async counterpart: `tokio::fs::copy("", "_").await` + +error: blocking function call detected in an async body + --> $DIR/unnecessary_blocking_ops.rs:27:5 + | +LL | io::copy(&mut r, &mut w).unwrap(); + | ^^^^^^^^---------------- + | | + | help: try using its async counterpart: `tokio::io::copy(&mut r, &mut w).await` + +error: blocking function call detected in an async body + --> $DIR/unnecessary_blocking_ops.rs:29:17 + | +LL | let _cont = io::read_to_string(io::stdin()).unwrap(); + | ^^^^^^^^^^^^^^^^^^------------- + | | + | help: try using its async counterpart: `unnecessary_blocking_ops::async_mod::read_to_string(io::stdin()).await` + +error: blocking function call detected in an async body + --> $DIR/unnecessary_blocking_ops.rs:31:5 + | +LL | fs::create_dir("").unwrap(); + | ^^^^^^^^^^^^^^ + +error: aborting due to 6 previous errors + diff --git a/tests/ui/unnecessary_blocking_ops.rs b/tests/ui/unnecessary_blocking_ops.rs index adadc87cae68..0b941d06e1fc 100644 --- a/tests/ui/unnecessary_blocking_ops.rs +++ b/tests/ui/unnecessary_blocking_ops.rs @@ -1,11 +1,10 @@ -//@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 std::{fs, io}; use tokio::io as tokio_io; mod totally_thread_safe { @@ -14,24 +13,29 @@ mod totally_thread_safe { pub async fn async_fn() { sleep(Duration::from_secs(1)); + //~^ ERROR: blocking function call detected in an async body fs::remove_dir("").unwrap(); + //~^ ERROR: blocking function call detected in an async body fs::copy("", "_").unwrap(); + //~^ ERROR: blocking function call detected in an async body let _ = fs::canonicalize(""); + //~^ ERROR: blocking function call detected in an async body { fs::write("", "").unwrap(); + //~^ ERROR: blocking function call detected in an async body let _ = io::stdin(); } let _stdout = io::stdout(); let mut r: &[u8] = b"hello"; let mut w: Vec = vec![]; io::copy(&mut r, &mut w).unwrap(); + //~^ ERROR: blocking function call detected in an async body } 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 = vec![]; tokio_io::copy(&mut r, &mut w).await; // don't lint, not blocking @@ -45,6 +49,7 @@ struct SomeType(u8); impl AsyncTrait for SomeType { async fn foo(&self) { sleep(Duration::from_secs(self.0 as _)); + //~^ ERROR: blocking function call detected in an async body } } @@ -52,10 +57,12 @@ fn do_something() {} fn closures() { let _ = async || sleep(Duration::from_secs(1)); + //~^ ERROR: blocking function call detected in an async body let async_closure = async move |_a: i32| { let _ = 1; do_something(); sleep(Duration::from_secs(1)); + //~^ ERROR: blocking function call detected in an async body }; let non_async_closure = |_a: i32| { sleep(Duration::from_secs(1)); // don't lint, not async diff --git a/tests/ui/unnecessary_blocking_ops.stderr b/tests/ui/unnecessary_blocking_ops.stderr index 4e71bf39d6d3..2f2118a622ac 100644 --- a/tests/ui/unnecessary_blocking_ops.stderr +++ b/tests/ui/unnecessary_blocking_ops.stderr @@ -1,8 +1,8 @@ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:16:5 + --> $DIR/unnecessary_blocking_ops.rs:15: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)]` @@ -11,61 +11,49 @@ 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 + --> $DIR/unnecessary_blocking_ops.rs:19: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 + --> $DIR/unnecessary_blocking_ops.rs:21: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 + --> $DIR/unnecessary_blocking_ops.rs:25: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 + --> $DIR/unnecessary_blocking_ops.rs:32: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 + --> $DIR/unnecessary_blocking_ops.rs:51: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 + --> $DIR/unnecessary_blocking_ops.rs:59: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 + --> $DIR/unnecessary_blocking_ops.rs:64: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 +error: aborting due to 9 previous errors