Skip to content

Commit 3d4c536

Browse files
committed
Auto merge of #11013 - Centri3:redundant_rest_pattern, r=giraffate
New lint [`redundant_at_rest_pattern`] Closes #11011 It's always a great feeling when a new lint triggers on clippy itself 😄 changelog: New lint [`redundant_at_rest_pattern`]
2 parents 9020937 + 3376c71 commit 3d4c536

13 files changed

+173
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5137,6 +5137,7 @@ Released 2018-09-13
51375137
[`recursive_format_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#recursive_format_impl
51385138
[`redundant_allocation`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_allocation
51395139
[`redundant_async_block`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_async_block
5140+
[`redundant_at_rest_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_at_rest_pattern
51405141
[`redundant_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_clone
51415142
[`redundant_closure`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure
51425143
[`redundant_closure_call`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_closure_call

clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -431,6 +431,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
431431
crate::misc_early::DOUBLE_NEG_INFO,
432432
crate::misc_early::DUPLICATE_UNDERSCORE_ARGUMENT_INFO,
433433
crate::misc_early::MIXED_CASE_HEX_LITERALS_INFO,
434+
crate::misc_early::REDUNDANT_AT_REST_PATTERN_INFO,
434435
crate::misc_early::REDUNDANT_PATTERN_INFO,
435436
crate::misc_early::SEPARATED_LITERAL_SUFFIX_INFO,
436437
crate::misc_early::UNNEEDED_FIELD_PATTERN_INFO,

clippy_lints/src/methods/needless_collect.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ impl<'tcx> Visitor<'tcx> for IterFunctionVisitor<'_, 'tcx> {
322322

323323
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
324324
// Check function calls on our collection
325-
if let ExprKind::MethodCall(method_name, recv, [args @ ..], _) = &expr.kind {
325+
if let ExprKind::MethodCall(method_name, recv, args, _) = &expr.kind {
326326
if method_name.ident.name == sym!(collect) && is_trait_method(self.cx, expr, sym::Iterator) {
327327
self.current_mutably_captured_ids = get_captured_ids(self.cx, self.cx.typeck_results().expr_ty(recv));
328328
self.visit_expr(recv);

clippy_lints/src/methods/str_splitn.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ fn parse_iter_usage<'tcx>(
289289
) -> Option<IterUsage> {
290290
let (kind, span) = match iter.next() {
291291
Some((_, Node::Expr(e))) if e.span.ctxt() == ctxt => {
292-
let ExprKind::MethodCall(name, _, [args @ ..], _) = e.kind else {
292+
let ExprKind::MethodCall(name, _, args, _) = e.kind else {
293293
return None;
294294
};
295295
let did = cx.typeck_results().type_dependent_def_id(e.hir_id)?;

clippy_lints/src/misc_early/mod.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ mod builtin_type_shadow;
22
mod double_neg;
33
mod literal_suffix;
44
mod mixed_case_hex_literals;
5+
mod redundant_at_rest_pattern;
56
mod redundant_pattern;
67
mod unneeded_field_pattern;
78
mod unneeded_wildcard_pattern;
@@ -318,6 +319,36 @@ declare_clippy_lint! {
318319
"tuple patterns with a wildcard pattern (`_`) is next to a rest pattern (`..`)"
319320
}
320321

322+
declare_clippy_lint! {
323+
/// ### What it does
324+
/// Checks for `[all @ ..]` patterns.
325+
///
326+
/// ### Why is this bad?
327+
/// In all cases, `all` works fine and can often make code simpler, as you possibly won't need
328+
/// to convert from say a `Vec` to a slice by dereferencing.
329+
///
330+
/// ### Example
331+
/// ```rust,ignore
332+
/// if let [all @ ..] = &*v {
333+
/// // NOTE: Type is a slice here
334+
/// println!("all elements: {all:#?}");
335+
/// }
336+
/// ```
337+
/// Use instead:
338+
/// ```rust,ignore
339+
/// if let all = v {
340+
/// // NOTE: Type is a `Vec` here
341+
/// println!("all elements: {all:#?}");
342+
/// }
343+
/// // or
344+
/// println!("all elements: {v:#?}");
345+
/// ```
346+
#[clippy::version = "1.72.0"]
347+
pub REDUNDANT_AT_REST_PATTERN,
348+
complexity,
349+
"checks for `[all @ ..]` where `all` would suffice"
350+
}
351+
321352
declare_lint_pass!(MiscEarlyLints => [
322353
UNNEEDED_FIELD_PATTERN,
323354
DUPLICATE_UNDERSCORE_ARGUMENT,
@@ -329,6 +360,7 @@ declare_lint_pass!(MiscEarlyLints => [
329360
BUILTIN_TYPE_SHADOW,
330361
REDUNDANT_PATTERN,
331362
UNNEEDED_WILDCARD_PATTERN,
363+
REDUNDANT_AT_REST_PATTERN,
332364
]);
333365

334366
impl EarlyLintPass for MiscEarlyLints {
@@ -345,6 +377,7 @@ impl EarlyLintPass for MiscEarlyLints {
345377

346378
unneeded_field_pattern::check(cx, pat);
347379
redundant_pattern::check(cx, pat);
380+
redundant_at_rest_pattern::check(cx, pat);
348381
unneeded_wildcard_pattern::check(cx, pat);
349382
}
350383

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use rustc_ast::{Pat, PatKind};
3+
use rustc_errors::Applicability;
4+
use rustc_lint::{EarlyContext, LintContext};
5+
use rustc_middle::lint::in_external_macro;
6+
7+
use super::REDUNDANT_AT_REST_PATTERN;
8+
9+
pub(super) fn check(cx: &EarlyContext<'_>, pat: &Pat) {
10+
if !in_external_macro(cx.sess(), pat.span)
11+
&& let PatKind::Slice(slice) = &pat.kind
12+
&& let [one] = &**slice
13+
&& let PatKind::Ident(annotation, ident, Some(rest)) = &one.kind
14+
&& let PatKind::Rest = rest.kind
15+
{
16+
span_lint_and_sugg(
17+
cx,
18+
REDUNDANT_AT_REST_PATTERN,
19+
pat.span,
20+
"using a rest pattern to bind an entire slice to a local",
21+
"this is better represented with just the binding",
22+
format!("{}{ident}", annotation.prefix_str()),
23+
Applicability::MachineApplicable,
24+
);
25+
}
26+
}

clippy_utils/src/sugg.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -942,7 +942,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
942942
},
943943
// item is used in a call
944944
// i.e.: `Call`: `|x| please(x)` or `MethodCall`: `|x| [1, 2, 3].contains(x)`
945-
ExprKind::Call(_, [call_args @ ..]) | ExprKind::MethodCall(_, _, [call_args @ ..], _) => {
945+
ExprKind::Call(_, call_args) | ExprKind::MethodCall(_, _, call_args, _) => {
946946
let expr = self.cx.tcx.hir().expect_expr(cmt.hir_id);
947947
let arg_ty_kind = self.cx.typeck_results().expr_ty(expr).kind();
948948

tests/ui/manual_let_else_match.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
#![allow(unused_braces, unused_variables, dead_code)]
2-
#![allow(clippy::collapsible_else_if, clippy::let_unit_value)]
2+
#![allow(
3+
clippy::collapsible_else_if,
4+
clippy::let_unit_value,
5+
clippy::redundant_at_rest_pattern
6+
)]
37
#![warn(clippy::manual_let_else)]
48
// Ensure that we don't conflict with match -> if let lints
59
#![warn(clippy::single_match_else, clippy::single_match)]

tests/ui/manual_let_else_match.stderr

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this could be rewritten as `let...else`
2-
--> $DIR/manual_let_else_match.rs:32:5
2+
--> $DIR/manual_let_else_match.rs:36:5
33
|
44
LL | / let v = match g() {
55
LL | | Some(v_some) => v_some,
@@ -10,7 +10,7 @@ LL | | };
1010
= note: `-D clippy::manual-let-else` implied by `-D warnings`
1111

1212
error: this could be rewritten as `let...else`
13-
--> $DIR/manual_let_else_match.rs:37:5
13+
--> $DIR/manual_let_else_match.rs:41:5
1414
|
1515
LL | / let v = match g() {
1616
LL | | Some(v_some) => v_some,
@@ -19,7 +19,7 @@ LL | | };
1919
| |______^ help: consider writing: `let Some(v) = g() else { return };`
2020

2121
error: this could be rewritten as `let...else`
22-
--> $DIR/manual_let_else_match.rs:44:9
22+
--> $DIR/manual_let_else_match.rs:48:9
2323
|
2424
LL | / let v = match h() {
2525
LL | | (Some(v), None) | (None, Some(v)) => v,
@@ -28,7 +28,7 @@ LL | | };
2828
| |__________^ help: consider writing: `let ((Some(v), None) | (None, Some(v))) = h() else { continue };`
2929

3030
error: this could be rewritten as `let...else`
31-
--> $DIR/manual_let_else_match.rs:49:9
31+
--> $DIR/manual_let_else_match.rs:53:9
3232
|
3333
LL | / let v = match build_enum() {
3434
LL | | Variant::Bar(v) | Variant::Baz(v) => v,
@@ -37,7 +37,7 @@ LL | | };
3737
| |__________^ help: consider writing: `let (Variant::Bar(v) | Variant::Baz(v)) = build_enum() else { continue };`
3838

3939
error: this could be rewritten as `let...else`
40-
--> $DIR/manual_let_else_match.rs:57:5
40+
--> $DIR/manual_let_else_match.rs:61:5
4141
|
4242
LL | / let v = match f() {
4343
LL | | Ok(v) => v,
@@ -46,7 +46,7 @@ LL | | };
4646
| |______^ help: consider writing: `let Ok(v) = f() else { return };`
4747

4848
error: this could be rewritten as `let...else`
49-
--> $DIR/manual_let_else_match.rs:63:5
49+
--> $DIR/manual_let_else_match.rs:67:5
5050
|
5151
LL | / let v = match f().map_err(|_| ()) {
5252
LL | | Ok(v) => v,
@@ -55,7 +55,7 @@ LL | | };
5555
| |______^ help: consider writing: `let Ok(v) = f().map_err(|_| ()) else { return };`
5656

5757
error: this could be rewritten as `let...else`
58-
--> $DIR/manual_let_else_match.rs:70:5
58+
--> $DIR/manual_let_else_match.rs:74:5
5959
|
6060
LL | / let _value = match f {
6161
LL | | Variant::Bar(v) | Variant::Baz(v) => v,
@@ -64,7 +64,7 @@ LL | | };
6464
| |______^ help: consider writing: `let (Variant::Bar(_value) | Variant::Baz(_value)) = f else { return };`
6565

6666
error: this could be rewritten as `let...else`
67-
--> $DIR/manual_let_else_match.rs:75:5
67+
--> $DIR/manual_let_else_match.rs:79:5
6868
|
6969
LL | / let _value = match Some(build_enum()) {
7070
LL | | Some(Variant::Bar(v) | Variant::Baz(v)) => v,
@@ -73,7 +73,7 @@ LL | | };
7373
| |______^ help: consider writing: `let Some(Variant::Bar(_value) | Variant::Baz(_value)) = Some(build_enum()) else { return };`
7474

7575
error: this could be rewritten as `let...else`
76-
--> $DIR/manual_let_else_match.rs:81:5
76+
--> $DIR/manual_let_else_match.rs:85:5
7777
|
7878
LL | / let data = match data.as_slice() {
7979
LL | | [data @ .., 0, 0, 0, 0] | [data @ .., 0, 0] | [data @ .., 0] => data,

tests/ui/match_on_vec_items.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#![warn(clippy::match_on_vec_items)]
2-
#![allow(clippy::useless_vec)]
2+
#![allow(clippy::redundant_at_rest_pattern, clippy::useless_vec)]
33

44
fn match_with_wildcard() {
55
let arr = vec![0, 1, 2, 3];
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(irrefutable_let_patterns, unused)]
4+
#![warn(clippy::redundant_at_rest_pattern)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
fn main() {
10+
if let a = [()] {}
11+
if let ref a = [()] {}
12+
if let mut a = [()] {}
13+
if let ref mut a = [()] {}
14+
let v = vec![()];
15+
if let a = &*v {}
16+
let s = &[()];
17+
if let a = s {}
18+
// Don't lint
19+
if let [..] = &*v {}
20+
if let [a] = &*v {}
21+
if let [()] = &*v {}
22+
if let [first, rest @ ..] = &*v {}
23+
if let a = [()] {}
24+
external! {
25+
if let [a @ ..] = [()] {}
26+
}
27+
}

tests/ui/redundant_at_rest_pattern.rs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(irrefutable_let_patterns, unused)]
4+
#![warn(clippy::redundant_at_rest_pattern)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
fn main() {
10+
if let [a @ ..] = [()] {}
11+
if let [ref a @ ..] = [()] {}
12+
if let [mut a @ ..] = [()] {}
13+
if let [ref mut a @ ..] = [()] {}
14+
let v = vec![()];
15+
if let [a @ ..] = &*v {}
16+
let s = &[()];
17+
if let [a @ ..] = s {}
18+
// Don't lint
19+
if let [..] = &*v {}
20+
if let [a] = &*v {}
21+
if let [()] = &*v {}
22+
if let [first, rest @ ..] = &*v {}
23+
if let a = [()] {}
24+
external! {
25+
if let [a @ ..] = [()] {}
26+
}
27+
}
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
error: using a rest pattern to bind an entire slice to a local
2+
--> $DIR/redundant_at_rest_pattern.rs:10:12
3+
|
4+
LL | if let [a @ ..] = [()] {}
5+
| ^^^^^^^^ help: this is better represented with just the binding: `a`
6+
|
7+
= note: `-D clippy::redundant-at-rest-pattern` implied by `-D warnings`
8+
9+
error: using a rest pattern to bind an entire slice to a local
10+
--> $DIR/redundant_at_rest_pattern.rs:11:12
11+
|
12+
LL | if let [ref a @ ..] = [()] {}
13+
| ^^^^^^^^^^^^ help: this is better represented with just the binding: `ref a`
14+
15+
error: using a rest pattern to bind an entire slice to a local
16+
--> $DIR/redundant_at_rest_pattern.rs:12:12
17+
|
18+
LL | if let [mut a @ ..] = [()] {}
19+
| ^^^^^^^^^^^^ help: this is better represented with just the binding: `mut a`
20+
21+
error: using a rest pattern to bind an entire slice to a local
22+
--> $DIR/redundant_at_rest_pattern.rs:13:12
23+
|
24+
LL | if let [ref mut a @ ..] = [()] {}
25+
| ^^^^^^^^^^^^^^^^ help: this is better represented with just the binding: `ref mut a`
26+
27+
error: using a rest pattern to bind an entire slice to a local
28+
--> $DIR/redundant_at_rest_pattern.rs:15:12
29+
|
30+
LL | if let [a @ ..] = &*v {}
31+
| ^^^^^^^^ help: this is better represented with just the binding: `a`
32+
33+
error: using a rest pattern to bind an entire slice to a local
34+
--> $DIR/redundant_at_rest_pattern.rs:17:12
35+
|
36+
LL | if let [a @ ..] = s {}
37+
| ^^^^^^^^ help: this is better represented with just the binding: `a`
38+
39+
error: aborting due to 6 previous errors
40+

0 commit comments

Comments
 (0)