Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New lint [tuple_array_conversions] #11020

Merged
merged 4 commits into from
Jul 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5262,6 +5262,7 @@ Released 2018-09-13
[`trivial_regex`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivial_regex
[`trivially_copy_pass_by_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#trivially_copy_pass_by_ref
[`try_err`]: https://rust-lang.github.io/rust-clippy/master/index.html#try_err
[`tuple_array_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions
[`type_complexity`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_complexity
[`type_repetition_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds
[`unchecked_duration_subtraction`]: https://rust-lang.github.io/rust-clippy/master/index.html#unchecked_duration_subtraction
Expand Down
1 change: 1 addition & 0 deletions book/src/lint_configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ The minimum rust version that the project supports
* [`manual_rem_euclid`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_rem_euclid)
* [`manual_retain`](https://rust-lang.github.io/rust-clippy/master/index.html#manual_retain)
* [`type_repetition_in_bounds`](https://rust-lang.github.io/rust-clippy/master/index.html#type_repetition_in_bounds)
* [`tuple_array_conversions`](https://rust-lang.github.io/rust-clippy/master/index.html#tuple_array_conversions)


## `cognitive-complexity-threshold`
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -624,6 +624,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::transmute::UNSOUND_COLLECTION_TRANSMUTE_INFO,
crate::transmute::USELESS_TRANSMUTE_INFO,
crate::transmute::WRONG_TRANSMUTE_INFO,
crate::tuple_array_conversions::TUPLE_ARRAY_CONVERSIONS_INFO,
crate::types::BORROWED_BOX_INFO,
crate::types::BOX_COLLECTION_INFO,
crate::types::LINKEDLIST_INFO,
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ mod to_digit_is_some;
mod trailing_empty_array;
mod trait_bounds;
mod transmute;
mod tuple_array_conversions;
mod types;
mod undocumented_unsafe_blocks;
mod unicode;
Expand Down Expand Up @@ -1072,6 +1073,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
});
store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
store.register_early_pass(|| Box::new(visibility::Visibility));
store.register_late_pass(move |_| Box::new(tuple_array_conversions::TupleArrayConversions { msrv: msrv() }));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
235 changes: 235 additions & 0 deletions clippy_lints/src/tuple_array_conversions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,235 @@
use clippy_utils::{
diagnostics::span_lint_and_help,
is_from_proc_macro,
msrvs::{self, Msrv},
path_to_local,
};
use rustc_ast::LitKind;
use rustc_hir::{Expr, ExprKind, HirId, Node, Pat};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::{lint::in_external_macro, ty};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use std::iter::once;

declare_clippy_lint! {
/// ### What it does
/// Checks for tuple<=>array conversions that are not done with `.into()`.
///
/// ### Why is this bad?
/// It's unnecessary complexity. `.into()` works for tuples<=>arrays at or below 12 elements and
/// conveys the intent a lot better, while also leaving less room for hard to spot bugs!
///
/// ### Example
/// ```rust,ignore
/// let t1 = &[(1, 2), (3, 4)];
/// let v1: Vec<[u32; 2]> = t1.iter().map(|&(a, b)| [a, b]).collect();
/// ```
/// Use instead:
/// ```rust,ignore
/// let t1 = &[(1, 2), (3, 4)];
/// let v1: Vec<[u32; 2]> = t1.iter().map(|&t| t.into()).collect();
/// ```
#[clippy::version = "1.72.0"]
pub TUPLE_ARRAY_CONVERSIONS,
complexity,
"checks for tuple<=>array conversions that are not done with `.into()`"
}
impl_lint_pass!(TupleArrayConversions => [TUPLE_ARRAY_CONVERSIONS]);

#[derive(Clone)]
pub struct TupleArrayConversions {
pub msrv: Msrv,
}

impl LateLintPass<'_> for TupleArrayConversions {
fn check_expr<'tcx>(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
if !in_external_macro(cx.sess(), expr.span) && self.msrv.meets(msrvs::TUPLE_ARRAY_CONVERSIONS) {
match expr.kind {
ExprKind::Array(elements) if (1..=12).contains(&elements.len()) => check_array(cx, expr, elements),
ExprKind::Tup(elements) if (1..=12).contains(&elements.len()) => check_tuple(cx, expr, elements),
_ => {},
}
}
}

extract_msrv_attr!(LateContext);
}

#[expect(
clippy::blocks_in_if_conditions,
reason = "not a FP, but this is much easier to understand"
)]
fn check_array<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &'tcx [Expr<'tcx>]) {
if should_lint(
cx,
elements,
// This is cursed.
Some,
|(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Tuple(len) if len.len() == elements.len()
);
}

false
},
) || should_lint(
cx,
elements,
|(i, expr)| {
if let ExprKind::Field(path, field) = expr.kind && field.as_str() == i.to_string() {
return Some((i, path));
};

None
},
|(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Tuple(len) if len.len() == elements.len()
);
}

false
},
) {
emit_lint(cx, expr, ToType::Array);
}
}

#[expect(
clippy::blocks_in_if_conditions,
reason = "not a FP, but this is much easier to understand"
)]
#[expect(clippy::cast_possible_truncation)]
fn check_tuple<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, elements: &'tcx [Expr<'tcx>]) {
if should_lint(cx, elements, Some, |(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Array(_, len) if len.eval_target_usize(cx.tcx, cx.param_env) as usize == elements.len()
);
}

false
}) || should_lint(
cx,
elements,
|(i, expr)| {
if let ExprKind::Index(path, index) = expr.kind
&& let ExprKind::Lit(lit) = index.kind
&& let LitKind::Int(val, _) = lit.node
&& val as usize == i
{
return Some((i, path));
};

None
},
|(first_id, local)| {
if let Node::Pat(pat) = local
&& let parent = parent_pat(cx, pat)
&& parent.hir_id == first_id
{
return matches!(
cx.typeck_results().pat_ty(parent).peel_refs().kind(),
ty::Array(_, len) if len.eval_target_usize(cx.tcx, cx.param_env) as usize == elements.len()
);
}

false
},
) {
emit_lint(cx, expr, ToType::Tuple);
}
}

/// Walks up the `Pat` until it's reached the final containing `Pat`.
fn parent_pat<'tcx>(cx: &LateContext<'tcx>, start: &'tcx Pat<'tcx>) -> &'tcx Pat<'tcx> {
let mut end = start;
for (_, node) in cx.tcx.hir().parent_iter(start.hir_id) {
if let Node::Pat(pat) = node {
end = pat;
} else {
break;
}
}
end
}

#[derive(Clone, Copy)]
enum ToType {
Array,
Tuple,
}

impl ToType {
fn msg(self) -> &'static str {
match self {
ToType::Array => "it looks like you're trying to convert a tuple to an array",
ToType::Tuple => "it looks like you're trying to convert an array to a tuple",
}
}

fn help(self) -> &'static str {
match self {
ToType::Array => "use `.into()` instead, or `<[T; N]>::from` if type annotations are needed",
ToType::Tuple => "use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed",
}
}
}

fn emit_lint<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, to_type: ToType) -> bool {
if !is_from_proc_macro(cx, expr) {
span_lint_and_help(
cx,
TUPLE_ARRAY_CONVERSIONS,
expr.span,
to_type.msg(),
None,
to_type.help(),
);

return true;
}

false
}

fn should_lint<'tcx>(
cx: &LateContext<'tcx>,
elements: &'tcx [Expr<'tcx>],
map: impl FnMut((usize, &'tcx Expr<'tcx>)) -> Option<(usize, &Expr<'_>)>,
predicate: impl FnMut((HirId, &Node<'tcx>)) -> bool,
) -> bool {
if let Some(elements) = elements
.iter()
.enumerate()
.map(map)
.collect::<Option<Vec<_>>>()
&& let Some(locals) = elements
.iter()
.map(|(_, element)| path_to_local(element).and_then(|local| cx.tcx.hir().find(local)))
.collect::<Option<Vec<_>>>()
&& let [first, rest @ ..] = &*locals
&& let Node::Pat(first_pat) = first
&& let parent = parent_pat(cx, first_pat).hir_id
&& rest.iter().chain(once(first)).map(|i| (parent, i)).all(predicate)
{
return true;
}

false
}
2 changes: 1 addition & 1 deletion clippy_lints/src/upper_case_acronyms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ fn correct_ident(ident: &str) -> String {

let mut ident = fragments.clone().next().unwrap();
for (ref prev, ref curr) in fragments.tuple_windows() {
if [prev, curr]
if <[&String; 2]>::from((prev, curr))
.iter()
.all(|s| s.len() == 1 && s.chars().next().unwrap().is_ascii_uppercase())
{
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/utils/conf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ define_Conf! {
///
/// Suppress lints whenever the suggested change would cause breakage for other crates.
(avoid_breaking_exported_api: bool = true),
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS.
/// Lint: MANUAL_SPLIT_ONCE, MANUAL_STR_REPEAT, CLONED_INSTEAD_OF_COPIED, REDUNDANT_FIELD_NAMES, OPTION_MAP_UNWRAP_OR, REDUNDANT_STATIC_LIFETIMES, FILTER_MAP_NEXT, CHECKED_CONVERSIONS, MANUAL_RANGE_CONTAINS, USE_SELF, MEM_REPLACE_WITH_DEFAULT, MANUAL_NON_EXHAUSTIVE, OPTION_AS_REF_DEREF, MAP_UNWRAP_OR, MATCH_LIKE_MATCHES_MACRO, MANUAL_STRIP, MISSING_CONST_FOR_FN, UNNESTED_OR_PATTERNS, FROM_OVER_INTO, PTR_AS_PTR, IF_THEN_SOME_ELSE_NONE, APPROX_CONSTANT, DEPRECATED_CFG_ATTR, INDEX_REFUTABLE_SLICE, MAP_CLONE, BORROW_AS_PTR, MANUAL_BITS, ERR_EXPECT, CAST_ABS_TO_UNSIGNED, UNINLINED_FORMAT_ARGS, MANUAL_CLAMP, MANUAL_LET_ELSE, UNCHECKED_DURATION_SUBTRACTION, COLLAPSIBLE_STR_REPLACE, SEEK_FROM_CURRENT, SEEK_REWIND, UNNECESSARY_LAZY_EVALUATIONS, TRANSMUTE_PTR_TO_REF, ALMOST_COMPLETE_RANGE, NEEDLESS_BORROW, DERIVABLE_IMPLS, MANUAL_IS_ASCII_CHECK, MANUAL_REM_EUCLID, MANUAL_RETAIN, TYPE_REPETITION_IN_BOUNDS, TUPLE_ARRAY_CONVERSIONS.
///
/// The minimum rust version that the project supports
(msrv: Option<String> = None),
Expand Down
1 change: 1 addition & 0 deletions clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ macro_rules! msrv_aliases {

// names may refer to stabilized feature flags or library items
msrv_aliases! {
1,71,0 { TUPLE_ARRAY_CONVERSIONS }
1,70,0 { OPTION_IS_SOME_AND }
1,68,0 { PATH_MAIN_SEPARATOR_STR }
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
Expand Down
73 changes: 73 additions & 0 deletions tests/ui/tuple_array_conversions.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
//@aux-build:proc_macros.rs:proc-macro
#![allow(clippy::no_effect, clippy::useless_vec, unused)]
#![warn(clippy::tuple_array_conversions)]

#[macro_use]
extern crate proc_macros;

fn main() {
let x = [1, 2];
let x = (x[0], x[1]);
let x = [x.0, x.1];
let x = &[1, 2];
let x = (x[0], x[1]);

let t1: &[(u32, u32)] = &[(1, 2), (3, 4)];
let v1: Vec<[u32; 2]> = t1.iter().map(|&(a, b)| [a, b]).collect();
t1.iter().for_each(|&(a, b)| _ = [a, b]);
let t2: Vec<(u32, u32)> = v1.iter().map(|&[a, b]| (a, b)).collect();
t1.iter().for_each(|&(a, b)| _ = [a, b]);
// Do not lint
let v2: Vec<[u32; 2]> = t1.iter().map(|&t| t.into()).collect();
let t3: Vec<(u32, u32)> = v2.iter().map(|&v| v.into()).collect();
let x = [1; 13];
let x = (
x[0], x[1], x[2], x[3], x[4], x[5], x[6], x[7], x[8], x[9], x[10], x[11], x[12],
);
let x = [x.0, x.1, x.2, x.3, x.4, x.5, x.6, x.7, x.8, x.9, x.10, x.11, x.12];
let x = (1, 2);
let x = (x.0, x.1);
let x = [1, 2];
let x = [x[0], x[1]];
let x = vec![1, 2];
let x = (x[0], x[1]);
let x = [1; 3];
let x = (x[0],);
let x = (1, 2, 3);
let x = [x.0];
let x = (1, 2);
let y = (1, 2);
[x.0, y.0];
[x.0, y.1];
let x = [x.0, x.0];
let x = (x[0], x[0]);
external! {
let t1: &[(u32, u32)] = &[(1, 2), (3, 4)];
let v1: Vec<[u32; 2]> = t1.iter().map(|&(a, b)| [a, b]).collect();
let t2: Vec<(u32, u32)> = v1.iter().map(|&[a, b]| (a, b)).collect();
}
with_span! {
span
let t1: &[(u32, u32)] = &[(1, 2), (3, 4)];
let v1: Vec<[u32; 2]> = t1.iter().map(|&(a, b)| [a, b]).collect();
let t2: Vec<(u32, u32)> = v1.iter().map(|&[a, b]| (a, b)).collect();
}
}

#[clippy::msrv = "1.70.0"]
fn msrv_too_low() {
let x = [1, 2];
let x = (x[0], x[1]);
let x = [x.0, x.1];
let x = &[1, 2];
let x = (x[0], x[1]);
}

#[clippy::msrv = "1.71.0"]
fn msrv_juust_right() {
let x = [1, 2];
let x = (x[0], x[1]);
let x = [x.0, x.1];
let x = &[1, 2];
let x = (x[0], x[1]);
}
Loading