Auto merge of #4683 - HMPerson1:inefficient_to_string, r=Manishearth

Add `inefficient_to_string` lint

Closes #4586

changelog: Add `inefficient_to_string` lint, which checks for calling `to_string` on `&&str`, which would bypass the `str`'s specialization

clippy_lints/src/lib.rs

@@ -808,6 +808,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         methods::EXPECT_FUN_CALL,
         methods::FILTER_NEXT,
         methods::FLAT_MAP_IDENTITY,
+        methods::INEFFICIENT_TO_STRING,
         methods::INTO_ITER_ON_ARRAY,
         methods::INTO_ITER_ON_REF,
         methods::ITER_CLONED_COLLECT,
@@ -1184,6 +1185,7 @@ pub fn register_plugins(reg: &mut rustc_driver::plugin::Registry<'_>, conf: &Con
         loops::MANUAL_MEMCPY,
         loops::NEEDLESS_COLLECT,
         methods::EXPECT_FUN_CALL,
+        methods::INEFFICIENT_TO_STRING,
         methods::ITER_NTH,
         methods::OR_FUN_CALL,
         methods::SINGLE_CHAR_PATTERN,

clippy_lints/src/methods/inefficient_to_string.rs

@@ -0,0 +1,55 @@
+use super::INEFFICIENT_TO_STRING;
+use crate::utils::{match_def_path, paths, snippet_with_applicability, span_lint_and_then, walk_ptrs_ty_depth};
+use if_chain::if_chain;
+use rustc::hir;
+use rustc::lint::LateContext;
+use rustc::ty::{self, Ty};
+use rustc_errors::Applicability;
+
+/// Checks for the `INEFFICIENT_TO_STRING` lint
+pub fn lint<'tcx>(cx: &LateContext<'_, 'tcx>, expr: &hir::Expr, arg: &hir::Expr, arg_ty: Ty<'tcx>) {
+    if_chain! {
+        if let Some(to_string_meth_did) = cx.tables.type_dependent_def_id(expr.hir_id);
+        if match_def_path(cx, to_string_meth_did, &paths::TO_STRING_METHOD);
+        if let Some(substs) = cx.tables.node_substs_opt(expr.hir_id);
+        let self_ty = substs.type_at(0);
+        let (deref_self_ty, deref_count) = walk_ptrs_ty_depth(self_ty);
+        if deref_count >= 1;
+        if specializes_tostring(cx, deref_self_ty);
+        then {
+            span_lint_and_then(
+                cx,
+                INEFFICIENT_TO_STRING,
+                expr.span,
+                &format!("calling `to_string` on `{}`", arg_ty),
+                |db| {
+                    db.help(&format!(
+                        "`{}` implements `ToString` through a slower blanket impl, but `{}` has a fast specialization of `ToString`",
+                        self_ty, deref_self_ty
+                    ));
+                    let mut applicability = Applicability::MachineApplicable;
+                    let arg_snippet = snippet_with_applicability(cx, arg.span, "..", &mut applicability);
+                    db.span_suggestion(
+                        expr.span,
+                        "try dereferencing the receiver",
+                        format!("({}{}).to_string()", "*".repeat(deref_count), arg_snippet),
+                        applicability,
+                    );
+                },
+            );
+        }
+    }
+}
+
+/// Returns whether `ty` specializes `ToString`.
+/// Currently, these are `str`, `String`, and `Cow<'_, str>`.
+fn specializes_tostring(cx: &LateContext<'_, '_>, ty: Ty<'_>) -> bool {
+    match ty.kind {
+        ty::Str => true,
+        ty::Adt(adt, substs) => {
+            match_def_path(cx, adt.did, &paths::STRING)
+                || (match_def_path(cx, adt.did, &paths::COW) && substs.type_at(1).is_str())
+        },
+        _ => false,
+    }
+}

clippy_lints/src/methods/mod.rs

@@ -1,3 +1,4 @@
+mod inefficient_to_string;
 mod manual_saturating_arithmetic;
 mod option_map_unwrap_or;
 mod unnecessary_filter_map;
@@ -589,6 +590,29 @@ declare_clippy_lint! {
     "using `clone` on `&&T`"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for usage of `.to_string()` on an `&&T` where
+    /// `T` implements `ToString` directly (like `&&str` or `&&String`).
+    ///
+    /// **Why is this bad?** This bypasses the specialized implementation of
+    /// `ToString` and instead goes through the more expensive string formatting
+    /// facilities.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    /// ```rust
+    /// // Generic implementation for `T: Display` is used (slow)
+    /// ["foo", "bar"].iter().map(|s| s.to_string());
+    ///
+    /// // OK, the specialized impl is used
+    /// ["foo", "bar"].iter().map(|&s| s.to_string());
+    /// ```
+    pub INEFFICIENT_TO_STRING,
+    perf,
+    "using `to_string` on `&&T` where `T: ToString`"
+}
+
 declare_clippy_lint! {
     /// **What it does:** Checks for `new` not returning `Self`.
     ///
@@ -1029,6 +1053,7 @@ declare_lint_pass!(Methods => [
     CLONE_ON_COPY,
     CLONE_ON_REF_PTR,
     CLONE_DOUBLE_REF,
+    INEFFICIENT_TO_STRING,
     NEW_RET_NO_SELF,
     SINGLE_CHAR_PATTERN,
     SEARCH_IS_SOME,
@@ -1122,6 +1147,9 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Methods {
                     lint_clone_on_copy(cx, expr, &args[0], self_ty);
                     lint_clone_on_ref_ptr(cx, expr, &args[0]);
                 }
+                if args.len() == 1 && method_call.ident.name == sym!(to_string) {
+                    inefficient_to_string::lint(cx, expr, &args[0], self_ty);
+                }
 
                 match self_ty.kind {
                     ty::Ref(_, ty, _) if ty.kind == ty::Str => {

clippy_lints/src/replace_consts.rs

@@ -39,7 +39,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ReplaceConsts {
             if let hir::ExprKind::Path(ref qp) = expr.kind;
             if let Res::Def(DefKind::Const, def_id) = cx.tables.qpath_res(qp, expr.hir_id);
             then {
-                for (const_path, repl_snip) in &REPLACEMENTS {
+                for &(ref const_path, repl_snip) in &REPLACEMENTS {
                     if match_def_path(cx, def_id, const_path) {
                         span_lint_and_sugg(
                             cx,

clippy_lints/src/utils/attrs.rs

@@ -71,7 +71,7 @@ pub fn get_attr<'a>(
                     })
             {
                 let mut db = sess.struct_span_err(attr_segments[1].ident.span, "Usage of deprecated attribute");
-                match deprecation_status {
+                match *deprecation_status {
                     DeprecationStatus::Deprecated => {
                         db.emit();
                         false