add SuggestedPath config type;

Author: J-ZhengLi

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;
@@ -39,6 +41,31 @@ const DEFAULT_DOC_VALID_IDENTS: &[&str] = &[
 ];
 const DEFAULT_DISALLOWED_NAMES: &[&str] = &["foo", "baz", "quux"];
 const DEFAULT_ALLOWED_IDENTS_BELOW_MIN_CHARS: &[&str] = &["i", "j", "x", "y", "z", "w", "n"];
+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)]
@@ -591,24 +618,22 @@ define_Conf! {
     (allowed_wildcard_imports: FxHashSet<String> = FxHashSet::default()),
     /// Lint: UNNECESSARY_BLOCKING_OPS.
     ///
-    /// List of additional blocking function paths to check.
+    /// List of additional blocking function paths to check, with optional suggestions for each path.
     ///
     /// #### Example
     ///
     /// ```toml
-    /// blocking-ops = ["my_crate::some_blocking_fn"]
+    /// blocking-ops = [ "my_crate::blocking_foo" ]
     /// ```
-    (blocking_ops: Vec<String> = <_>::default()),
-    /// Lint: UNNECESSARY_BLOCKING_OPS.
     ///
-    /// List of additional blocking function paths to check, with replacement suggestion function paths.
-    ///
-    /// #### Example
+    /// Or, if you are sure that some functions can be replaced by a suggested one:
     ///
     /// ```toml
-    /// blocking-ops-with-suggestions = [["my_crate::some_blocking_fn" , "my_crate::use_this_instead"]]
+    /// blocking-ops = [
+    ///     { path = "my_crate::blocking_foo", suggestion = "my_crate::async::async_foo" }
+    /// ]
     /// ```
-    (blocking_ops_with_suggestions: Vec<[String; 2]> = <_>::default()),
+    (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/lib.rs

@@ -547,7 +547,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         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,
@@ -1137,7 +1136,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
     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

@@ -1,16 +1,17 @@
-use std::ops::ControlFlow;
-
+use clippy_config::types::SuggestedPath;
 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, Diagnostic};
+use rustc_errors::{Applicability, Diag};
 use rustc_hir::def_id::DefId;
-use rustc_hir::hir_id::CRATE_HIR_ID;
-use rustc_hir::{Body, Expr, ExprKind, GeneratorKind, HirIdSet};
-use rustc_lint::{LateContext, LateLintPass};
-use rustc_session::{declare_tool_lint, impl_lint_pass};
+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! {
@@ -43,131 +44,118 @@ 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<String>,
-    blocking_ops_with_suggs: Vec<[String; 2]>,
-    /// Map of resolved funtion def_id with suggestion string after checking crate
+    blocking_ops: Vec<SuggestedPath>,
+    /// Map of resolved funtion `def_id` with suggestion string after checking crate
     id_with_suggs: FxHashMap<DefId, Option<String>>,
-    /// Keep track of visited block ids to skip checking the same bodies in `check_body` calls
-    visited_block: HirIdSet,
+    /// Tracking whether a body is async after entering it.
+    body_asyncness: Vec<bool>,
 }
 
 impl UnnecessaryBlockingOps {
-    pub(crate) fn new(blocking_ops: Vec<String>, blocking_ops_with_suggs: Vec<[String; 2]>) -> Self {
+    pub(crate) fn new(blocking_ops: Vec<SuggestedPath>) -> Self {
         Self {
             blocking_ops,
-            blocking_ops_with_suggs,
             id_with_suggs: FxHashMap::default(),
-            visited_block: HirIdSet::default(),
+            body_asyncness: vec![],
         }
     }
 }
 
 impl_lint_pass!(UnnecessaryBlockingOps => [UNNECESSARY_BLOCKING_OPS]);
 
-static HARD_CODED_BLOCKING_OPS: [&[&str]; 21] = [
-    &["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"],
-];
-
 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
-            .into_iter()
-            .map(|p| (p.to_vec(), None))
-            // Chain configured functions without suggestions
-            .chain(
-                self.blocking_ops
-                    .iter()
-                    .map(|p| (p.split("::").collect::<Vec<_>>(), None)),
-            )
-            // Chain configured functions with suggestions
-            .chain(
-                self.blocking_ops_with_suggs
-                    .iter()
-                    .map(|[p, s]| (p.split("::").collect::<Vec<_>>(), Some(s.as_str()))),
-            );
-        for (path, maybe_sugg_str) in full_fn_list {
+        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)
-            || self.visited_block.contains(&body.value.hir_id)
-        {
+        if is_lint_allowed(cx, UNNECESSARY_BLOCKING_OPS, body.value.hir_id) {
             return;
         }
-        if let Some(GeneratorKind::Async(_)) = body.generator_kind() {
-            for_each_expr_with_closures(cx, body, |ex| {
-                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);
-                                }
-                            }
-                        );
+        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);
                     }
-                    _ => {}
-                }
-                ControlFlow::<()>::Continue(())
-            });
+                },
+            );
         }
     }
+
+    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) {
-    let mut applicability = Applicability::Unspecified;
+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::Unspecified,
-    );
+    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

@@ -23,6 +23,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
@@ -102,6 +103,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
@@ -181,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