Skip to content

Commit 7d8ef2d

Browse files
committed
fix: search_is_some suggests wrongly inside macro
1 parent 7546381 commit 7d8ef2d

File tree

4 files changed

+41
-4
lines changed

4 files changed

+41
-4
lines changed

clippy_utils/src/sugg.rs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@ use crate::ty::expr_sig;
66
use crate::{get_parent_expr_for_hir, higher};
77
use rustc_ast::ast;
88
use rustc_ast::util::parser::AssocOp;
9+
use rustc_data_structures::fx::FxHashSet;
910
use rustc_errors::Applicability;
10-
use rustc_hir as hir;
11-
use rustc_hir::{Closure, ExprKind, HirId, MutTy, TyKind};
11+
use rustc_hir::{self as hir, Closure, ExprKind, HirId, MutTy, Node, TyKind};
1212
use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
1313
use rustc_lint::{EarlyContext, LateContext, LintContext};
1414
use rustc_middle::hir::place::ProjectionKind;
@@ -753,8 +753,10 @@ pub fn deref_closure_args(cx: &LateContext<'_>, closure: &hir::Expr<'_>) -> Opti
753753
let mut visitor = DerefDelegate {
754754
cx,
755755
closure_span: closure.span,
756+
closure_arg_id: closure_body.params[0].pat.hir_id,
756757
closure_arg_is_type_annotated_double_ref,
757758
next_pos: closure.span.lo(),
759+
checked_borrows: FxHashSet::default(),
758760
suggestion_start: String::new(),
759761
applicability: Applicability::MachineApplicable,
760762
};
@@ -780,10 +782,15 @@ struct DerefDelegate<'a, 'tcx> {
780782
cx: &'a LateContext<'tcx>,
781783
/// The span of the input closure to adapt
782784
closure_span: Span,
785+
/// The `hir_id` of the closure argument being checked
786+
closure_arg_id: HirId,
783787
/// Indicates if the arg of the closure is a type annotated double reference
784788
closure_arg_is_type_annotated_double_ref: bool,
785789
/// last position of the span to gradually build the suggestion
786790
next_pos: BytePos,
791+
/// `hir_id` that has been checked. This is used to avoid checking the same `hir_id` multiple
792+
/// times when inside macro expansions.
793+
checked_borrows: FxHashSet<HirId>,
787794
/// starting part of the gradually built suggestion
788795
suggestion_start: String,
789796
/// confidence on the built suggestion
@@ -847,9 +854,15 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
847854

848855
fn use_cloned(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
849856

857+
#[expect(clippy::too_many_lines)]
850858
fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {
851859
if let PlaceBase::Local(id) = cmt.place.base {
852860
let span = self.cx.tcx.hir_span(cmt.hir_id);
861+
if !self.checked_borrows.insert(cmt.hir_id) {
862+
// already checked this span and hir_id, skip
863+
return;
864+
}
865+
853866
let start_span = Span::new(self.next_pos, span.lo(), span.ctxt(), None);
854867
let mut start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
855868

@@ -858,7 +871,11 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
858871
// full identifier that includes projection (i.e.: `fp.field`)
859872
let ident_str_with_proj = snippet(self.cx, span, "..").to_string();
860873

861-
if cmt.place.projections.is_empty() {
874+
if let Node::Pat(pat) = self.cx.tcx.hir_node(id)
875+
&& pat.hir_id != self.closure_arg_id
876+
{
877+
let _ = write!(self.suggestion_start, "{start_snip}{ident_str_with_proj}");
878+
} else if cmt.place.projections.is_empty() {
862879
// handle item without any projection, that needs an explicit borrowing
863880
// i.e.: suggest `&x` instead of `x`
864881
let _: fmt::Result = write!(self.suggestion_start, "{start_snip}&{ident_str}");

tests/ui/search_is_some_fixable_some.fixed

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,3 +289,10 @@ mod issue9120 {
289289
//~^ search_is_some
290290
}
291291
}
292+
293+
fn issue15102() {
294+
let values = [None, Some(3)];
295+
let has_even = values.iter().any(|v| matches!(&v, Some(x) if x % 2 == 0));
296+
//~^ search_is_some
297+
println!("{has_even}");
298+
}

tests/ui/search_is_some_fixable_some.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -297,3 +297,10 @@ mod issue9120 {
297297
//~^ search_is_some
298298
}
299299
}
300+
301+
fn issue15102() {
302+
let values = [None, Some(3)];
303+
let has_even = values.iter().find(|v| matches!(v, Some(x) if x % 2 == 0)).is_some();
304+
//~^ search_is_some
305+
println!("{has_even}");
306+
}

tests/ui/search_is_some_fixable_some.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,5 +289,11 @@ error: called `is_some()` after searching an `Iterator` with `find`
289289
LL | let _ = v.iter().find(|x: &&u32| (*arg_no_deref_dyn)(x)).is_some();
290290
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(|x: &u32| (*arg_no_deref_dyn)(&x))`
291291

292-
error: aborting due to 46 previous errors
292+
error: called `is_some()` after searching an `Iterator` with `find`
293+
--> tests/ui/search_is_some_fixable_some.rs:303:34
294+
|
295+
LL | let has_even = values.iter().find(|v| matches!(v, Some(x) if x % 2 == 0)).is_some();
296+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `any(|v| matches!(&v, Some(x) if x % 2 == 0))`
297+
298+
error: aborting due to 47 previous errors
293299

0 commit comments

Comments
 (0)