methods: suggest correct replacement for `to_string()` (fixes #232)

diff --git a/src/methods.rs b/src/methods.rs
index 07693e11d996589389f58fa8286ac07afcc1c57a..bfe2f7984a90549bf90d6735d21a88a10f10d675 100644 (file)
--- 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 394204bedfc714e11f5ab4089b064523b8d36150..1b01d558094a30b453e326dc9974f161fbcd6f8c 100644 (file)
--- 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 91d3b72de8409922255ca348d1377a7d71664fed..811c44ef85cb846b538dea588ba684d75c15a494 100755 (executable)
--- 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 680ebb73dea43e978e2b07d30340a0a67ee217ec..7e21294a3d16a3c1dcb5df27bb166d8f8411426b 100755 (executable)
--- 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);
 }