Add lint for `string.extend(string.chars())`

fixes #792

diff --git a/README.md b/README.md
index bc21bd28d780e856d7bdccd117715d97f7969876..db291adefaf3d3a8c8c848f3132fbe641f6a83d8 100644 (file)
--- a/README.md
+++ b/README.md
@@ -327,7 +327,7 @@ name
 [single_match_else](https://github.com/Manishearth/rust-clippy/wiki#single_match_else)                                 | allow   | a match statement with a two arms where the second arm's pattern is a wildcard instead of `if let`
 [string_add](https://github.com/Manishearth/rust-clippy/wiki#string_add)                                               | allow   | using `x + ..` where x is a `String` instead of `push_str()`
 [string_add_assign](https://github.com/Manishearth/rust-clippy/wiki#string_add_assign)                                 | allow   | using `x = x + ..` where x is a `String` instead of `push_str()`
-[string_extend_chars](https://github.com/Manishearth/rust-clippy/wiki#string_extend_chars)                             | warn    | using `x.extend(s.chars())` where s is a `&str`
+[string_extend_chars](https://github.com/Manishearth/rust-clippy/wiki#string_extend_chars)                             | warn    | using `x.extend(s.chars())` where s is a `&str` or `String`
 [string_lit_as_bytes](https://github.com/Manishearth/rust-clippy/wiki#string_lit_as_bytes)                             | warn    | calling `as_bytes` on a string literal instead of using a byte string literal
 [stutter](https://github.com/Manishearth/rust-clippy/wiki#stutter)                                                     | allow   | type names prefixed/postfixed with their containing module's name
 [suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting)   | warn    | suspicious formatting of `*=`, `-=` or `!=`

diff --git a/clippy_lints/src/methods.rs b/clippy_lints/src/methods.rs
index 527b3228d69309ebe42a4d4f2a6c451299264dbd..9506c8e838ada02be90a27b4781692a3f02b9ce3 100644 (file)
--- a/clippy_lints/src/methods.rs
+++ b/clippy_lints/src/methods.rs
@@ -491,7 +491,7 @@
 }
 
 /// **What it does:** Checks for the use of `.extend(s.chars())` where s is a
-/// `&str`.
+/// `&str` or `String`.
 ///
 /// **Why is this bad?** `.push_str(s)` is clearer and faster
 ///
@@ -500,20 +500,24 @@
 /// **Example:**
 /// ```rust
 /// let abc = "abc";
+/// let def = String::from("def");
 /// let mut s = String::new();
 /// s.extend(abc.chars());
+/// s.extend(def.chars());
 /// ```
 /// The correct use would be:
 /// ```rust
 /// let abc = "abc";
+/// let def = String::from("def");
 /// let mut s = String::new();
 /// s.push_str(abc);
+/// s.push_str(def.as_str());
 /// ```
 
 declare_lint! {
     pub STRING_EXTEND_CHARS,
     Warn,
-    "using `x.extend(s.chars())` where s is a `&str`"
+    "using `x.extend(s.chars())` where s is a `&str` or `String`"
 }
 
 
@@ -839,19 +843,26 @@ fn lint_string_extend(cx: &LateContext, expr: &hir::Expr, args: &MethodArgs) {
     if let Some(arglists) = method_chain_args(arg, &["chars"]) {
         let target = &arglists[0][0];
         let (self_ty, _) = walk_ptrs_ty_depth(cx.tcx.tables().expr_ty(target));
-        if self_ty.sty == ty::TyStr {
-            span_lint_and_then(
-                cx,
-                STRING_EXTEND_CHARS,
-                expr.span,
-                "calling `.extend(_.chars())`",
-                |db| {
-                    db.span_suggestion(expr.span, "try this",
-                            format!("{}.push_str({})",
-                                    snippet(cx, args[0].span, "_"),
-                                    snippet(cx, target.span, "_")));
-                });
-        }
+        let extra_suggestion = if self_ty.sty == ty::TyStr {
+            ""
+        } else if match_type(cx, self_ty, &paths::STRING) {
+            ".as_str()"
+        } else {
+            return;
+        };
+
+        span_lint_and_then(
+            cx,
+            STRING_EXTEND_CHARS,
+            expr.span,
+            "calling `.extend(_.chars())`",
+            |db| {
+                db.span_suggestion(expr.span, "try this",
+                        format!("{}.push_str({}{})",
+                                snippet(cx, args[0].span, "_"),
+                                snippet(cx, target.span, "_"),
+                                extra_suggestion));
+            });
     }
 }
 

diff --git a/tests/compile-fail/methods.rs b/tests/compile-fail/methods.rs
index 658eda65f6716fdf62f9589499632f473c8517b7..0e30876c3e5c173302736c1c8ee2d18637fe1dc2 100644 (file)
--- a/tests/compile-fail/methods.rs
+++ b/tests/compile-fail/methods.rs
@@ -535,6 +535,7 @@ fn use_extend_from_slice() {
 
 fn str_extend_chars() {
     let abc = "abc";
+    let def = String::from("def");
     let mut s = String::new();
 
     s.push_str(abc);
@@ -549,6 +550,12 @@ fn str_extend_chars() {
     //~|HELP try this
     //~|SUGGESTION s.push_str("abc")
 
+    s.push_str(def.as_str());
+    s.extend(def.chars());
+    //~^ERROR calling `.extend(_.chars())`
+    //~|HELP try this
+    //~|SUGGESTION s.push_str(def.as_str())
+
     s.extend(abc.chars().skip(1));
     s.extend("abc".chars().skip(1));
     s.extend(['a', 'b', 'c'].iter());