Auto merge of #5378 - Centril:unnested-or-pats, r=flip1995,phansch

New lint: `unnested_or_patterns`

changelog: Adds a lint `unnested_or_patterns`, suggesting `Some(0 | 2)` as opposed to `Some(0) | Some(2)`. The lint only fires on compilers capable of using `#![feature(or_patterns)]`.

- The lint is primarily encoded as a pure algorithm which to unnest or-patterns in an `ast::Pat` (`fn unnest_or_patterns`) through a `MutVisitor`. After that is done, and assuming that any change was detected, then `pprust::pat_to_string` is used to simply convert the transformed pattern into a suggestion.

- The PR introduces a module `utils::ast_utils` with a bunch of functions for spanless & nodeless equality comparisons of ASTs.

cc rust-lang/rust#54883

Author: bors

CHANGELOG.md

@@ -1683,6 +1683,7 @@ Released 2018-09-13
 [`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
 [`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern
 [`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern
+[`unnested_or_patterns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnested_or_patterns
 [`unreachable`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreachable
 [`unreadable_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#unreadable_literal
 [`unsafe_derive_deserialize`]: https://rust-lang.github.io/rust-clippy/master/index.html#unsafe_derive_deserialize

clippy_lints/src/await_holding_lock.rs

@@ -54,18 +54,13 @@ declare_lint_pass!(AwaitHoldingLock => [AWAIT_HOLDING_LOCK]);
 impl LateLintPass<'_, '_> for AwaitHoldingLock {
     fn check_body(&mut self, cx: &LateContext<'_, '_>, body: &'_ Body<'_>) {
         use AsyncGeneratorKind::{Block, Closure, Fn};
-        match body.generator_kind {
-            Some(GeneratorKind::Async(Block))
-            | Some(GeneratorKind::Async(Closure))
-            | Some(GeneratorKind::Async(Fn)) => {
-                let body_id = BodyId {
-                    hir_id: body.value.hir_id,
-                };
-                let def_id = cx.tcx.hir().body_owner_def_id(body_id);
-                let tables = cx.tcx.typeck_tables_of(def_id);
-                check_interior_types(cx, &tables.generator_interior_types, body.value.span);
-            },
-            _ => {},
+        if let Some(GeneratorKind::Async(Block | Closure | Fn)) = body.generator_kind {
+            let body_id = BodyId {
+                hir_id: body.value.hir_id,
+            };
+            let def_id = cx.tcx.hir().body_owner_def_id(body_id);
+            let tables = cx.tcx.typeck_tables_of(def_id);
+            check_interior_types(cx, &tables.generator_interior_types, body.value.span);
         }
     }
 }

clippy_lints/src/formatting.rs

@@ -112,12 +112,8 @@ declare_lint_pass!(Formatting => [
 impl EarlyLintPass for Formatting {
     fn check_block(&mut self, cx: &EarlyContext<'_>, block: &Block) {
         for w in block.stmts.windows(2) {
-            match (&w[0].kind, &w[1].kind) {
-                (&StmtKind::Expr(ref first), &StmtKind::Expr(ref second))
-                | (&StmtKind::Expr(ref first), &StmtKind::Semi(ref second)) => {
-                    check_missing_else(cx, first, second);
-                },
-                _ => (),
+            if let (StmtKind::Expr(first), StmtKind::Expr(second) | StmtKind::Semi(second)) = (&w[0].kind, &w[1].kind) {
+                check_missing_else(cx, first, second);
             }
         }
     }

clippy_lints/src/lib.rs

@@ -1,5 +1,6 @@
 // error-pattern:cargo-clippy
 
+#![feature(bindings_after_at)]
 #![feature(box_syntax)]
 #![feature(box_patterns)]
 #![feature(or_patterns)]
@@ -12,6 +13,7 @@
 #![cfg_attr(feature = "deny-warnings", deny(warnings))]
 #![feature(crate_visibility_modifier)]
 #![feature(concat_idents)]
+#![feature(drain_filter)]
 
 // FIXME: switch to something more ergonomic here, once available.
 // (Currently there is no way to opt into sysroot crates without `extern crate`.)
@@ -319,6 +321,7 @@ mod types;
 mod unicode;
 mod unnamed_address;
 mod unnecessary_sort_by;
+mod unnested_or_patterns;
 mod unsafe_removed_from_name;
 mod unused_io_amount;
 mod unused_self;
@@ -836,6 +839,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &unnamed_address::FN_ADDRESS_COMPARISONS,
         &unnamed_address::VTABLE_ADDRESS_COMPARISONS,
         &unnecessary_sort_by::UNNECESSARY_SORT_BY,
+        &unnested_or_patterns::UNNESTED_OR_PATTERNS,
         &unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME,
         &unused_io_amount::UNUSED_IO_AMOUNT,
         &unused_self::UNUSED_SELF,
@@ -1073,6 +1077,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(move || box non_expressive_names::NonExpressiveNames {
         single_char_binding_names_threshold,
     });
+    store.register_early_pass(|| box unnested_or_patterns::UnnestedOrPatterns);
 
     store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1433,6 +1438,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&unnamed_address::FN_ADDRESS_COMPARISONS),
         LintId::of(&unnamed_address::VTABLE_ADDRESS_COMPARISONS),
         LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
+        LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
         LintId::of(&unsafe_removed_from_name::UNSAFE_REMOVED_FROM_NAME),
         LintId::of(&unused_io_amount::UNUSED_IO_AMOUNT),
         LintId::of(&unwrap::PANICKING_UNWRAP),
@@ -1616,6 +1622,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&types::UNNECESSARY_CAST),
         LintId::of(&types::VEC_BOX),
         LintId::of(&unnecessary_sort_by::UNNECESSARY_SORT_BY),
+        LintId::of(&unnested_or_patterns::UNNESTED_OR_PATTERNS),
         LintId::of(&unwrap::UNNECESSARY_UNWRAP),
         LintId::of(&useless_conversion::USELESS_CONVERSION),
         LintId::of(&zero_div_zero::ZERO_DIVIDED_BY_ZERO),

clippy_lints/src/methods/manual_saturating_arithmetic.rs

@@ -57,7 +57,7 @@ pub fn lint(cx: &LateContext<'_, '_>, expr: &hir::Expr<'_>, args: &[&[hir::Expr<
         );
     } else {
         match (mm, arith) {
-            (MinMax::Max, "add") | (MinMax::Max, "mul") | (MinMax::Min, "sub") => (),
+            (MinMax::Max, "add" | "mul") | (MinMax::Min, "sub") => (),
             _ => return,
         }
 

clippy_lints/src/methods/mod.rs

@@ -1403,9 +1403,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
                 lint_search_is_some(cx, expr, "rposition", arg_lists[1], arg_lists[0], method_spans[1])
             },
             ["extend", ..] => lint_extend(cx, expr, arg_lists[0]),
-            ["as_ptr", "unwrap"] | ["as_ptr", "expect"] => {
-                lint_cstring_as_ptr(cx, expr, &arg_lists[1][0], &arg_lists[0][0])
-            },
+            ["as_ptr", "unwrap" | "expect"] => lint_cstring_as_ptr(cx, expr, &arg_lists[1][0], &arg_lists[0][0]),
             ["nth", "iter"] => lint_iter_nth(cx, expr, &arg_lists, false),
             ["nth", "iter_mut"] => lint_iter_nth(cx, expr, &arg_lists, true),
             ["nth", ..] => lint_iter_nth_zero(cx, expr, arg_lists[0]),
@@ -1418,12 +1416,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
             ["filter_map", ..] => unnecessary_filter_map::lint(cx, expr, arg_lists[0]),
             ["count", "map"] => lint_suspicious_map(cx, expr),
             ["assume_init"] => lint_maybe_uninit(cx, &arg_lists[0][0], expr),
-            ["unwrap_or", arith @ "checked_add"]
-            | ["unwrap_or", arith @ "checked_sub"]
-            | ["unwrap_or", arith @ "checked_mul"] => {
+            ["unwrap_or", arith @ ("checked_add" | "checked_sub" | "checked_mul")] => {
                 manual_saturating_arithmetic::lint(cx, expr, &arg_lists, &arith["checked_".len()..])
             },
-            ["add"] | ["offset"] | ["sub"] | ["wrapping_offset"] | ["wrapping_add"] | ["wrapping_sub"] => {
+            ["add" | "offset" | "sub" | "wrapping_offset" | "wrapping_add" | "wrapping_sub"] => {
                 check_pointer_offset(cx, expr, arg_lists[0])
             },
             ["is_file", ..] => lint_filetype_is_file(cx, expr, arg_lists[0]),
@@ -1829,8 +1825,7 @@ fn lint_expect_fun_call(
             hir::ExprKind::Call(fun, _) => {
                 if let hir::ExprKind::Path(ref p) = fun.kind {
                     match cx.tables.qpath_res(p, fun.hir_id) {
-                        hir::def::Res::Def(hir::def::DefKind::Fn, def_id)
-                        | hir::def::Res::Def(hir::def::DefKind::AssocFn, def_id) => matches!(
+                        hir::def::Res::Def(hir::def::DefKind::Fn | hir::def::DefKind::AssocFn, def_id) => matches!(
                             cx.tcx.fn_sig(def_id).output().skip_binder().kind,
                             ty::Ref(ty::ReStatic, ..)
                         ),

clippy_lints/src/misc.rs

@@ -275,17 +275,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MiscLints {
             return;
         }
         for arg in iter_input_pats(decl, body) {
-            match arg.pat.kind {
-                PatKind::Binding(BindingAnnotation::Ref, ..) | PatKind::Binding(BindingAnnotation::RefMut, ..) => {
-                    span_lint(
-                        cx,
-                        TOPLEVEL_REF_ARG,
-                        arg.pat.span,
-                        "`ref` directly on a function argument is ignored. Consider using a reference type \
-                         instead.",
-                    );
-                },
-                _ => {},
+            if let PatKind::Binding(BindingAnnotation::Ref | BindingAnnotation::RefMut, ..) = arg.pat.kind {
+                span_lint(
+                    cx,
+                    TOPLEVEL_REF_ARG,
+                    arg.pat.span,
+                    "`ref` directly on a function argument is ignored. \
+                    Consider using a reference type instead.",
+                );
             }
         }
     }

clippy_lints/src/no_effect.rs

@@ -147,7 +147,7 @@ fn reduce_expression<'a>(cx: &LateContext<'_, '_>, expr: &'a Expr<'a>) -> Option
             if let ExprKind::Path(ref qpath) = callee.kind {
                 let res = qpath_res(cx, qpath, callee.hir_id);
                 match res {
-                    Res::Def(DefKind::Struct, ..) | Res::Def(DefKind::Variant, ..) | Res::Def(DefKind::Ctor(..), _)
+                    Res::Def(DefKind::Struct | DefKind::Variant | DefKind::Ctor(..), ..)
                         if !has_drop(cx, cx.tables.expr_ty(expr)) =>
                     {
                         Some(args.iter().collect())

clippy_lints/src/suspicious_trait_impl.rs

@@ -71,8 +71,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SuspiciousImpl {
                 if let hir::Node::Expr(e) = cx.tcx.hir().get(parent_expr) {
                     match e.kind {
                         hir::ExprKind::Binary(..)
-                        | hir::ExprKind::Unary(hir::UnOp::UnNot, _)
-                        | hir::ExprKind::Unary(hir::UnOp::UnNeg, _)
+                        | hir::ExprKind::Unary(hir::UnOp::UnNot | hir::UnOp::UnNeg, _)
                         | hir::ExprKind::AssignOp(..) => return,
                         _ => {},
                     }
@@ -191,8 +190,7 @@ impl<'a, 'tcx> Visitor<'tcx> for BinaryExprVisitor {
     fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) {
         match expr.kind {
             hir::ExprKind::Binary(..)
-            | hir::ExprKind::Unary(hir::UnOp::UnNot, _)
-            | hir::ExprKind::Unary(hir::UnOp::UnNeg, _)
+            | hir::ExprKind::Unary(hir::UnOp::UnNot | hir::UnOp::UnNeg, _)
             | hir::ExprKind::AssignOp(..) => self.in_binary_expr = true,
             _ => {},
         }