Merge branch 'master' into if_then_panic

Author: Labelray

.github/ISSUE_TEMPLATE/blank_issue.md

@@ -8,7 +8,7 @@ about: Create a blank issue.
 Additional labels can be added to this issue by including the following command
 (without the space after the @ symbol):
 
-`@rustbot label +<label>`
+@ rustbot label +<label>
 
 Common labels for this issue type are:
 * C-an-interesting-project

.github/ISSUE_TEMPLATE/bug_report.md

@@ -36,7 +36,7 @@ LLVM version: 10.0
 Additional labels can be added to this issue by including the following command
 (without the space after the @ symbol):
 
-`@rustbot label +<label>`
+@ rustbot label +<label>
 
 Common labels for this issue type are:
 * `I-suggestion-causes-error`

.github/ISSUE_TEMPLATE/false_positive.md

@@ -37,7 +37,7 @@ LLVM version: 10.0
 Additional labels can be added to this issue by including the following command
 (without the space after the @ symbol):
 
-`@rustbot label +<label>`
+@ rustbot label +<label>
 
 Common labels for this issue type are:
 * I-suggestion-causes-error

CHANGELOG.md

@@ -1050,7 +1050,7 @@ Released 2020-11-19
   [#5913](https://github.com/rust-lang/rust-clippy/pull/5913)
 * Add example of false positive to [`ptr_arg`] docs.
   [#5885](https://github.com/rust-lang/rust-clippy/pull/5885)
-* [`box_vec`], [`vec_box`] and [`borrowed_box`]: add link to the documentation of `Box`
+* [`box_vec`](https://rust-lang.github.io/rust-clippy/master/index.html#box_collection), [`vec_box`] and [`borrowed_box`]: add link to the documentation of `Box`
   [#6023](https://github.com/rust-lang/rust-clippy/pull/6023)
 
 ## Rust 1.47
@@ -2570,7 +2570,7 @@ Released 2018-09-13
 [`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
 [`borrowed_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#borrowed_box
-[`box_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_vec
+[`box_collection`]: https://rust-lang.github.io/rust-clippy/master/index.html#box_collection
 [`boxed_local`]: https://rust-lang.github.io/rust-clippy/master/index.html#boxed_local
 [`branches_sharing_code`]: https://rust-lang.github.io/rust-clippy/master/index.html#branches_sharing_code
 [`builtin_type_shadow`]: https://rust-lang.github.io/rust-clippy/master/index.html#builtin_type_shadow
@@ -2907,6 +2907,7 @@ Released 2018-09-13
 [`reversed_empty_ranges`]: https://rust-lang.github.io/rust-clippy/master/index.html#reversed_empty_ranges
 [`same_functions_in_if_condition`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_functions_in_if_condition
 [`same_item_push`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_item_push
+[`same_name_method`]: https://rust-lang.github.io/rust-clippy/master/index.html#same_name_method
 [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
 [`self_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_assignment
 [`self_named_constructors`]: https://rust-lang.github.io/rust-clippy/master/index.html#self_named_constructors

README.md

@@ -18,7 +18,7 @@ You can choose how much Clippy is supposed to ~~annoy~~ help you by changing the
 | `clippy::style`       | code that should be written in a more idiomatic way                                 | **warn**      |
 | `clippy::complexity`  | code that does something simple but in a complex way                                | **warn**      |
 | `clippy::perf`        | code that can be written to run faster                                              | **warn**      |
-| `clippy::pedantic`    | lints which are rather strict or might have false positives                         | allow         |
+| `clippy::pedantic`    | lints which are rather strict or have occasional false positives                    | allow         |
 | `clippy::nursery`     | new lints that are still under development                                          | allow         |
 | `clippy::cargo`       | lints for the cargo manifest                                                        | allow         |
 

clippy_lints/src/formatting.rs

@@ -286,34 +286,39 @@ fn check_array(cx: &EarlyContext<'_>, expr: &Expr) {
 }
 
 fn check_missing_else(cx: &EarlyContext<'_>, first: &Expr, second: &Expr) {
-    if !differing_macro_contexts(first.span, second.span)
-        && !first.span.from_expansion()
-        && is_if(first)
-        && (is_block(second) || is_if(second))
-    {
-        // where the else would be
-        let else_span = first.span.between(second.span);
+    if_chain! {
+        if !differing_macro_contexts(first.span, second.span);
+        if !first.span.from_expansion();
+        if let ExprKind::If(cond_expr, ..) = &first.kind;
+        if is_block(second) || is_if(second);
 
-        if let Some(else_snippet) = snippet_opt(cx, else_span) {
-            if !else_snippet.contains('\n') {
-                let (looks_like, next_thing) = if is_if(second) {
-                    ("an `else if`", "the second `if`")
-                } else {
-                    ("an `else {..}`", "the next block")
-                };
+        // Proc-macros can give weird spans. Make sure this is actually an `if`.
+        if let Some(if_snip) = snippet_opt(cx, first.span.until(cond_expr.span));
+        if if_snip.starts_with("if");
 
-                span_lint_and_note(
-                    cx,
-                    SUSPICIOUS_ELSE_FORMATTING,
-                    else_span,
-                    &format!("this looks like {} but the `else` is missing", looks_like),
-                    None,
-                    &format!(
-                        "to remove this lint, add the missing `else` or add a new line before {}",
-                        next_thing,
-                    ),
-                );
-            }
+        // If there is a line break between the two expressions, don't lint.
+        // If there is a non-whitespace character, this span came from a proc-macro.
+        let else_span = first.span.between(second.span);
+        if let Some(else_snippet) = snippet_opt(cx, else_span);
+        if !else_snippet.chars().any(|c| c == '\n' || !c.is_whitespace());
+        then {
+            let (looks_like, next_thing) = if is_if(second) {
+                ("an `else if`", "the second `if`")
+            } else {
+                ("an `else {..}`", "the next block")
+            };
+
+            span_lint_and_note(
+                cx,
+                SUSPICIOUS_ELSE_FORMATTING,
+                else_span,
+                &format!("this looks like {} but the `else` is missing", looks_like),
+                None,
+                &format!(
+                    "to remove this lint, add the missing `else` or add a new line before {}",
+                    next_thing,
+                ),
+            );
         }
     }
 }

clippy_lints/src/lib.rs

@@ -332,6 +332,7 @@ mod reference;
 mod regex;
 mod repeat_once;
 mod returns;
+mod same_name_method;
 mod self_assignment;
 mod self_named_constructors;
 mod semicolon_if_nothing_returned;
@@ -912,6 +913,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         repeat_once::REPEAT_ONCE,
         returns::LET_AND_RETURN,
         returns::NEEDLESS_RETURN,
+        same_name_method::SAME_NAME_METHOD,
         self_assignment::SELF_ASSIGNMENT,
         self_named_constructors::SELF_NAMED_CONSTRUCTORS,
         semicolon_if_nothing_returned::SEMICOLON_IF_NOTHING_RETURNED,
@@ -956,7 +958,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         transmuting_null::TRANSMUTING_NULL,
         try_err::TRY_ERR,
         types::BORROWED_BOX,
-        types::BOX_VEC,
+        types::BOX_COLLECTION,
         types::LINKEDLIST,
         types::OPTION_OPTION,
         types::RC_BUFFER,
@@ -1055,6 +1057,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(panic_unimplemented::UNIMPLEMENTED),
         LintId::of(panic_unimplemented::UNREACHABLE),
         LintId::of(pattern_type_mismatch::PATTERN_TYPE_MISMATCH),
+        LintId::of(same_name_method::SAME_NAME_METHOD),
         LintId::of(shadow::SHADOW_REUSE),
         LintId::of(shadow::SHADOW_SAME),
         LintId::of(strings::STRING_ADD),
@@ -1139,6 +1142,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(needless_continue::NEEDLESS_CONTINUE),
         LintId::of(needless_for_each::NEEDLESS_FOR_EACH),
         LintId::of(needless_pass_by_value::NEEDLESS_PASS_BY_VALUE),
+        LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
         LintId::of(non_expressive_names::SIMILAR_NAMES),
         LintId::of(pass_by_ref_or_value::LARGE_TYPES_PASSED_BY_VALUE),
         LintId::of(pass_by_ref_or_value::TRIVIALLY_COPY_PASS_BY_REF),
@@ -1396,7 +1400,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),
         LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
         LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
-        LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
         LintId::of(non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS),
         LintId::of(open_options::NONSENSICAL_OPEN_OPTIONS),
         LintId::of(option_env_unwrap::OPTION_ENV_UNWRAP),
@@ -1454,7 +1457,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(transmuting_null::TRANSMUTING_NULL),
         LintId::of(try_err::TRY_ERR),
         LintId::of(types::BORROWED_BOX),
-        LintId::of(types::BOX_VEC),
+        LintId::of(types::BOX_COLLECTION),
         LintId::of(types::REDUNDANT_ALLOCATION),
         LintId::of(types::TYPE_COMPLEXITY),
         LintId::of(types::VEC_BOX),
@@ -1571,7 +1574,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),
         LintId::of(non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
         LintId::of(non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
-        LintId::of(non_expressive_names::MANY_SINGLE_CHAR_NAMES),
         LintId::of(ptr::CMP_NULL),
         LintId::of(ptr::PTR_ARG),
         LintId::of(ptr_eq::PTR_EQ),
@@ -1794,7 +1796,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(redundant_clone::REDUNDANT_CLONE),
         LintId::of(slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
         LintId::of(stable_sort_primitive::STABLE_SORT_PRIMITIVE),
-        LintId::of(types::BOX_VEC),
+        LintId::of(types::BOX_COLLECTION),
         LintId::of(types::REDUNDANT_ALLOCATION),
         LintId::of(vec::USELESS_VEC),
         LintId::of(vec_init_then_push::VEC_INIT_THEN_PUSH),
@@ -1927,6 +1929,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(move || Box::new(unnested_or_patterns::UnnestedOrPatterns::new(msrv)));
 
     store.register_late_pass(|| Box::new(size_of_in_element_count::SizeOfInElementCount));
+    store.register_late_pass(|| Box::new(same_name_method::SameNameMethod));
     store.register_late_pass(|| Box::new(map_clone::MapClone));
     store.register_late_pass(|| Box::new(map_err_ignore::MapErrIgnore));
     store.register_late_pass(|| Box::new(shadow::Shadow));
@@ -2195,6 +2198,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) {
     ls.register_renamed("clippy::cyclomatic_complexity", "clippy::cognitive_complexity");
     ls.register_renamed("clippy::const_static_lifetime", "clippy::redundant_static_lifetimes");
     ls.register_renamed("clippy::option_and_then_some", "clippy::bind_instead_of_map");
+    ls.register_renamed("clippy::box_vec", "clippy::box_collection");
     ls.register_renamed("clippy::block_in_if_condition_expr", "clippy::blocks_in_if_conditions");
     ls.register_renamed("clippy::block_in_if_condition_stmt", "clippy::blocks_in_if_conditions");
     ls.register_renamed("clippy::option_map_unwrap_or", "clippy::map_unwrap_or");

clippy_lints/src/loops/while_let_on_iterator.rs

@@ -8,7 +8,7 @@ use clippy_utils::{
 use if_chain::if_chain;
 use rustc_errors::Applicability;
 use rustc_hir::intravisit::{walk_expr, ErasedMap, NestedVisitorMap, Visitor};
-use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, UnOp};
+use rustc_hir::{def::Res, Expr, ExprKind, HirId, Local, PatKind, QPath, UnOp};
 use rustc_lint::LateContext;
 use rustc_span::{symbol::sym, Span, Symbol};
 
@@ -47,13 +47,8 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
     // If the iterator is a field or the iterator is accessed after the loop is complete it needs to be
     // borrowed mutably. TODO: If the struct can be partially moved from and the struct isn't used
     // afterwards a mutable borrow of a field isn't necessary.
-    let ref_mut = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) {
-        if cx.typeck_results().node_type(iter_expr.hir_id).ref_mutability() == Some(Mutability::Mut) {
-            // Reborrow for mutable references. It may not be possible to get a mutable reference here.
-            "&mut *"
-        } else {
-            "&mut "
-        }
+    let by_ref = if !iter_expr.fields.is_empty() || needs_mutable_borrow(cx, &iter_expr, loop_expr) {
+        ".by_ref()"
     } else {
         ""
     };
@@ -65,7 +60,7 @@ pub(super) fn check(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
         expr.span.with_hi(scrutinee_expr.span.hi()),
         "this loop could be written as a `for` loop",
         "try",
-        format!("for {} in {}{}", loop_var, ref_mut, iterator),
+        format!("for {} in {}{}", loop_var, iterator, by_ref),
         applicability,
     );
 }

clippy_lints/src/methods/mod.rs

@@ -264,6 +264,8 @@ declare_clippy_lint! {
     /// The method signature is controlled by the trait and often `&self` is required for all types that implement the trait
     /// (see e.g. the `std::string::ToString` trait).
     ///
+    /// Clippy allows `Pin<&Self>` and `Pin<&mut Self>` if `&self` and `&mut self` is required.
+    ///
     /// Please find more info here:
     /// https://rust-lang.github.io/api-guidelines/naming.html#ad-hoc-conversions-follow-as_-to_-into_-conventions-c-conv
     ///

clippy_lints/src/mut_key.rs

@@ -3,7 +3,7 @@ use clippy_utils::trait_ref_of_method;
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::TypeFoldable;
-use rustc_middle::ty::{Adt, Array, RawPtr, Ref, Slice, Tuple, Ty, TypeAndMut};
+use rustc_middle::ty::{Adt, Array, Ref, Slice, Tuple, Ty};
 use rustc_session::{declare_lint_pass, declare_tool_lint};
 use rustc_span::source_map::Span;
 use rustc_span::symbol::sym;
@@ -19,10 +19,29 @@ declare_clippy_lint! {
     /// so having types with interior mutability is a bad idea.
     ///
     /// ### Known problems
-    /// It's correct to use a struct, that contains interior mutability
-    /// as a key, when its `Hash` implementation doesn't access any of the interior mutable types.
-    /// However, this lint is unable to recognize this, so it causes a false positive in theses cases.
-    /// The `bytes` crate is a great example of this.
+    ///
+    /// #### False Positives
+    /// It's correct to use a struct that contains interior mutability as a key, when its
+    /// implementation of `Hash` or `Ord` doesn't access any of the interior mutable types.
+    /// However, this lint is unable to recognize this, so it will often cause false positives in
+    /// theses cases.  The `bytes` crate is a great example of this.
+    ///
+    /// #### False Negatives
+    /// For custom `struct`s/`enum`s, this lint is unable to check for interior mutability behind
+    /// indirection.  For example, `struct BadKey<'a>(&'a Cell<usize>)` will be seen as immutable
+    /// and cause a false negative if its implementation of `Hash`/`Ord` accesses the `Cell`.
+    ///
+    /// This lint does check a few cases for indirection.  Firstly, using some standard library
+    /// types (`Option`, `Result`, `Box`, `Rc`, `Arc`, `Vec`, `VecDeque`, `BTreeMap` and
+    /// `BTreeSet`) directly as keys (e.g. in `HashMap<Box<Cell<usize>>, ()>`) **will** trigger the
+    /// lint, because the impls of `Hash`/`Ord` for these types directly call `Hash`/`Ord` on their
+    /// contained type.
+    ///
+    /// Secondly, the implementations of `Hash` and `Ord` for raw pointers (`*const T` or `*mut T`)
+    /// apply only to the **address** of the contained value.  Therefore, interior mutability
+    /// behind raw pointers (e.g. in `HashSet<*mut Cell<usize>>`) can't impact the value of `Hash`
+    /// or `Ord`, and therefore will not trigger this link.  For more info, see issue
+    /// [#6745](https://github.com/rust-lang/rust-clippy/issues/6745).
     ///
     /// ### Example
     /// ```rust
@@ -103,30 +122,52 @@ fn check_sig<'tcx>(cx: &LateContext<'tcx>, item_hir_id: hir::HirId, decl: &hir::
 fn check_ty<'tcx>(cx: &LateContext<'tcx>, span: Span, ty: Ty<'tcx>) {
     let ty = ty.peel_refs();
     if let Adt(def, substs) = ty.kind() {
-        if [sym::hashmap_type, sym::BTreeMap, sym::hashset_type, sym::BTreeMap]
+        let is_keyed_type = [sym::hashmap_type, sym::BTreeMap, sym::hashset_type, sym::BTreeSet]
             .iter()
-            .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did))
-            && is_mutable_type(cx, substs.type_at(0), span)
-        {
+            .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did));
+        if is_keyed_type && is_interior_mutable_type(cx, substs.type_at(0), span) {
             span_lint(cx, MUTABLE_KEY_TYPE, span, "mutable key type");
         }
     }
 }
 
-fn is_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bool {
+/// Determines if a type contains interior mutability which would affect its implementation of
+/// [`Hash`] or [`Ord`].
+fn is_interior_mutable_type<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span) -> bool {
     match *ty.kind() {
-        RawPtr(TypeAndMut { ty: inner_ty, mutbl }) | Ref(_, inner_ty, mutbl) => {
-            mutbl == hir::Mutability::Mut || is_mutable_type(cx, inner_ty, span)
-        },
-        Slice(inner_ty) => is_mutable_type(cx, inner_ty, span),
+        Ref(_, inner_ty, mutbl) => mutbl == hir::Mutability::Mut || is_interior_mutable_type(cx, inner_ty, span),
+        Slice(inner_ty) => is_interior_mutable_type(cx, inner_ty, span),
         Array(inner_ty, size) => {
-            size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0) && is_mutable_type(cx, inner_ty, span)
+            size.try_eval_usize(cx.tcx, cx.param_env).map_or(true, |u| u != 0)
+                && is_interior_mutable_type(cx, inner_ty, span)
         },
-        Tuple(..) => ty.tuple_fields().any(|ty| is_mutable_type(cx, ty, span)),
-        Adt(..) => {
-            !ty.has_escaping_bound_vars()
-                && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok()
-                && !ty.is_freeze(cx.tcx.at(span), cx.param_env)
+        Tuple(..) => ty.tuple_fields().any(|ty| is_interior_mutable_type(cx, ty, span)),
+        Adt(def, substs) => {
+            // Special case for collections in `std` who's impl of `Hash` or `Ord` delegates to
+            // that of their type parameters.  Note: we don't include `HashSet` and `HashMap`
+            // because they have no impl for `Hash` or `Ord`.
+            let is_std_collection = [
+                sym::option_type,
+                sym::result_type,
+                sym::LinkedList,
+                sym::vec_type,
+                sym::vecdeque_type,
+                sym::BTreeMap,
+                sym::BTreeSet,
+                sym::Rc,
+                sym::Arc,
+            ]
+            .iter()
+            .any(|diag_item| cx.tcx.is_diagnostic_item(*diag_item, def.did));
+            let is_box = Some(def.did) == cx.tcx.lang_items().owned_box();
+            if is_std_collection || is_box {
+                // The type is mutable if any of its type parameters are
+                substs.types().any(|ty| is_interior_mutable_type(cx, ty, span))
+            } else {
+                !ty.has_escaping_bound_vars()
+                    && cx.tcx.layout_of(cx.param_env.and(ty)).is_ok()
+                    && !ty.is_freeze(cx.tcx.at(span), cx.param_env)
+            }
         },
         _ => false,
     }