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

Add `inefficient_to_string` lint

Closes rust-lang#4586

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

Author: bors

CHANGELOG.md

@@ -1033,6 +1033,7 @@ Released 2018-09-13
 [`inconsistent_digit_grouping`]: https://rust-lang.github.io/rust-clippy/master/index.html#inconsistent_digit_grouping
 [`indexing_slicing`]: https://rust-lang.github.io/rust-clippy/master/index.html#indexing_slicing
 [`ineffective_bit_mask`]: https://rust-lang.github.io/rust-clippy/master/index.html#ineffective_bit_mask
+[`inefficient_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inefficient_to_string
 [`infallible_destructuring_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#infallible_destructuring_match
 [`infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#infinite_iter
 [`inherent_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#inherent_to_string

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 325 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 326 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
 
 We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:
 

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

src/lintlist/mod.rs

@@ -6,7 +6,7 @@ pub use lint::Lint;
 pub use lint::LINT_LEVELS;
 
 // begin lint list, do not remove this comment, it’s used in `update_lints`
-pub const ALL_LINTS: [Lint; 325] = [
+pub const ALL_LINTS: [Lint; 326] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -735,6 +735,13 @@ pub const ALL_LINTS: [Lint; 325] = [
         deprecation: None,
         module: "bit_mask",
     },
+    Lint {
+        name: "inefficient_to_string",
+        group: "perf",
+        desc: "using `to_string` on `&&T` where `T: ToString`",
+        deprecation: None,
+        module: "methods",
+    },
     Lint {
         name: "infallible_destructuring_match",
         group: "style",

tests/ui/inefficient_to_string.fixed

@@ -0,0 +1,32 @@
+// run-rustfix
+#![deny(clippy::inefficient_to_string)]
+
+use std::borrow::Cow;
+use std::string::ToString;
+
+fn main() {
+    let rstr: &str = "hello";
+    let rrstr: &&str = &rstr;
+    let rrrstr: &&&str = &rrstr;
+    let _: String = rstr.to_string();
+    let _: String = (*rrstr).to_string();
+    let _: String = (**rrrstr).to_string();
+
+    let string: String = String::from("hello");
+    let rstring: &String = &string;
+    let rrstring: &&String = &rstring;
+    let rrrstring: &&&String = &rrstring;
+    let _: String = string.to_string();
+    let _: String = rstring.to_string();
+    let _: String = (*rrstring).to_string();
+    let _: String = (**rrrstring).to_string();
+
+    let cow: Cow<'_, str> = Cow::Borrowed("hello");
+    let rcow: &Cow<'_, str> = &cow;
+    let rrcow: &&Cow<'_, str> = &rcow;
+    let rrrcow: &&&Cow<'_, str> = &rrcow;
+    let _: String = cow.to_string();
+    let _: String = rcow.to_string();
+    let _: String = (*rrcow).to_string();
+    let _: String = (**rrrcow).to_string();
+}

tests/ui/inefficient_to_string.rs

@@ -0,0 +1,32 @@
+// run-rustfix
+#![deny(clippy::inefficient_to_string)]
+
+use std::borrow::Cow;
+use std::string::ToString;
+
+fn main() {
+    let rstr: &str = "hello";
+    let rrstr: &&str = &rstr;
+    let rrrstr: &&&str = &rrstr;
+    let _: String = rstr.to_string();
+    let _: String = rrstr.to_string();
+    let _: String = rrrstr.to_string();
+
+    let string: String = String::from("hello");
+    let rstring: &String = &string;
+    let rrstring: &&String = &rstring;
+    let rrrstring: &&&String = &rrstring;
+    let _: String = string.to_string();
+    let _: String = rstring.to_string();
+    let _: String = rrstring.to_string();
+    let _: String = rrrstring.to_string();
+
+    let cow: Cow<'_, str> = Cow::Borrowed("hello");
+    let rcow: &Cow<'_, str> = &cow;
+    let rrcow: &&Cow<'_, str> = &rcow;
+    let rrrcow: &&&Cow<'_, str> = &rrcow;
+    let _: String = cow.to_string();
+    let _: String = rcow.to_string();
+    let _: String = rrcow.to_string();
+    let _: String = rrrcow.to_string();
+}