Rollup merge of #74324 - flip1995:clippyup, r=Manishearth

Update Clippy

~~I'm not sure, if we can/should land this before beta is branched.~~ (Nvm, beta is already branched) The last Clippy update was 3 weeks ago: #73660

This includes, besides other minor things:

- New lints
- One lint deprecation
- One lint was moved to pedantic
- Some FP fixes
- I think an ICE fix?

cc @Mark-Simulacrum

r? @Manishearth

---

We probably should also think of some process when and how often we should sync Clippy to the rust repo, so that we don't end up with those huge updates. Maybe every 2 weeks? Or even every week? cc @rust-lang/clippy

src/tools/clippy/.github/ISSUE_TEMPLATE/new_lint.md

@@ -12,7 +12,7 @@ labels: L-lint
 
 - Kind: *See <https://github.com/rust-lang/rust-clippy/blob/master/README.md#clippy> for list of lint kinds*
 
-*What benefit of this lint over old code?*
+*What is the advantage of the recommended code over the original code*
 
 For example:
 - Remove bounce checking inserted by ...

src/tools/clippy/.github/workflows/clippy_bors.yml

@@ -240,7 +240,8 @@ jobs:
         - 'Geal/nom'
         - 'rust-lang/stdarch'
         - 'serde-rs/serde'
-        - 'chronotope/chrono'
+        # FIXME: chrono currently cannot be compiled with `--all-targets`
+        # - 'chronotope/chrono'
         - 'hyperium/hyper'
         - 'rust-random/rand'
         - 'rust-lang/futures-rs'

src/tools/clippy/CHANGELOG.md

@@ -1352,6 +1352,7 @@ Released 2018-09-13
 [`bad_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#bad_bit_mask
 [`bind_instead_of_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#bind_instead_of_map
 [`blacklisted_name`]: https://rust-lang.github.io/rust-clippy/master/index.html#blacklisted_name
+[`blanket_clippy_restriction_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#blanket_clippy_restriction_lints
 [`blocks_in_if_conditions`]: https://rust-lang.github.io/rust-clippy/master/index.html#blocks_in_if_conditions
 [`bool_comparison`]: https://rust-lang.github.io/rust-clippy/master/index.html#bool_comparison
 [`borrow_interior_mutable_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrow_interior_mutable_const
@@ -1508,9 +1509,11 @@ Released 2018-09-13
 [`map_clone`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_clone
 [`map_entry`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_entry
 [`map_flatten`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten
+[`map_identity`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_identity
 [`map_unwrap_or`]: https://rust-lang.github.io/rust-clippy/master/index.html#map_unwrap_or
 [`match_as_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_as_ref
 [`match_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_bool
+[`match_like_matches_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_like_matches_macro
 [`match_on_vec_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_on_vec_items
 [`match_overlapping_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_overlapping_arm
 [`match_ref_pats`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_ref_pats
@@ -1575,6 +1578,7 @@ Released 2018-09-13
 [`op_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#op_ref
 [`option_as_ref_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_as_ref_deref
 [`option_env_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_env_unwrap
+[`option_if_let_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_if_let_else
 [`option_map_or_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_or_none
 [`option_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_map_unit_fn
 [`option_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#option_option
@@ -1586,6 +1590,7 @@ Released 2018-09-13
 [`panicking_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#panicking_unwrap
 [`partialeq_ne_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#partialeq_ne_impl
 [`path_buf_push_overwrite`]: https://rust-lang.github.io/rust-clippy/master/index.html#path_buf_push_overwrite
+[`pattern_type_mismatch`]: https://rust-lang.github.io/rust-clippy/master/index.html#pattern_type_mismatch
 [`possible_missing_comma`]: https://rust-lang.github.io/rust-clippy/master/index.html#possible_missing_comma
 [`precedence`]: https://rust-lang.github.io/rust-clippy/master/index.html#precedence
 [`print_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#print_literal
@@ -1612,6 +1617,7 @@ Released 2018-09-13
 [`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
 [`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
 [`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro
+[`repeat_once`]: https://rust-lang.github.io/rust-clippy/master/index.html#repeat_once
 [`replace_consts`]: https://rust-lang.github.io/rust-clippy/master/index.html#replace_consts
 [`rest_pat_in_fully_bound_structs`]: https://rust-lang.github.io/rust-clippy/master/index.html#rest_pat_in_fully_bound_structs
 [`result_map_or_into_option`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_or_into_option

src/tools/clippy/CONTRIBUTING.md

@@ -245,7 +245,7 @@ this to work, you will need the fix of `git subtree` available
 [here][gitgitgadget-pr].
 
 [gitgitgadget-pr]: https://github.com/gitgitgadget/git/pull/493
-[subtree]: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#external-dependencies-subtree
+[subtree]: https://rustc-dev-guide.rust-lang.org/contributing.html#external-dependencies-subtree
 [`rust-lang/rust`]: https://github.com/rust-lang/rust
 
 ## Issue and PR triage

src/tools/clippy/clippy_lints/src/attrs.rs

@@ -2,8 +2,8 @@
 
 use crate::reexport::Name;
 use crate::utils::{
-    first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_sugg,
-    span_lint_and_then, without_block_comments,
+    first_line_of_span, is_present_in_source, match_def_path, paths, snippet_opt, span_lint, span_lint_and_help,
+    span_lint_and_sugg, span_lint_and_then, without_block_comments,
 };
 use if_chain::if_chain;
 use rustc_ast::ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
@@ -17,7 +17,7 @@ use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
-use rustc_span::symbol::Symbol;
+use rustc_span::symbol::{Symbol, SymbolStr};
 use semver::Version;
 
 static UNIX_SYSTEMS: &[&str] = &[
@@ -182,6 +182,29 @@ declare_clippy_lint! {
     "unknown_lints for scoped Clippy lints"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for `warn`/`deny`/`forbid` attributes targeting the whole clippy::restriction category.
+    ///
+    /// **Why is this bad?** Restriction lints sometimes are in contrast with other lints or even go against idiomatic rust.
+    /// These lints should only be enabled on a lint-by-lint basis and with careful consideration.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// Bad:
+    /// ```rust
+    /// #![deny(clippy::restriction)]
+    /// ```
+    ///
+    /// Good:
+    /// ```rust
+    /// #![deny(clippy::as_conversions)]
+    /// ```
+    pub BLANKET_CLIPPY_RESTRICTION_LINTS,
+    style,
+    "enabling the complete restriction group"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for `#[cfg_attr(rustfmt, rustfmt_skip)]` and suggests to replace it
     /// with `#[rustfmt::skip]`.
@@ -249,15 +272,17 @@ declare_lint_pass!(Attributes => [
     DEPRECATED_SEMVER,
     USELESS_ATTRIBUTE,
     UNKNOWN_CLIPPY_LINTS,
+    BLANKET_CLIPPY_RESTRICTION_LINTS,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Attributes {
     fn check_attribute(&mut self, cx: &LateContext<'tcx>, attr: &'tcx Attribute) {
         if let Some(items) = &attr.meta_item_list() {
             if let Some(ident) = attr.ident() {
-                match &*ident.as_str() {
+                let ident = &*ident.as_str();
+                match ident {
                     "allow" | "warn" | "deny" | "forbid" => {
-                        check_clippy_lint_names(cx, items);
+                        check_clippy_lint_names(cx, ident, items);
                     },
                     _ => {},
                 }
@@ -363,38 +388,43 @@ impl<'tcx> LateLintPass<'tcx> for Attributes {
     }
 }
 
-#[allow(clippy::single_match_else)]
-fn check_clippy_lint_names(cx: &LateContext<'_>, items: &[NestedMetaItem]) {
-    let lint_store = cx.lints();
-    for lint in items {
+fn check_clippy_lint_names(cx: &LateContext<'_>, ident: &str, items: &[NestedMetaItem]) {
+    fn extract_name(lint: &NestedMetaItem) -> Option<SymbolStr> {
         if_chain! {
             if let Some(meta_item) = lint.meta_item();
             if meta_item.path.segments.len() > 1;
             if let tool_name = meta_item.path.segments[0].ident;
             if tool_name.as_str() == "clippy";
-            let name = meta_item.path.segments.last().unwrap().ident.name;
-            if let CheckLintNameResult::Tool(Err((None, _))) = lint_store.check_lint_name(
-                &name.as_str(),
-                Some(tool_name.name),
-            );
+            let lint_name = meta_item.path.segments.last().unwrap().ident.name;
             then {
+                return Some(lint_name.as_str());
+            }
+        }
+        None
+    }
+
+    let lint_store = cx.lints();
+    for lint in items {
+        if let Some(lint_name) = extract_name(lint) {
+            if let CheckLintNameResult::Tool(Err((None, _))) =
+                lint_store.check_lint_name(&lint_name, Some(sym!(clippy)))
+            {
                 span_lint_and_then(
                     cx,
                     UNKNOWN_CLIPPY_LINTS,
                     lint.span(),
-                    &format!("unknown clippy lint: clippy::{}", name),
+                    &format!("unknown clippy lint: clippy::{}", lint_name),
                     |diag| {
-                        let name_lower = name.as_str().to_lowercase();
-                        let symbols = lint_store.get_lints().iter().map(
-                            |l| Symbol::intern(&l.name_lower())
-                        ).collect::<Vec<_>>();
-                        let sugg = find_best_match_for_name(
-                            symbols.iter(),
-                            &format!("clippy::{}", name_lower),
-                            None,
-                        );
-                        if name.as_str().chars().any(char::is_uppercase)
-                            && lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok() {
+                        let name_lower = lint_name.to_lowercase();
+                        let symbols = lint_store
+                            .get_lints()
+                            .iter()
+                            .map(|l| Symbol::intern(&l.name_lower()))
+                            .collect::<Vec<_>>();
+                        let sugg = find_best_match_for_name(symbols.iter(), &format!("clippy::{}", name_lower), None);
+                        if lint_name.chars().any(char::is_uppercase)
+                            && lint_store.find_lints(&format!("clippy::{}", name_lower)).is_ok()
+                        {
                             diag.span_suggestion(
                                 lint.span(),
                                 "lowercase the lint name",
@@ -409,10 +439,19 @@ fn check_clippy_lint_names(cx: &LateContext<'_>, items: &[NestedMetaItem]) {
                                 Applicability::MachineApplicable,
                             );
                         }
-                    }
+                    },
+                );
+            } else if lint_name == "restriction" && ident != "allow" {
+                span_lint_and_help(
+                    cx,
+                    BLANKET_CLIPPY_RESTRICTION_LINTS,
+                    lint.span(),
+                    "restriction lints are not meant to be all enabled",
+                    None,
+                    "try enabling only the lints you really need",
                 );
             }
-        };
+        }
     }
 }
 
@@ -442,15 +481,14 @@ fn is_relevant_trait(cx: &LateContext<'_>, item: &TraitItem<'_>) -> bool {
 }
 
 fn is_relevant_block(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, block: &Block<'_>) -> bool {
-    if let Some(stmt) = block.stmts.first() {
-        match &stmt.kind {
+    block.stmts.first().map_or(
+        block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e)),
+        |stmt| match &stmt.kind {
             StmtKind::Local(_) => true,
             StmtKind::Expr(expr) | StmtKind::Semi(expr) => is_relevant_expr(cx, tables, expr),
             _ => false,
-        }
-    } else {
-        block.expr.as_ref().map_or(false, |e| is_relevant_expr(cx, tables, e))
-    }
+        },
+    )
 }
 
 fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &Expr<'_>) -> bool {
@@ -460,11 +498,10 @@ fn is_relevant_expr(cx: &LateContext<'_>, tables: &ty::TypeckTables<'_>, expr: &
         ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
         ExprKind::Call(path_expr, _) => {
             if let ExprKind::Path(qpath) = &path_expr.kind {
-                if let Some(fun_id) = tables.qpath_res(qpath, path_expr.hir_id).opt_def_id() {
-                    !match_def_path(cx, fun_id, &paths::BEGIN_PANIC)
-                } else {
-                    true
-                }
+                tables
+                    .qpath_res(qpath, path_expr.hir_id)
+                    .opt_def_id()
+                    .map_or(true, |fun_id| !match_def_path(cx, fun_id, &paths::BEGIN_PANIC))
             } else {
                 true
             }

src/tools/clippy/clippy_lints/src/await_holding_lock.rs

@@ -11,7 +11,7 @@ declare_clippy_lint! {
     /// non-async-aware MutexGuard.
     ///
     /// **Why is this bad?** The Mutex types found in syd::sync and parking_lot
-    /// are not designed to operator in an async context across await points.
+    /// are not designed to operate in an async context across await points.
     ///
     /// There are two potential solutions. One is to use an asynx-aware Mutex
     /// type. Many asynchronous foundation crates provide such a Mutex type. The

src/tools/clippy/clippy_lints/src/collapsible_if.rs

@@ -115,7 +115,7 @@ fn check_collapsible_maybe_if_let(cx: &EarlyContext<'_>, else_: &ast::Expr) {
                 COLLAPSIBLE_IF,
                 block.span,
                 "this `else { if .. }` block can be collapsed",
-                "try",
+                "collapse nested if block",
                 snippet_block_with_applicability(cx, else_.span, "..", Some(block.span), &mut applicability).into_owned(),
                 applicability,
             );
@@ -142,7 +142,7 @@ fn check_collapsible_no_if_let(cx: &EarlyContext<'_>, expr: &ast::Expr, check: &
                 let rhs = Sugg::ast(cx, check_inner, "..");
                 diag.span_suggestion(
                     expr.span,
-                    "try",
+                    "collapse nested if block",
                     format!(
                         "if {} {}",
                         lhs.and(&rhs),

src/tools/clippy/clippy_lints/src/comparison_chain.rs

@@ -122,8 +122,5 @@ impl<'tcx> LateLintPass<'tcx> for ComparisonChain {
 }
 
 fn kind_is_cmp(kind: BinOpKind) -> bool {
-    match kind {
-        BinOpKind::Lt | BinOpKind::Gt | BinOpKind::Eq => true,
-        _ => false,
-    }
+    matches!(kind, BinOpKind::Lt | BinOpKind::Gt | BinOpKind::Eq)
 }

src/tools/clippy/clippy_lints/src/deprecated_lints.rs

@@ -153,5 +153,13 @@ declare_deprecated_lint! {
     ///
     /// **Deprecation reason:** Associated-constants are now preferred.
     pub REPLACE_CONSTS,
-    "associated-constants `MIN`/`MAX` of integers are prefer to `{min,max}_value()` and module constants"
+    "associated-constants `MIN`/`MAX` of integers are prefered to `{min,max}_value()` and module constants"
+}
+
+declare_deprecated_lint! {
+    /// **What it does:** Nothing. This lint has been deprecated.
+    ///
+    /// **Deprecation reason:** The regex! macro does not exist anymore.
+    pub REGEX_MACRO,
+    "the regex! macro has been removed from the regex crate in 2018"
 }

src/tools/clippy/clippy_lints/src/dereference.rs

@@ -73,9 +73,10 @@ impl<'tcx> LateLintPass<'tcx> for Dereferencing {
 fn lint_deref(cx: &LateContext<'_>, method_name: &str, call_expr: &Expr<'_>, var_span: Span, expr_span: Span) {
     match method_name {
         "deref" => {
-            if cx.tcx.lang_items().deref_trait().map_or(false, |id| {
+            let impls_deref_trait = cx.tcx.lang_items().deref_trait().map_or(false, |id| {
                 implements_trait(cx, cx.tables().expr_ty(&call_expr), id, &[])
-            }) {
+            });
+            if impls_deref_trait {
                 span_lint_and_sugg(
                     cx,
                     EXPLICIT_DEREF_METHODS,
@@ -88,9 +89,10 @@ fn lint_deref(cx: &LateContext<'_>, method_name: &str, call_expr: &Expr<'_>, var
             }
         },
         "deref_mut" => {
-            if cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| {
+            let impls_deref_mut_trait = cx.tcx.lang_items().deref_mut_trait().map_or(false, |id| {
                 implements_trait(cx, cx.tables().expr_ty(&call_expr), id, &[])
-            }) {
+            });
+            if impls_deref_mut_trait {
                 span_lint_and_sugg(
                     cx,
                     EXPLICIT_DEREF_METHODS,

src/tools/clippy/clippy_lints/src/eq_op.rs

@@ -214,20 +214,20 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
 }
 
 fn is_valid_operator(op: BinOp) -> bool {
-    match op.node {
+    matches!(
+        op.node,
         BinOpKind::Sub
-        | BinOpKind::Div
-        | BinOpKind::Eq
-        | BinOpKind::Lt
-        | BinOpKind::Le
-        | BinOpKind::Gt
-        | BinOpKind::Ge
-        | BinOpKind::Ne
-        | BinOpKind::And
-        | BinOpKind::Or
-        | BinOpKind::BitXor
-        | BinOpKind::BitAnd
-        | BinOpKind::BitOr => true,
-        _ => false,
-    }
+            | BinOpKind::Div
+            | BinOpKind::Eq
+            | BinOpKind::Lt
+            | BinOpKind::Le
+            | BinOpKind::Gt
+            | BinOpKind::Ge
+            | BinOpKind::Ne
+            | BinOpKind::And
+            | BinOpKind::Or
+            | BinOpKind::BitXor
+            | BinOpKind::BitAnd
+            | BinOpKind::BitOr
+    )
 }

src/tools/clippy/clippy_lints/src/escape.rs

@@ -105,10 +105,7 @@ fn is_argument(map: rustc_middle::hir::map::Map<'_>, id: HirId) -> bool {
         _ => return false,
     }
 
-    match map.find(map.get_parent_node(id)) {
-        Some(Node::Param(_)) => true,
-        _ => false,
-    }
+    matches!(map.find(map.get_parent_node(id)), Some(Node::Param(_)))
 }
 
 impl<'a, 'tcx> Delegate<'tcx> for EscapeDelegate<'a, 'tcx> {