Auto merge of rust-lang#93001 - flip1995:clippyup, r=Manishearth

Out of cycle Clippy update I want to do an out-of-cycle sync for rust-lang/rust-clippy#8295, and possibly backport this to stable together with rust-lang#92938. If this doesn't get backported to stable, then I at least want to backport it to beta. r? `@Manishearth`
rust-lang-ci · Jan 18, 2022 · e4ff903 · e4ff903
2 parents 86b1581 + 0fc3fda
commit e4ff903
Show file tree

Hide file tree

Showing 43 changed files with 754 additions and 100 deletions.
diff --git a/src/tools/clippy/CHANGELOG.md b/src/tools/clippy/CHANGELOG.md
@@ -3049,6 +3049,7 @@ Released 2018-09-13
 [`iter_not_returning_iterator`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_not_returning_iterator
 [`iter_nth`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth
 [`iter_nth_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_nth_zero
+[`iter_overeager_cloned`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_overeager_cloned
 [`iter_skip_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#iter_skip_next
 [`iterator_step_by_zero`]: https://rust-lang.github.io/rust-clippy/master/index.html#iterator_step_by_zero
 [`just_underscores_and_digits`]: https://rust-lang.github.io/rust-clippy/master/index.html#just_underscores_and_digits

diff --git a/src/tools/clippy/COPYRIGHT b/src/tools/clippy/COPYRIGHT
@@ -1,4 +1,4 @@
-Copyright 2014-2021 The Rust Project Developers
+Copyright 2014-2022 The Rust Project Developers
 
 Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
 http://www.apache.org/licenses/LICENSE-2.0> or the MIT license

diff --git a/src/tools/clippy/LICENSE-APACHE b/src/tools/clippy/LICENSE-APACHE
@@ -186,7 +186,7 @@ APPENDIX: How to apply the Apache License to your work.
    same "printed page" as the copyright notice for easier
    identification within third-party archives.
 
-Copyright 2014-2021 The Rust Project Developers
+Copyright 2014-2022 The Rust Project Developers
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.

diff --git a/src/tools/clippy/LICENSE-MIT b/src/tools/clippy/LICENSE-MIT
@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2014-2021 The Rust Project Developers
+Copyright (c) 2014-2022 The Rust Project Developers
 
 Permission is hereby granted, free of charge, to any
 person obtaining a copy of this software and associated

diff --git a/src/tools/clippy/README.md b/src/tools/clippy/README.md
@@ -238,7 +238,7 @@ If you want to contribute to Clippy, you can find more information in [CONTRIBUT
 
 ## License
 
-Copyright 2014-2021 The Rust Project Developers
+Copyright 2014-2022 The Rust Project Developers
 
 Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
 [https://www.apache.org/licenses/LICENSE-2.0](https://www.apache.org/licenses/LICENSE-2.0)> or the MIT license

diff --git a/src/tools/clippy/clippy_lints/src/bit_mask.rs b/src/tools/clippy/clippy_lints/src/bit_mask.rs
@@ -18,14 +18,14 @@ declare_clippy_lint! {
     /// {`!=`, `>=`, `>`, `!=`, `>=`, `>`}) can be determined from the following
     /// table:
     ///
-    /// |Comparison  |Bit Op|Example     |is always|Formula               |
-    /// |------------|------|------------|---------|----------------------|
-    /// |`==` or `!=`| `&`  |`x & 2 == 3`|`false`  |`c & m != c`          |
-    /// |`<`  or `>=`| `&`  |`x & 2 < 3` |`true`   |`m < c`               |
-    /// |`>`  or `<=`| `&`  |`x & 1 > 1` |`false`  |`m <= c`              |
-    /// |`==` or `!=`| `|`  |`x | 1 == 0`|`false`  |`c | m != c`          |
-    /// |`<`  or `>=`| `|`  |`x | 1 < 1` |`false`  |`m >= c`              |
-    /// |`<=` or `>` | `|`  |`x | 1 > 0` |`true`   |`m > c`               |
+    /// |Comparison  |Bit Op|Example      |is always|Formula               |
+    /// |------------|------|-------------|---------|----------------------|
+    /// |`==` or `!=`| `&`  |`x & 2 == 3` |`false`  |`c & m != c`          |
+    /// |`<`  or `>=`| `&`  |`x & 2 < 3`  |`true`   |`m < c`               |
+    /// |`>`  or `<=`| `&`  |`x & 1 > 1`  |`false`  |`m <= c`              |
+    /// |`==` or `!=`| `\|` |`x \| 1 == 0`|`false`  |`c \| m != c`         |
+    /// |`<`  or `>=`| `\|` |`x \| 1 < 1` |`false`  |`m >= c`              |
+    /// |`<=` or `>` | `\|` |`x \| 1 > 0` |`true`   |`m > c`               |
     ///
     /// ### Why is this bad?
     /// If the bits that the comparison cares about are always
@@ -53,10 +53,10 @@ declare_clippy_lint! {
     /// without changing the outcome. The basic structure can be seen in the
     /// following table:
     ///
-    /// |Comparison| Bit Op  |Example    |equals |
-    /// |----------|---------|-----------|-------|
-    /// |`>` / `<=`|`|` / `^`|`x | 2 > 3`|`x > 3`|
-    /// |`<` / `>=`|`|` / `^`|`x ^ 1 < 4`|`x < 4`|
+    /// |Comparison| Bit Op   |Example     |equals |
+    /// |----------|----------|------------|-------|
+    /// |`>` / `<=`|`\|` / `^`|`x \| 2 > 3`|`x > 3`|
+    /// |`<` / `>=`|`\|` / `^`|`x ^ 1 < 4` |`x < 4`|
     ///
     /// ### Why is this bad?
     /// Not equally evil as [`bad_bit_mask`](#bad_bit_mask),

diff --git a/src/tools/clippy/clippy_lints/src/copies.rs b/src/tools/clippy/clippy_lints/src/copies.rs
@@ -183,7 +183,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
                 lint_same_cond(cx, &conds);
                 lint_same_fns_in_if_cond(cx, &conds);
                 // Block duplication
-                lint_same_then_else(cx, &blocks, conds.len() == blocks.len(), expr);
+                lint_same_then_else(cx, &conds, &blocks, conds.len() == blocks.len(), expr);
             }
         }
     }
@@ -192,6 +192,7 @@ impl<'tcx> LateLintPass<'tcx> for CopyAndPaste {
 /// Implementation of `BRANCHES_SHARING_CODE` and `IF_SAME_THEN_ELSE` if the blocks are equal.
 fn lint_same_then_else<'tcx>(
     cx: &LateContext<'tcx>,
+    conds: &[&'tcx Expr<'_>],
     blocks: &[&Block<'tcx>],
     has_conditional_else: bool,
     expr: &'tcx Expr<'_>,
@@ -204,7 +205,7 @@ fn lint_same_then_else<'tcx>(
     // Check if each block has shared code
     let has_expr = blocks[0].expr.is_some();
 
-    let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, blocks) {
+    let (start_eq, mut end_eq, expr_eq) = if let Some(block_eq) = scan_block_for_eq(cx, conds, blocks) {
         (block_eq.start_eq, block_eq.end_eq, block_eq.expr_eq)
     } else {
         return;
@@ -316,14 +317,14 @@ struct BlockEqual {
 
 /// This function can also trigger the `IF_SAME_THEN_ELSE` in which case it'll return `None` to
 /// abort any further processing and avoid duplicate lint triggers.
-fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option<BlockEqual> {
+fn scan_block_for_eq(cx: &LateContext<'_>, conds: &[&Expr<'_>], blocks: &[&Block<'_>]) -> Option<BlockEqual> {
     let mut start_eq = usize::MAX;
     let mut end_eq = usize::MAX;
     let mut expr_eq = true;
-    let mut iter = blocks.windows(2);
-    while let Some(&[win0, win1]) = iter.next() {
-        let l_stmts = win0.stmts;
-        let r_stmts = win1.stmts;
+    let mut iter = blocks.windows(2).enumerate();
+    while let Some((i, &[block0, block1])) = iter.next() {
+        let l_stmts = block0.stmts;
+        let r_stmts = block1.stmts;
 
         // `SpanlessEq` now keeps track of the locals and is therefore context sensitive clippy#6752.
         // The comparison therefore needs to be done in a way that builds the correct context.
@@ -340,22 +341,26 @@ fn scan_block_for_eq(cx: &LateContext<'_>, blocks: &[&Block<'_>]) -> Option<Bloc
             it1.zip(it2)
                 .fold(0, |acc, (l, r)| if evaluator.eq_stmt(l, r) { acc + 1 } else { 0 })
         };
-        let block_expr_eq = both(&win0.expr, &win1.expr, |l, r| evaluator.eq_expr(l, r));
+        let block_expr_eq = both(&block0.expr, &block1.expr, |l, r| evaluator.eq_expr(l, r));
 
         // IF_SAME_THEN_ELSE
         if_chain! {
             if block_expr_eq;
             if l_stmts.len() == r_stmts.len();
             if l_stmts.len() == current_start_eq;
-            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win0.hir_id);
-            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, win1.hir_id);
+            // `conds` may have one last item than `blocks`.
+            // Any `i` from `blocks.windows(2)` will exist in `conds`, but `i+1` may not exist on the last iteration.
+            if !matches!(conds[i].kind, ExprKind::Let(..));
+            if !matches!(conds.get(i + 1).map(|e| &e.kind), Some(ExprKind::Let(..)));
+            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block0.hir_id);
+            if !is_lint_allowed(cx, IF_SAME_THEN_ELSE, block1.hir_id);
             then {
                 span_lint_and_note(
                     cx,
                     IF_SAME_THEN_ELSE,
-                    win0.span,
+                    block0.span,
                     "this `if` has identical blocks",
-                    Some(win1.span),
+                    Some(block1.span),
                     "same as this",
                 );
 

diff --git a/src/tools/clippy/clippy_lints/src/format.rs b/src/tools/clippy/clippy_lints/src/format.rs
@@ -9,7 +9,7 @@ use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::symbol::kw;
-use rustc_span::{sym, Span};
+use rustc_span::{sym, BytePos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -84,7 +84,22 @@ impl<'tcx> LateLintPass<'tcx> for UselessFormat {
                         ExprKind::MethodCall(path, ..) => path.ident.name.as_str() == "to_string",
                         _ => false,
                     };
-                    let sugg = if is_new_string {
+                    let sugg = if format_args.format_string_span.contains(value.span) {
+                        // Implicit argument. e.g. `format!("{x}")` span points to `{x}`
+                        let spdata = value.span.data();
+                        let span = Span::new(
+                            spdata.lo + BytePos(1),
+                            spdata.hi - BytePos(1),
+                            spdata.ctxt,
+                            spdata.parent
+                        );
+                        let snip = snippet_with_applicability(cx, span, "..", &mut applicability);
+                        if is_new_string {
+                            snip.into()
+                        } else {
+                            format!("{snip}.to_string()")
+                        }
+                    } else if is_new_string {
                         snippet_with_applicability(cx, value.span, "..", &mut applicability).into_owned()
                     } else {
                         let sugg = Sugg::hir_with_applicability(cx, value, "<arg>", &mut applicability);

diff --git a/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/src/tools/clippy/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs
@@ -42,8 +42,7 @@ fn check_raw_ptr<'tcx>(
     let expr = &body.value;
     if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) {
         let raw_ptrs = iter_input_pats(decl, body)
-            .zip(decl.inputs.iter())
-            .filter_map(|(arg, ty)| raw_ptr_arg(arg, ty))
+            .filter_map(|arg| raw_ptr_arg(cx, arg))
             .collect::<HirIdSet>();
 
         if !raw_ptrs.is_empty() {
@@ -59,8 +58,12 @@ fn check_raw_ptr<'tcx>(
     }
 }
 
-fn raw_ptr_arg(arg: &hir::Param<'_>, ty: &hir::Ty<'_>) -> Option<hir::HirId> {
-    if let (&hir::PatKind::Binding(_, id, _, _), &hir::TyKind::Ptr(_)) = (&arg.pat.kind, &ty.kind) {
+fn raw_ptr_arg(cx: &LateContext<'_>, arg: &hir::Param<'_>) -> Option<hir::HirId> {
+    if let (&hir::PatKind::Binding(_, id, _, _), Some(&ty::RawPtr(_))) = (
+        &arg.pat.kind,
+        cx.maybe_typeck_results()
+            .map(|typeck_results| typeck_results.pat_ty(arg.pat).kind()),
+    ) {
         Some(id)
     } else {
         None

diff --git a/src/tools/clippy/clippy_lints/src/iter_not_returning_iterator.rs b/src/tools/clippy/clippy_lints/src/iter_not_returning_iterator.rs
@@ -66,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for IterNotReturningIterator {
 
 fn check_sig(cx: &LateContext<'_>, name: &str, sig: &FnSig<'_>, fn_id: LocalDefId) {
     if sig.decl.implicit_self.has_implicit_self() {
-        let ret_ty = cx.tcx.fn_sig(fn_id).skip_binder().output();
+        let ret_ty = cx.tcx.erase_late_bound_regions(cx.tcx.fn_sig(fn_id).output());
         let ret_ty = cx
             .tcx
             .try_normalize_erasing_regions(cx.param_env, ret_ty)

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_all.rs b/src/tools/clippy/clippy_lints/src/lib.register_all.rs
@@ -156,6 +156,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(methods::ITER_NEXT_SLICE),
     LintId::of(methods::ITER_NTH),
     LintId::of(methods::ITER_NTH_ZERO),
+    LintId::of(methods::ITER_OVEREAGER_CLONED),
     LintId::of(methods::ITER_SKIP_NEXT),
     LintId::of(methods::MANUAL_FILTER_MAP),
     LintId::of(methods::MANUAL_FIND_MAP),
@@ -249,7 +250,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(reference::REF_IN_DEREF),
     LintId::of(regex::INVALID_REGEX),
     LintId::of(repeat_once::REPEAT_ONCE),
-    LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
     LintId::of(returns::LET_AND_RETURN),
     LintId::of(returns::NEEDLESS_RETURN),
     LintId::of(self_assignment::SELF_ASSIGNMENT),

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_lints.rs b/src/tools/clippy/clippy_lints/src/lib.register_lints.rs
@@ -288,6 +288,7 @@ store.register_lints(&[
     methods::ITER_NEXT_SLICE,
     methods::ITER_NTH,
     methods::ITER_NTH_ZERO,
+    methods::ITER_OVEREAGER_CLONED,
     methods::ITER_SKIP_NEXT,
     methods::MANUAL_FILTER_MAP,
     methods::MANUAL_FIND_MAP,

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs b/src/tools/clippy/clippy_lints/src/lib.register_pedantic.rs
@@ -81,6 +81,7 @@ store.register_group(true, "clippy::pedantic", Some("clippy_pedantic"), vec![
     LintId::of(ranges::RANGE_PLUS_ONE),
     LintId::of(redundant_else::REDUNDANT_ELSE),
     LintId::of(ref_option_ref::REF_OPTION_REF),
+    LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
     LintId::of(semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED),
     LintId::of(strings::STRING_ADD_ASSIGN),
     LintId::of(trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS),

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_perf.rs b/src/tools/clippy/clippy_lints/src/lib.register_perf.rs
@@ -14,6 +14,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
     LintId::of(methods::EXPECT_FUN_CALL),
     LintId::of(methods::EXTEND_WITH_DRAIN),
     LintId::of(methods::ITER_NTH),
+    LintId::of(methods::ITER_OVEREAGER_CLONED),
     LintId::of(methods::MANUAL_STR_REPEAT),
     LintId::of(methods::OR_FUN_CALL),
     LintId::of(methods::SINGLE_CHAR_PATTERN),

diff --git a/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs b/src/tools/clippy/clippy_lints/src/lib.register_suspicious.rs
@@ -16,7 +16,6 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
     LintId::of(methods::SUSPICIOUS_MAP),
     LintId::of(mut_key::MUTABLE_KEY_TYPE),
     LintId::of(octal_escapes::OCTAL_ESCAPES),
-    LintId::of(return_self_not_must_use::RETURN_SELF_NOT_MUST_USE),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL),
     LintId::of(suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL),
 ])
diff --git a/src/tools/clippy/clippy_lints/src/methods/implicit_clone.rs b/src/tools/clippy/clippy_lints/src/methods/implicit_clone.rs
@@ -1,31 +1,40 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet_with_context;
+use clippy_utils::ty::peel_mid_ty_refs;
 use clippy_utils::{is_diag_item_method, is_diag_trait_item};
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
 use rustc_middle::ty::TyS;
-use rustc_span::{sym, Span};
+use rustc_span::sym;
 
 use super::IMPLICIT_CLONE;
 
-pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, span: Span) {
+pub fn check(cx: &LateContext<'_>, method_name: &str, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) {
     if_chain! {
         if let Some(method_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
         if is_clone_like(cx, method_name, method_def_id);
         let return_type = cx.typeck_results().expr_ty(expr);
-        let input_type = cx.typeck_results().expr_ty(recv).peel_refs();
+        let input_type = cx.typeck_results().expr_ty(recv);
+        let (input_type, ref_count) = peel_mid_ty_refs(input_type);
         if let Some(ty_name) = input_type.ty_adt_def().map(|adt_def| cx.tcx.item_name(adt_def.did));
         if TyS::same_type(return_type, input_type);
         then {
+            let mut app = Applicability::MachineApplicable;
+            let recv_snip = snippet_with_context(cx, recv.span, expr.span.ctxt(), "..", &mut app).0;
             span_lint_and_sugg(
                 cx,
                 IMPLICIT_CLONE,
-                span,
+                expr.span,
                 &format!("implicitly cloning a `{}` by calling `{}` on its dereferenced type", ty_name, method_name),
                 "consider using",
-                "clone".to_string(),
-                Applicability::MachineApplicable
+                if ref_count > 1 {
+                    format!("({}{}).clone()", "*".repeat(ref_count - 1), recv_snip)
+                } else {
+                    format!("{}.clone()", recv_snip)
+                },
+                app,
             );
         }
     }

diff --git a/src/tools/clippy/clippy_lints/src/methods/iter_overeager_cloned.rs b/src/tools/clippy/clippy_lints/src/methods/iter_overeager_cloned.rs
@@ -0,0 +1,62 @@
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::source::snippet;
+use clippy_utils::ty::{get_iterator_item_ty, is_copy};
+use itertools::Itertools;
+use rustc_errors::Applicability;
+use rustc_hir as hir;
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use std::ops::Not;
+
+use super::ITER_OVEREAGER_CLONED;
+use crate::redundant_clone::REDUNDANT_CLONE;
+
+/// lint overeager use of `cloned()` for `Iterator`s
+pub(super) fn check<'tcx>(
+    cx: &LateContext<'tcx>,
+    expr: &'tcx hir::Expr<'_>,
+    recv: &'tcx hir::Expr<'_>,
+    name: &str,
+    map_arg: &[hir::Expr<'_>],
+) {
+    // Check if it's iterator and get type associated with `Item`.
+    let inner_ty = match get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv)) {
+        Some(ty) => ty,
+        _ => return,
+    };
+
+    match inner_ty.kind() {
+        ty::Ref(_, ty, _) if !is_copy(cx, ty) => {},
+        _ => return,
+    };
+
+    let (lint, preserve_cloned) = match name {
+        "count" => (REDUNDANT_CLONE, false),
+        _ => (ITER_OVEREAGER_CLONED, true),
+    };
+    let wildcard_params = map_arg.is_empty().not().then(|| "...").unwrap_or_default();
+    let msg = format!(
+        "called `cloned().{}({})` on an `Iterator`. It may be more efficient to call `{}({}){}` instead",
+        name,
+        wildcard_params,
+        name,
+        wildcard_params,
+        preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
+    );
+
+    span_lint_and_sugg(
+        cx,
+        lint,
+        expr.span,
+        &msg,
+        "try this",
+        format!(
+            "{}.{}({}){}",
+            snippet(cx, recv.span, ".."),
+            name,
+            map_arg.iter().map(|a| snippet(cx, a.span, "..")).join(", "),
+            preserve_cloned.then(|| ".cloned()").unwrap_or_default(),
+        ),
+        Applicability::MachineApplicable,
+    );
+}