Skip to content

Commit 52fea21

Browse files
authored
Fix tuple_array_conversions FP when binded vars are used before conversion (#16197)
Closes #16192 changelog: [`tuple_array_conversions`] fix FP when binded vars are used before conversion
2 parents 70a4d95 + 3551aeb commit 52fea21

File tree

3 files changed

+61
-13
lines changed

3 files changed

+61
-13
lines changed

clippy_lints/src/tuple_array_conversions.rs

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,16 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_help;
3-
use clippy_utils::is_from_proc_macro;
43
use clippy_utils::msrvs::{self, Msrv};
54
use clippy_utils::res::MaybeResPath;
6-
use clippy_utils::visitors::for_each_local_use_after_expr;
5+
use clippy_utils::visitors::local_used_once;
6+
use clippy_utils::{get_enclosing_block, is_from_proc_macro};
77
use itertools::Itertools;
88
use rustc_ast::LitKind;
99
use rustc_hir::{Expr, ExprKind, Node, PatKind};
1010
use rustc_lint::{LateContext, LateLintPass, LintContext};
1111
use rustc_middle::ty::{self, Ty};
1212
use rustc_session::impl_lint_pass;
1313
use std::iter::once;
14-
use std::ops::ControlFlow;
1514

1615
declare_clippy_lint! {
1716
/// ### What it does
@@ -86,7 +85,7 @@ fn check_array<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &
8685
ExprKind::Path(_) => Some(elements.iter().collect()),
8786
_ => None,
8887
})
89-
&& all_bindings_are_for_conv(cx, &[ty], expr, elements, &locals, ToType::Array)
88+
&& all_bindings_are_for_conv(cx, &[ty], elements, &locals, ToType::Array)
9089
&& !is_from_proc_macro(cx, expr)
9190
{
9291
span_lint_and_help(
@@ -123,7 +122,7 @@ fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &
123122
ExprKind::Path(_) => Some(elements.iter().collect()),
124123
_ => None,
125124
})
126-
&& all_bindings_are_for_conv(cx, tys, expr, elements, &locals, ToType::Tuple)
125+
&& all_bindings_are_for_conv(cx, tys, elements, &locals, ToType::Tuple)
127126
&& !is_from_proc_macro(cx, expr)
128127
{
129128
span_lint_and_help(
@@ -148,7 +147,6 @@ fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &
148147
fn all_bindings_are_for_conv<'tcx>(
149148
cx: &LateContext<'tcx>,
150149
final_tys: &[Ty<'tcx>],
151-
expr: &Expr<'_>,
152150
elements: &[Expr<'_>],
153151
locals: &[&Expr<'_>],
154152
kind: ToType,
@@ -166,13 +164,30 @@ fn all_bindings_are_for_conv<'tcx>(
166164
_ => None,
167165
})
168166
.all_equal()
169-
// Fix #11124, very convenient utils function! ❤️
170-
&& locals
171-
.iter()
172-
.all(|&l| for_each_local_use_after_expr(cx, l, expr.hir_id, |_| ControlFlow::Break::<()>(())).is_continue())
167+
&& locals.iter().zip(local_parents.iter()).all(|(&l, &parent)| {
168+
if let Node::LetStmt(_) = parent {
169+
return true;
170+
}
171+
172+
let Some(b) = get_enclosing_block(cx, l) else {
173+
return true;
174+
};
175+
local_used_once(cx, b, l).is_some()
176+
})
173177
&& local_parents.first().is_some_and(|node| {
174178
let Some(ty) = match node {
175-
Node::Pat(pat) => Some(pat.hir_id),
179+
Node::Pat(pat)
180+
if let PatKind::Tuple(pats, _) | PatKind::Slice(pats, None, []) = &pat.kind
181+
&& pats.iter().zip(locals.iter()).all(|(p, l)| {
182+
if let PatKind::Binding(_, id, _, _) = p.kind {
183+
id == *l
184+
} else {
185+
true
186+
}
187+
}) =>
188+
{
189+
Some(pat.hir_id)
190+
},
176191
Node::LetStmt(l) => Some(l.hir_id),
177192
_ => None,
178193
}
@@ -186,7 +201,9 @@ fn all_bindings_are_for_conv<'tcx>(
186201
tys.len() == elements.len() && tys.iter().chain(final_tys.iter().copied()).all_equal()
187202
},
188203
(ToType::Tuple, ty::Array(ty, len)) => {
189-
let Some(len) = len.try_to_target_usize(cx.tcx) else { return false };
204+
let Some(len) = len.try_to_target_usize(cx.tcx) else {
205+
return false;
206+
};
190207
len as usize == elements.len() && final_tys.iter().chain(once(ty)).all_equal()
191208
},
192209
_ => false,

tests/ui/tuple_array_conversions.rs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,3 +116,26 @@ fn msrv_juust_right() {
116116
let x = &[1, 2];
117117
let x = (x[0], x[1]);
118118
}
119+
120+
fn issue16192() {
121+
fn do_something(tuple: (u32, u32)) {}
122+
fn produce_array() -> [u32; 2] {
123+
[1, 2]
124+
}
125+
126+
let [a, b] = produce_array();
127+
for tuple in [(a, b), (b, a)] {
128+
do_something(tuple);
129+
}
130+
131+
let [a, b] = produce_array();
132+
let x = b;
133+
do_something((a, b));
134+
135+
let [a, b] = produce_array();
136+
do_something((b, a));
137+
138+
let [a, b] = produce_array();
139+
do_something((a, b));
140+
//~^ tuple_array_conversions
141+
}

tests/ui/tuple_array_conversions.stderr

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,5 +80,13 @@ LL | let x = [x.0, x.1];
8080
|
8181
= help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed
8282

83-
error: aborting due to 10 previous errors
83+
error: it looks like you're trying to convert an array to a tuple
84+
--> tests/ui/tuple_array_conversions.rs:139:18
85+
|
86+
LL | do_something((a, b));
87+
| ^^^^^^
88+
|
89+
= help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed
90+
91+
error: aborting due to 11 previous errors
8492

0 commit comments

Comments
 (0)