Skip to content

Commit 0c0525e

Browse files
committed
fix: search_is_some suggests wrongly inside macro
1 parent 62fd159 commit 0c0525e

File tree

4 files changed

+40
-4
lines changed

4 files changed

+40
-4
lines changed

clippy_utils/src/sugg.rs

Lines changed: 19 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;
@@ -743,8 +743,10 @@ pub fn deref_closure_args(cx: &LateContext<'_>, closure: &hir::Expr<'_>) -> Opti
743743
let mut visitor = DerefDelegate {
744744
cx,
745745
closure_span: closure.span,
746+
closure_arg_id: closure_body.params[0].pat.hir_id,
746747
closure_arg_is_type_annotated_double_ref,
747748
next_pos: closure.span.lo(),
749+
checked_borrows: FxHashSet::default(),
748750
suggestion_start: String::new(),
749751
applicability: Applicability::MachineApplicable,
750752
};
@@ -770,10 +772,15 @@ struct DerefDelegate<'a, 'tcx> {
770772
cx: &'a LateContext<'tcx>,
771773
/// The span of the input closure to adapt
772774
closure_span: Span,
775+
/// The `hir_id` of the closure argument being checked
776+
closure_arg_id: HirId,
773777
/// Indicates if the arg of the closure is a type annotated double reference
774778
closure_arg_is_type_annotated_double_ref: bool,
775779
/// last position of the span to gradually build the suggestion
776780
next_pos: BytePos,
781+
/// `hir_id` that has been checked. This is used to avoid checking the same hir_id multiple
782+
/// times when inside macro expansions.
783+
checked_borrows: FxHashSet<HirId>,
777784
/// starting part of the gradually built suggestion
778785
suggestion_start: String,
779786
/// confidence on the built suggestion
@@ -840,6 +847,11 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
840847
fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {
841848
if let PlaceBase::Local(id) = cmt.place.base {
842849
let span = self.cx.tcx.hir_span(cmt.hir_id);
850+
if !self.checked_borrows.insert(cmt.hir_id) {
851+
// already checked this span and hir_id, skip
852+
return;
853+
}
854+
843855
let start_span = Span::new(self.next_pos, span.lo(), span.ctxt(), None);
844856
let mut start_snip = snippet_with_applicability(self.cx, start_span, "..", &mut self.applicability);
845857

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

851-
if cmt.place.projections.is_empty() {
863+
if let Node::Pat(pat) = self.cx.tcx.hir_node(id)
864+
&& pat.hir_id != self.closure_arg_id
865+
{
866+
let _ = write!(self.suggestion_start, "{start_snip}{ident_str_with_proj}");
867+
} else if cmt.place.projections.is_empty() {
852868
// handle item without any projection, that needs an explicit borrowing
853869
// i.e.: suggest `&x` instead of `x`
854870
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)