Merge remote-tracking branch 'upstream/master' into rustup

clippy_lints/src/methods/bytes_nth.rs

@@ -22,7 +22,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, recv: &'tcx E
         cx,
         BYTES_NTH,
         expr.span,
-        &format!("called `.byte().nth()` on a `{}`", caller_type),
+        &format!("called `.bytes().nth()` on a `{}`", caller_type),
         "try",
         format!(
             "{}.as_bytes().get({})",

clippy_lints/src/methods/err_expect.rs

@@ -0,0 +1,60 @@
+use super::ERR_EXPECT;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::ty::implements_trait;
+use clippy_utils::{meets_msrv, msrvs, ty::is_type_diagnostic_item};
+use rustc_errors::Applicability;
+use rustc_lint::LateContext;
+use rustc_middle::ty;
+use rustc_middle::ty::Ty;
+use rustc_semver::RustcVersion;
+use rustc_span::{sym, Span};
+
+pub(super) fn check(
+    cx: &LateContext<'_>,
+    _expr: &rustc_hir::Expr<'_>,
+    recv: &rustc_hir::Expr<'_>,
+    msrv: Option<&RustcVersion>,
+    expect_span: Span,
+    err_span: Span,
+) {
+    if_chain! {
+        if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv), sym::Result);
+        // Test the version to make sure the lint can be showed (expect_err has been
+        // introduced in rust 1.17.0 : https://github.com/rust-lang/rust/pull/38982)
+        if meets_msrv(msrv, &msrvs::EXPECT_ERR);
+
+        // Grabs the `Result<T, E>` type
+        let result_type = cx.typeck_results().expr_ty(recv);
+        // Tests if the T type in a `Result<T, E>` is not None
+        if let Some(data_type) = get_data_type(cx, result_type);
+        // Tests if the T type in a `Result<T, E>` implements debug
+        if has_debug_impl(data_type, cx);
+
+        then {
+            span_lint_and_sugg(
+                cx,
+                ERR_EXPECT,
+                err_span.to(expect_span),
+                "called `.err().expect()` on a `Result` value",
+                "try",
+                "expect_err".to_string(),
+                Applicability::MachineApplicable
+        );
+        }
+    };
+}
+
+/// Given a `Result<T, E>` type, return its data (`T`).
+fn get_data_type<'a>(cx: &LateContext<'_>, ty: Ty<'a>) -> Option<Ty<'a>> {
+    match ty.kind() {
+        ty::Adt(_, substs) if is_type_diagnostic_item(cx, ty, sym::Result) => substs.types().next(),
+        _ => None,
+    }
+}
+
+/// Given a type, very if the Debug trait has been impl'd
+fn has_debug_impl<'tcx>(ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
+    cx.tcx
+        .get_diagnostic_item(sym::Debug)
+        .map_or(false, |debug| implements_trait(cx, ty, debug, &[]))
+}

clippy_lints/src/methods/implicit_clone.rs

@@ -48,13 +48,11 @@ pub fn is_clone_like(cx: &LateContext<'_>, method_name: &str, method_def_id: hir
         "to_os_string" => is_diag_item_method(cx, method_def_id, sym::OsStr),
         "to_owned" => is_diag_trait_item(cx, method_def_id, sym::ToOwned),
         "to_path_buf" => is_diag_item_method(cx, method_def_id, sym::Path),
-        "to_vec" => {
-            cx.tcx.impl_of_method(method_def_id)
-                .filter(|&impl_did| {
-                    cx.tcx.type_of(impl_did).is_slice() && cx.tcx.impl_trait_ref(impl_did).is_none()
-                })
-                .is_some()
-        },
+        "to_vec" => cx
+            .tcx
+            .impl_of_method(method_def_id)
+            .filter(|&impl_did| cx.tcx.type_of(impl_did).is_slice() && cx.tcx.impl_trait_ref(impl_did).is_none())
+            .is_some(),
         _ => false,
     }
 }

clippy_lints/src/methods/iter_overeager_cloned.rs

@@ -1,11 +1,12 @@
 use clippy_utils::diagnostics::span_lint_and_sugg;
 use clippy_utils::source::snippet;
-use clippy_utils::ty::{get_iterator_item_ty, is_copy};
+use clippy_utils::ty::{get_iterator_item_ty, implements_trait, is_copy};
 use itertools::Itertools;
 use rustc_errors::Applicability;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
 use rustc_middle::ty;
+use rustc_span::sym;
 use std::ops::Not;
 
 use super::ITER_OVEREAGER_CLONED;
@@ -20,9 +21,16 @@ pub(super) fn check<'tcx>(
     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,
+    let inner_ty = if_chain! {
+        if let Some(iterator_trait_id) = cx.tcx.get_diagnostic_item(sym::Iterator);
+        let recv_ty = cx.typeck_results().expr_ty(recv);
+        if implements_trait(cx, recv_ty, iterator_trait_id, &[]);
+        if let Some(inner_ty) = get_iterator_item_ty(cx, cx.typeck_results().expr_ty_adjusted(recv));
+        then {
+            inner_ty
+        } else {
+            return;
+        }
     };
 
     match inner_ty.kind() {

clippy_lints/src/methods/map_identity.rs

@@ -13,6 +13,7 @@ pub(super) fn check(
     expr: &hir::Expr<'_>,
     caller: &hir::Expr<'_>,
     map_arg: &hir::Expr<'_>,
+    name: &str,
     _map_span: Span,
 ) {
     let caller_ty = cx.typeck_results().expr_ty(caller);
@@ -29,7 +30,7 @@ pub(super) fn check(
                 MAP_IDENTITY,
                 sugg_span,
                 "unnecessary map of the identity function",
-                "remove the call to `map`",
+                &format!("remove the call to `{}`", name),
                 String::new(),
                 Applicability::MachineApplicable,
             )

clippy_lints/src/methods/mod.rs

@@ -9,6 +9,7 @@ mod chars_next_cmp_with_unwrap;
 mod clone_on_copy;
 mod clone_on_ref_ptr;
 mod cloned_instead_of_copied;
+mod err_expect;
 mod expect_fun_call;
 mod expect_used;
 mod extend_with_drain;
@@ -362,6 +363,29 @@ declare_clippy_lint! {
     "using `ok().expect()`, which gives worse error messages than calling `expect` directly on the Result"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for `.err().expect()` calls on the `Result` type.
+    ///
+    /// ### Why is this bad?
+    /// `.expect_err()` can be called directly to avoid the extra type conversion from `err()`.
+    ///
+    /// ### Example
+    /// ```should_panic
+    /// let x: Result<u32, &str> = Ok(10);
+    /// x.err().expect("Testing err().expect()");
+    /// ```
+    /// Use instead:
+    /// ```should_panic
+    /// let x: Result<u32, &str> = Ok(10);
+    /// x.expect_err("Testing expect_err");
+    /// ```
+    #[clippy::version = "1.61.0"]
+    pub ERR_EXPECT,
+    style,
+    r#"using `.err().expect("")` when `.expect_err("")` can be used"#
+}
+
 declare_clippy_lint! {
     /// ### What it does
     /// Checks for usages of `_.unwrap_or_else(Default::default)` on `Option` and
@@ -2055,7 +2079,7 @@ declare_clippy_lint! {
     /// Checks for use of `.collect::<Vec<String>>().join("")` on iterators.
     ///
     /// ### Why is this bad?
-    /// `.collect::<String>()` is more concise and usually more performant
+    /// `.collect::<String>()` is more concise and might be more performant
     ///
     /// ### Example
     /// ```rust
@@ -2070,9 +2094,12 @@ declare_clippy_lint! {
     /// println!("{}", output);
     /// ```
     /// ### Known problems
-    /// While `.collect::<String>()` is more performant in most cases, there are cases where
+    /// While `.collect::<String>()` is sometimes more performant, there are cases where
     /// using `.collect::<String>()` over `.collect::<Vec<String>>().join("")`
     /// will prevent loop unrolling and will result in a negative performance impact.
+    ///
+    /// Additionlly, differences have been observed between aarch64 and x86_64 assembly output,
+    /// with aarch64 tending to producing faster assembly in more cases when using `.collect::<String>()`
     #[clippy::version = "1.61.0"]
     pub UNNECESSARY_JOIN,
     pedantic,
@@ -2165,6 +2192,7 @@ impl_lint_pass!(Methods => [
     NEEDLESS_SPLITN,
     UNNECESSARY_TO_OWNED,
     UNNECESSARY_JOIN,
+    ERR_EXPECT,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -2428,6 +2456,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
             },
             ("expect", [_]) => match method_call(recv) {
                 Some(("ok", [recv], _)) => ok_expect::check(cx, expr, recv),
+                Some(("err", [recv], err_span)) => err_expect::check(cx, expr, recv, msrv, span, err_span),
                 _ => expect_used::check(cx, expr, recv),
             },
             ("extend", [arg]) => {
@@ -2472,7 +2501,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                     }
                 }
             },
-            ("map", [m_arg]) => {
+            (name @ ("map" | "map_err"), [m_arg]) => {
                 if let Some((name, [recv2, args @ ..], span2)) = method_call(recv) {
                     match (name, args) {
                         ("as_mut", []) => option_as_ref_deref::check(cx, expr, recv2, m_arg, true, msrv),
@@ -2484,7 +2513,7 @@ fn check_methods<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, msrv: Optio
                         _ => {},
                     }
                 }
-                map_identity::check(cx, expr, recv, m_arg, span);
+                map_identity::check(cx, expr, recv, m_arg, name, span);
             },
             ("map_or", [def, map]) => option_map_or_none::check(cx, expr, recv, def, map),
             (name @ "next", args @ []) => {