diff --git a/clippy_lints/src/infallible_destructuring_match.rs b/clippy_lints/src/infallible_destructuring_match.rs new file mode 100644 index 00000000000..a2b3846b986 --- /dev/null +++ b/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, ".."), + ), + ); + } + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 890d7a6e22e..9b005016e48 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/tests/ui/infallible_destructuring_match.rs b/tests/ui/infallible_destructuring_match.rs new file mode 100644 index 00000000000..270272261b5 --- /dev/null +++ b/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 = 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() {} diff --git a/tests/ui/infallible_destructuring_match.stderr b/tests/ui/infallible_destructuring_match.stderr new file mode 100644 index 00000000000..8ee73bbfde8 --- /dev/null +++ b/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 +