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

fixes #792
This commit is contained in:
Phil Turnbull 2016-11-19 10:36:23 -05:00
parent fa78b09fa7
commit 73a73638c0
3 changed files with 34 additions and 16 deletions

View file

@ -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 `!=`

View file

@ -491,7 +491,7 @@ declare_lint! {
}
/// **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 @@ declare_lint! {
/// **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));
});
}
}

View file

@ -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());