Skip to content

Commit 99982e6

Browse files
committed
fix: tuple_array_conversions FP when binded vars are used before conversion
1 parent 64f43bf commit 99982e6

File tree

3 files changed

+56
-11
lines changed

3 files changed

+56
-11
lines changed

clippy_lints/src/tuple_array_conversions.rs

Lines changed: 24 additions & 10 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,
@@ -167,12 +165,28 @@ fn all_bindings_are_for_conv<'tcx>(
167165
})
168166
.all_equal()
169167
// 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())
168+
&& locals.iter().zip(local_parents.iter()).all(|(&l, &parent)| {
169+
if let Node::LetStmt(_) = parent {
170+
return true;
171+
}
172+
173+
let Some(b) = get_enclosing_block(cx, l) else {
174+
return true;
175+
};
176+
local_used_once(cx, b, l).is_some()
177+
})
173178
&& local_parents.first().is_some_and(|node| {
174179
let Some(ty) = match node {
175-
Node::Pat(pat) => Some(pat.hir_id),
180+
Node::Pat(pat)
181+
if let PatKind::Tuple(pats, _) | PatKind::Slice(pats, None, []) = &pat.kind
182+
&& pats.iter().zip(locals.iter()).all(|(p, l)| {
183+
if let PatKind::Binding(_, id, _, _) = p.kind {
184+
id == *l
185+
} else {
186+
true
187+
}
188+
})
189+
=> Some(pat.hir_id),
176190
Node::LetStmt(l) => Some(l.hir_id),
177191
_ => None,
178192
}

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)