Skip to content

Commit 86a10f0

Browse files
committed
fix: double_ended_iterator_last FP when iter has side effects
1 parent 781fdab commit 86a10f0

File tree

4 files changed

+56
-1
lines changed

4 files changed

+56
-1
lines changed

clippy_lints/src/methods/double_ended_iterator_last.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::ty::implements_trait;
2+
use clippy_utils::ty::{implements_trait, is_iter_with_side_effects};
33
use clippy_utils::{is_mutable, is_trait_method, path_to_local};
44
use rustc_errors::Applicability;
55
use rustc_hir::{Expr, Node, PatKind};
@@ -27,6 +27,8 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &'_ Expr<'_>, self_expr: &'_ Exp
2727
&& let Some(last_def) = cx.tcx.provided_trait_methods(item).find(|m| m.name().as_str() == "last")
2828
// if the resolved method is the same as the provided definition
2929
&& fn_def.def_id() == last_def.def_id
30+
&& let self_ty = cx.typeck_results().expr_ty(self_expr)
31+
&& !is_iter_with_side_effects(cx, self_ty)
3032
{
3133
let mut sugg = vec![(call_span, String::from("next_back()"))];
3234
let mut dont_apply = false;

clippy_utils/src/ty/mod.rs

+31
Original file line numberDiff line numberDiff line change
@@ -1376,3 +1376,34 @@ pub fn option_arg_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<Ty<'t
13761376
_ => None,
13771377
}
13781378
}
1379+
1380+
/// Check if `ty` is an `Iterator` and has side effects when iterated over. Currently, this only
1381+
/// checks if the `ty` contains mutable captures, and thus may be imcomplete.
1382+
pub fn is_iter_with_side_effects<'tcx>(cx: &LateContext<'tcx>, iter_ty: Ty<'tcx>) -> bool {
1383+
let Some(iter_trait) = cx.tcx.lang_items().iterator_trait() else {
1384+
return false;
1385+
};
1386+
1387+
is_iter_with_side_effects_impl(cx, iter_ty, iter_trait)
1388+
}
1389+
1390+
fn is_iter_with_side_effects_impl<'tcx>(cx: &LateContext<'tcx>, iter_ty: Ty<'tcx>, iter_trait: DefId) -> bool {
1391+
if implements_trait(cx, iter_ty, iter_trait, &[])
1392+
&& let ty::Adt(_, args) = iter_ty.kind()
1393+
{
1394+
return args.types().any(|arg_ty| {
1395+
if let ty::Closure(_, closure_args) = arg_ty.kind()
1396+
&& let Some(captures) = closure_args.types().next_back()
1397+
{
1398+
captures
1399+
.tuple_fields()
1400+
.iter()
1401+
.any(|capture_ty| matches!(capture_ty.ref_mutability(), Some(Mutability::Mut)))
1402+
} else {
1403+
is_iter_with_side_effects_impl(cx, arg_ty, iter_trait)
1404+
}
1405+
});
1406+
}
1407+
1408+
false
1409+
}

tests/ui/double_ended_iterator_last.fixed

+11
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,14 @@ fn drop_order() {
9090
//~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
9191
println!("Done");
9292
}
93+
94+
fn issue_14444() {
95+
let mut squares = vec![];
96+
let last_square = [1, 2, 3]
97+
.into_iter()
98+
.map(|x| {
99+
squares.push(x * x);
100+
Some(x * x)
101+
})
102+
.last();
103+
}

tests/ui/double_ended_iterator_last.rs

+11
Original file line numberDiff line numberDiff line change
@@ -90,3 +90,14 @@ fn drop_order() {
9090
//~^ ERROR: called `Iterator::last` on a `DoubleEndedIterator`
9191
println!("Done");
9292
}
93+
94+
fn issue_14444() {
95+
let mut squares = vec![];
96+
let last_square = [1, 2, 3]
97+
.into_iter()
98+
.map(|x| {
99+
squares.push(x * x);
100+
Some(x * x)
101+
})
102+
.last();
103+
}

0 commit comments

Comments
 (0)