Skip to content
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 @@ -5131,6 +5131,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
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 @@ -430,6 +430,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
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,
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/needless_collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ impl<'tcx> Visitor<'tcx> for IterFunctionVisitor<'_, '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);
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/methods/str_splitn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ fn parse_iter_usage<'tcx>(
) -> Option<IterUsage> {
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)?;
Expand Down
33 changes: 33 additions & 0 deletions clippy_lints/src/misc_early/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ mod builtin_type_shadow;
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;
Expand Down Expand Up @@ -318,6 +319,36 @@ declare_clippy_lint! {
"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:#?}");
/// }
Comment on lines +339 to +342
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It warns irrefutable_let_patterns in this case, so can we add the case that simply print?

It would be better to detect irrefutable_let_patterns, but IMO this lint is still worth it as is.

Suggested change
/// if let all = v {
/// // NOTE: Type is a `Vec` here
/// println!("all elements: {all:#?}");
/// }
/// if let all = v {
/// // NOTE: Type is a `Vec` here
/// println!("all elements: {all:#?}");
/// }
/// // or
/// println!("all elements: {v:#?}");

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine. Many lints clippy or not will still lint even if it'll give a warning, and then that lint will hopefully be applied as well by the programmer. Though yeah I'm fine with adding just the println!

/// // 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,
Expand All @@ -329,6 +360,7 @@ declare_lint_pass!(MiscEarlyLints => [
BUILTIN_TYPE_SHADOW,
REDUNDANT_PATTERN,
UNNEEDED_WILDCARD_PATTERN,
REDUNDANT_AT_REST_PATTERN,
]);

impl EarlyLintPass for MiscEarlyLints {
Expand All @@ -345,6 +377,7 @@ impl EarlyLintPass for MiscEarlyLints {

unneeded_field_pattern::check(cx, pat);
redundant_pattern::check(cx, pat);
redundant_at_rest_pattern::check(cx, pat);
unneeded_wildcard_pattern::check(cx, pat);
}

Expand Down
26 changes: 26 additions & 0 deletions clippy_lints/src/misc_early/redundant_at_rest_pattern.rs
Original file line number Diff line number Diff line change
@@ -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,
);
}
}
2 changes: 1 addition & 1 deletion clippy_utils/src/sugg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -942,7 +942,7 @@ impl<'tcx> Delegate<'tcx> for DerefDelegate<'_, 'tcx> {
},
// 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();

Expand Down
6 changes: 5 additions & 1 deletion tests/ui/manual_let_else_match.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down
18 changes: 9 additions & 9 deletions tests/ui/manual_let_else_match.stderr
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/match_on_vec_items.rs
Original file line number Diff line number Diff line change
@@ -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];
Expand Down
27 changes: 27 additions & 0 deletions tests/ui/redundant_at_rest_pattern.fixed
Original file line number Diff line number Diff line change
@@ -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 @ ..] = [()] {}
}
}
27 changes: 27 additions & 0 deletions tests/ui/redundant_at_rest_pattern.rs
Original file line number Diff line number Diff line change
@@ -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 @ ..] = [()] {}
}
}
40 changes: 40 additions & 0 deletions tests/ui/redundant_at_rest_pattern.stderr
Original file line number Diff line number Diff line change
@@ -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