Skip to content

Commit 3e3ae13

Browse files
committed
implemented basic lint logic for [unnecessary_blocking_ops]
1 parent 124f1b0 commit 3e3ae13

File tree

7 files changed

+330
-0
lines changed

7 files changed

+330
-0
lines changed

Diff for: CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5433,6 +5433,7 @@ Released 2018-09-13
54335433
[`unit_hash`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_hash
54345434
[`unit_return_expecting_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#unit_return_expecting_ord
54355435
[`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
5436+
[`unnecessary_blocking_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_blocking_ops
54365437
[`unnecessary_box_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns
54375438
[`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
54385439
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map

Diff for: clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
674674
crate::unit_types::UNIT_CMP_INFO,
675675
crate::unnamed_address::FN_ADDRESS_COMPARISONS_INFO,
676676
crate::unnamed_address::VTABLE_ADDRESS_COMPARISONS_INFO,
677+
crate::unnecessary_blocking_ops::UNNECESSARY_BLOCKING_OPS_INFO,
677678
crate::unnecessary_box_returns::UNNECESSARY_BOX_RETURNS_INFO,
678679
crate::unnecessary_map_on_constructor::UNNECESSARY_MAP_ON_CONSTRUCTOR_INFO,
679680
crate::unnecessary_owned_empty_strings::UNNECESSARY_OWNED_EMPTY_STRINGS_INFO,

Diff for: clippy_lints/src/lib.rs

+9
Original file line numberDiff line numberDiff line change
@@ -332,6 +332,7 @@ mod uninit_vec;
332332
mod unit_return_expecting_ord;
333333
mod unit_types;
334334
mod unnamed_address;
335+
mod unnecessary_blocking_ops;
335336
mod unnecessary_box_returns;
336337
mod unnecessary_map_on_constructor;
337338
mod unnecessary_owned_empty_strings;
@@ -1121,6 +1122,14 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
11211122
))
11221123
});
11231124
store.register_late_pass(move |_| Box::new(manual_hash_one::ManualHashOne::new(msrv())));
1125+
let blocking_ops = conf.blocking_ops.clone();
1126+
let blocking_ops_with_suggs = conf.blocking_ops_with_suggestions.clone();
1127+
store.register_late_pass(move |_| {
1128+
Box::new(unnecessary_blocking_ops::UnnecessaryBlockingOps::new(
1129+
blocking_ops.clone(),
1130+
blocking_ops_with_suggs.clone(),
1131+
))
1132+
});
11241133
// add lints here, do not remove this comment, it's used in `new_lint`
11251134
}
11261135

Diff for: clippy_lints/src/unnecessary_blocking_ops.rs

+162
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
use std::ops::ControlFlow;
2+
3+
use clippy_utils::diagnostics::span_lint_and_then;
4+
use clippy_utils::visitors::for_each_expr_with_closures;
5+
use clippy_utils::{def_path_def_ids, fn_def_id, is_lint_allowed};
6+
use rustc_data_structures::fx::FxHashMap;
7+
use rustc_errors::Applicability;
8+
use rustc_hir::def_id::DefId;
9+
use rustc_hir::hir_id::CRATE_HIR_ID;
10+
use rustc_hir::{Body, ExprKind, GeneratorKind, HirIdSet};
11+
use rustc_lint::{LateContext, LateLintPass};
12+
use rustc_session::{declare_tool_lint, impl_lint_pass};
13+
14+
declare_clippy_lint! {
15+
/// ### What it does
16+
/// Checks for async function or async closure with blocking operations that
17+
/// could be replaced with their async counterpart.
18+
///
19+
/// ### Why is this bad?
20+
/// Blocking a thread prevents tasks being swapped, causing other tasks to stop running
21+
/// until the thread is no longer blocked, which might not be as expected in an async context.
22+
///
23+
/// ### Known problems
24+
/// Not all blocking operations can be detected, as for now, this lint only detects a small
25+
/// set of functions in standard library by default. And some of the suggestions might need
26+
/// additional features to work properly.
27+
///
28+
/// ### Example
29+
/// ```rust
30+
/// use std::time::Duration;
31+
/// pub async fn foo() {
32+
/// std::thread::sleep(Duration::from_secs(5));
33+
/// }
34+
/// ```
35+
/// Use instead:
36+
/// ```rust
37+
/// use std::time::Duration;
38+
/// pub async fn foo() {
39+
/// tokio::time::sleep(Duration::from_secs(5));
40+
/// }
41+
/// ```
42+
#[clippy::version = "1.74.0"]
43+
pub UNNECESSARY_BLOCKING_OPS,
44+
nursery,
45+
"blocking operations in an async context"
46+
}
47+
48+
pub(crate) struct UnnecessaryBlockingOps {
49+
blocking_ops: Vec<String>,
50+
blocking_ops_with_suggs: Vec<[String; 2]>,
51+
/// Map of resolved funtion def_id with suggestion string after checking crate
52+
id_with_suggs: FxHashMap<DefId, String>,
53+
/// Keep track of visited block ids to skip checking the same bodies in `check_body` calls
54+
visited_block: HirIdSet,
55+
}
56+
57+
impl UnnecessaryBlockingOps {
58+
pub(crate) fn new(blocking_ops: Vec<String>, blocking_ops_with_suggs: Vec<[String; 2]>) -> Self {
59+
Self {
60+
blocking_ops,
61+
blocking_ops_with_suggs,
62+
id_with_suggs: FxHashMap::default(),
63+
visited_block: HirIdSet::default(),
64+
}
65+
}
66+
}
67+
68+
impl_lint_pass!(UnnecessaryBlockingOps => [UNNECESSARY_BLOCKING_OPS]);
69+
70+
// TODO: Should we throw away all suggestions and and give full control to user configurations?
71+
// this feels like a free ad for tokio :P
72+
static HARD_CODED_BLOCKING_OPS_WITH_SUGG: [[&str; 2]; 26] = [
73+
// Sleep
74+
["std::thread::sleep", "tokio::time::sleep"],
75+
// IO functions
76+
["std::io::copy", "tokio::io::copy"],
77+
["std::io::empty", "tokio::io::empty"],
78+
["std::io::repeat", "tokio::io::repeat"],
79+
["std::io::sink", "tokio::io::sink"],
80+
["std::io::stderr", "tokio::io::stderr"],
81+
["std::io::stdin", "tokio::io::stdin"],
82+
["std::io::stdout", "tokio::io::stdout"],
83+
// Filesystem functions
84+
["std::fs::try_exists", "tokio::fs::try_exists"],
85+
["std::fs::canonicalize", "tokio::fs::canonicalize"],
86+
["std::fs::copy", "tokio::fs::copy"],
87+
["std::fs::create_dir", "tokio::fs::create_dir"],
88+
["std::fs::create_dir_all", "tokio::fs::create_dir_all"],
89+
["std::fs::hard_link", "tokio::fs::hard_link"],
90+
["std::fs::metadata", "tokio::fs::metadata"],
91+
["std::fs::read", "tokio::fs::read"],
92+
["std::fs::read_dir", "tokio::fs::read_dir"],
93+
["std::fs::read_to_string", "tokio::fs::read_to_string"],
94+
["std::fs::remove_dir", "tokio::fs::remove_dir"],
95+
["std::fs::remove_dir_all", "tokio::fs::remove_dir_all"],
96+
["std::fs::remove_file", "tokio::fs::remove_file"],
97+
["std::fs::rename", "tokio::fs::rename"],
98+
["std::fs::set_permissions", "tokio::fs::set_permissions"],
99+
["std::fs::soft_link", "tokio::fs::soft_link"],
100+
["std::fs::symlink_metadata", "tokio::fs::symlink_metadata"],
101+
["std::fs::write", "tokio::fs::write"],
102+
];
103+
104+
impl<'tcx> LateLintPass<'tcx> for UnnecessaryBlockingOps {
105+
fn check_crate(&mut self, cx: &LateContext<'tcx>) {
106+
// Avoids processing and storing a long list of paths if this lint was allowed entirely
107+
if is_lint_allowed(cx, UNNECESSARY_BLOCKING_OPS, CRATE_HIR_ID) {
108+
return;
109+
}
110+
111+
let full_fn_list = HARD_CODED_BLOCKING_OPS_WITH_SUGG
112+
.into_iter()
113+
// Chain configured functions without suggestions
114+
.chain(self.blocking_ops.iter().map(|p| [p, ""]))
115+
// Chain configured functions with suggestions
116+
.chain(
117+
self.blocking_ops_with_suggs
118+
.iter()
119+
.map(|[p, s]| [p.as_str(), s.as_str()]),
120+
);
121+
122+
for [path_str, sugg_path_str] in full_fn_list {
123+
let path = path_str.split("::").collect::<Vec<_>>();
124+
for did in def_path_def_ids(cx, &path) {
125+
self.id_with_suggs.insert(did, sugg_path_str.to_string());
126+
}
127+
}
128+
}
129+
130+
fn check_body(&mut self, cx: &LateContext<'tcx>, body: &'tcx Body<'tcx>) {
131+
if self.visited_block.contains(&body.value.hir_id) {
132+
return;
133+
}
134+
if let Some(GeneratorKind::Async(_)) = body.generator_kind() {
135+
for_each_expr_with_closures(cx, body, |ex| {
136+
if let ExprKind::Block(block, _) = ex.kind {
137+
self.visited_block.insert(block.hir_id);
138+
} else if let Some(call_did) = fn_def_id(cx, ex) &&
139+
let Some(replace_sugg) = self.id_with_suggs.get(&call_did)
140+
{
141+
span_lint_and_then(
142+
cx,
143+
UNNECESSARY_BLOCKING_OPS,
144+
ex.span,
145+
"blocking function call detected in an async body",
146+
|diag| {
147+
if !replace_sugg.is_empty() {
148+
diag.span_suggestion(
149+
ex.span,
150+
"try using an async counterpart such as",
151+
replace_sugg,
152+
Applicability::Unspecified,
153+
);
154+
}
155+
}
156+
);
157+
}
158+
ControlFlow::<()>::Continue(())
159+
});
160+
}
161+
}
162+
}

Diff for: clippy_lints/src/utils/conf.rs

+20
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,26 @@ define_Conf! {
586586
/// for _ in &mut *rmvec {}
587587
/// ```
588588
(enforce_iter_loop_reborrow: bool = false),
589+
/// Lint: UNNECESSARY_BLOCKING_OPS.
590+
///
591+
/// List of additional blocking function paths to check.
592+
///
593+
/// #### Example
594+
///
595+
/// ```toml
596+
/// blocking-ops = ["my_crate::some_blocking_fn"]
597+
/// ```
598+
(blocking_ops: Vec<String> = <_>::default()),
599+
/// Lint: UNNECESSARY_BLOCKING_OPS.
600+
///
601+
/// List of additional blocking function paths to check, with replacement suggestion function paths.
602+
///
603+
/// #### Example
604+
///
605+
/// ```toml
606+
/// blocking-ops-with-suggestions = [["my_crate::some_blocking_fn" , "my_crate::use_this_instead"]]
607+
/// ```
608+
(blocking_ops_with_suggestions: Vec<[String; 2]> = <_>::default()),
589609
}
590610

591611
/// Search for the configuration file.

Diff for: tests/ui/unnecessary_blocking_ops.rs

+66
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
//@no-rustfix
2+
#![feature(async_fn_in_trait)]
3+
#![feature(async_closure)]
4+
#![allow(incomplete_features)]
5+
#![warn(clippy::unnecessary_blocking_ops)]
6+
use std::{fs, io};
7+
use std::thread::sleep;
8+
use std::time::Duration;
9+
use tokio::io as tokio_io;
10+
11+
mod totally_thread_safe {
12+
pub async fn sleep(_dur: std::time::Duration) {}
13+
}
14+
15+
pub async fn async_fn() {
16+
sleep(Duration::from_secs(1));
17+
fs::remove_dir("").unwrap();
18+
fs::copy("", "_").unwrap();
19+
let _ = fs::canonicalize("");
20+
21+
{
22+
fs::write("", "").unwrap();
23+
let _ = io::stdin();
24+
}
25+
let _stdout = io::stdout();
26+
let mut r: &[u8] = b"hello";
27+
let mut w: Vec<u8> = vec![];
28+
io::copy(&mut r, &mut w).unwrap();
29+
}
30+
31+
pub async fn non_blocking() {
32+
totally_thread_safe::sleep(Duration::from_secs(1)).await; // don't lint, not blocking
33+
34+
35+
let mut r: &[u8] = b"hello";
36+
let mut w: Vec<u8> = vec![];
37+
tokio_io::copy(&mut r, &mut w).await; // don't lint, not blocking
38+
}
39+
40+
trait AsyncTrait {
41+
async fn foo(&self);
42+
}
43+
44+
struct SomeType(u8);
45+
impl AsyncTrait for SomeType {
46+
async fn foo(&self) {
47+
sleep(Duration::from_secs(self.0 as _));
48+
}
49+
}
50+
51+
fn do_something() {}
52+
53+
fn closures() {
54+
let _ = async || sleep(Duration::from_secs(1));
55+
let async_closure = async move |_a: i32| {
56+
let _ = 1;
57+
do_something();
58+
sleep(Duration::from_secs(1));
59+
};
60+
let non_async_closure = |_a: i32| {
61+
sleep(Duration::from_secs(1)); // don't lint, not async
62+
do_something();
63+
};
64+
}
65+
66+
fn main() {}

Diff for: tests/ui/unnecessary_blocking_ops.stderr

+71
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
error: blocking function call detected in an async body
2+
--> $DIR/unnecessary_blocking_ops.rs:16:5
3+
|
4+
LL | sleep(Duration::from_secs(1));
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
6+
|
7+
= note: `-D clippy::unnecessary-blocking-ops` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_blocking_ops)]`
9+
10+
error: blocking function call detected in an async body
11+
--> $DIR/unnecessary_blocking_ops.rs:17:5
12+
|
13+
LL | fs::remove_dir("").unwrap();
14+
| ^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::remove_dir`
15+
16+
error: blocking function call detected in an async body
17+
--> $DIR/unnecessary_blocking_ops.rs:18:5
18+
|
19+
LL | fs::copy("", "_").unwrap();
20+
| ^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::copy`
21+
22+
error: blocking function call detected in an async body
23+
--> $DIR/unnecessary_blocking_ops.rs:19:13
24+
|
25+
LL | let _ = fs::canonicalize("");
26+
| ^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::canonicalize`
27+
28+
error: blocking function call detected in an async body
29+
--> $DIR/unnecessary_blocking_ops.rs:22:9
30+
|
31+
LL | fs::write("", "").unwrap();
32+
| ^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::fs::write`
33+
34+
error: blocking function call detected in an async body
35+
--> $DIR/unnecessary_blocking_ops.rs:23:17
36+
|
37+
LL | let _ = io::stdin();
38+
| ^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::io::stdin`
39+
40+
error: blocking function call detected in an async body
41+
--> $DIR/unnecessary_blocking_ops.rs:25:19
42+
|
43+
LL | let _stdout = io::stdout();
44+
| ^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::io::stdout`
45+
46+
error: blocking function call detected in an async body
47+
--> $DIR/unnecessary_blocking_ops.rs:28:5
48+
|
49+
LL | io::copy(&mut r, &mut w).unwrap();
50+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::io::copy`
51+
52+
error: blocking function call detected in an async body
53+
--> $DIR/unnecessary_blocking_ops.rs:47:9
54+
|
55+
LL | sleep(Duration::from_secs(self.0 as _));
56+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
57+
58+
error: blocking function call detected in an async body
59+
--> $DIR/unnecessary_blocking_ops.rs:54:22
60+
|
61+
LL | let _ = async || sleep(Duration::from_secs(1));
62+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
63+
64+
error: blocking function call detected in an async body
65+
--> $DIR/unnecessary_blocking_ops.rs:58:9
66+
|
67+
LL | sleep(Duration::from_secs(1));
68+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using an async counterpart such as: `tokio::time::sleep`
69+
70+
error: aborting due to 11 previous errors
71+

0 commit comments

Comments
 (0)