Auto merge of #8707 - OneSignal:await-invalid-types, r=llogiq

Add `await_holding_invalid_type` lint

changelog: [`await_holding_invalid_type`]

This lint allows users to create a denylist of types which are not allowed to be
held across await points. This is essentially a re-implementation of the
language-level [`must_not_suspend`
lint](https://github.com/rust-lang/rust/issues/83310). That lint has a lot of
work still to be done before it will reach Rust stable, and in the meantime
there are a lot of types which can trip up developers if they are used
improperly.

I originally implemented this specifically for `tracing::span::Entered`, until I discovered #8434 and read the commentary on that PR. Given this implementation is fully user configurable, doesn't tie clippy to any one particular crate, and introduces no additional dependencies, it seems more appropriate.

11 files changed, 212 insertions, 40 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 900a1dbfe43..aaa71a9affc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3283,6 +3283,7 @@ Released 2018-09-13
 [`assign_op_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_op_pattern
 [`assign_ops`]: https://rust-lang.github.io/rust-clippy/master/index.html#assign_ops
 [`async_yields_async`]: https://rust-lang.github.io/rust-clippy/master/index.html#async_yields_async
+[`await_holding_invalid_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_invalid_type
 [`await_holding_lock`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_lock
 [`await_holding_refcell_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#await_holding_refcell_ref
 [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs
index 4592ca72748..5b7c4591504 100644
--- a/clippy_lints/src/await_holding_invalid.rs
+++ b/clippy_lints/src/await_holding_invalid.rs
@@ -1,12 +1,15 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::{match_def_path, paths};
+use rustc_data_structures::fx::FxHashMap;
 use rustc_hir::def_id::DefId;
-use rustc_hir::{AsyncGeneratorKind, Body, BodyId, GeneratorKind};
+use rustc_hir::{def::Res, AsyncGeneratorKind, Body, BodyId, GeneratorKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::GeneratorInteriorTypeCause;
-use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::Span;
 
+use crate::utils::conf::DisallowedType;
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for calls to await while holding a non-async-aware MutexGuard.
@@ -127,9 +130,75 @@ declare_clippy_lint! {
     "inside an async function, holding a `RefCell` ref while calling `await`"
 }
 
-declare_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF]);
+declare_clippy_lint! {
+    /// ### What it does
+    /// Allows users to configure types which should not be held across `await`
+    /// suspension points.
+    ///
+    /// ### Why is this bad?
+    /// There are some types which are perfectly "safe" to be used concurrently
+    /// from a memory access perspective but will cause bugs at runtime if they
+    /// are held in such a way.
+    ///
+    /// ### Known problems
+    ///
+    /// ### Example
+    ///
+    /// ```toml
+    /// await-holding-invalid-types = [
+    ///   # You can specify a type name
+    ///   "CustomLockType",
+    ///   # You can (optionally) specify a reason
+    ///   { path = "OtherCustomLockType", reason = "Relies on a thread local" }
+    /// ]
+    /// ```
+    ///
+    /// ```rust
+    /// # async fn baz() {}
+    /// struct CustomLockType;
+    /// struct OtherCustomLockType;
+    /// async fn foo() {
+    ///   let _x = CustomLockType;
+    ///   let _y = OtherCustomLockType;
+    ///   baz().await; // Lint violation
+    /// }
+    /// ```
+    #[clippy::version = "1.49.0"]
+    pub AWAIT_HOLDING_INVALID_TYPE,
+    suspicious,
+    "holding a type across an await point which is not allowed to be held as per the configuration"
+}
+
+impl_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF, AWAIT_HOLDING_INVALID_TYPE]);
+
+#[derive(Debug)]
+pub struct AwaitHolding {
+    conf_invalid_types: Vec<DisallowedType>,
+    def_ids: FxHashMap<DefId, DisallowedType>,
+}
+
+impl AwaitHolding {
+    pub(crate) fn new(conf_invalid_types: Vec<DisallowedType>) -> Self {
+        Self {
+            conf_invalid_types,
+            def_ids: FxHashMap::default(),
+        }
+    }
+}
 
 impl LateLintPass<'_> for AwaitHolding {
+    fn check_crate(&mut self, cx: &LateContext<'_>) {
+        for conf in &self.conf_invalid_types {
+            let path = match conf {
+                DisallowedType::Simple(path) | DisallowedType::WithReason { path, .. } => path,
+            };
+            let segs: Vec<_> = path.split("::").collect();
+            if let Res::Def(_, id) = clippy_utils::def_path_res(cx, &segs) {
+                self.def_ids.insert(id, conf.clone());
+            }
+        }
+    }
+
     fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
         use AsyncGeneratorKind::{Block, Closure, Fn};
         if let Some(GeneratorKind::Async(Block | Closure | Fn)) = body.generator_kind {
@@ -137,7 +206,7 @@ impl LateLintPass<'_> for AwaitHolding {
                 hir_id: body.value.hir_id,
             };
             let typeck_results = cx.tcx.typeck_body(body_id);
-            check_interior_types(
+            self.check_interior_types(
                 cx,
                 typeck_results.generator_interior_types.as_ref().skip_binder(),
                 body.value.span,
@@ -146,46 +215,68 @@ impl LateLintPass<'_> for AwaitHolding {
     }
 }
 
-fn check_interior_types(cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorTypeCause<'_>], span: Span) {
-    for ty_cause in ty_causes {
-        if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind() {
-            if is_mutex_guard(cx, adt.did()) {
-                span_lint_and_then(
-                    cx,
-                    AWAIT_HOLDING_LOCK,
-                    ty_cause.span,
-                    "this `MutexGuard` is held across an `await` point",
-                    |diag| {
-                        diag.help(
-                            "consider using an async-aware `Mutex` type or ensuring the \
+impl AwaitHolding {
+    fn check_interior_types(&self, cx: &LateContext<'_>, ty_causes: &[GeneratorInteriorTypeCause<'_>], span: Span) {
+        for ty_cause in ty_causes {
+            if let rustc_middle::ty::Adt(adt, _) = ty_cause.ty.kind() {
+                if is_mutex_guard(cx, adt.did()) {
+                    span_lint_and_then(
+                        cx,
+                        AWAIT_HOLDING_LOCK,
+                        ty_cause.span,
+                        "this `MutexGuard` is held across an `await` point",
+                        |diag| {
+                            diag.help(
+                                "consider using an async-aware `Mutex` type or ensuring the \
                                 `MutexGuard` is dropped before calling await",
-                        );
-                        diag.span_note(
-                            ty_cause.scope_span.unwrap_or(span),
-                            "these are all the `await` points this lock is held through",
-                        );
-                    },
-                );
-            }
-            if is_refcell_ref(cx, adt.did()) {
-                span_lint_and_then(
-                    cx,
-                    AWAIT_HOLDING_REFCELL_REF,
-                    ty_cause.span,
-                    "this `RefCell` reference is held across an `await` point",
-                    |diag| {
-                        diag.help("ensure the reference is dropped before calling `await`");
-                        diag.span_note(
-                            ty_cause.scope_span.unwrap_or(span),
-                            "these are all the `await` points this reference is held through",
-                        );
-                    },
-                );
+                            );
+                            diag.span_note(
+                                ty_cause.scope_span.unwrap_or(span),
+                                "these are all the `await` points this lock is held through",
+                            );
+                        },
+                    );
+                } else if is_refcell_ref(cx, adt.did()) {
+                    span_lint_and_then(
+                        cx,
+                        AWAIT_HOLDING_REFCELL_REF,
+                        ty_cause.span,
+                        "this `RefCell` reference is held across an `await` point",
+                        |diag| {
+                            diag.help("ensure the reference is dropped before calling `await`");
+                            diag.span_note(
+                                ty_cause.scope_span.unwrap_or(span),
+                                "these are all the `await` points this reference is held through",
+                            );
+                        },
+                    );
+                } else if let Some(disallowed) = self.def_ids.get(&adt.did()) {
+                    emit_invalid_type(cx, ty_cause.span, disallowed);
+                }
             }
         }
     }
 }
 
+fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedType) {
+    let (type_name, reason) = match disallowed {
+        DisallowedType::Simple(path) => (path, &None),
+        DisallowedType::WithReason { path, reason } => (path, reason),
+    };
+
+    span_lint_and_then(
+        cx,
+        AWAIT_HOLDING_INVALID_TYPE,
+        span,
+        &format!("`{type_name}` may not be held across an `await` point per `clippy.toml`",),
+        |diag| {
+            if let Some(reason) = reason {
+                diag.note(reason.clone());
+            }
+        },
+    );
+}
+
 fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool {
     match_def_path(cx, def_id, &paths::MUTEX_GUARD)
         || match_def_path(cx, def_id, &paths::RWLOCK_READ_GUARD)
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index d091503c4dd..a2e9e07ac08 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -14,6 +14,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(attrs::DEPRECATED_SEMVER),
     LintId::of(attrs::MISMATCHED_TARGET_OS),
     LintId::of(attrs::USELESS_ATTRIBUTE),
+    LintId::of(await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE),
     LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
     LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
     LintId::of(bit_mask::BAD_BIT_MASK),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index a189c27b510..78b727f4cdf 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/clippy_lints/src/lib.register_lints.rs
@@ -52,6 +52,7 @@ store.register_lints(&[
     attrs::INLINE_ALWAYS,
     attrs::MISMATCHED_TARGET_OS,
     attrs::USELESS_ATTRIBUTE,
+    await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE,
     await_holding_invalid::AWAIT_HOLDING_LOCK,
     await_holding_invalid::AWAIT_HOLDING_REFCELL_REF,
     bit_mask::BAD_BIT_MASK,
diff --git a/clippy_lints/src/lib.register_suspicious.rs b/clippy_lints/src/lib.register_suspicious.rs
index 82f45b5fd58..7a881bfe399 100644
--- a/clippy_lints/src/lib.register_suspicious.rs
+++ b/clippy_lints/src/lib.register_suspicious.rs
@@ -5,6 +5,7 @@
 store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec![
     LintId::of(assign_ops::MISREFACTORED_ASSIGN_OP),
     LintId::of(attrs::BLANKET_CLIPPY_RESTRICTION_LINTS),
+    LintId::of(await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE),
     LintId::of(await_holding_invalid::AWAIT_HOLDING_LOCK),
     LintId::of(await_holding_invalid::AWAIT_HOLDING_REFCELL_REF),
     LintId::of(casts::CAST_ABS_TO_UNSIGNED),
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index 159e7ac683a..27c602818a6 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -515,7 +515,12 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
 
     store.register_late_pass(|| Box::new(utils::dump_hir::DumpHir));
     store.register_late_pass(|| Box::new(utils::author::Author));
-    store.register_late_pass(|| Box::new(await_holding_invalid::AwaitHolding));
+    let await_holding_invalid_types = conf.await_holding_invalid_types.clone();
+    store.register_late_pass(move || {
+        Box::new(await_holding_invalid::AwaitHolding::new(
+            await_holding_invalid_types.clone(),
+        ))
+    });
     store.register_late_pass(|| Box::new(serde_api::SerdeApi));
     let vec_box_size_threshold = conf.vec_box_size_threshold;
     let type_complexity_threshold = conf.type_complexity_threshold;
diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs
index 271c3a3dd18..e6abe499b43 100644
--- a/clippy_lints/src/utils/conf.rs
+++ b/clippy_lints/src/utils/conf.rs
@@ -310,6 +310,8 @@ define_Conf! {
     /// the slice pattern that is suggested. If more elements would be necessary, the lint is suppressed.
     /// For example, `[_, _, _, e, ..]` is a slice pattern with 4 elements.
     (max_suggested_slice_pattern_length: u64 = 3),
+    /// Lint: AWAIT_HOLDING_INVALID_TYPE
+    (await_holding_invalid_types: Vec<crate::utils::conf::DisallowedType> = Vec::new()),
 }
 
 /// Search for the configuration file.
diff --git a/tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.rs b/tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.rs
new file mode 100644
index 00000000000..fbef5c4564b
--- /dev/null
+++ b/tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.rs
@@ -0,0 +1,41 @@
+#![warn(clippy::await_holding_invalid_type)]
+use std::net::Ipv4Addr;
+
+async fn bad() -> u32 {
+    let _x = String::from("hello");
+    baz().await
+}
+
+async fn bad_reason() -> u32 {
+    let _x = Ipv4Addr::new(127, 0, 0, 1);
+    baz().await
+}
+
+async fn good() -> u32 {
+    {
+        let _x = String::from("hi!");
+        let _y = Ipv4Addr::new(127, 0, 0, 1);
+    }
+    baz().await;
+    let _x = String::from("hi!");
+    47
+}
+
+async fn baz() -> u32 {
+    42
+}
+
+#[allow(clippy::manual_async_fn)]
+fn block_bad() -> impl std::future::Future<Output = u32> {
+    async move {
+        let _x = String::from("hi!");
+        baz().await
+    }
+}
+
+fn main() {
+    good();
+    bad();
+    bad_reason();
+    block_bad();
+}
diff --git a/tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.stderr b/tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.stderr
new file mode 100644
index 00000000000..62c45b54634
--- /dev/null
+++ b/tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.stderr
@@ -0,0 +1,25 @@
+error: `std::string::String` may not be held across an `await` point per `clippy.toml`
+  --> $DIR/await_holding_invalid_type.rs:5:9
+   |
+LL |     let _x = String::from("hello");
+   |         ^^
+   |
+   = note: `-D clippy::await-holding-invalid-type` implied by `-D warnings`
+   = note: strings are bad
+
+error: `std::net::Ipv4Addr` may not be held across an `await` point per `clippy.toml`
+  --> $DIR/await_holding_invalid_type.rs:10:9
+   |
+LL |     let _x = Ipv4Addr::new(127, 0, 0, 1);
+   |         ^^
+
+error: `std::string::String` may not be held across an `await` point per `clippy.toml`
+  --> $DIR/await_holding_invalid_type.rs:31:13
+   |
+LL |         let _x = String::from("hi!");
+   |             ^^
+   |
+   = note: strings are bad
+
+error: aborting due to 3 previous errors
+
diff --git a/tests/ui-toml/await_holding_invalid_type/clippy.toml b/tests/ui-toml/await_holding_invalid_type/clippy.toml
new file mode 100644
index 00000000000..79990096b84
--- /dev/null
+++ b/tests/ui-toml/await_holding_invalid_type/clippy.toml
@@ -0,0 +1,4 @@
+await-holding-invalid-types = [
+    { path = "std::string::String", reason = "strings are bad" },
+    "std::net::Ipv4Addr",
+]
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 00ddbd608a7..f60afb8d4e3 100644
--- a/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
+++ b/tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr
@@ -1,4 +1,4 @@
-error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `max-suggested-slice-pattern-length`, `third-party` at line 5 column 1
+error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `avoid-breaking-exported-api`, `msrv`, `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `pass-by-value-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-trait-bounds`, `max-struct-bools`, `max-fn-params-bools`, `warn-on-all-wildcard-imports`, `disallowed-methods`, `disallowed-types`, `unreadable-literal-lint-fractions`, `upper-case-acronyms-aggressive`, `cargo-ignore-publish`, `standard-macro-braces`, `enforced-import-renames`, `allowed-scripts`, `enable-raw-pointer-heuristic-for-send`, `max-suggested-slice-pattern-length`, `await-holding-invalid-types`, `third-party` at line 5 column 1
 
 error: aborting due to previous error