diff --git a/src/tools/clippy/book/src/lint_configuration.md b/src/tools/clippy/book/src/lint_configuration.md index c87f8e9a68de1..64d0bf9b62f69 100644 --- a/src/tools/clippy/book/src/lint_configuration.md +++ b/src/tools/clippy/book/src/lint_configuration.md @@ -928,6 +928,7 @@ The minimum rust version that the project supports. Defaults to the `rust-versio * [`manual_let_else`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_let_else) * [`manual_midpoint`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_midpoint) * [`manual_non_exhaustive`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_non_exhaustive) +* [`manual_noop_waker`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_noop_waker) * [`manual_option_as_slice`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_option_as_slice) * [`manual_pattern_char_comparison`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_pattern_char_comparison) * [`manual_range_contains`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_range_contains) diff --git a/src/tools/clippy/clippy_config/src/conf.rs b/src/tools/clippy/clippy_config/src/conf.rs index 41099f94b0448..465e88a783ed8 100644 --- a/src/tools/clippy/clippy_config/src/conf.rs +++ b/src/tools/clippy/clippy_config/src/conf.rs @@ -789,6 +789,7 @@ define_Conf! { manual_let_else, manual_midpoint, manual_non_exhaustive, + manual_noop_waker, manual_option_as_slice, manual_pattern_char_comparison, manual_range_contains, diff --git a/src/tools/clippy/clippy_lints/src/lib.rs b/src/tools/clippy/clippy_lints/src/lib.rs index 68a8f51e7f4d3..61c54678c4b2a 100644 --- a/src/tools/clippy/clippy_lints/src/lib.rs +++ b/src/tools/clippy/clippy_lints/src/lib.rs @@ -866,7 +866,7 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co Box::new(move |_| Box::new(manual_take::ManualTake::new(conf))), Box::new(|_| Box::new(manual_checked_ops::ManualCheckedOps)), Box::new(move |tcx| Box::new(manual_pop_if::ManualPopIf::new(tcx, conf))), - Box::new(|_| Box::new(manual_noop_waker::ManualNoopWaker)), + Box::new(move |_| Box::new(manual_noop_waker::ManualNoopWaker::new(conf))), // add late passes here, used by `cargo dev new_lint` ]; store.late_passes.extend(late_lints); diff --git a/src/tools/clippy/clippy_lints/src/manual_noop_waker.rs b/src/tools/clippy/clippy_lints/src/manual_noop_waker.rs index c5de39dbf7f90..fb0e8a1d363f4 100644 --- a/src/tools/clippy/clippy_lints/src/manual_noop_waker.rs +++ b/src/tools/clippy/clippy_lints/src/manual_noop_waker.rs @@ -1,8 +1,10 @@ +use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::{is_empty_block, sym}; use rustc_hir::{ImplItemKind, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_session::declare_lint_pass; +use rustc_session::impl_lint_pass; declare_clippy_lint! { /// ### What it does @@ -35,7 +37,17 @@ declare_clippy_lint! { "manual implementations of noop wakers can be simplified using Waker::noop()" } -declare_lint_pass!(ManualNoopWaker => [MANUAL_NOOP_WAKER]); +impl_lint_pass!(ManualNoopWaker => [MANUAL_NOOP_WAKER]); + +pub struct ManualNoopWaker { + msrv: Msrv, +} + +impl ManualNoopWaker { + pub fn new(conf: &'static Conf) -> Self { + Self { msrv: conf.msrv } + } +} impl<'tcx> LateLintPass<'tcx> for ManualNoopWaker { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { @@ -43,6 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualNoopWaker { && let Some(trait_ref) = imp.of_trait && let Some(trait_id) = trait_ref.trait_ref.trait_def_id() && cx.tcx.is_diagnostic_item(sym::Wake, trait_id) + && self.msrv.meets(cx, msrvs::WAKER_NOOP) { for impl_item_ref in imp.items { let impl_item = cx diff --git a/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs b/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs index cb784d1ff6600..d86b05e5c8824 100644 --- a/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs +++ b/src/tools/clippy/clippy_lints/src/matches/collapsible_match.rs @@ -23,8 +23,20 @@ use super::{COLLAPSIBLE_MATCH, pat_contains_disallowed_or}; pub(super) fn check_match<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arms: &'tcx [Arm<'_>], msrv: Msrv) { if let Some(els_arm) = arms.iter().rfind(|arm| arm_is_wild_like(cx, arm)) { - for arm in arms { - check_arm(cx, true, arm.pat, expr, arm.body, arm.guard, Some(els_arm.body), msrv); + let last_non_wildcard = arms.iter().rposition(|arm| !arm_is_wild_like(cx, arm)); + for (idx, arm) in arms.iter().enumerate() { + let only_wildcards_after = last_non_wildcard.is_none_or(|lnw| idx >= lnw); + check_arm( + cx, + true, + arm.pat, + expr, + arm.body, + arm.guard, + Some(els_arm.body), + msrv, + only_wildcards_after, + ); } } } @@ -37,7 +49,7 @@ pub(super) fn check_if_let<'tcx>( let_expr: &'tcx Expr<'_>, msrv: Msrv, ) { - check_arm(cx, false, pat, let_expr, body, None, else_expr, msrv); + check_arm(cx, false, pat, let_expr, body, None, else_expr, msrv, false); } #[expect(clippy::too_many_arguments, clippy::too_many_lines)] @@ -50,6 +62,7 @@ fn check_arm<'tcx>( outer_guard: Option<&'tcx Expr<'tcx>>, outer_else_body: Option<&'tcx Expr<'tcx>>, msrv: Msrv, + only_wildcards_after: bool, ) { let inner_expr = peel_blocks_with_stmt(outer_then_body); if let Some(inner) = IfLetOrMatch::parse(cx, inner_expr) @@ -126,6 +139,7 @@ fn check_arm<'tcx>( ); }); } else if outer_is_match // Leave if-let to the `collapsible_if` lint + && only_wildcards_after // adding a guard allows fall-through; unsafe if other arms follow && let Some(inner) = If::hir(inner_expr) && outer_pat.span.eq_ctxt(inner.cond.span) && match (outer_else_body, inner.r#else) { diff --git a/src/tools/clippy/clippy_lints/src/methods/mod.rs b/src/tools/clippy/clippy_lints/src/methods/mod.rs index b39aec6e521ce..80dce6a53640a 100644 --- a/src/tools/clippy/clippy_lints/src/methods/mod.rs +++ b/src/tools/clippy/clippy_lints/src/methods/mod.rs @@ -5217,7 +5217,6 @@ impl Methods { format_collect::check(cx, expr, m_arg, m_ident_span); }, Some((sym::take, take_self_arg, [take_arg], _, _)) => { - #[expect(clippy::collapsible_match)] if self.msrv.meets(cx, msrvs::STR_REPEAT) { manual_str_repeat::check(cx, expr, recv, take_self_arg, take_arg); } @@ -5542,9 +5541,7 @@ impl Methods { (sym::open, [_]) => { open_options::check(cx, expr, recv); }, - (sym::or_else, [arg]) => - { - #[expect(clippy::collapsible_match)] + (sym::or_else, [arg]) => { if !bind_instead_of_map::check_or_else_err(cx, expr, recv, arg) { unnecessary_lazy_eval::check(cx, expr, recv, arg, "or"); } @@ -5649,9 +5646,7 @@ impl Methods { (sym::try_into, []) if cx.ty_based_def(expr).opt_parent(cx).is_diag_item(cx, sym::TryInto) => { unnecessary_fallible_conversions::check_method(cx, expr); }, - (sym::to_owned, []) => - { - #[expect(clippy::collapsible_match)] + (sym::to_owned, []) => { if !suspicious_to_owned::check(cx, expr, span) { implicit_clone::check(cx, name, expr, recv); } diff --git a/src/tools/clippy/clippy_utils/src/msrvs.rs b/src/tools/clippy/clippy_utils/src/msrvs.rs index 4f9a064bf7a6a..a56e729c70bbb 100644 --- a/src/tools/clippy/clippy_utils/src/msrvs.rs +++ b/src/tools/clippy/clippy_utils/src/msrvs.rs @@ -28,7 +28,7 @@ msrv_aliases! { 1,88,0 { LET_CHAINS } 1,87,0 { OS_STR_DISPLAY, INT_MIDPOINT, CONST_CHAR_IS_DIGIT, UNSIGNED_IS_MULTIPLE_OF, INTEGER_SIGN_CAST } 1,86,0 { VEC_POP_IF } - 1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL } + 1,85,0 { UINT_FLOAT_MIDPOINT, CONST_SIZE_OF_VAL, WAKER_NOOP } 1,84,0 { CONST_OPTION_AS_SLICE, MANUAL_DANGLING_PTR } 1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_MUT_REFS, CONST_UNWRAP } 1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP, SPECIALIZED_TO_STRING_FOR_REFS } diff --git a/src/tools/clippy/tests/ui/collapsible_match.rs b/src/tools/clippy/tests/ui/collapsible_match.rs index 98f2fcfdf479f..51cb0168c735b 100644 --- a/src/tools/clippy/tests/ui/collapsible_match.rs +++ b/src/tools/clippy/tests/ui/collapsible_match.rs @@ -390,6 +390,26 @@ fn take(t: T) {} fn main() {} +// https://github.com/rust-lang/rust-clippy/issues/16875 +// Adding a match guard allows fall-through to subsequent arms, which changes semantics +// when non-wildcard arms follow the arm being collapsed. +fn issue16875(a: Option<&str>, b: i32) -> i32 { + let mut res = 0; + // should NOT lint: `_ if b == 1` is not wild-like (has a guard), so collapsing + // `Some(_)` into `Some(_) if b == 0` would let `_ if b == 1` match Some values + // that previously fell through to the no-op arm body. + match a { + Some(_) => { + if b == 0 { + res = 1; + } + }, + _ if b == 1 => res = 2, + _ => {}, + } + res +} + fn issue16705(x: Option) { fn takes_ownership(s: String) -> bool { true diff --git a/src/tools/clippy/tests/ui/collapsible_match_fixable.fixed b/src/tools/clippy/tests/ui/collapsible_match_fixable.fixed index db76530aee144..8e5934d3693a3 100644 --- a/src/tools/clippy/tests/ui/collapsible_match_fixable.fixed +++ b/src/tools/clippy/tests/ui/collapsible_match_fixable.fixed @@ -28,3 +28,18 @@ fn issue16558() { _ => 1, }; } + +// https://github.com/rust-lang/rust-clippy/issues/16875 +// lint still fires when only wildcard-like arms follow (fall-through is harmless) +fn issue16875(a: Option<&str>, b: i32) -> i32 { + let mut res = 0; + match a { + Some(_) + if b == 0 => { + //~^ collapsible_match + res = 1; + }, + _ => {}, + } + res +} diff --git a/src/tools/clippy/tests/ui/collapsible_match_fixable.rs b/src/tools/clippy/tests/ui/collapsible_match_fixable.rs index 94bf1d6bfdfab..e2ccae4559612 100644 --- a/src/tools/clippy/tests/ui/collapsible_match_fixable.rs +++ b/src/tools/clippy/tests/ui/collapsible_match_fixable.rs @@ -29,3 +29,19 @@ fn issue16558() { _ => 1, }; } + +// https://github.com/rust-lang/rust-clippy/issues/16875 +// lint still fires when only wildcard-like arms follow (fall-through is harmless) +fn issue16875(a: Option<&str>, b: i32) -> i32 { + let mut res = 0; + match a { + Some(_) => { + if b == 0 { + //~^ collapsible_match + res = 1; + } + }, + _ => {}, + } + res +} diff --git a/src/tools/clippy/tests/ui/collapsible_match_fixable.stderr b/src/tools/clippy/tests/ui/collapsible_match_fixable.stderr index 4d501cbd0993d..0be47076fa472 100644 --- a/src/tools/clippy/tests/ui/collapsible_match_fixable.stderr +++ b/src/tools/clippy/tests/ui/collapsible_match_fixable.stderr @@ -46,5 +46,23 @@ LL | LL ~ , | -error: aborting due to 3 previous errors +error: this `if` can be collapsed into the outer `match` + --> tests/ui/collapsible_match_fixable.rs:39:13 + | +LL | / if b == 0 { +LL | | +LL | | res = 1; +LL | | } + | |_____________^ + | +help: collapse nested if block + | +LL ~ Some(_) +LL ~ if b == 0 => { +LL | +LL | res = 1; +LL ~ }, + | + +error: aborting due to 4 previous errors diff --git a/src/tools/clippy/tests/ui/manual_noop_waker.rs b/src/tools/clippy/tests/ui/manual_noop_waker.rs index 9b4dd90e273cf..ecfbffeb008f4 100644 --- a/src/tools/clippy/tests/ui/manual_noop_waker.rs +++ b/src/tools/clippy/tests/ui/manual_noop_waker.rs @@ -38,3 +38,28 @@ mod custom_module { fn wake_by_ref(self: &Arc) {} } } + +#[clippy::msrv = "1.84"] +mod msrv_1_84 { + use std::sync::Arc; + use std::task::Wake; + + struct CustomWaker; + impl Wake for CustomWaker { + fn wake(self: Arc) {} + fn wake_by_ref(self: &Arc) {} + } +} + +#[clippy::msrv = "1.85"] +mod msrv_1_85 { + use std::sync::Arc; + use std::task::Wake; + + struct CustomWaker; + impl Wake for CustomWaker { + //~^ manual_noop_waker + fn wake(self: Arc) {} + fn wake_by_ref(self: &Arc) {} + } +} diff --git a/src/tools/clippy/tests/ui/manual_noop_waker.stderr b/src/tools/clippy/tests/ui/manual_noop_waker.stderr index b3b30f96a08f1..4a01a4d0b47aa 100644 --- a/src/tools/clippy/tests/ui/manual_noop_waker.stderr +++ b/src/tools/clippy/tests/ui/manual_noop_waker.stderr @@ -16,5 +16,13 @@ LL | impl Wake for MyWakerPartial { | = help: use `std::task::Waker::noop()` instead -error: aborting due to 2 previous errors +error: manual implementation of a no-op waker + --> tests/ui/manual_noop_waker.rs:60:10 + | +LL | impl Wake for CustomWaker { + | ^^^^ + | + = help: use `std::task::Waker::noop()` instead + +error: aborting due to 3 previous errors