From f6876f2b9682a3635a403a9d1049ddd94a24eb3e Mon Sep 17 00:00:00 2001 From: John Costa Date: Tue, 7 Jul 2026 12:31:35 -0700 Subject: [PATCH] manual_filter: don't eat comments in the `and_then` suggestion The `opt.and_then(|x| { /* comment */ if .. { Some(x) } else { None } })` to `filter` rewrite drops any comment inside the closure. Mark the suggestion `MaybeIncorrect` when the replaced span contains a comment, so `clippy --fix` no longer applies it silently. This matches the existing `span_contains_comment` handling in manual_ok_err / manual_unwrap_or / manual_flatten. --- clippy_lints/src/matches/manual_filter.rs | 10 ++++++++-- tests/ui/manual_filter_unfixable.rs | 15 +++++++++++++++ tests/ui/manual_filter_unfixable.stderr | 16 ++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) create mode 100644 tests/ui/manual_filter_unfixable.rs create mode 100644 tests/ui/manual_filter_unfixable.stderr diff --git a/clippy_lints/src/matches/manual_filter.rs b/clippy_lints/src/matches/manual_filter.rs index 9c2d60eb9c57..942a08bf07c4 100644 --- a/clippy_lints/src/matches/manual_filter.rs +++ b/clippy_lints/src/matches/manual_filter.rs @@ -1,10 +1,10 @@ -use clippy_utils::as_some_expr; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::res::{MaybeDef, MaybeQPath, MaybeResPath}; use clippy_utils::source::snippet_with_context; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_copy; use clippy_utils::visitors::contains_unsafe_block; +use clippy_utils::{as_some_expr, span_contains_comment}; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionNone; use rustc_hir::{Arm, Expr, ExprKind, HirId, Pat, PatKind}; @@ -118,7 +118,13 @@ pub(crate) fn check_and_then_method<'tcx>( call_span, "manual implementation of `Option::filter`", |diag| { - let mut applicability = Applicability::MachineApplicable; + // A comment inside the replaced `and_then` closure (for example, one explaining the + // predicate) would be dropped by the rewrite, so don't auto-apply the fix then (#17376). + let mut applicability = if span_contains_comment(cx, call_span) { + Applicability::MaybeIncorrect + } else { + Applicability::MachineApplicable + }; let mut cond_snip = Sugg::hir_with_context(cx, some_expr.expr, expr_span_ctxt, "..", &mut applicability); diff --git a/tests/ui/manual_filter_unfixable.rs b/tests/ui/manual_filter_unfixable.rs new file mode 100644 index 000000000000..0212e2fbd450 --- /dev/null +++ b/tests/ui/manual_filter_unfixable.rs @@ -0,0 +1,15 @@ +//@no-rustfix: the suggestion drops the closure's comment, so it is not machine-applicable + +#![warn(clippy::manual_filter)] + +fn issue17376(opt: Option) { + // Rewriting to `filter` would drop the comment below, so the suggestion must not be + // machine-applicable here. Otherwise `clippy --fix` would silently eat the comment (#17376). + opt.and_then(|x| { + //~^ manual_filter + // keep this explanation + if x < 10 { Some(x) } else { None } + }); +} + +fn main() {} diff --git a/tests/ui/manual_filter_unfixable.stderr b/tests/ui/manual_filter_unfixable.stderr new file mode 100644 index 000000000000..00b267e97ac4 --- /dev/null +++ b/tests/ui/manual_filter_unfixable.stderr @@ -0,0 +1,16 @@ +error: manual implementation of `Option::filter` + --> tests/ui/manual_filter_unfixable.rs:8:9 + | +LL | opt.and_then(|x| { + | _________^ +LL | | +LL | | // keep this explanation +LL | | if x < 10 { Some(x) } else { None } +LL | | }); + | |______^ help: try: `filter(|&x| x < 10)` + | + = note: `-D clippy::manual-filter` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::manual_filter)]` + +error: aborting due to 1 previous error +