From 4844325faf392beb47c6e6d3231dfbba55f926c2 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Thu, 14 Apr 2022 16:52:10 -0700 Subject: [PATCH 01/10] 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. --- CHANGELOG.md | 1 + Cargo.toml | 1 + clippy_lints/src/await_holding_invalid.rs | 165 ++++++++++++++---- clippy_lints/src/lib.register_all.rs | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_suspicious.rs | 1 + clippy_lints/src/lib.rs | 7 +- clippy_lints/src/utils/conf.rs | 2 + tests/compile-test.rs | 4 + .../await_holding_invalid_type.rs | 41 +++++ .../await_holding_invalid_type.stderr | 25 +++ .../await_holding_invalid_type/clippy.toml | 4 + .../toml_unknown_key/conf_unknown_key.stderr | 2 +- 13 files changed, 215 insertions(+), 40 deletions(-) create mode 100644 tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.rs create mode 100644 tests/ui-toml/await_holding_invalid_type/await_holding_invalid_type.stderr create mode 100644 tests/ui-toml/await_holding_invalid_type/clippy.toml diff --git a/CHANGELOG.md b/CHANGELOG.md index 39786c08156..7aa4884d103 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3182,6 +3182,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/Cargo.toml b/Cargo.toml index dd6518d5241..1fb532deb9b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,6 +50,7 @@ futures = "0.3" parking_lot = "0.11.2" tokio = { version = "1", features = ["io-util"] } rustc-semver = "1.1" +tracing = "0.1" [build-dependencies] rustc_tools_util = { version = "0.2", path = "rustc_tools_util" } diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 4592ca72748..36b4f7faa19 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,73 @@ 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 + /// + /// The `tracing` library has types which should not be held across `await` + /// points. + /// + /// ```toml + /// await-holding-invalid-types = [ + /// "tracing::span::Entered", + /// "tracing::span::EnteredSpan", + /// ] + /// ``` + /// + /// ```rust + /// # async fn baz() {} + /// async fn foo() { + /// let _entered = tracing::info_span!("baz").entered(); + /// baz().await; + /// } + /// ``` + #[clippy::version = "1.49.0"] + pub AWAIT_HOLDING_INVALID_TYPE, + suspicious, + "inside an async function, holding a type across an await point which is not safe to be held across an await point" +} + +impl_lint_pass!(AwaitHolding => [AWAIT_HOLDING_LOCK, AWAIT_HOLDING_REFCELL_REF, AWAIT_HOLDING_INVALID_TYPE]); + +#[derive(Debug)] +pub struct AwaitHolding { + conf_invalid_types: Vec, + def_ids: FxHashMap, +} + +impl AwaitHolding { + pub(crate) fn new(conf_invalid_types: Vec) -> 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 +204,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 +213,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 according to config",), + |diag| { + if let Some(reason) = reason { + diag.note(format!("{reason} (according to clippy.toml)")); + } + }, + ); +} + 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 2fa59733365..be57e1038fd 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 c608f634291..8a97233dc5d 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 88e8a0cc2af..e31f71fe5da 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::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 = Vec::new()), } /// Search for the configuration file. diff --git a/tests/compile-test.rs b/tests/compile-test.rs index c9710e3db8e..f437553bca1 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -35,6 +35,7 @@ static TEST_DEPENDENCIES: &[&str] = &[ "tokio", "parking_lot", "rustc_semver", + "tracing", ]; // Test dependencies may need an `extern crate` here to ensure that they show up @@ -59,6 +60,9 @@ extern crate rustc_semver; extern crate syn; #[allow(unused_extern_crates)] extern crate tokio; +#[allow(unused_extern_crates, unused_imports)] +#[macro_use] +extern crate tracing; /// Produces a string with an `--extern` flag for all UI test crate /// dependencies. 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 { + 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..eebdd624d24 --- /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 according to config + --> $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 (according to clippy.toml) + +error: `std::net::Ipv4Addr` may not be held across an `await` point according to config + --> $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 according to config + --> $DIR/await_holding_invalid_type.rs:31:13 + | +LL | let _x = String::from("hi!"); + | ^^ + | + = note: strings are bad (according to clippy.toml) + +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 From 3cd8b5a5ef81b43434f1fd0e3574263a286d152a Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Fri, 15 Apr 2022 14:45:58 -0700 Subject: [PATCH 02/10] fixup! Add `await_holding_invalid_type` lint --- Cargo.toml | 1 - tests/compile-test.rs | 4 ---- 2 files changed, 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1fb532deb9b..dd6518d5241 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -50,7 +50,6 @@ futures = "0.3" parking_lot = "0.11.2" tokio = { version = "1", features = ["io-util"] } rustc-semver = "1.1" -tracing = "0.1" [build-dependencies] rustc_tools_util = { version = "0.2", path = "rustc_tools_util" } diff --git a/tests/compile-test.rs b/tests/compile-test.rs index f437553bca1..c9710e3db8e 100644 --- a/tests/compile-test.rs +++ b/tests/compile-test.rs @@ -35,7 +35,6 @@ static TEST_DEPENDENCIES: &[&str] = &[ "tokio", "parking_lot", "rustc_semver", - "tracing", ]; // Test dependencies may need an `extern crate` here to ensure that they show up @@ -60,9 +59,6 @@ extern crate rustc_semver; extern crate syn; #[allow(unused_extern_crates)] extern crate tokio; -#[allow(unused_extern_crates, unused_imports)] -#[macro_use] -extern crate tracing; /// Produces a string with an `--extern` flag for all UI test crate /// dependencies. From 534419282e94e618054815e643932f6f39e48000 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Fri, 15 Apr 2022 14:54:19 -0700 Subject: [PATCH 03/10] fixup! Add `await_holding_invalid_type` lint --- clippy_lints/src/await_holding_invalid.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 36b4f7faa19..51dd5f4937a 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -144,21 +144,21 @@ declare_clippy_lint! { /// /// ### Example /// - /// The `tracing` library has types which should not be held across `await` - /// points. + /// Strings are, of course, safe to hold across await points. This example + /// exists only to show how this lint might be configured for third-party + /// crates. /// /// ```toml /// await-holding-invalid-types = [ - /// "tracing::span::Entered", - /// "tracing::span::EnteredSpan", + /// "std::string::String", /// ] /// ``` /// /// ```rust /// # async fn baz() {} /// async fn foo() { - /// let _entered = tracing::info_span!("baz").entered(); - /// baz().await; + /// let _x = String::from("hello!"); + /// baz().await; // Lint violation /// } /// ``` #[clippy::version = "1.49.0"] From 6d0baf2b16c709284868c3e3b0809cc336bc6243 Mon Sep 17 00:00:00 2001 From: Lily Mara <54288692+lilymara-onesignal@users.noreply.github.com> Date: Mon, 18 Apr 2022 10:31:57 -0700 Subject: [PATCH 04/10] Update clippy_lints/src/await_holding_invalid.rs Co-authored-by: llogiq --- clippy_lints/src/await_holding_invalid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 51dd5f4937a..8f2b67176ec 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -164,7 +164,7 @@ declare_clippy_lint! { #[clippy::version = "1.49.0"] pub AWAIT_HOLDING_INVALID_TYPE, suspicious, - "inside an async function, holding a type across an await point which is not safe to be held across an await point" + "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]); From 228ce783bc75b778272958c3a4d3b8b990a74401 Mon Sep 17 00:00:00 2001 From: Lily Mara <54288692+lilymara-onesignal@users.noreply.github.com> Date: Mon, 18 Apr 2022 10:32:03 -0700 Subject: [PATCH 05/10] Update clippy_lints/src/await_holding_invalid.rs Co-authored-by: llogiq --- clippy_lints/src/await_holding_invalid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 8f2b67176ec..24c9f49d9f8 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -266,7 +266,7 @@ fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedTy cx, AWAIT_HOLDING_INVALID_TYPE, span, - &format!("`{type_name}` may not be held across an `await` point according to config",), + &format!("`{type_name}` may not be held across an `await` point per `clippy.toml`",), |diag| { if let Some(reason) = reason { diag.note(format!("{reason} (according to clippy.toml)")); From ee3ebb35f4f077176657b11d2b05b37f23742baf Mon Sep 17 00:00:00 2001 From: Lily Mara <54288692+lilymara-onesignal@users.noreply.github.com> Date: Mon, 18 Apr 2022 10:32:11 -0700 Subject: [PATCH 06/10] Update clippy_lints/src/await_holding_invalid.rs Co-authored-by: llogiq --- clippy_lints/src/await_holding_invalid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 24c9f49d9f8..be905f67ec8 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -269,7 +269,7 @@ fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedTy &format!("`{type_name}` may not be held across an `await` point per `clippy.toml`",), |diag| { if let Some(reason) = reason { - diag.note(format!("{reason} (according to clippy.toml)")); + diag.note(reason.clone()); } }, ); From a511072c2cb29f4a30fbd357c7e4b6771b9f6a29 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Mon, 18 Apr 2022 10:34:17 -0700 Subject: [PATCH 07/10] fixup! Add `await_holding_invalid_type` lint --- clippy_lints/src/await_holding_invalid.rs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index be905f67ec8..5659651c842 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -144,20 +144,17 @@ declare_clippy_lint! { /// /// ### Example /// - /// Strings are, of course, safe to hold across await points. This example - /// exists only to show how this lint might be configured for third-party - /// crates. - /// /// ```toml /// await-holding-invalid-types = [ - /// "std::string::String", + /// "CustomLockType", /// ] /// ``` /// /// ```rust /// # async fn baz() {} + /// struct CustomLockType; /// async fn foo() { - /// let _x = String::from("hello!"); + /// let _x = CustomLockType; /// baz().await; // Lint violation /// } /// ``` From 7e26edce65df4e89be23aa22bcb3d3a2db723bf7 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Mon, 18 Apr 2022 11:16:35 -0700 Subject: [PATCH 08/10] fixup! Add `await_holding_invalid_type` lint --- clippy_lints/src/await_holding_invalid.rs | 5 +++++ .../await_holding_invalid_type.stderr | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 5659651c842..022f82ebf51 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -146,15 +146,20 @@ declare_clippy_lint! { /// /// ```toml /// await-holding-invalid-types = [ + /// # You can specify a type name /// "CustomLockType", + /// # You can (optionally) specify a reason + /// { type = "OtherCustomLockType", reason = "Relies on a thread local" } /// ] /// ``` /// /// ```rust /// # async fn baz() {} /// struct CustomLockType; + /// struct OtherCustomLockType; /// async fn foo() { /// let _x = CustomLockType; + /// let _y = CustomLockType; /// baz().await; // Lint violation /// } /// ``` 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 index eebdd624d24..62c45b54634 100644 --- 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 @@ -1,25 +1,25 @@ -error: `std::string::String` may not be held across an `await` point according to config +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 (according to clippy.toml) + = note: strings are bad -error: `std::net::Ipv4Addr` may not be held across an `await` point according to config +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 according to config +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 (according to clippy.toml) + = note: strings are bad error: aborting due to 3 previous errors From 8eccbbed68bbc4ad8c5f0ab17d905359e88e30fa Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Mon, 18 Apr 2022 11:30:59 -0700 Subject: [PATCH 09/10] fixup! Add `await_holding_invalid_type` lint --- clippy_lints/src/await_holding_invalid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 022f82ebf51..5c29afb947b 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -159,7 +159,7 @@ declare_clippy_lint! { /// struct OtherCustomLockType; /// async fn foo() { /// let _x = CustomLockType; - /// let _y = CustomLockType; + /// let _y = OtherCustomLockType; /// baz().await; // Lint violation /// } /// ``` From 05086858c4058400053e67d53d647ffa43945530 Mon Sep 17 00:00:00 2001 From: Lily Mara Date: Mon, 18 Apr 2022 11:32:28 -0700 Subject: [PATCH 10/10] fixup! Add `await_holding_invalid_type` lint --- clippy_lints/src/await_holding_invalid.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 5c29afb947b..5b7c4591504 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -149,7 +149,7 @@ declare_clippy_lint! { /// # You can specify a type name /// "CustomLockType", /// # You can (optionally) specify a reason - /// { type = "OtherCustomLockType", reason = "Relies on a thread local" } + /// { path = "OtherCustomLockType", reason = "Relies on a thread local" } /// ] /// ``` ///