Skip to content

Commit

Permalink
single_char_insert_str: lint using insert_str() on single-char litera…
Browse files Browse the repository at this point in the history
…ls and suggest insert()

changelog: add single_char_insert_str lint which lints using string.insert_str() with single char literals and suggests string.insert() with a char

Fixes #6026
  • Loading branch information
matthiaskrgr committed Sep 13, 2020
1 parent 231444d commit d02e4c8
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 2 deletions.
49 changes: 47 additions & 2 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1324,8 +1324,8 @@ declare_clippy_lint! {
}

declare_clippy_lint! {
/// **What it does:** Warns when using `push_str` with a single-character string literal,
/// and `push` with a `char` would work fine.
/// **What it does:** Warns when using `push_str` with a single-character string literal
/// where `push` with a `char` would work fine.
///
/// **Why is this bad?** It's less clear that we are pushing a single character.
///
Expand Down Expand Up @@ -1382,6 +1382,28 @@ declare_clippy_lint! {
"using unnecessary lazy evaluation, which can be replaced with simpler eager evaluation"
}

declare_clippy_lint! {
/// **What it does:** Warns when using `insert_str` with a single-character string literal
/// where `insert` with a `char` would work fine.
///
/// **Why is this bad?** It's less clear that we are inserting a single character.
///
/// **Known problems:** None
///
/// **Example:**
/// ```rust
/// let mut string = String::new();
/// string.insert_str(0, "R");
/// ```
/// Could be written as
/// ```rust
/// let mut string = String::new();
/// string.insert(0, 'R');
/// ```
pub SINGLE_CHAR_INSERT_STR,
style,
"`insert_str()` used with a single-character string literal as parameter"
}
declare_lint_pass!(Methods => [
UNWRAP_USED,
EXPECT_USED,
Expand All @@ -1402,6 +1424,7 @@ declare_lint_pass!(Methods => [
CLONE_DOUBLE_REF,
INEFFICIENT_TO_STRING,
NEW_RET_NO_SELF,
SINGLE_CHAR_INSERT_STR,
SINGLE_CHAR_PATTERN,
SINGLE_CHAR_PUSH_STR,
SEARCH_IS_SOME,
Expand Down Expand Up @@ -1532,6 +1555,9 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
if match_def_path(cx, fn_def_id, &paths::PUSH_STR) {
lint_single_char_push_string(cx, expr, args);
}
if match_def_path(cx, fn_def_id, &paths::INSERT_STR) {
lint_single_char_insert_string(cx, expr, args);
}
}

match self_ty.kind() {
Expand Down Expand Up @@ -3314,6 +3340,25 @@ fn lint_single_char_push_string(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args
}
}

/// lint for length-1 `str`s as argument for `insert_str`
fn lint_single_char_insert_string(cx: &LateContext<'_>, expr: &hir::Expr<'_>, args: &[hir::Expr<'_>]) {
let mut applicability = Applicability::MachineApplicable;
if let Some(extension_string) = get_hint_if_single_char_arg(cx, &args[2], &mut applicability) {
let base_string_snippet = snippet_with_applicability(cx, args[0].span, "_", &mut applicability);
let pos_arg = snippet(cx, args[1].span, "..");
let sugg = format!("{}.insert({}, {})", base_string_snippet, pos_arg, extension_string);
span_lint_and_sugg(
cx,
SINGLE_CHAR_INSERT_STR,
expr.span,
"calling `insert_str()` using a single-character string literal",
"consider using `insert` with a character literal",
sugg,
applicability,
);
}
}

/// Checks for the `USELESS_ASREF` lint.
fn lint_asref(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str, as_ref_args: &[hir::Expr<'_>]) {
// when we get here, we've already checked that the call name is "as_ref" or "as_mut"
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/utils/paths.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ pub const HASHMAP_ENTRY: [&str; 5] = ["std", "collections", "hash", "map", "Entr
pub const HASHSET: [&str; 5] = ["std", "collections", "hash", "set", "HashSet"];
pub const INDEX: [&str; 3] = ["core", "ops", "Index"];
pub const INDEX_MUT: [&str; 3] = ["core", "ops", "IndexMut"];
pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"];
pub const INTO: [&str; 3] = ["core", "convert", "Into"];
pub const INTO_ITERATOR: [&str; 5] = ["core", "iter", "traits", "collect", "IntoIterator"];
pub const IO_READ: [&str; 3] = ["std", "io", "Read"];
Expand Down
18 changes: 18 additions & 0 deletions tests/ui/single_char_insert_str.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-rustfix
#![warn(clippy::single_char_push_str)]

fn main() {
let mut string = String::new();
string.insert(0, 'R');
string.insert(1, '\'');

string.insert(0, 'u');
string.insert_str(2, "st");
string.insert_str(0, "");
string.insert(0, '\x52');
string.insert(0, '\u{0052}');
let x: usize = 2;
string.insert(x, 'a');
const Y: usize = 1;
string.insert(Y, 'a');
}
18 changes: 18 additions & 0 deletions tests/ui/single_char_insert_str.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// run-rustfix
#![warn(clippy::single_char_push_str)]

fn main() {
let mut string = String::new();
string.insert_str(0, "R");
string.insert_str(1, "'");

string.insert(0, 'u');
string.insert_str(2, "st");
string.insert_str(0, "");
string.insert_str(0, "\x52");
string.insert_str(0, "\u{0052}");
let x: usize = 2;
string.insert_str(x, r##"a"##);
const Y: usize = 1;
string.insert_str(Y, r##"a"##);
}
40 changes: 40 additions & 0 deletions tests/ui/single_char_insert_str.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
error: calling `insert_str()` using a single-character string literal
--> $DIR/single_char_insert_str.rs:6:5
|
LL | string.insert_str(0, "R");
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(0, 'R')`
|
= note: `-D clippy::single-char-insert-str` implied by `-D warnings`

error: calling `insert_str()` using a single-character string literal
--> $DIR/single_char_insert_str.rs:7:5
|
LL | string.insert_str(1, "'");
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(1, '/'')`

error: calling `insert_str()` using a single-character string literal
--> $DIR/single_char_insert_str.rs:12:5
|
LL | string.insert_str(0, "/x52");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(0, '/x52')`

error: calling `insert_str()` using a single-character string literal
--> $DIR/single_char_insert_str.rs:13:5
|
LL | string.insert_str(0, "/u{0052}");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(0, '/u{0052}')`

error: calling `insert_str()` using a single-character string literal
--> $DIR/single_char_insert_str.rs:15:5
|
LL | string.insert_str(x, r##"a"##);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(x, 'a')`

error: calling `insert_str()` using a single-character string literal
--> $DIR/single_char_insert_str.rs:17:5
|
LL | string.insert_str(Y, r##"a"##);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(Y, 'a')`

error: aborting due to 6 previous errors

0 comments on commit d02e4c8

Please sign in to comment.