Auto merge of #12823 - schvv31n:fix-iter-on-empty-collections, r=y21
Suppress `iter_on_empty_collections` if the iterator's concrete type is relied upon changelog: fixed #12807
This commit is contained in:
commit
76eee82e79
@ -1,8 +1,12 @@
|
|||||||
|
use std::iter::once;
|
||||||
|
|
||||||
use clippy_utils::diagnostics::span_lint_and_sugg;
|
use clippy_utils::diagnostics::span_lint_and_sugg;
|
||||||
use clippy_utils::source::snippet;
|
use clippy_utils::source::snippet;
|
||||||
use clippy_utils::{get_expr_use_or_unification_node, is_res_lang_ctor, path_res, std_or_core};
|
use clippy_utils::{get_expr_use_or_unification_node, is_res_lang_ctor, path_res, std_or_core};
|
||||||
|
|
||||||
use rustc_errors::Applicability;
|
use rustc_errors::Applicability;
|
||||||
|
use rustc_hir::def_id::DefId;
|
||||||
|
use rustc_hir::hir_id::HirId;
|
||||||
use rustc_hir::LangItem::{OptionNone, OptionSome};
|
use rustc_hir::LangItem::{OptionNone, OptionSome};
|
||||||
use rustc_hir::{Expr, ExprKind, Node};
|
use rustc_hir::{Expr, ExprKind, Node};
|
||||||
use rustc_lint::LateContext;
|
use rustc_lint::LateContext;
|
||||||
@ -25,7 +29,29 @@ impl IterType {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: &str, recv: &Expr<'_>) {
|
fn is_arg_ty_unified_in_fn<'tcx>(
|
||||||
|
cx: &LateContext<'tcx>,
|
||||||
|
fn_id: DefId,
|
||||||
|
arg_id: HirId,
|
||||||
|
args: impl IntoIterator<Item = &'tcx Expr<'tcx>>,
|
||||||
|
) -> bool {
|
||||||
|
let fn_sig = cx.tcx.fn_sig(fn_id).instantiate_identity();
|
||||||
|
let arg_id_in_args = args.into_iter().position(|e| e.hir_id == arg_id).unwrap();
|
||||||
|
let arg_ty_in_args = fn_sig.input(arg_id_in_args).skip_binder();
|
||||||
|
|
||||||
|
cx.tcx.predicates_of(fn_id).predicates.iter().any(|(clause, _)| {
|
||||||
|
clause
|
||||||
|
.as_projection_clause()
|
||||||
|
.and_then(|p| p.map_bound(|p| p.term.ty()).transpose())
|
||||||
|
.is_some_and(|ty| ty.skip_binder() == arg_ty_in_args)
|
||||||
|
}) || fn_sig
|
||||||
|
.inputs()
|
||||||
|
.iter()
|
||||||
|
.enumerate()
|
||||||
|
.any(|(i, ty)| i != arg_id_in_args && ty.skip_binder().walk().any(|arg| arg.as_type() == Some(arg_ty_in_args)))
|
||||||
|
}
|
||||||
|
|
||||||
|
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: &str, recv: &'tcx Expr<'tcx>) {
|
||||||
let item = match recv.kind {
|
let item = match recv.kind {
|
||||||
ExprKind::Array([]) => None,
|
ExprKind::Array([]) => None,
|
||||||
ExprKind::Array([e]) => Some(e),
|
ExprKind::Array([e]) => Some(e),
|
||||||
@ -43,6 +69,25 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, method_name: &str, re
|
|||||||
let is_unified = match get_expr_use_or_unification_node(cx.tcx, expr) {
|
let is_unified = match get_expr_use_or_unification_node(cx.tcx, expr) {
|
||||||
Some((Node::Expr(parent), child_id)) => match parent.kind {
|
Some((Node::Expr(parent), child_id)) => match parent.kind {
|
||||||
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id == child_id => false,
|
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id == child_id => false,
|
||||||
|
ExprKind::Call(
|
||||||
|
Expr {
|
||||||
|
kind: ExprKind::Path(path),
|
||||||
|
hir_id,
|
||||||
|
..
|
||||||
|
},
|
||||||
|
args,
|
||||||
|
) => cx
|
||||||
|
.typeck_results()
|
||||||
|
.qpath_res(path, *hir_id)
|
||||||
|
.opt_def_id()
|
||||||
|
.filter(|fn_id| cx.tcx.def_kind(fn_id).is_fn_like())
|
||||||
|
.is_some_and(|fn_id| is_arg_ty_unified_in_fn(cx, fn_id, child_id, args)),
|
||||||
|
ExprKind::MethodCall(_name, recv, args, _span) => is_arg_ty_unified_in_fn(
|
||||||
|
cx,
|
||||||
|
cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap(),
|
||||||
|
child_id,
|
||||||
|
once(recv).chain(args.iter()),
|
||||||
|
),
|
||||||
ExprKind::If(_, _, _)
|
ExprKind::If(_, _, _)
|
||||||
| ExprKind::Match(_, _, _)
|
| ExprKind::Match(_, _, _)
|
||||||
| ExprKind::Closure(_)
|
| ExprKind::Closure(_)
|
||||||
|
@ -20,6 +20,28 @@ fn array() {
|
|||||||
};
|
};
|
||||||
|
|
||||||
let _ = if false { ["test"].iter() } else { [].iter() };
|
let _ = if false { ["test"].iter() } else { [].iter() };
|
||||||
|
|
||||||
|
let smth = Some(vec![1, 2, 3]);
|
||||||
|
|
||||||
|
// Don't trigger when the empty collection iter is relied upon for its concrete type
|
||||||
|
// But do trigger if it is just an iterator, despite being an argument to a method
|
||||||
|
for i in smth.as_ref().map_or([].iter(), |s| s.iter()).chain(std::iter::empty()) {
|
||||||
|
println!("{i}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same as above, but for empty collection iters with extra layers
|
||||||
|
for i in smth.as_ref().map_or({ [].iter() }, |s| s.iter()) {
|
||||||
|
println!("{y}", y = i + 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same as above, but for regular function calls
|
||||||
|
for i in Option::map_or(smth.as_ref(), [].iter(), |s| s.iter()) {
|
||||||
|
println!("{i}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same as above, but when there are no predicates that mention the collection iter type.
|
||||||
|
let mut iter = [34, 228, 35].iter();
|
||||||
|
let _ = std::mem::replace(&mut iter, [].iter());
|
||||||
}
|
}
|
||||||
|
|
||||||
macro_rules! in_macros {
|
macro_rules! in_macros {
|
||||||
|
@ -20,6 +20,28 @@ fn array() {
|
|||||||
};
|
};
|
||||||
|
|
||||||
let _ = if false { ["test"].iter() } else { [].iter() };
|
let _ = if false { ["test"].iter() } else { [].iter() };
|
||||||
|
|
||||||
|
let smth = Some(vec![1, 2, 3]);
|
||||||
|
|
||||||
|
// Don't trigger when the empty collection iter is relied upon for its concrete type
|
||||||
|
// But do trigger if it is just an iterator, despite being an argument to a method
|
||||||
|
for i in smth.as_ref().map_or([].iter(), |s| s.iter()).chain([].iter()) {
|
||||||
|
println!("{i}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same as above, but for empty collection iters with extra layers
|
||||||
|
for i in smth.as_ref().map_or({ [].iter() }, |s| s.iter()) {
|
||||||
|
println!("{y}", y = i + 1);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same as above, but for regular function calls
|
||||||
|
for i in Option::map_or(smth.as_ref(), [].iter(), |s| s.iter()) {
|
||||||
|
println!("{i}");
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same as above, but when there are no predicates that mention the collection iter type.
|
||||||
|
let mut iter = [34, 228, 35].iter();
|
||||||
|
let _ = std::mem::replace(&mut iter, [].iter());
|
||||||
}
|
}
|
||||||
|
|
||||||
macro_rules! in_macros {
|
macro_rules! in_macros {
|
||||||
|
@ -37,5 +37,11 @@ error: `iter` call on an empty collection
|
|||||||
LL | assert_eq!(None.iter().next(), Option::<&i32>::None);
|
LL | assert_eq!(None.iter().next(), Option::<&i32>::None);
|
||||||
| ^^^^^^^^^^^ help: try: `std::iter::empty()`
|
| ^^^^^^^^^^^ help: try: `std::iter::empty()`
|
||||||
|
|
||||||
error: aborting due to 6 previous errors
|
error: `iter` call on an empty collection
|
||||||
|
--> tests/ui/iter_on_empty_collections.rs:28:66
|
||||||
|
|
|
||||||
|
LL | for i in smth.as_ref().map_or([].iter(), |s| s.iter()).chain([].iter()) {
|
||||||
|
| ^^^^^^^^^ help: try: `std::iter::empty()`
|
||||||
|
|
||||||
|
error: aborting due to 7 previous errors
|
||||||
|
|
||||||
|
Loading…
x
Reference in New Issue
Block a user