From 6fa34cca291995df922af6c822d0078db88504d5 Mon Sep 17 00:00:00 2001
From: Georg Brandl <georg@python.org>
Date: Tue, 25 Aug 2015 18:38:08 +0200
Subject: [PATCH] methods: suggest correct replacement for `to_string()` (fixes
 #232)

---
 src/methods.rs                | 16 +++++++++++++---
 src/utils.rs                  | 11 +++++++++++
 tests/compile-fail/methods.rs |  5 ++++-
 tests/compile-fail/strings.rs | 32 ++++++++++++++++----------------
 4 files changed, 44 insertions(+), 20 deletions(-)

diff --git a/src/methods.rs b/src/methods.rs
index 07693e11d99..bfe2f7984a9 100644
--- a/src/methods.rs
+++ b/src/methods.rs
@@ -1,8 +1,10 @@
 use syntax::ast::*;
 use rustc::lint::*;
 use rustc::middle::ty;
+use std::iter;
+use std::borrow::Cow;
 
-use utils::{span_lint, match_type, walk_ptrs_ty};
+use utils::{snippet, span_lint, match_type, walk_ptrs_ty_depth};
 use utils::{OPTION_PATH, RESULT_PATH, STRING_PATH};
 
 #[derive(Copy,Clone)]
@@ -24,7 +26,7 @@ fn get_lints(&self) -> LintArray {
 
     fn check_expr(&mut self, cx: &Context, expr: &Expr) {
         if let ExprMethodCall(ref ident, _, ref args) = expr.node {
-            let obj_ty = walk_ptrs_ty(cx.tcx.expr_ty(&args[0]));
+            let (obj_ty, ptr_depth) = walk_ptrs_ty_depth(cx.tcx.expr_ty(&args[0]));
             if ident.node.name == "unwrap" {
                 if match_type(cx, obj_ty, &OPTION_PATH) {
                     span_lint(cx, OPTION_UNWRAP_USED, expr.span,
@@ -39,7 +41,15 @@ fn check_expr(&mut self, cx: &Context, expr: &Expr) {
             }
             else if ident.node.name == "to_string" {
                 if obj_ty.sty == ty::TyStr {
-                    span_lint(cx, STR_TO_STRING, expr.span, "`str.to_owned()` is faster");
+                    let mut arg_str = snippet(cx, args[0].span, "_");
+                    if ptr_depth > 1 {
+                        arg_str = Cow::Owned(format!(
+                            "({}{})",
+                            iter::repeat('*').take(ptr_depth - 1).collect::<String>(),
+                            arg_str));
+                    }
+                    span_lint(cx, STR_TO_STRING, expr.span, &format!(
+                        "`{}.to_owned()` is faster", arg_str));
                 } else if match_type(cx, obj_ty, &STRING_PATH) {
                     span_lint(cx, STRING_TO_STRING, expr.span, "`String.to_string()` is a no-op; use \
                                                                 `clone()` to make a copy");
diff --git a/src/utils.rs b/src/utils.rs
index 394204bedfc..1b01d558094 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -158,6 +158,17 @@ pub fn walk_ptrs_ty(ty: ty::Ty) -> ty::Ty {
     }
 }
 
+/// return the base type for references and raw pointers, and count reference depth
+pub fn walk_ptrs_ty_depth(ty: ty::Ty) -> (ty::Ty, usize) {
+    fn inner(ty: ty::Ty, depth: usize) -> (ty::Ty, usize) {
+        match ty.sty {
+            ty::TyRef(_, ref tm) | ty::TyRawPtr(ref tm) => inner(tm.ty, depth + 1),
+            _ => (ty, depth)
+        }
+    }
+    inner(ty, 0)
+}
+
 /// Produce a nested chain of if-lets and ifs from the patterns:
 ///
 ///     if_let_chain! {
diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs
index 91d3b72de84..811c44ef85c 100755
--- a/tests/compile-fail/methods.rs
+++ b/tests/compile-fail/methods.rs
@@ -10,6 +10,9 @@ fn main() {
     let res: Result<i32, ()> = Ok(0);
     let _ = res.unwrap();  //~ERROR used unwrap() on a Result
 
-    let string = "str".to_string();  //~ERROR `str.to_owned()` is faster
+    let _ = "str".to_string();  //~ERROR `"str".to_owned()` is faster
+
+    let v = &"str";
+    let string = v.to_string();  //~ERROR `(*v).to_owned()` is faster
     let _again = string.to_string();  //~ERROR `String.to_string()` is a no-op
 }
diff --git a/tests/compile-fail/strings.rs b/tests/compile-fail/strings.rs
index 680ebb73dea..7e21294a3d1 100755
--- a/tests/compile-fail/strings.rs
+++ b/tests/compile-fail/strings.rs
@@ -4,29 +4,29 @@
 #[deny(string_add)]
 #[allow(string_add_assign)]
 fn add_only() { // ignores assignment distinction
-	let mut x = "".to_owned();
+    let mut x = "".to_owned();
 
     for _ in (1..3) {
         x = x + "."; //~ERROR you added something to a string.
     }
-    
+
     let y = "".to_owned();
     let z = y + "..."; //~ERROR you added something to a string.
-    
+
     assert_eq!(&x, &z);
 }
 
 #[deny(string_add_assign)]
 fn add_assign_only() {
-	let mut x = "".to_owned();
+    let mut x = "".to_owned();
 
     for _ in (1..3) {
         x = x + "."; //~ERROR you assigned the result of adding something to this string.
     }
-    
+
     let y = "".to_owned();
     let z = y + "...";
-    
+
     assert_eq!(&x, &z);
 }
 
@@ -37,20 +37,20 @@ fn both() {
     for _ in (1..3) {
         x = x + "."; //~ERROR you assigned the result of adding something to this string.
     }
-    
+
     let y = "".to_owned();
     let z = y + "..."; //~ERROR you added something to a string.
-    
+
     assert_eq!(&x, &z);
 }
 
 fn main() {
-	add_only();
-	add_assign_only();
-	both();
-	
-	// the add is only caught for String
-	let mut x = 1;
-	x = x + 1;
-	assert_eq!(2, x);
+    add_only();
+    add_assign_only();
+    both();
+
+    // the add is only caught for String
+    let mut x = 1;
+    x = x + 1;
+    assert_eq!(2, x);
 }