Merge pull request #2684 from 17cupsofcoffee/infallible-destructuring-match

Lint for destructuring tuple structs/variants with an infallible single-armed match

Author: oli-obk

clippy_lints/src/infallible_destructuring_match.rs

@@ -0,0 +1,79 @@
+use super::utils::{get_arg_name, match_var, remove_blocks, snippet, span_lint_and_sugg};
+use rustc::hir::*;
+use rustc::lint::*;
+
+/// **What it does:** Checks for matches being used to destructure a single-variant enum
+/// or tuple struct where a `let` will suffice.
+///
+/// **Why is this bad?** Just readability – `let` doesn't nest, whereas a `match` does.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ```rust
+/// enum Wrapper {
+///     Data(i32),
+/// }
+///
+/// let wrapper = Wrapper::Data(42);
+///
+/// let data = match wrapper {
+///     Wrapper::Data(i) => i,
+/// };
+/// ```
+///
+/// The correct use would be:
+/// ```rust
+/// enum Wrapper {
+///     Data(i32),
+/// }
+///
+/// let wrapper = Wrapper::Data(42);
+/// let Wrapper::Data(data) = wrapper;
+/// ```
+declare_clippy_lint! {
+    pub INFALLIBLE_DESTRUCTURING_MATCH,
+    style,
+    "a match statement with a single infallible arm instead of a `let`"
+}
+
+#[derive(Copy, Clone, Default)]
+pub struct Pass;
+
+impl LintPass for Pass {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(INFALLIBLE_DESTRUCTURING_MATCH)
+    }
+}
+
+impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    fn check_local(&mut self, cx: &LateContext<'a, 'tcx>, local: &'tcx Local) {
+        if_chain! {
+            if let Some(ref expr) = local.init;
+            if let Expr_::ExprMatch(ref target, ref arms, MatchSource::Normal) = expr.node;
+            if arms.len() == 1 && arms[0].pats.len() == 1 && arms[0].guard.is_none();
+            if let PatKind::TupleStruct(QPath::Resolved(None, ref variant_name), ref args, _) = arms[0].pats[0].node;
+            if args.len() == 1;
+            if let Some(arg) = get_arg_name(&args[0]);
+            let body = remove_blocks(&arms[0].body);
+            if match_var(body, arg);
+
+            then {
+                span_lint_and_sugg(
+                    cx,
+                    INFALLIBLE_DESTRUCTURING_MATCH,
+                    local.span,
+                    "you seem to be trying to use match to destructure a single infallible pattern. \
+                     Consider using `let`",
+                    "try this",
+                    format!(
+                        "let {}({}) = {};",
+                        snippet(cx, variant_name.span, ".."),
+                        snippet(cx, local.pat.span, ".."),
+                        snippet(cx, target.span, ".."),
+                    ),
+                );
+            }
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -133,6 +133,7 @@ pub mod identity_conversion;
 pub mod identity_op;
 pub mod if_let_redundant_pattern_matching;
 pub mod if_not_else;
+pub mod infallible_destructuring_match;
 pub mod infinite_iter;
 pub mod inline_fn_without_body;
 pub mod int_plus_one;
@@ -407,6 +408,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
     reg.register_late_lint_pass(box suspicious_trait_impl::SuspiciousImpl);
     reg.register_late_lint_pass(box redundant_field_names::RedundantFieldNames);
     reg.register_late_lint_pass(box map_unit_fn::Pass);
+    reg.register_late_lint_pass(box infallible_destructuring_match::Pass);
 
 
     reg.register_lint_group("clippy_restriction", vec![
@@ -520,6 +522,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         identity_conversion::IDENTITY_CONVERSION,
         identity_op::IDENTITY_OP,
         if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
+        infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
         infinite_iter::INFINITE_ITER,
         inline_fn_without_body::INLINE_FN_WITHOUT_BODY,
         int_plus_one::INT_PLUS_ONE,
@@ -688,6 +691,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry) {
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
         if_let_redundant_pattern_matching::IF_LET_REDUNDANT_PATTERN_MATCHING,
+        infallible_destructuring_match::INFALLIBLE_DESTRUCTURING_MATCH,
         len_zero::LEN_WITHOUT_IS_EMPTY,
         len_zero::LEN_ZERO,
         let_if_seq::USELESS_LET_IF_SEQ,

tests/ui/infallible_destructuring_match.rs

@@ -0,0 +1,83 @@
+#![feature(exhaustive_patterns)]
+#![allow(let_and_return)]
+
+enum SingleVariantEnum {
+    Variant(i32),
+}
+
+struct TupleStruct(i32);
+
+enum EmptyEnum {}
+
+fn infallible_destructuring_match_enum() {
+    let wrapper = SingleVariantEnum::Variant(0);
+
+    // This should lint!
+    let data = match wrapper {
+        SingleVariantEnum::Variant(i) => i,
+    };
+
+    // This shouldn't!
+    let data = match wrapper {
+        SingleVariantEnum::Variant(_) => -1,
+    };
+
+    // Neither should this!
+    let data = match wrapper {
+        SingleVariantEnum::Variant(i) => -1,
+    };
+
+    let SingleVariantEnum::Variant(data) = wrapper;
+}
+
+fn infallible_destructuring_match_struct() {
+    let wrapper = TupleStruct(0);
+
+    // This should lint!
+    let data = match wrapper {
+        TupleStruct(i) => i,
+    };
+
+    // This shouldn't!
+    let data = match wrapper {
+        TupleStruct(_) => -1,
+    };
+
+    // Neither should this!
+    let data = match wrapper {
+        TupleStruct(i) => -1,
+    };
+
+    let TupleStruct(data) = wrapper;
+}
+
+fn never_enum() {
+    let wrapper: Result<i32, !> = Ok(23);
+
+    // This should lint!
+    let data = match wrapper {
+        Ok(i) => i,
+    };
+
+    // This shouldn't!
+    let data = match wrapper {
+        Ok(_) => -1,
+    };
+
+    // Neither should this!
+    let data = match wrapper {
+        Ok(i) => -1,
+    };
+
+    let Ok(data) = wrapper;
+}
+
+impl EmptyEnum {
+    fn match_on(&self) -> ! {
+        // The lint shouldn't pick this up, as `let` won't work here!
+        let data = match *self {};
+        data
+    }
+}
+
+fn main() {}

tests/ui/infallible_destructuring_match.stderr

@@ -0,0 +1,28 @@
+error: you seem to be trying to use match to destructure a single infallible pattern. Consider using `let`
+  --> $DIR/infallible_destructuring_match.rs:16:5
+   |
+16 | /     let data = match wrapper {
+17 | |         SingleVariantEnum::Variant(i) => i,
+18 | |     };
+   | |______^ help: try this: `let SingleVariantEnum::Variant(data) = wrapper;`
+   |
+   = note: `-D infallible-destructuring-match` implied by `-D warnings`
+
+error: you seem to be trying to use match to destructure a single infallible pattern. Consider using `let`
+  --> $DIR/infallible_destructuring_match.rs:37:5
+   |
+37 | /     let data = match wrapper {
+38 | |         TupleStruct(i) => i,
+39 | |     };
+   | |______^ help: try this: `let TupleStruct(data) = wrapper;`
+
+error: you seem to be trying to use match to destructure a single infallible pattern. Consider using `let`
+  --> $DIR/infallible_destructuring_match.rs:58:5
+   |
+58 | /     let data = match wrapper {
+59 | |         Ok(i) => i,
+60 | |     };
+   | |______^ help: try this: `let Ok(data) = wrapper;`
+
+error: aborting due to 3 previous errors
+