Skip to content

Commit 445d419

Browse files
authored
Fix iter_on_single_items FP on function pointers and let stmts (#15013)
Closes #14981 changelog: [`iter_on_single_items`] fix FP on function pointers and let stmts
2 parents 18e8ac3 + 20670f7 commit 445d419

File tree

4 files changed

+85
-34
lines changed

4 files changed

+85
-34
lines changed

clippy_lints/src/methods/iter_on_single_or_empty_collections.rs

Lines changed: 36 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@ use std::iter::once;
22

33
use clippy_utils::diagnostics::span_lint_and_sugg;
44
use clippy_utils::source::snippet;
5+
use clippy_utils::ty::{ExprFnSig, expr_sig, ty_sig};
56
use clippy_utils::{get_expr_use_or_unification_node, is_res_lang_ctor, path_res, std_or_core, sym};
67

78
use rustc_errors::Applicability;
89
use rustc_hir::LangItem::{OptionNone, OptionSome};
9-
use rustc_hir::def_id::DefId;
1010
use rustc_hir::hir_id::HirId;
1111
use rustc_hir::{Expr, ExprKind, Node};
1212
use rustc_lint::LateContext;
13+
use rustc_middle::ty::Binder;
1314
use rustc_span::Symbol;
1415

1516
use super::{ITER_ON_EMPTY_COLLECTIONS, ITER_ON_SINGLE_ITEMS};
@@ -32,24 +33,34 @@ impl IterType {
3233

3334
fn is_arg_ty_unified_in_fn<'tcx>(
3435
cx: &LateContext<'tcx>,
35-
fn_id: DefId,
36+
fn_sig: ExprFnSig<'tcx>,
3637
arg_id: HirId,
3738
args: impl IntoIterator<Item = &'tcx Expr<'tcx>>,
39+
is_method: bool,
3840
) -> bool {
39-
let fn_sig = cx.tcx.fn_sig(fn_id).instantiate_identity();
4041
let arg_id_in_args = args.into_iter().position(|e| e.hir_id == arg_id).unwrap();
41-
let arg_ty_in_args = fn_sig.input(arg_id_in_args).skip_binder();
42+
let Some(arg_ty_in_args) = fn_sig.input(arg_id_in_args).map(Binder::skip_binder) else {
43+
return false;
44+
};
4245

43-
cx.tcx.predicates_of(fn_id).predicates.iter().any(|(clause, _)| {
44-
clause
45-
.as_projection_clause()
46-
.and_then(|p| p.map_bound(|p| p.term.as_type()).transpose())
47-
.is_some_and(|ty| ty.skip_binder() == arg_ty_in_args)
48-
}) || fn_sig
49-
.inputs()
50-
.iter()
51-
.enumerate()
52-
.any(|(i, ty)| i != arg_id_in_args && ty.skip_binder().walk().any(|arg| arg.as_type() == Some(arg_ty_in_args)))
46+
fn_sig
47+
.predicates_id()
48+
.map(|def_id| cx.tcx.predicates_of(def_id))
49+
.is_some_and(|generics| {
50+
generics.predicates.iter().any(|(clause, _)| {
51+
clause
52+
.as_projection_clause()
53+
.and_then(|p| p.map_bound(|p| p.term.as_type()).transpose())
54+
.is_some_and(|ty| ty.skip_binder() == arg_ty_in_args)
55+
})
56+
})
57+
|| (!is_method
58+
&& fn_sig.input(arg_id_in_args).is_some_and(|binder| {
59+
binder
60+
.skip_binder()
61+
.walk()
62+
.any(|arg| arg.as_type() == Some(arg_ty_in_args))
63+
}))
5364
}
5465

5566
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method_name: Symbol, recv: &'tcx Expr<'tcx>) {
@@ -70,33 +81,25 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, method
7081
let is_unified = match get_expr_use_or_unification_node(cx.tcx, expr) {
7182
Some((Node::Expr(parent), child_id)) => match parent.kind {
7283
ExprKind::If(e, _, _) | ExprKind::Match(e, _, _) if e.hir_id == child_id => false,
73-
ExprKind::Call(
74-
Expr {
75-
kind: ExprKind::Path(path),
76-
hir_id,
77-
..
78-
},
79-
args,
80-
) => cx
84+
ExprKind::Call(recv, args) => {
85+
expr_sig(cx, recv).is_some_and(|fn_sig| is_arg_ty_unified_in_fn(cx, fn_sig, child_id, args, false))
86+
},
87+
ExprKind::MethodCall(_name, recv, args, _span) => cx
8188
.typeck_results()
82-
.qpath_res(path, *hir_id)
83-
.opt_def_id()
84-
.filter(|fn_id| cx.tcx.def_kind(fn_id).is_fn_like())
85-
.is_some_and(|fn_id| is_arg_ty_unified_in_fn(cx, fn_id, child_id, args)),
86-
ExprKind::MethodCall(_name, recv, args, _span) => is_arg_ty_unified_in_fn(
87-
cx,
88-
cx.typeck_results().type_dependent_def_id(parent.hir_id).unwrap(),
89-
child_id,
90-
once(recv).chain(args.iter()),
91-
),
89+
.type_dependent_def_id(parent.hir_id)
90+
.and_then(|def_id| ty_sig(cx, cx.tcx.type_of(def_id).instantiate_identity()))
91+
.is_some_and(|fn_sig| {
92+
is_arg_ty_unified_in_fn(cx, fn_sig, child_id, once(recv).chain(args.iter()), true)
93+
}),
9294
ExprKind::If(_, _, _)
9395
| ExprKind::Match(_, _, _)
9496
| ExprKind::Closure(_)
9597
| ExprKind::Ret(_)
9698
| ExprKind::Break(_, _) => true,
9799
_ => false,
98100
},
99-
Some((Node::Stmt(_) | Node::LetStmt(_), _)) => false,
101+
Some((Node::LetStmt(let_stmt), _)) => let_stmt.ty.is_some(),
102+
Some((Node::Stmt(_), _)) => false,
100103
_ => true,
101104
};
102105

clippy_utils/src/ty/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,7 @@ pub fn all_predicates_of(tcx: TyCtxt<'_>, id: DefId) -> impl Iterator<Item = &(t
582582
}
583583

584584
/// A signature for a function like type.
585-
#[derive(Clone, Copy)]
585+
#[derive(Clone, Copy, Debug)]
586586
pub enum ExprFnSig<'tcx> {
587587
Sig(Binder<'tcx, FnSig<'tcx>>, Option<DefId>),
588588
Closure(Option<&'tcx FnDecl<'tcx>>, Binder<'tcx, FnSig<'tcx>>),

tests/ui/iter_on_single_items.fixed

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,27 @@ fn main() {
6666
custom_option::custom_option();
6767
in_macros!();
6868
}
69+
70+
mod issue14981 {
71+
use std::option::IntoIter;
72+
fn takes_into_iter(_: impl IntoIterator<Item = i32>) {}
73+
74+
fn let_stmt() {
75+
macro_rules! x {
76+
($e:expr) => {
77+
let _: IntoIter<i32> = $e;
78+
};
79+
}
80+
x!(Some(5).into_iter());
81+
}
82+
83+
fn fn_ptr() {
84+
fn some_func(_: IntoIter<i32>) -> IntoIter<i32> {
85+
todo!()
86+
}
87+
some_func(Some(5).into_iter());
88+
89+
const C: fn(IntoIter<i32>) -> IntoIter<i32> = <IntoIter<i32> as IntoIterator>::into_iter;
90+
C(Some(5).into_iter());
91+
}
92+
}

tests/ui/iter_on_single_items.rs

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,27 @@ fn main() {
6666
custom_option::custom_option();
6767
in_macros!();
6868
}
69+
70+
mod issue14981 {
71+
use std::option::IntoIter;
72+
fn takes_into_iter(_: impl IntoIterator<Item = i32>) {}
73+
74+
fn let_stmt() {
75+
macro_rules! x {
76+
($e:expr) => {
77+
let _: IntoIter<i32> = $e;
78+
};
79+
}
80+
x!(Some(5).into_iter());
81+
}
82+
83+
fn fn_ptr() {
84+
fn some_func(_: IntoIter<i32>) -> IntoIter<i32> {
85+
todo!()
86+
}
87+
some_func(Some(5).into_iter());
88+
89+
const C: fn(IntoIter<i32>) -> IntoIter<i32> = <IntoIter<i32> as IntoIterator>::into_iter;
90+
C(Some(5).into_iter());
91+
}
92+
}

0 commit comments

Comments
 (0)