Auto merge of #6037 - matthiaskrgr:single_char_insert, r=flip1995

single_char_insert_str: lint using insert_str() on single-char literals and suggest insert() Fixes #6026 changelog: add single_char_insert_str lint which lints using string.insert_str() with single char literals and suggests string.insert() with a char
rust-lang · Nov 3, 2020 · 3ee9c2e · 3ee9c2e
2 parents c45255b + f8ac1f9
commit 3ee9c2e
Show file tree

Hide file tree

Showing 14 changed files with 278 additions and 125 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,7 +22,7 @@ Current beta, release 2020-11-19
 * [`map_err_ignore`] [#5998](https://github.com/rust-lang/rust-clippy/pull/5998)
 * [`rc_buffer`] [#6044](https://github.com/rust-lang/rust-clippy/pull/6044)
 * [`to_string_in_display`] [#5831](https://github.com/rust-lang/rust-clippy/pull/5831)
-* [`single_char_push_str`] [#5881](https://github.com/rust-lang/rust-clippy/pull/5881)
+* `single_char_push_str` [#5881](https://github.com/rust-lang/rust-clippy/pull/5881)
 
 ### Moves and Deprecations
 
@@ -1939,8 +1939,8 @@ Released 2018-09-13
 [`should_assert_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_assert_eq
 [`should_implement_trait`]: https://rust-lang.github.io/rust-clippy/master/index.html#should_implement_trait
 [`similar_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#similar_names
+[`single_char_add_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_add_str
 [`single_char_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_pattern
-[`single_char_push_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_char_push_str
 [`single_component_path_imports`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_component_path_imports
 [`single_element_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_element_loop
 [`single_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match

diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs
@@ -714,8 +714,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::RESULT_MAP_OR_INTO_OPTION,
         &methods::SEARCH_IS_SOME,
         &methods::SHOULD_IMPLEMENT_TRAIT,
+        &methods::SINGLE_CHAR_ADD_STR,
         &methods::SINGLE_CHAR_PATTERN,
-        &methods::SINGLE_CHAR_PUSH_STR,
         &methods::SKIP_WHILE_NEXT,
         &methods::STRING_EXTEND_CHARS,
         &methods::SUSPICIOUS_MAP,
@@ -1439,8 +1439,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION),
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
+        LintId::of(&methods::SINGLE_CHAR_ADD_STR),
         LintId::of(&methods::SINGLE_CHAR_PATTERN),
-        LintId::of(&methods::SINGLE_CHAR_PUSH_STR),
         LintId::of(&methods::SKIP_WHILE_NEXT),
         LintId::of(&methods::STRING_EXTEND_CHARS),
         LintId::of(&methods::SUSPICIOUS_MAP),
@@ -1632,7 +1632,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::OPTION_MAP_OR_NONE),
         LintId::of(&methods::RESULT_MAP_OR_INTO_OPTION),
         LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
-        LintId::of(&methods::SINGLE_CHAR_PUSH_STR),
+        LintId::of(&methods::SINGLE_CHAR_ADD_STR),
         LintId::of(&methods::STRING_EXTEND_CHARS),
         LintId::of(&methods::UNNECESSARY_FOLD),
         LintId::of(&methods::UNNECESSARY_LAZY_EVALUATIONS),
@@ -1959,6 +1959,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) {
     ls.register_renamed("clippy::for_loop_over_result", "clippy::for_loops_over_fallibles");
     ls.register_renamed("clippy::identity_conversion", "clippy::useless_conversion");
     ls.register_renamed("clippy::zero_width_space", "clippy::invisible_characters");
+    ls.register_renamed("clippy::single_char_push_str", "clippy::single_char_add_str");
 }
 
 // only exists to let the dogfood integration test works.

diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
@@ -1290,8 +1290,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`/`insert_str` with a single-character string literal
+    /// where `push`/`insert` with a `char` would work fine.
     ///
     /// **Why is this bad?** It's less clear that we are pushing a single character.
     ///
@@ -1300,16 +1300,18 @@ declare_clippy_lint! {
     /// **Example:**
     /// ```rust
     /// let mut string = String::new();
+    /// string.insert_str(0, "R");
     /// string.push_str("R");
     /// ```
     /// Could be written as
     /// ```rust
     /// let mut string = String::new();
+    /// string.insert(0, 'R');
     /// string.push('R');
     /// ```
-    pub SINGLE_CHAR_PUSH_STR,
+    pub SINGLE_CHAR_ADD_STR,
     style,
-    "`push_str()` used with a single-character string literal as parameter"
+    "`push_str()` or `insert_str()` used with a single-character string literal as parameter"
 }
 
 declare_clippy_lint! {
@@ -1390,7 +1392,7 @@ declare_lint_pass!(Methods => [
     INEFFICIENT_TO_STRING,
     NEW_RET_NO_SELF,
     SINGLE_CHAR_PATTERN,
-    SINGLE_CHAR_PUSH_STR,
+    SINGLE_CHAR_ADD_STR,
     SEARCH_IS_SOME,
     FILTER_NEXT,
     SKIP_WHILE_NEXT,
@@ -1521,6 +1523,8 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 if let Some(fn_def_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id) {
                     if match_def_path(cx, fn_def_id, &paths::PUSH_STR) {
                         lint_single_char_push_string(cx, expr, args);
+                    } else if match_def_path(cx, fn_def_id, &paths::INSERT_STR) {
+                        lint_single_char_insert_string(cx, expr, args);
                     }
                 }
 
@@ -3202,7 +3206,7 @@ fn get_hint_if_single_char_arg(
         if let hir::ExprKind::Lit(lit) = &arg.kind;
         if let ast::LitKind::Str(r, style) = lit.node;
         let string = r.as_str();
-        if string.len() == 1;
+        if string.chars().count() == 1;
         then {
             let snip = snippet_with_applicability(cx, arg.span, &string, applicability);
             let ch = if let ast::StrStyle::Raw(nhash) = style {
@@ -3241,11 +3245,12 @@ fn lint_single_char_pattern(cx: &LateContext<'_>, _expr: &hir::Expr<'_>, arg: &h
 fn lint_single_char_push_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[1], &mut applicability) {
-        let base_string_snippet = snippet_with_applicability(cx, args[0].span, "..", &mut applicability);
+        let base_string_snippet =
+            snippet_with_applicability(cx, args[0].span.source_callsite(), "..", &mut applicability);
         let sugg = format!("{}.push({})", base_string_snippet, extension_string);
         span_lint_and_sugg(
             cx,
-            SINGLE_CHAR_PUSH_STR,
+            SINGLE_CHAR_ADD_STR,
             expr.span,
             "calling `push_str()` using a single-character string literal",
             "consider using `push` with a character literal",
@@ -3255,6 +3260,26 @@ 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.source_callsite(), "_", &mut applicability);
+        let pos_arg = snippet_with_applicability(cx, args[1].span, "..", &mut applicability);
+        let sugg = format!("{}.insert({}, {})", base_string_snippet, pos_arg, extension_string);
+        span_lint_and_sugg(
+            cx,
+            SINGLE_CHAR_ADD_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"

diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs
@@ -52,6 +52,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"];

diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs
@@ -2147,16 +2147,16 @@ vec![
         module: "non_expressive_names",
     },
     Lint {
-        name: "single_char_pattern",
-        group: "perf",
-        desc: "using a single-character str where a char could be used, e.g., `_.split(\"x\")`",
+        name: "single_char_add_str",
+        group: "style",
+        desc: "`push_str()` or `insert_str()` used with a single-character string literal as parameter",
         deprecation: None,
         module: "methods",
     },
     Lint {
-        name: "single_char_push_str",
-        group: "style",
-        desc: "`push_str()` used with a single-character string literal as parameter",
+        name: "single_char_pattern",
+        group: "perf",
+        desc: "using a single-character str where a char could be used, e.g., `_.split(\"x\")`",
         deprecation: None,
         module: "methods",
     },

diff --git a/tests/ui/single_char_add_str.fixed b/tests/ui/single_char_add_str.fixed
@@ -0,0 +1,45 @@
+// run-rustfix
+#![warn(clippy::single_char_add_str)]
+
+macro_rules! get_string {
+    () => {
+        String::from("Hello world!")
+    };
+}
+
+fn main() {
+    // `push_str` tests
+
+    let mut string = String::new();
+    string.push('R');
+    string.push('\'');
+
+    string.push('u');
+    string.push_str("st");
+    string.push_str("");
+    string.push('\x52');
+    string.push('\u{0052}');
+    string.push('a');
+
+    get_string!().push('ö');
+
+    // `insert_str` tests
+
+    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');
+    string.insert(Y, '"');
+    string.insert(Y, '\'');
+
+    get_string!().insert(1, '?');
+}
diff --git a/tests/ui/single_char_add_str.rs b/tests/ui/single_char_add_str.rs
@@ -0,0 +1,45 @@
+// run-rustfix
+#![warn(clippy::single_char_add_str)]
+
+macro_rules! get_string {
+    () => {
+        String::from("Hello world!")
+    };
+}
+
+fn main() {
+    // `push_str` tests
+
+    let mut string = String::new();
+    string.push_str("R");
+    string.push_str("'");
+
+    string.push('u');
+    string.push_str("st");
+    string.push_str("");
+    string.push_str("\x52");
+    string.push_str("\u{0052}");
+    string.push_str(r##"a"##);
+
+    get_string!().push_str("ö");
+
+    // `insert_str` tests
+
+    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"##);
+    string.insert_str(Y, r##"""##);
+    string.insert_str(Y, r##"'"##);
+
+    get_string!().insert_str(1, "?");
+}
diff --git a/tests/ui/single_char_add_str.stderr b/tests/ui/single_char_add_str.stderr
@@ -0,0 +1,94 @@
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:14:5
+   |
+LL |     string.push_str("R");
+   |     ^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('R')`
+   |
+   = note: `-D clippy::single-char-add-str` implied by `-D warnings`
+
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:15:5
+   |
+LL |     string.push_str("'");
+   |     ^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('/'')`
+
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:20:5
+   |
+LL |     string.push_str("/x52");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('/x52')`
+
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:21:5
+   |
+LL |     string.push_str("/u{0052}");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('/u{0052}')`
+
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:22:5
+   |
+LL |     string.push_str(r##"a"##);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('a')`
+
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:24:5
+   |
+LL |     get_string!().push_str("ö");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `get_string!().push('ö')`
+
+error: calling `insert_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:29:5
+   |
+LL |     string.insert_str(0, "R");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(0, 'R')`
+
+error: calling `insert_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:30: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_add_str.rs:35: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_add_str.rs:36: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_add_str.rs:38: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_add_str.rs:40:5
+   |
+LL |     string.insert_str(Y, r##"a"##);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(Y, 'a')`
+
+error: calling `insert_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:41:5
+   |
+LL |     string.insert_str(Y, r##"""##);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(Y, '"')`
+
+error: calling `insert_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:42:5
+   |
+LL |     string.insert_str(Y, r##"'"##);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `string.insert(Y, '/'')`
+
+error: calling `insert_str()` using a single-character string literal
+  --> $DIR/single_char_add_str.rs:44:5
+   |
+LL |     get_string!().insert_str(1, "?");
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `insert` with a character literal: `get_string!().insert(1, '?')`
+
+error: aborting due to 15 previous errors
+
diff --git a/tests/ui/single_char_pattern.fixed b/tests/ui/single_char_pattern.fixed
@@ -12,15 +12,9 @@ fn main() {
 
     let y = "x";
     x.split(y);
-    // Not yet testing for multi-byte characters
-    // Changing `r.len() == 1` to `r.chars().count() == 1` in `lint_clippy::single_char_pattern`
-    // should have done this but produced an ICE
-    //
-    // We may not want to suggest changing these anyway
-    // See: https://github.com/rust-lang/rust-clippy/issues/650#issuecomment-184328984
-    x.split("ß");
-    x.split("ℝ");
-    x.split("💣");
+    x.split('ß');
+    x.split('ℝ');
+    x.split('💣');
     // Can't use this lint for unicode code points which don't fit in a char
     x.split("❤️");
     x.contains('x');