Add `inefficient_to_string` lint

8 files changed, 181 insertions, 2 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9dd04af611f..42ab00001d9 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/README.md b/README.md
index 986d920ac96..5023538c5ed 100644
--- a/README.md
+++ b/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:
 
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
index e8962c6e207..bae904a445e 100644
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@@ -806,6 +806,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,
@@ -1182,6 +1183,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,
diff --git a/clippy_lints/src/methods/inefficient_to_string.rs b/clippy_lints/src/methods/inefficient_to_string.rs
new file mode 100644
index 00000000000..35c634cc52d
--- /dev/null
+++ b/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 the blanket impl, but `{}` specializes `ToString` directly",
+                        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,
+    }
+}
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index 848e3bcb881..efa283b823d 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/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;
@@ -590,6 +591,29 @@ declare_clippy_lint! {
 }
 
 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`.
     ///
     /// **Why is this bad?** As a convention, `new` methods are used to make a new
@@ -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 => {
diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
index d9ae1f70d42..1575f0e3027 100644
--- a/src/lintlist/mod.rs
+++ b/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",
@@ -736,6 +736,13 @@ pub const ALL_LINTS: [Lint; 325] = [
         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",
         desc: "a match statement with a single infallible arm instead of a `let`",
diff --git a/tests/ui/inefficient_to_string.rs b/tests/ui/inefficient_to_string.rs
new file mode 100644
index 00000000000..a9f8f244c19
--- /dev/null
+++ b/tests/ui/inefficient_to_string.rs
@@ -0,0 +1,31 @@
+#![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();
+}
diff --git a/tests/ui/inefficient_to_string.stderr b/tests/ui/inefficient_to_string.stderr
new file mode 100644
index 00000000000..70e3c1e82c7
--- /dev/null
+++ b/tests/ui/inefficient_to_string.stderr
@@ -0,0 +1,55 @@
+error: calling `to_string` on `&&str`
+  --> $DIR/inefficient_to_string.rs:11:21
+   |
+LL |     let _: String = rrstr.to_string();
+   |                     ^^^^^^^^^^^^^^^^^ help: try dereferencing the receiver: `(*rrstr).to_string()`
+   |
+note: lint level defined here
+  --> $DIR/inefficient_to_string.rs:1:9
+   |
+LL | #![deny(clippy::inefficient_to_string)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: `&str` implements `ToString` through the blanket impl, but `str` specializes `ToString` directly
+
+error: calling `to_string` on `&&&str`
+  --> $DIR/inefficient_to_string.rs:12:21
+   |
+LL |     let _: String = rrrstr.to_string();
+   |                     ^^^^^^^^^^^^^^^^^^ help: try dereferencing the receiver: `(**rrrstr).to_string()`
+   |
+   = help: `&&str` implements `ToString` through the blanket impl, but `str` specializes `ToString` directly
+
+error: calling `to_string` on `&&std::string::String`
+  --> $DIR/inefficient_to_string.rs:20:21
+   |
+LL |     let _: String = rrstring.to_string();
+   |                     ^^^^^^^^^^^^^^^^^^^^ help: try dereferencing the receiver: `(*rrstring).to_string()`
+   |
+   = help: `&std::string::String` implements `ToString` through the blanket impl, but `std::string::String` specializes `ToString` directly
+
+error: calling `to_string` on `&&&std::string::String`
+  --> $DIR/inefficient_to_string.rs:21:21
+   |
+LL |     let _: String = rrrstring.to_string();
+   |                     ^^^^^^^^^^^^^^^^^^^^^ help: try dereferencing the receiver: `(**rrrstring).to_string()`
+   |
+   = help: `&&std::string::String` implements `ToString` through the blanket impl, but `std::string::String` specializes `ToString` directly
+
+error: calling `to_string` on `&&std::borrow::Cow<'_, str>`
+  --> $DIR/inefficient_to_string.rs:29:21
+   |
+LL |     let _: String = rrcow.to_string();
+   |                     ^^^^^^^^^^^^^^^^^ help: try dereferencing the receiver: `(*rrcow).to_string()`
+   |
+   = help: `&std::borrow::Cow<'_, str>` implements `ToString` through the blanket impl, but `std::borrow::Cow<'_, str>` specializes `ToString` directly
+
+error: calling `to_string` on `&&&std::borrow::Cow<'_, str>`
+  --> $DIR/inefficient_to_string.rs:30:21
+   |
+LL |     let _: String = rrrcow.to_string();
+   |                     ^^^^^^^^^^^^^^^^^^ help: try dereferencing the receiver: `(**rrrcow).to_string()`
+   |
+   = help: `&&std::borrow::Cow<'_, str>` implements `ToString` through the blanket impl, but `std::borrow::Cow<'_, str>` specializes `ToString` directly
+
+error: aborting due to 6 previous errors
+