diff --git a/CHANGELOG.md b/CHANGELOG.md index 4debe3048572..b13faf39283a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5137,6 +5137,7 @@ Released 2018-09-13 [`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl [`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation [`redundant_async_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_async_block +[`redundant_at_rest_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_at_rest_pattern [`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone [`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure [`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 46f4082f0c7f..3e627a2bd28e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -431,6 +431,7 @@ crate::misc_early::DOUBLE_NEG_INFO, crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO, crate::misc_early::MIXED_CASE_HEX_LITERALS_INFO, + crate::misc_early::REDUNDANT_AT_REST_PATTERN_INFO, crate::misc_early::REDUNDANT_PATTERN_INFO, crate::misc_early::SEPARATED_LITERAL_SUFFIX_INFO, crate::misc_early::UNNEEDED_FIELD_PATTERN_INFO, diff --git a/clippy_lints/src/methods/needless_collect.rs b/clippy_lints/src/methods/needless_collect.rs index 99f810c27cf8..3c3919dffe5f 100644 --- a/clippy_lints/src/methods/needless_collect.rs +++ b/clippy_lints/src/methods/needless_collect.rs @@ -322,7 +322,7 @@ fn visit_block(&mut self, block: &'tcx Block<'tcx>) { fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { // Check function calls on our collection - if let ExprKind::MethodCall(method_name, recv, [args @ ..], _) = &expr.kind { + if let ExprKind::MethodCall(method_name, recv, args, _) = &expr.kind { if method_name.ident.name == sym!(collect) && is_trait_method(self.cx, expr, sym::Iterator) { self.current_mutably_captured_ids = get_captured_ids(self.cx, self.cx.typeck_results().expr_ty(recv)); self.visit_expr(recv); diff --git a/clippy_lints/src/methods/str_splitn.rs b/clippy_lints/src/methods/str_splitn.rs index 5ea12c441840..88a3c2620a40 100644 --- a/clippy_lints/src/methods/str_splitn.rs +++ b/clippy_lints/src/methods/str_splitn.rs @@ -289,7 +289,7 @@ fn parse_iter_usage<'tcx>( ) -> Option { let (kind, span) = match iter.next() { Some((_, Node::Expr(e))) if e.span.ctxt() == ctxt => { - let ExprKind::MethodCall(name, _, [args @ ..], _) = e.kind else { + let ExprKind::MethodCall(name, _, args, _) = e.kind else { return None; }; let did = cx.typeck_results().type_dependent_def_id(e.hir_id)?; diff --git a/clippy_lints/src/misc_early/mod.rs b/clippy_lints/src/misc_early/mod.rs index 78253ab6cdc4..b226b8781237 100644 --- a/clippy_lints/src/misc_early/mod.rs +++ b/clippy_lints/src/misc_early/mod.rs @@ -2,6 +2,7 @@ mod double_neg; mod literal_suffix; mod mixed_case_hex_literals; +mod redundant_at_rest_pattern; mod redundant_pattern; mod unneeded_field_pattern; mod unneeded_wildcard_pattern; @@ -318,6 +319,36 @@ "tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`)" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `[all @ ..]` patterns. + /// + /// ### Why is this bad? + /// In all cases, `all` works fine and can often make code simpler, as you possibly won't need + /// to convert from say a `Vec` to a slice by dereferencing. + /// + /// ### Example + /// ```rust,ignore + /// if let [all @ ..] = &*v { + /// // NOTE: Type is a slice here + /// println!("all elements: {all:#?}"); + /// } + /// ``` + /// Use instead: + /// ```rust,ignore + /// if let all = v { + /// // NOTE: Type is a `Vec` here + /// println!("all elements: {all:#?}"); + /// } + /// // or + /// println!("all elements: {v:#?}"); + /// ``` + #[clippy::version = "1.72.0"] + pub REDUNDANT_AT_REST_PATTERN, + complexity, + "checks for `[all @ ..]` where `all` would suffice" +} + declare_lint_pass!(MiscEarlyLints => [ UNNEEDED_FIELD_PATTERN, DUPLICATE_UNDERSCORE_ARGUMENT, @@ -329,6 +360,7 @@ BUILTIN_TYPE_SHADOW, REDUNDANT_PATTERN, UNNEEDED_WILDCARD_PATTERN, + REDUNDANT_AT_REST_PATTERN, ]); impl EarlyLintPass for MiscEarlyLints { @@ -345,6 +377,7 @@ fn check_pat(&mut self, cx: &EarlyContext<'_>, pat: &Pat) { unneeded_field_pattern::check(cx, pat); redundant_pattern::check(cx, pat); + redundant_at_rest_pattern::check(cx, pat); unneeded_wildcard_pattern::check(cx, pat); } diff --git a/clippy_lints/src/misc_early/redundant_at_rest_pattern.rs b/clippy_lints/src/misc_early/redundant_at_rest_pattern.rs new file mode 100644 index 000000000000..0c81ee5eced8 --- /dev/null +++ b/clippy_lints/src/misc_early/redundant_at_rest_pattern.rs @@ -0,0 +1,26 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_ast::{Pat, PatKind}; +use rustc_errors::Applicability; +use rustc_lint::{EarlyContext, LintContext}; +use rustc_middle::lint::in_external_macro; + +use super::REDUNDANT_AT_REST_PATTERN; + +pub(super) fn check(cx: &EarlyContext<'_>, pat: &Pat) { + if !in_external_macro(cx.sess(), pat.span) + && let PatKind::Slice(slice) = &pat.kind + && let [one] = &**slice + && let PatKind::Ident(annotation, ident, Some(rest)) = &one.kind + && let PatKind::Rest = rest.kind + { + span_lint_and_sugg( + cx, + REDUNDANT_AT_REST_PATTERN, + pat.span, + "using a rest pattern to bind an entire slice to a local", + "this is better represented with just the binding", + format!("{}{ident}", annotation.prefix_str()), + Applicability::MachineApplicable, + ); + } +} diff --git a/clippy_utils/src/sugg.rs b/clippy_utils/src/sugg.rs index f477524eec5c..bc3e58169e61 100644 --- a/clippy_utils/src/sugg.rs +++ b/clippy_utils/src/sugg.rs @@ -942,7 +942,7 @@ fn borrow(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) { }, // item is used in a call // i.e.: `Call`: `|x| please(x)` or `MethodCall`: `|x| [1, 2, 3].contains(x)` - ExprKind::Call(_, [call_args @ ..]) | ExprKind::MethodCall(_, _, [call_args @ ..], _) => { + ExprKind::Call(_, call_args) | ExprKind::MethodCall(_, _, call_args, _) => { let expr = self.cx.tcx.hir().expect_expr(cmt.hir_id); let arg_ty_kind = self.cx.typeck_results().expr_ty(expr).kind(); diff --git a/tests/ui/manual_let_else_match.rs b/tests/ui/manual_let_else_match.rs index 07cd5a53a7c4..73ff69eec4ed 100644 --- a/tests/ui/manual_let_else_match.rs +++ b/tests/ui/manual_let_else_match.rs @@ -1,5 +1,9 @@ #![allow(unused_braces, unused_variables, dead_code)] -#![allow(clippy::collapsible_else_if, clippy::let_unit_value)] +#![allow( + clippy::collapsible_else_if, + clippy::let_unit_value, + clippy::redundant_at_rest_pattern +)] #![warn(clippy::manual_let_else)] // Ensure that we don't conflict with match -> if let lints #![warn(clippy::single_match_else, clippy::single_match)] diff --git a/tests/ui/manual_let_else_match.stderr b/tests/ui/manual_let_else_match.stderr index ead2f28e609a..3fd9a637605b 100644 --- a/tests/ui/manual_let_else_match.stderr +++ b/tests/ui/manual_let_else_match.stderr @@ -1,5 +1,5 @@ error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:32:5 + --> $DIR/manual_let_else_match.rs:36:5 | LL | / let v = match g() { LL | | Some(v_some) => v_some, @@ -10,7 +10,7 @@ LL | | }; = note: `-D clippy::manual-let-else` implied by `-D warnings` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:37:5 + --> $DIR/manual_let_else_match.rs:41:5 | LL | / let v = match g() { LL | | Some(v_some) => v_some, @@ -19,7 +19,7 @@ LL | | }; | |______^ help: consider writing: `let Some(v) = g() else { return };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:44:9 + --> $DIR/manual_let_else_match.rs:48:9 | LL | / let v = match h() { LL | | (Some(v), None) | (None, Some(v)) => v, @@ -28,7 +28,7 @@ LL | | }; | |__________^ help: consider writing: `let ((Some(v), None) | (None, Some(v))) = h() else { continue };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:49:9 + --> $DIR/manual_let_else_match.rs:53:9 | LL | / let v = match build_enum() { LL | | Variant::Bar(v) | Variant::Baz(v) => v, @@ -37,7 +37,7 @@ LL | | }; | |__________^ help: consider writing: `let (Variant::Bar(v) | Variant::Baz(v)) = build_enum() else { continue };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:57:5 + --> $DIR/manual_let_else_match.rs:61:5 | LL | / let v = match f() { LL | | Ok(v) => v, @@ -46,7 +46,7 @@ LL | | }; | |______^ help: consider writing: `let Ok(v) = f() else { return };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:63:5 + --> $DIR/manual_let_else_match.rs:67:5 | LL | / let v = match f().map_err(|_| ()) { LL | | Ok(v) => v, @@ -55,7 +55,7 @@ LL | | }; | |______^ help: consider writing: `let Ok(v) = f().map_err(|_| ()) else { return };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:70:5 + --> $DIR/manual_let_else_match.rs:74:5 | LL | / let _value = match f { LL | | Variant::Bar(v) | Variant::Baz(v) => v, @@ -64,7 +64,7 @@ LL | | }; | |______^ help: consider writing: `let (Variant::Bar(_value) | Variant::Baz(_value)) = f else { return };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:75:5 + --> $DIR/manual_let_else_match.rs:79:5 | LL | / let _value = match Some(build_enum()) { LL | | Some(Variant::Bar(v) | Variant::Baz(v)) => v, @@ -73,7 +73,7 @@ LL | | }; | |______^ help: consider writing: `let Some(Variant::Bar(_value) | Variant::Baz(_value)) = Some(build_enum()) else { return };` error: this could be rewritten as `let...else` - --> $DIR/manual_let_else_match.rs:81:5 + --> $DIR/manual_let_else_match.rs:85:5 | LL | / let data = match data.as_slice() { LL | | [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0] => data, diff --git a/tests/ui/match_on_vec_items.rs b/tests/ui/match_on_vec_items.rs index d82a736963a2..cf9c279cd2f1 100644 --- a/tests/ui/match_on_vec_items.rs +++ b/tests/ui/match_on_vec_items.rs @@ -1,5 +1,5 @@ #![warn(clippy::match_on_vec_items)] -#![allow(clippy::useless_vec)] +#![allow(clippy::redundant_at_rest_pattern, clippy::useless_vec)] fn match_with_wildcard() { let arr = vec![0, 1, 2, 3]; diff --git a/tests/ui/redundant_at_rest_pattern.fixed b/tests/ui/redundant_at_rest_pattern.fixed new file mode 100644 index 000000000000..080cf13b5dac --- /dev/null +++ b/tests/ui/redundant_at_rest_pattern.fixed @@ -0,0 +1,27 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow(irrefutable_let_patterns, unused)] +#![warn(clippy::redundant_at_rest_pattern)] + +#[macro_use] +extern crate proc_macros; + +fn main() { + if let a = [()] {} + if let ref a = [()] {} + if let mut a = [()] {} + if let ref mut a = [()] {} + let v = vec![()]; + if let a = &*v {} + let s = &[()]; + if let a = s {} + // Don't lint + if let [..] = &*v {} + if let [a] = &*v {} + if let [()] = &*v {} + if let [first, rest @ ..] = &*v {} + if let a = [()] {} + external! { + if let [a @ ..] = [()] {} + } +} diff --git a/tests/ui/redundant_at_rest_pattern.rs b/tests/ui/redundant_at_rest_pattern.rs new file mode 100644 index 000000000000..a8a802829564 --- /dev/null +++ b/tests/ui/redundant_at_rest_pattern.rs @@ -0,0 +1,27 @@ +//@run-rustfix +//@aux-build:proc_macros.rs:proc-macro +#![allow(irrefutable_let_patterns, unused)] +#![warn(clippy::redundant_at_rest_pattern)] + +#[macro_use] +extern crate proc_macros; + +fn main() { + if let [a @ ..] = [()] {} + if let [ref a @ ..] = [()] {} + if let [mut a @ ..] = [()] {} + if let [ref mut a @ ..] = [()] {} + let v = vec![()]; + if let [a @ ..] = &*v {} + let s = &[()]; + if let [a @ ..] = s {} + // Don't lint + if let [..] = &*v {} + if let [a] = &*v {} + if let [()] = &*v {} + if let [first, rest @ ..] = &*v {} + if let a = [()] {} + external! { + if let [a @ ..] = [()] {} + } +} diff --git a/tests/ui/redundant_at_rest_pattern.stderr b/tests/ui/redundant_at_rest_pattern.stderr new file mode 100644 index 000000000000..e2a4d9ffd57c --- /dev/null +++ b/tests/ui/redundant_at_rest_pattern.stderr @@ -0,0 +1,40 @@ +error: using a rest pattern to bind an entire slice to a local + --> $DIR/redundant_at_rest_pattern.rs:10:12 + | +LL | if let [a @ ..] = [()] {} + | ^^^^^^^^ help: this is better represented with just the binding: `a` + | + = note: `-D clippy::redundant-at-rest-pattern` implied by `-D warnings` + +error: using a rest pattern to bind an entire slice to a local + --> $DIR/redundant_at_rest_pattern.rs:11:12 + | +LL | if let [ref a @ ..] = [()] {} + | ^^^^^^^^^^^^ help: this is better represented with just the binding: `ref a` + +error: using a rest pattern to bind an entire slice to a local + --> $DIR/redundant_at_rest_pattern.rs:12:12 + | +LL | if let [mut a @ ..] = [()] {} + | ^^^^^^^^^^^^ help: this is better represented with just the binding: `mut a` + +error: using a rest pattern to bind an entire slice to a local + --> $DIR/redundant_at_rest_pattern.rs:13:12 + | +LL | if let [ref mut a @ ..] = [()] {} + | ^^^^^^^^^^^^^^^^ help: this is better represented with just the binding: `ref mut a` + +error: using a rest pattern to bind an entire slice to a local + --> $DIR/redundant_at_rest_pattern.rs:15:12 + | +LL | if let [a @ ..] = &*v {} + | ^^^^^^^^ help: this is better represented with just the binding: `a` + +error: using a rest pattern to bind an entire slice to a local + --> $DIR/redundant_at_rest_pattern.rs:17:12 + | +LL | if let [a @ ..] = s {} + | ^^^^^^^^ help: this is better represented with just the binding: `a` + +error: aborting due to 6 previous errors +