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

Update Clippy

Biweekly Clippy update (2 days late, since I wanted to wait for rust-lang#75098)

r? @Manishearth

CHANGELOG.md

@@ -6,13 +6,13 @@ document.
 
 ## Unreleased / In Rust Nightly
 
-[c2c07fa...master](https://github.com/rust-lang/rust-clippy/compare/7ea7cd1...master)
+[c2c07fa...master](https://github.com/rust-lang/rust-clippy/compare/c2c07fa...master)
 
 ## Rust 1.46
 
 Current beta, release 2020-08-27
 
-[7ea7cd1...c2c07fa](https://github.com/rust-lang/rust-clippy/compare/7ea7cd1...master)
+[7ea7cd1...c2c07fa](https://github.com/rust-lang/rust-clippy/compare/7ea7cd1...c2c07fa)
 
 ### New lints
 
@@ -1454,6 +1454,7 @@ Released 2018-09-13
 [`deprecated_semver`]: https://rust-lang.github.io/rust-clippy/master/index.html#deprecated_semver
 [`deref_addrof`]: https://rust-lang.github.io/rust-clippy/master/index.html#deref_addrof
 [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
+[`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
 [`diverging_sub_expression`]: https://rust-lang.github.io/rust-clippy/master/index.html#diverging_sub_expression
 [`doc_markdown`]: https://rust-lang.github.io/rust-clippy/master/index.html#doc_markdown
 [`double_comparisons`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_comparisons
@@ -1615,6 +1616,7 @@ Released 2018-09-13
 [`mutex_atomic`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_atomic
 [`mutex_integer`]: https://rust-lang.github.io/rust-clippy/master/index.html#mutex_integer
 [`naive_bytecount`]: https://rust-lang.github.io/rust-clippy/master/index.html#naive_bytecount
+[`needless_arbitrary_self_type`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_arbitrary_self_type
 [`needless_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_bool
 [`needless_borrow`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrow
 [`needless_borrowed_reference`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_borrowed_reference
@@ -1686,6 +1688,7 @@ Released 2018-09-13
 [`result_map_unit_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#result_map_unit_fn
 [`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
 [`search_is_some`]: https://rust-lang.github.io/rust-clippy/master/index.html#search_is_some
 [`serde_api_misuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#serde_api_misuse
 [`shadow_reuse`]: https://rust-lang.github.io/rust-clippy/master/index.html#shadow_reuse
@@ -1701,6 +1704,7 @@ Released 2018-09-13
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else
 [`skip_while_next`]: https://rust-lang.github.io/rust-clippy/master/index.html#skip_while_next
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
+[`stable_sort_primitive`]: https://rust-lang.github.io/rust-clippy/master/index.html#stable_sort_primitive
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add
 [`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign
@@ -1723,6 +1727,7 @@ Released 2018-09-13
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
 [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
 [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
+[`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds
 [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str
 [`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int
 [`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool

CONTRIBUTING.md

@@ -28,11 +28,14 @@ All contributors are expected to follow the [Rust Code of Conduct].
 
 ## Getting started
 
-High level approach:
+**Note: If this is your first time contributing to Clippy, you should
+first read the [Basics docs](doc/basics.md).**
+
+### High level approach
 
 1. Find something to fix/improve
 2. Change code (likely some file in `clippy_lints/src/`)
-3. Follow the instructions in the [docs for writing lints](doc/adding_lints.md) such as running the `setup-toolchain.sh` script
+3. Follow the instructions in the [Basics docs](doc/basics.md) to get set up
 4. Run `cargo test` in the root directory and wiggle code until it passes
 5. Open a PR (also can be done after 2. if you run into problems)
 
@@ -95,16 +98,16 @@ quick read.
 
 ## Getting code-completion for rustc internals to work
 
-Unfortunately, [`rust-analyzer`][ra_homepage] does not (yet?) understand how Clippy uses compiler-internals 
-using `extern crate` and it also needs to be able to read the source files of the rustc-compiler which are not 
-available via a `rustup` component at the time of writing.  
-To work around this, you need to have a copy of the [rustc-repo][rustc_repo] available which can be obtained via  
-`git clone https://github.com/rust-lang/rust/`.  
-Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies 
-which rust-analyzer will be able to understand.  
-Run `cargo dev ra-setup --repo-path <repo-path>` where `<repo-path>` is an absolute path to the rustc repo 
-you just cloned.  
-The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to 
+Unfortunately, [`rust-analyzer`][ra_homepage] does not (yet?) understand how Clippy uses compiler-internals
+using `extern crate` and it also needs to be able to read the source files of the rustc-compiler which are not
+available via a `rustup` component at the time of writing.
+To work around this, you need to have a copy of the [rustc-repo][rustc_repo] available which can be obtained via
+`git clone https://github.com/rust-lang/rust/`.
+Then you can run a `cargo dev` command to automatically make Clippy use the rustc-repo via path-dependencies
+which rust-analyzer will be able to understand.
+Run `cargo dev ra-setup --repo-path <repo-path>` where `<repo-path>` is an absolute path to the rustc repo
+you just cloned.
+The command will add path-dependencies pointing towards rustc-crates inside the rustc repo to
 Clippys `Cargo.toml`s and should allow rust-analyzer to understand most of the types that Clippy uses.
 Just make sure to remove the dependencies again before finally making a pull request!
 

clippy_dev/src/ra_setup.rs

@@ -68,10 +68,11 @@ fn inject_deps_into_manifest(
     });
 
     // format a new [dependencies]-block with the new deps we need to inject
-    let mut all_deps = String::from("[dependencies]\n");
+    let mut all_deps = String::from("[target.'cfg(NOT_A_PLATFORM)'.dependencies]\n");
     new_deps.for_each(|dep_line| {
         all_deps.push_str(&dep_line);
     });
+    all_deps.push_str("\n[dependencies]\n");
 
     // replace "[dependencies]" with
     // [dependencies]

clippy_lints/src/attrs.rs

@@ -1,6 +1,5 @@
 //! checks for attributes
 
-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_help,
     span_lint_and_sugg, span_lint_and_then, without_block_comments,
@@ -517,7 +516,7 @@ fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>
     }
 }
 
-fn check_attrs(cx: &LateContext<'_>, span: Span, name: Name, attrs: &[Attribute]) {
+fn check_attrs(cx: &LateContext<'_>, span: Span, name: Symbol, attrs: &[Attribute]) {
     if span.from_expansion() {
         return;
     }
@@ -606,7 +605,7 @@ fn check_empty_line_after_outer_attr(cx: &EarlyContext<'_>, item: &rustc_ast::as
                         cx,
                         EMPTY_LINE_AFTER_OUTER_ATTR,
                         begin_of_attr_to_item,
-                        "Found an empty line after an outer attribute. \
+                        "found an empty line after an outer attribute. \
                         Perhaps you forgot to add a `!` to make it an inner attribute?",
                     );
                 }

clippy_lints/src/bytecount.rs

@@ -82,8 +82,8 @@ impl<'tcx> LateLintPass<'tcx> for ByteCount {
                             cx,
                             NAIVE_BYTECOUNT,
                             expr.span,
-                            "You appear to be counting bytes the naive way",
-                            "Consider using the bytecount crate",
+                            "you appear to be counting bytes the naive way",
+                            "consider using the bytecount crate",
                             format!("bytecount::count({}, {})",
                                     snippet_with_applicability(cx, haystack.span, "..", &mut applicability),
                                     snippet_with_applicability(cx, needle.span, "..", &mut applicability)),

clippy_lints/src/checked_conversions.rs

@@ -66,7 +66,7 @@ impl<'tcx> LateLintPass<'tcx> for CheckedConversions {
                     cx,
                     CHECKED_CONVERSIONS,
                     item.span,
-                    "Checked cast can be simplified.",
+                    "checked cast can be simplified",
                     "try",
                     format!("{}::try_from({}).is_ok()", to_type, snippet),
                     applicability,

clippy_lints/src/default_trait_access.rs

@@ -61,7 +61,7 @@ impl<'tcx> LateLintPass<'tcx> for DefaultTraitAccess {
                                 cx,
                                 DEFAULT_TRAIT_ACCESS,
                                 expr.span,
-                                &format!("Calling `{}` is more clear than this expression", replacement),
+                                &format!("calling `{}` is more clear than this expression", replacement),
                                 "try",
                                 replacement,
                                 Applicability::Unspecified, // First resolve the TODO above

clippy_lints/src/derive.rs

@@ -1,6 +1,7 @@
 use crate::utils::paths;
 use crate::utils::{
-    is_automatically_derived, is_copy, match_path, span_lint_and_help, span_lint_and_note, span_lint_and_then,
+    get_trait_def_id, is_allowed, is_automatically_derived, is_copy, match_path, span_lint_and_help,
+    span_lint_and_note, span_lint_and_then,
 };
 use if_chain::if_chain;
 use rustc_hir::def_id::DefId;
@@ -43,6 +44,57 @@ declare_clippy_lint! {
     "deriving `Hash` but implementing `PartialEq` explicitly"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for deriving `Ord` but implementing `PartialOrd`
+    /// explicitly or vice versa.
+    ///
+    /// **Why is this bad?** The implementation of these traits must agree (for
+    /// example for use with `sort`) so it’s probably a bad idea to use a
+    /// default-generated `Ord` implementation with an explicitly defined
+    /// `PartialOrd`. In particular, the following must hold for any type
+    /// implementing `Ord`:
+    ///
+    /// ```text
+    /// k1.cmp(&k2) == k1.partial_cmp(&k2).unwrap()
+    /// ```
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// ```rust,ignore
+    /// #[derive(Ord, PartialEq, Eq)]
+    /// struct Foo;
+    ///
+    /// impl PartialOrd for Foo {
+    ///     ...
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// #[derive(PartialEq, Eq)]
+    /// struct Foo;
+    ///
+    /// impl PartialOrd for Foo {
+    ///     fn partial_cmp(&self, other: &Foo) -> Option<Ordering> {
+    ///        Some(self.cmp(other))
+    ///     }
+    /// }
+    ///
+    /// impl Ord for Foo {
+    ///     ...
+    /// }
+    /// ```
+    /// or, if you don't need a custom ordering:
+    /// ```rust,ignore
+    /// #[derive(Ord, PartialOrd, PartialEq, Eq)]
+    /// struct Foo;
+    /// ```
+    pub DERIVE_ORD_XOR_PARTIAL_ORD,
+    correctness,
+    "deriving `Ord` but implementing `PartialOrd` explicitly"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for explicit `Clone` implementations for `Copy`
     /// types.
@@ -103,7 +155,12 @@ declare_clippy_lint! {
     "deriving `serde::Deserialize` on a type that has methods using `unsafe`"
 }
 
-declare_lint_pass!(Derive => [EXPL_IMPL_CLONE_ON_COPY, DERIVE_HASH_XOR_EQ, UNSAFE_DERIVE_DESERIALIZE]);
+declare_lint_pass!(Derive => [
+    EXPL_IMPL_CLONE_ON_COPY,
+    DERIVE_HASH_XOR_EQ,
+    DERIVE_ORD_XOR_PARTIAL_ORD,
+    UNSAFE_DERIVE_DESERIALIZE
+]);
 
 impl<'tcx> LateLintPass<'tcx> for Derive {
     fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'_>) {
@@ -116,6 +173,7 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
             let is_automatically_derived = is_automatically_derived(&*item.attrs);
 
             check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
+            check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);
 
             if is_automatically_derived {
                 check_unsafe_derive_deserialize(cx, item, trait_ref, ty);
@@ -180,6 +238,60 @@ fn check_hash_peq<'tcx>(
     }
 }
 
+/// Implementation of the `DERIVE_ORD_XOR_PARTIAL_ORD` lint.
+fn check_ord_partial_ord<'tcx>(
+    cx: &LateContext<'tcx>,
+    span: Span,
+    trait_ref: &TraitRef<'_>,
+    ty: Ty<'tcx>,
+    ord_is_automatically_derived: bool,
+) {
+    if_chain! {
+        if let Some(ord_trait_def_id) = get_trait_def_id(cx, &paths::ORD);
+        if let Some(partial_ord_trait_def_id) = cx.tcx.lang_items().partial_ord_trait();
+        if let Some(def_id) = &trait_ref.trait_def_id();
+        if *def_id == ord_trait_def_id;
+        then {
+            // Look for the PartialOrd implementations for `ty`
+            cx.tcx.for_each_relevant_impl(partial_ord_trait_def_id, ty, |impl_id| {
+                let partial_ord_is_automatically_derived = is_automatically_derived(&cx.tcx.get_attrs(impl_id));
+
+                if partial_ord_is_automatically_derived == ord_is_automatically_derived {
+                    return;
+                }
+
+                let trait_ref = cx.tcx.impl_trait_ref(impl_id).expect("must be a trait implementation");
+
+                // Only care about `impl PartialOrd<Foo> for Foo`
+                // For `impl PartialOrd<B> for A, input_types is [A, B]
+                if trait_ref.substs.type_at(1) == ty {
+                    let mess = if partial_ord_is_automatically_derived {
+                        "you are implementing `Ord` explicitly but have derived `PartialOrd`"
+                    } else {
+                        "you are deriving `Ord` but have implemented `PartialOrd` explicitly"
+                    };
+
+                    span_lint_and_then(
+                        cx,
+                        DERIVE_ORD_XOR_PARTIAL_ORD,
+                        span,
+                        mess,
+                        |diag| {
+                            if let Some(local_def_id) = impl_id.as_local() {
+                                let hir_id = cx.tcx.hir().as_local_hir_id(local_def_id);
+                                diag.span_note(
+                                    cx.tcx.hir().span(hir_id),
+                                    "`PartialOrd` implemented here"
+                                );
+                            }
+                        }
+                    );
+                }
+            });
+        }
+    }
+}
+
 /// Implementation of the `EXPL_IMPL_CLONE_ON_COPY` lint.
 fn check_copy_clone<'tcx>(cx: &LateContext<'tcx>, item: &Item<'_>, trait_ref: &TraitRef<'_>, ty: Ty<'tcx>) {
     if match_path(&trait_ref.path, &paths::CLONE_TRAIT) {
@@ -242,7 +354,9 @@ fn check_unsafe_derive_deserialize<'tcx>(
     if_chain! {
         if match_path(&trait_ref.path, &paths::SERDE_DESERIALIZE);
         if let ty::Adt(def, _) = ty.kind;
-        if def.did.is_local();
+        if let Some(local_def_id) = def.did.as_local();
+        let adt_hir_id = cx.tcx.hir().as_local_hir_id(local_def_id);
+        if !is_allowed(cx, UNSAFE_DERIVE_DESERIALIZE, adt_hir_id);
         if cx.tcx.inherent_impls(def.did)
             .iter()
             .map(|imp_did| item_from_def_id(cx, *imp_did))

clippy_lints/src/double_comparison.rs

@@ -60,7 +60,7 @@ impl<'tcx> DoubleComparisons {
                     cx,
                     DOUBLE_COMPARISONS,
                     span,
-                    "This binary expression can be simplified",
+                    "this binary expression can be simplified",
                     "try",
                     sugg,
                     applicability,

clippy_lints/src/double_parens.rs

@@ -45,41 +45,28 @@ impl EarlyLintPass for DoubleParens {
             return;
         }
 
+        let msg: &str = "consider removing unnecessary double parentheses";
+
         match expr.kind {
             ExprKind::Paren(ref in_paren) => match in_paren.kind {
                 ExprKind::Paren(_) | ExprKind::Tup(_) => {
-                    span_lint(
-                        cx,
-                        DOUBLE_PARENS,
-                        expr.span,
-                        "Consider removing unnecessary double parentheses",
-                    );
+                    span_lint(cx, DOUBLE_PARENS, expr.span, &msg);
                 },
                 _ => {},
             },
             ExprKind::Call(_, ref params) => {
                 if params.len() == 1 {
                     let param = &params[0];
                     if let ExprKind::Paren(_) = param.kind {
-                        span_lint(
-                            cx,
-                            DOUBLE_PARENS,
-                            param.span,
-                            "Consider removing unnecessary double parentheses",
-                        );
+                        span_lint(cx, DOUBLE_PARENS, param.span, &msg);
                     }
                 }
             },
             ExprKind::MethodCall(_, ref params, _) => {
                 if params.len() == 2 {
                     let param = &params[1];
                     if let ExprKind::Paren(_) = param.kind {
-                        span_lint(
-                            cx,
-                            DOUBLE_PARENS,
-                            param.span,
-                            "Consider removing unnecessary double parentheses",
-                        );
+                        span_lint(cx, DOUBLE_PARENS, param.span, &msg);
                     }
                 }
             },

clippy_lints/src/drop_bounds.rs

@@ -33,11 +33,11 @@ declare_clippy_lint! {
     /// ```
     pub DROP_BOUNDS,
     correctness,
-    "Bounds of the form `T: Drop` are useless"
+    "bounds of the form `T: Drop` are useless"
 }
 
-const DROP_BOUNDS_SUMMARY: &str = "Bounds of the form `T: Drop` are useless. \
-                                   Use `std::mem::needs_drop` to detect if a type has drop glue.";
+const DROP_BOUNDS_SUMMARY: &str = "bounds of the form `T: Drop` are useless, \
+                                   use `std::mem::needs_drop` to detect if a type has drop glue";
 
 declare_lint_pass!(DropBounds => [DROP_BOUNDS]);