Skip to content

Commit b7cc5c4

Browse files
authored
Fix search_is_some suggests wrongly inside macro (#15135)
Closes #15102 changelog: [`search_is_some`] fix wrong suggestions inside macro
2 parents 7b3bd5b + 222ebd0 commit b7cc5c4

File tree

6 files changed

+73
-5
lines changed

6 files changed

+73
-5
lines changed

clippy_utils/src/sugg.rs

Lines changed: 21 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`s 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,12 @@ 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+
// Make sure to get in all projections if we're on a `matches!`
875+
if let Node::Pat(pat) = self.cx.tcx.hir_node(id)
876+
&& pat.hir_id != self.closure_arg_id
877+
{
878+
let _ = write!(self.suggestion_start, "{start_snip}{ident_str_with_proj}");
879+
} else if cmt.place.projections.is_empty() {
862880
// handle item without any projection, that needs an explicit borrowing
863881
// i.e.: suggest `&x` instead of `x`
864882
let _: fmt::Result = write!(self.suggestion_start, "{start_snip}&{ident_str}");

tests/ui/search_is_some.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,3 +87,18 @@ fn is_none() {
8787
let _ = (0..1).find(some_closure).is_none();
8888
//~^ search_is_some
8989
}
90+
91+
#[allow(clippy::match_like_matches_macro)]
92+
fn issue15102() {
93+
let values = [None, Some(3)];
94+
let has_even = values
95+
//~^ search_is_some
96+
.iter()
97+
.find(|v| match v {
98+
Some(x) if x % 2 == 0 => true,
99+
_ => false,
100+
})
101+
.is_some();
102+
103+
println!("{has_even}");
104+
}

tests/ui/search_is_some.stderr

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,5 +90,20 @@ error: called `is_none()` after searching an `Iterator` with `find`
9090
LL | let _ = (0..1).find(some_closure).is_none();
9191
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `!(0..1).any(some_closure)`
9292

93-
error: aborting due to 8 previous errors
93+
error: called `is_some()` after searching an `Iterator` with `find`
94+
--> tests/ui/search_is_some.rs:94:20
95+
|
96+
LL | let has_even = values
97+
| ____________________^
98+
LL | |
99+
LL | | .iter()
100+
LL | | .find(|v| match v {
101+
... |
102+
LL | | })
103+
LL | | .is_some();
104+
| |__________________^
105+
|
106+
= help: this is more succinctly expressed by calling `any()`
107+
108+
error: aborting due to 9 previous errors
94109

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)