From 3194a5134ddeac5ca86b93a8cc5dd849d43a9d2e Mon Sep 17 00:00:00 2001 From: J-ZhengLi Date: Mon, 1 Apr 2024 10:33:24 +0800 Subject: [PATCH] change catagory to `pedantic` & add a test case & api adjustments --- clippy_lints/src/unnecessary_blocking_ops.rs | 61 ++++++++++++++----- .../toml_unknown_key/conf_unknown_key.stderr | 1 + .../unnecessary_blocking_ops.rs | 2 + .../unnecessary_blocking_ops.stderr | 20 +++--- tests/ui/unnecessary_blocking_ops.rs | 7 +++ tests/ui/unnecessary_blocking_ops.stderr | 26 +++++--- 6 files changed, 84 insertions(+), 33 deletions(-) diff --git a/clippy_lints/src/unnecessary_blocking_ops.rs b/clippy_lints/src/unnecessary_blocking_ops.rs index 27a96970ec38..716b875d331b 100644 --- a/clippy_lints/src/unnecessary_blocking_ops.rs +++ b/clippy_lints/src/unnecessary_blocking_ops.rs @@ -3,9 +3,12 @@ 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, Diagnostic}; +use rustc_errors::{Applicability, Diag}; use rustc_hir::def_id::DefId; -use rustc_hir::{Body, CoroutineKind, Expr, ExprKind}; +use rustc_hir::{ + Body, BodyId, Closure, ClosureKind, CoroutineDesugaring, CoroutineKind, Expr, ExprKind, ImplItem, ImplItemKind, + Item, ItemKind, Node, TraitItem, TraitItemKind, +}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::impl_lint_pass; use rustc_span::Span; @@ -40,15 +43,16 @@ declare_clippy_lint! { /// ``` #[clippy::version = "1.74.0"] pub UNNECESSARY_BLOCKING_OPS, - nursery, + pedantic, "blocking operations in an async context" } pub(crate) struct UnnecessaryBlockingOps { blocking_ops: Vec, - /// Map of resolved funtion def_id with suggestion string after checking crate + /// Map of resolved funtion `def_id` with suggestion string after checking crate id_with_suggs: FxHashMap>, - is_in_async: bool, + /// Tracking whether a body is async after entering it. + body_asyncness: Vec, } impl UnnecessaryBlockingOps { @@ -56,7 +60,7 @@ impl UnnecessaryBlockingOps { Self { blocking_ops, id_with_suggs: FxHashMap::default(), - is_in_async: false, + body_asyncness: vec![], } } } @@ -108,14 +112,11 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps { if is_lint_allowed(cx, UNNECESSARY_BLOCKING_OPS, body.value.hir_id) { return; } - - if let Some(CoroutineKind::Async(_)) = body.coroutine_kind() { - self.is_in_async = true; - } + self.body_asyncness.push(in_async_body(cx, body.id())); } fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if self.is_in_async + if 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) @@ -134,14 +135,12 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps { } } - fn check_body_post(&mut self, _: &LateContext<'tcx>, body: &'tcx Body<'tcx>) { - if !matches!(body.coroutine_kind(), Some(CoroutineKind::Async(_))) { - self.is_in_async = false; - } + fn check_body_post(&mut self, _: &LateContext<'tcx>, _: &'tcx Body<'tcx>) { + self.body_asyncness.pop(); } } -fn make_suggestion(diag: &mut Diagnostic, cx: &LateContext<'_>, expr: &Expr<'_>, fn_span: Span, sugg_fn_path: &str) { +fn make_suggestion(diag: &mut Diag<'_, ()>, 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); @@ -153,3 +152,33 @@ fn make_suggestion(diag: &mut Diagnostic, cx: &LateContext<'_>, expr: &Expr<'_>, Applicability::Unspecified, ); } + +/// 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, + } +} diff --git a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr index 3d9495d08774..75d3e97241ff 100644 --- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr +++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr @@ -183,6 +183,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 diff --git a/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs index 4fce92aa70ef..130b68fad756 100644 --- a/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs +++ b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs @@ -30,6 +30,8 @@ pub async fn async_fn() { //~^ ERROR: blocking function call detected in an async body fs::create_dir("").unwrap(); //~^ ERROR: blocking function call detected in an async body + blocking_mod::sleep(Duration::from_secs(1)); + //~^ 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 index de2414295939..0d1666b9c571 100644 --- a/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.stderr +++ b/tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.stderr @@ -1,5 +1,5 @@ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:19:5 + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:19:5 | LL | sleep(Duration::from_secs(1)); | ^^^^^------------------------ @@ -10,7 +10,7 @@ LL | sleep(Duration::from_secs(1)); = 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 + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:21:5 | LL | fs::remove_dir("").unwrap(); | ^^^^^^^^^^^^^^---- @@ -18,7 +18,7 @@ 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 + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:23:5 | LL | fs::copy("", "_").unwrap(); | ^^^^^^^^--------- @@ -26,7 +26,7 @@ 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 + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:27:5 | LL | io::copy(&mut r, &mut w).unwrap(); | ^^^^^^^^---------------- @@ -34,7 +34,7 @@ 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 + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:29:17 | LL | let _cont = io::read_to_string(io::stdin()).unwrap(); | ^^^^^^^^^^^^^^^^^^------------- @@ -42,10 +42,16 @@ 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 + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:31:5 | LL | fs::create_dir("").unwrap(); | ^^^^^^^^^^^^^^ -error: aborting due to 6 previous errors +error: blocking function call detected in an async body + --> tests/ui-toml/unnecessary_blocking_ops/unnecessary_blocking_ops.rs:33:5 + | +LL | blocking_mod::sleep(Duration::from_secs(1)); + | ^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 7 previous errors diff --git a/tests/ui/unnecessary_blocking_ops.rs b/tests/ui/unnecessary_blocking_ops.rs index 969ed4f99377..f80d977e7c5e 100644 --- a/tests/ui/unnecessary_blocking_ops.rs +++ b/tests/ui/unnecessary_blocking_ops.rs @@ -69,4 +69,11 @@ fn closures() { }; } +fn thread_spawn() { + std::thread::spawn(|| sleep(Duration::from_secs(1))); + std::thread::spawn(async || {}); + std::thread::spawn(async || sleep(Duration::from_secs(1))); + //~^ ERROR: blocking function call detected in an async body +} + fn main() {} diff --git a/tests/ui/unnecessary_blocking_ops.stderr b/tests/ui/unnecessary_blocking_ops.stderr index 5dc5a9f60b2c..e0a5cea992ae 100644 --- a/tests/ui/unnecessary_blocking_ops.stderr +++ b/tests/ui/unnecessary_blocking_ops.stderr @@ -1,5 +1,5 @@ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:14:5 + --> tests/ui/unnecessary_blocking_ops.rs:14:5 | LL | sleep(Duration::from_secs(1)); | ^^^^^ @@ -8,52 +8,58 @@ LL | sleep(Duration::from_secs(1)); = 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:16:5 + --> tests/ui/unnecessary_blocking_ops.rs:16:5 | LL | fs::remove_dir("").unwrap(); | ^^^^^^^^^^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:18:5 + --> tests/ui/unnecessary_blocking_ops.rs:18:5 | LL | fs::copy("", "_").unwrap(); | ^^^^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:20:13 + --> tests/ui/unnecessary_blocking_ops.rs:20:13 | LL | let _ = fs::canonicalize(""); | ^^^^^^^^^^^^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:24:9 + --> tests/ui/unnecessary_blocking_ops.rs:24:9 | LL | fs::write("", "").unwrap(); | ^^^^^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:31:5 + --> tests/ui/unnecessary_blocking_ops.rs:31:5 | LL | io::copy(&mut r, &mut w).unwrap(); | ^^^^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:50:9 + --> tests/ui/unnecessary_blocking_ops.rs:50:9 | LL | sleep(Duration::from_secs(self.0 as _)); | ^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:58:22 + --> tests/ui/unnecessary_blocking_ops.rs:58:22 | LL | let _ = async || sleep(Duration::from_secs(1)); | ^^^^^ error: blocking function call detected in an async body - --> $DIR/unnecessary_blocking_ops.rs:63:9 + --> tests/ui/unnecessary_blocking_ops.rs:63:9 | LL | sleep(Duration::from_secs(1)); | ^^^^^ -error: aborting due to 9 previous errors +error: blocking function call detected in an async body + --> tests/ui/unnecessary_blocking_ops.rs:75:33 + | +LL | std::thread::spawn(async || sleep(Duration::from_secs(1))); + | ^^^^^ + +error: aborting due to 10 previous errors