diff --git a/CHANGELOG.md b/CHANGELOG.md index 4debe3048572..04917871c406 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/book/src/lint_configuration.md b/book/src/lint_configuration.md index 8bb3a57ba9a3..ae0b51403167 100644 --- a/book/src/lint_configuration.md +++ b/book/src/lint_configuration.md @@ -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` diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 46f4082f0c7f..316645e4066a 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -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, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 5ab28b5c70c1..87329ee5e14a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -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; @@ -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` } diff --git a/clippy_lints/src/tuple_array_conversions.rs b/clippy_lints/src/tuple_array_conversions.rs new file mode 100644 index 000000000000..bd983306508b --- /dev/null +++ b/clippy_lints/src/tuple_array_conversions.rs @@ -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::>>() + && let Some(locals) = elements + .iter() + .map(|(_, element)| path_to_local(element).and_then(|local| cx.tcx.hir().find(local))) + .collect::>>() + && 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 +} diff --git a/clippy_lints/src/upper_case_acronyms.rs b/clippy_lints/src/upper_case_acronyms.rs index 1d2d3eb12e11..4df1e3299ed9 100644 --- a/clippy_lints/src/upper_case_acronyms.rs +++ b/clippy_lints/src/upper_case_acronyms.rs @@ -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()) { diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index 972e24aac512..890639646228 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -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 = None), diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs index 5aacca451465..1f70f11bd677 100644 --- a/clippy_utils/src/msrvs.rs +++ b/clippy_utils/src/msrvs.rs @@ -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 } diff --git a/tests/ui/tuple_array_conversions.rs b/tests/ui/tuple_array_conversions.rs new file mode 100644 index 000000000000..f96a7c97f1aa --- /dev/null +++ b/tests/ui/tuple_array_conversions.rs @@ -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]); +} diff --git a/tests/ui/tuple_array_conversions.stderr b/tests/ui/tuple_array_conversions.stderr new file mode 100644 index 000000000000..be653e8efb79 --- /dev/null +++ b/tests/ui/tuple_array_conversions.stderr @@ -0,0 +1,83 @@ +error: it looks like you're trying to convert an array to a tuple + --> $DIR/tuple_array_conversions.rs:10:13 + | +LL | let x = (x[0], x[1]); + | ^^^^^^^^^^^^ + | + = help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed + = note: `-D clippy::tuple-array-conversions` implied by `-D warnings` + +error: it looks like you're trying to convert a tuple to an array + --> $DIR/tuple_array_conversions.rs:11:13 + | +LL | let x = [x.0, x.1]; + | ^^^^^^^^^^ + | + = help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed + +error: it looks like you're trying to convert an array to a tuple + --> $DIR/tuple_array_conversions.rs:13:13 + | +LL | let x = (x[0], x[1]); + | ^^^^^^^^^^^^ + | + = help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed + +error: it looks like you're trying to convert a tuple to an array + --> $DIR/tuple_array_conversions.rs:16:53 + | +LL | let v1: Vec<[u32; 2]> = t1.iter().map(|&(a, b)| [a, b]).collect(); + | ^^^^^^ + | + = help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed + +error: it looks like you're trying to convert a tuple to an array + --> $DIR/tuple_array_conversions.rs:17:38 + | +LL | t1.iter().for_each(|&(a, b)| _ = [a, b]); + | ^^^^^^ + | + = help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed + +error: it looks like you're trying to convert an array to a tuple + --> $DIR/tuple_array_conversions.rs:18:55 + | +LL | let t2: Vec<(u32, u32)> = v1.iter().map(|&[a, b]| (a, b)).collect(); + | ^^^^^^ + | + = help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed + +error: it looks like you're trying to convert a tuple to an array + --> $DIR/tuple_array_conversions.rs:19:38 + | +LL | t1.iter().for_each(|&(a, b)| _ = [a, b]); + | ^^^^^^ + | + = help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed + +error: it looks like you're trying to convert an array to a tuple + --> $DIR/tuple_array_conversions.rs:69:13 + | +LL | let x = (x[0], x[1]); + | ^^^^^^^^^^^^ + | + = help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed + +error: it looks like you're trying to convert a tuple to an array + --> $DIR/tuple_array_conversions.rs:70:13 + | +LL | let x = [x.0, x.1]; + | ^^^^^^^^^^ + | + = help: use `.into()` instead, or `<[T; N]>::from` if type annotations are needed + +error: it looks like you're trying to convert an array to a tuple + --> $DIR/tuple_array_conversions.rs:72:13 + | +LL | let x = (x[0], x[1]); + | ^^^^^^^^^^^^ + | + = help: use `.into()` instead, or `<(T0, T1, ..., Tn)>::from` if type annotations are needed + +error: aborting due to 10 previous errors +