Add new lint: match with a single binding statement
- Lint name: MATCH_SINGLE_BINDING
This commit is contained in:
@@ -605,6 +605,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
&matches::MATCH_OVERLAPPING_ARM,
|
&matches::MATCH_OVERLAPPING_ARM,
|
||||||
&matches::MATCH_REF_PATS,
|
&matches::MATCH_REF_PATS,
|
||||||
&matches::MATCH_WILD_ERR_ARM,
|
&matches::MATCH_WILD_ERR_ARM,
|
||||||
|
&matches::MATCH_SINGLE_BINDING,
|
||||||
&matches::SINGLE_MATCH,
|
&matches::SINGLE_MATCH,
|
||||||
&matches::SINGLE_MATCH_ELSE,
|
&matches::SINGLE_MATCH_ELSE,
|
||||||
&matches::WILDCARD_ENUM_MATCH_ARM,
|
&matches::WILDCARD_ENUM_MATCH_ARM,
|
||||||
@@ -1206,6 +1207,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
|
LintId::of(&matches::MATCH_OVERLAPPING_ARM),
|
||||||
LintId::of(&matches::MATCH_REF_PATS),
|
LintId::of(&matches::MATCH_REF_PATS),
|
||||||
LintId::of(&matches::MATCH_WILD_ERR_ARM),
|
LintId::of(&matches::MATCH_WILD_ERR_ARM),
|
||||||
|
LintId::of(&matches::MATCH_SINGLE_BINDING),
|
||||||
LintId::of(&matches::SINGLE_MATCH),
|
LintId::of(&matches::SINGLE_MATCH),
|
||||||
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
|
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
|
||||||
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
|
LintId::of(&mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
|
||||||
@@ -1483,6 +1485,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
|
|||||||
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
|
LintId::of(&map_unit_fn::OPTION_MAP_UNIT_FN),
|
||||||
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
|
LintId::of(&map_unit_fn::RESULT_MAP_UNIT_FN),
|
||||||
LintId::of(&matches::MATCH_AS_REF),
|
LintId::of(&matches::MATCH_AS_REF),
|
||||||
|
LintId::of(&matches::MATCH_SINGLE_BINDING),
|
||||||
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
|
LintId::of(&matches::WILDCARD_IN_OR_PATTERNS),
|
||||||
LintId::of(&methods::CHARS_NEXT_CMP),
|
LintId::of(&methods::CHARS_NEXT_CMP),
|
||||||
LintId::of(&methods::CLONE_ON_COPY),
|
LintId::of(&methods::CLONE_ON_COPY),
|
||||||
|
|||||||
@@ -3,9 +3,9 @@ use crate::utils::paths;
|
|||||||
use crate::utils::sugg::Sugg;
|
use crate::utils::sugg::Sugg;
|
||||||
use crate::utils::usage::is_unused;
|
use crate::utils::usage::is_unused;
|
||||||
use crate::utils::{
|
use crate::utils::{
|
||||||
expr_block, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks, snippet,
|
span_lint_and_help, span_lint_and_note,
|
||||||
snippet_with_applicability, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
|
expr_block, in_macro, is_allowed, is_expn_of, is_wild, match_qpath, match_type, multispan_sugg, remove_blocks,
|
||||||
walk_ptrs_ty,
|
snippet, snippet_with_applicability, span_lint_and_sugg, span_lint_and_then,
|
||||||
};
|
};
|
||||||
use if_chain::if_chain;
|
use if_chain::if_chain;
|
||||||
use rustc::lint::in_external_macro;
|
use rustc::lint::in_external_macro;
|
||||||
@@ -245,6 +245,33 @@ declare_clippy_lint! {
|
|||||||
"a wildcard pattern used with others patterns in same match arm"
|
"a wildcard pattern used with others patterns in same match arm"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
declare_clippy_lint! {
|
||||||
|
/// **What it does:** Checks for useless match that binds to only one value.
|
||||||
|
///
|
||||||
|
/// **Why is this bad?** Readability and needless complexity.
|
||||||
|
///
|
||||||
|
/// **Known problems:** This situation frequently happen in macros, so can't lint there.
|
||||||
|
///
|
||||||
|
/// **Example:**
|
||||||
|
/// ```rust
|
||||||
|
/// # let a = 1;
|
||||||
|
/// # let b = 2;
|
||||||
|
///
|
||||||
|
/// // Bad
|
||||||
|
/// match (a, b) {
|
||||||
|
/// (c, d) => {
|
||||||
|
/// // useless match
|
||||||
|
/// }
|
||||||
|
/// }
|
||||||
|
///
|
||||||
|
/// // Good
|
||||||
|
/// let (c, d) = (a, b);
|
||||||
|
/// ```
|
||||||
|
pub MATCH_SINGLE_BINDING,
|
||||||
|
complexity,
|
||||||
|
"a match with a single binding instead of using `let` statement"
|
||||||
|
}
|
||||||
|
|
||||||
declare_lint_pass!(Matches => [
|
declare_lint_pass!(Matches => [
|
||||||
SINGLE_MATCH,
|
SINGLE_MATCH,
|
||||||
MATCH_REF_PATS,
|
MATCH_REF_PATS,
|
||||||
@@ -254,7 +281,8 @@ declare_lint_pass!(Matches => [
|
|||||||
MATCH_WILD_ERR_ARM,
|
MATCH_WILD_ERR_ARM,
|
||||||
MATCH_AS_REF,
|
MATCH_AS_REF,
|
||||||
WILDCARD_ENUM_MATCH_ARM,
|
WILDCARD_ENUM_MATCH_ARM,
|
||||||
WILDCARD_IN_OR_PATTERNS
|
WILDCARD_IN_OR_PATTERNS,
|
||||||
|
MATCH_SINGLE_BINDING,
|
||||||
]);
|
]);
|
||||||
|
|
||||||
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
|
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
|
||||||
@@ -270,6 +298,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Matches {
|
|||||||
check_wild_enum_match(cx, ex, arms);
|
check_wild_enum_match(cx, ex, arms);
|
||||||
check_match_as_ref(cx, ex, arms, expr);
|
check_match_as_ref(cx, ex, arms, expr);
|
||||||
check_wild_in_or_pats(cx, arms);
|
check_wild_in_or_pats(cx, arms);
|
||||||
|
check_match_single_binding(cx, ex, arms, expr);
|
||||||
}
|
}
|
||||||
if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
|
if let ExprKind::Match(ref ex, ref arms, _) = expr.kind {
|
||||||
check_match_ref_pats(cx, ex, arms, expr);
|
check_match_ref_pats(cx, ex, arms, expr);
|
||||||
@@ -712,6 +741,29 @@ fn check_wild_in_or_pats(cx: &LateContext<'_, '_>, arms: &[Arm<'_>]) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn check_match_single_binding(cx: &LateContext<'_, '_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: &Expr<'_>) {
|
||||||
|
if in_macro(expr.span) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if arms.len() == 1 {
|
||||||
|
let bind_names = arms[0].pat.span;
|
||||||
|
let matched_vars = ex.span;
|
||||||
|
span_lint_and_sugg(
|
||||||
|
cx,
|
||||||
|
MATCH_SINGLE_BINDING,
|
||||||
|
expr.span,
|
||||||
|
"this match could be written as a `let` statement",
|
||||||
|
"try this",
|
||||||
|
format!(
|
||||||
|
"let {} = {};",
|
||||||
|
snippet(cx, bind_names, ".."),
|
||||||
|
snippet(cx, matched_vars, "..")
|
||||||
|
),
|
||||||
|
Applicability::HasPlaceholders,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Gets all arms that are unbounded `PatRange`s.
|
/// Gets all arms that are unbounded `PatRange`s.
|
||||||
fn all_ranges<'a, 'tcx>(
|
fn all_ranges<'a, 'tcx>(
|
||||||
cx: &LateContext<'a, 'tcx>,
|
cx: &LateContext<'a, 'tcx>,
|
||||||
|
|||||||
25
tests/ui/match_single_binding.rs
Normal file
25
tests/ui/match_single_binding.rs
Normal file
@@ -0,0 +1,25 @@
|
|||||||
|
#![warn(clippy::match_single_binding)]
|
||||||
|
#[allow(clippy::many_single_char_names)]
|
||||||
|
|
||||||
|
fn main() {
|
||||||
|
let a = 1;
|
||||||
|
let b = 2;
|
||||||
|
let c = 3;
|
||||||
|
// Lint
|
||||||
|
match (a, b, c) {
|
||||||
|
(x, y, z) => {
|
||||||
|
println!("{} {} {}", x, y, z);
|
||||||
|
},
|
||||||
|
}
|
||||||
|
// Ok
|
||||||
|
match a {
|
||||||
|
2 => println!("2"),
|
||||||
|
_ => println!("Not 2"),
|
||||||
|
}
|
||||||
|
// Ok
|
||||||
|
let d = Some(5);
|
||||||
|
match d {
|
||||||
|
Some(d) => println!("5"),
|
||||||
|
_ => println!("None"),
|
||||||
|
}
|
||||||
|
}
|
||||||
14
tests/ui/match_single_binding.stderr
Normal file
14
tests/ui/match_single_binding.stderr
Normal file
@@ -0,0 +1,14 @@
|
|||||||
|
error: this match could be written as a `let` statement
|
||||||
|
--> $DIR/match_single_binding.rs:9:5
|
||||||
|
|
|
||||||
|
LL | / match (a, b, c) {
|
||||||
|
LL | | (x, y, z) => {
|
||||||
|
LL | | println!("{} {} {}", x, y, z);
|
||||||
|
LL | | },
|
||||||
|
LL | | }
|
||||||
|
| |_____^ help: try this: `let (x, y, z) = (a, b, c);`
|
||||||
|
|
|
||||||
|
= note: `-D clippy::match-single-binding` implied by `-D warnings`
|
||||||
|
|
||||||
|
error: aborting due to previous error
|
||||||
|
|
||||||
Reference in New Issue
Block a user