Merge lint with single_char_pattern

clippy_lints/src/lib.rs

@@ -286,7 +286,6 @@ mod repeat_once;
 mod returns;
 mod serde_api;
 mod shadow;
-mod single_char_push_str;
 mod single_component_path_imports;
 mod slow_vector_initialization;
 mod stable_sort_primitive;
@@ -676,6 +675,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &methods::SEARCH_IS_SOME,
         &methods::SHOULD_IMPLEMENT_TRAIT,
         &methods::SINGLE_CHAR_PATTERN,
+        &methods::SINGLE_CHAR_PUSH_STR,
         &methods::SKIP_WHILE_NEXT,
         &methods::STRING_EXTEND_CHARS,
         &methods::SUSPICIOUS_MAP,
@@ -773,7 +773,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &shadow::SHADOW_REUSE,
         &shadow::SHADOW_SAME,
         &shadow::SHADOW_UNRELATED,
-        &single_char_push_str::SINGLE_CHAR_PUSH_STR,
         &single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS,
         &slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
         &stable_sort_primitive::STABLE_SORT_PRIMITIVE,
@@ -930,7 +929,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_late_pass(move || box escape::BoxedLocal{too_large_for_stack});
     store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented);
     store.register_late_pass(|| box strings::StringLitAsBytes);
-    store.register_late_pass(|| box single_char_push_str::SingleCharPushStrPass);
     store.register_late_pass(|| box derive::Derive);
     store.register_late_pass(|| box types::CharLitAsU8);
     store.register_late_pass(|| box vec::UselessVec);
@@ -1346,6 +1344,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&methods::SEARCH_IS_SOME),
         LintId::of(&methods::SHOULD_IMPLEMENT_TRAIT),
         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),
@@ -1412,7 +1411,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
         LintId::of(&serde_api::SERDE_API_MISUSE),
-        LintId::of(&single_char_push_str::SINGLE_CHAR_PUSH_STR),
         LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
         LintId::of(&slow_vector_initialization::SLOW_VECTOR_INITIALIZATION),
         LintId::of(&stable_sort_primitive::STABLE_SORT_PRIMITIVE),
@@ -1527,6 +1525,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::STRING_EXTEND_CHARS),
         LintId::of(&methods::UNNECESSARY_FOLD),
         LintId::of(&methods::WRONG_SELF_CONVENTION),
@@ -1551,7 +1550,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&regex::TRIVIAL_REGEX),
         LintId::of(&returns::NEEDLESS_RETURN),
         LintId::of(&returns::UNUSED_UNIT),
-        LintId::of(&single_char_push_str::SINGLE_CHAR_PUSH_STR),
         LintId::of(&single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS),
         LintId::of(&strings::STRING_LIT_AS_BYTES),
         LintId::of(&tabs_in_doc_comments::TABS_IN_DOC_COMMENTS),

clippy_lints/src/methods/mod.rs

@@ -1306,6 +1306,29 @@ declare_clippy_lint! {
     "using `.iter().next()` on a sliced array, which can be shortened to just `.get()`"
 }
 
+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.
+    ///
+    /// **Why is this bad?** it's less clear that we are pushing a single character
+    ///
+    /// **Known problems:** None
+    ///
+    /// **Example:**
+    /// ```
+    /// let mut string = String::new();
+    /// string.push_str("R");
+    /// ```
+    /// Could be written as
+    /// ```
+    /// let mut string = String::new();
+    /// string.push('R');
+    /// ```
+    pub SINGLE_CHAR_PUSH_STR,
+    style,
+    "`push_str()` used with a single-character string literal as parameter"
+}
+
 declare_lint_pass!(Methods => [
     UNWRAP_USED,
     EXPECT_USED,
@@ -1327,6 +1350,7 @@ declare_lint_pass!(Methods => [
     INEFFICIENT_TO_STRING,
     NEW_RET_NO_SELF,
     SINGLE_CHAR_PATTERN,
+    SINGLE_CHAR_PUSH_STR,
     SEARCH_IS_SOME,
     TEMPORARY_CSTRING_AS_PTR,
     FILTER_NEXT,
@@ -1441,6 +1465,12 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                     inefficient_to_string::lint(cx, expr, &args[0], self_ty);
                 }
 
+                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);
+                    }
+                }
+
                 match self_ty.kind {
                     ty::Ref(_, ty, _) if ty.kind == ty::Str => {
                         for &(method, pos) in &PATTERN_METHODS {
@@ -3124,15 +3154,18 @@ fn lint_chars_last_cmp_with_unwrap<'tcx>(cx: &LateContext<'tcx>, info: &BinaryEx
     }
 }
 
-/// lint for length-1 `str`s for methods in `PATTERN_METHODS`
-fn lint_single_char_pattern<'tcx>(cx: &LateContext<'tcx>, _expr: &'tcx hir::Expr<'_>, arg: &'tcx hir::Expr<'_>) {
+fn get_hint_if_single_char_arg<'tcx>(
+    cx: &LateContext<'tcx>,
+    arg: &'tcx hir::Expr<'_>,
+    applicability: &mut Applicability,
+) -> Option<String> {
     if_chain! {
         if let hir::ExprKind::Lit(lit) = &arg.kind;
         if let ast::LitKind::Str(r, style) = lit.node;
-        if r.as_str().len() == 1;
+        let string = r.as_str();
+        if string.len() == 1;
         then {
-            let mut applicability = Applicability::MachineApplicable;
-            let snip = snippet_with_applicability(cx, arg.span, "..", &mut applicability);
+            let snip = snippet_with_applicability(cx, arg.span, &string, applicability);
             let ch = if let ast::StrStyle::Raw(nhash) = style {
                 let nhash = nhash as usize;
                 // for raw string: r##"a"##
@@ -3142,19 +3175,47 @@ fn lint_single_char_pattern<'tcx>(cx: &LateContext<'tcx>, _expr: &'tcx hir::Expr
                 &snip[1..(snip.len() - 1)]
             };
             let hint = format!("'{}'", if ch == "'" { "\\'" } else { ch });
-            span_lint_and_sugg(
-                cx,
-                SINGLE_CHAR_PATTERN,
-                arg.span,
-                "single-character string constant used as pattern",
-                "try using a `char` instead",
-                hint,
-                applicability,
-            );
+            Some(hint)
+        } else {
+            None
         }
     }
 }
 
+/// lint for length-1 `str`s for methods in `PATTERN_METHODS`
+fn lint_single_char_pattern<'tcx>(cx: &LateContext<'tcx>, _expr: &'tcx hir::Expr<'_>, arg: &'tcx hir::Expr<'_>) {
+    let mut applicability = Applicability::MachineApplicable;
+    if let Some(hint) = get_hint_if_single_char_arg(cx, arg, &mut applicability) {
+        span_lint_and_sugg(
+            cx,
+            SINGLE_CHAR_PATTERN,
+            arg.span,
+            "single-character string constant used as pattern",
+            "try using a `char` instead",
+            hint,
+            applicability,
+        );
+    }
+}
+
+/// lint for length-1 `str`s as argument for `push_str`
+fn lint_single_char_push_string<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>, args: &'tcx [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 sugg = format!("{}.push({})", base_string_snippet, extension_string);
+        span_lint_and_sugg(
+            cx,
+            SINGLE_CHAR_PUSH_STR,
+            expr.span,
+            "calling `push_str()` using a single-character string literal",
+            "consider using `push` 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"

src/lintlist/mod.rs

@@ -2003,7 +2003,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         group: "style",
         desc: "`push_str()` used with a single-character string literal as parameter",
         deprecation: None,
-        module: "single_char_push_str",
+        module: "methods",
     },
     Lint {
         name: "single_component_path_imports",

tests/ui/single_char_push_str.fixed

@@ -7,4 +7,9 @@ fn main() {
     string.push('\'');
 
     string.push('u');
+    string.push_str("st");
+    string.push_str("");
+    string.push('\x52');
+    string.push('\u{0052}');
+    string.push('a');
 }

tests/ui/single_char_push_str.rs

@@ -7,4 +7,9 @@ fn main() {
     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"##);
 }

tests/ui/single_char_push_str.stderr

@@ -12,5 +12,23 @@ error: calling `push_str()` using a single-character string literal
 LL |     string.push_str("'");
    |     ^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('/'')`
 
-error: aborting due to 2 previous errors
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_push_str.rs:12: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_push_str.rs:13: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_push_str.rs:14:5
+   |
+LL |     string.push_str(r##"a"##);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('a')`
+
+error: aborting due to 5 previous errors