Lint push_str with a single-character string literal

Fixes #5875

Author: wiomoc

CHANGELOG.md

@@ -1697,6 +1697,7 @@ Released 2018-09-13
 [`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_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_match`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match
 [`single_match_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#single_match_else

clippy_lints/src/lib.rs

@@ -286,6 +286,7 @@ 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;
@@ -772,6 +773,7 @@ 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,
@@ -928,6 +930,7 @@ 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);
@@ -1409,6 +1412,7 @@ 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),
@@ -1547,6 +1551,7 @@ 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/single_char_push_str.rs

@@ -0,0 +1,62 @@
+use crate::utils::{match_def_path, paths, snippet_with_applicability, span_lint_and_sugg};
+use if_chain::if_chain;
+use rustc_ast::ast::LitKind;
+use rustc_errors::Applicability;
+use rustc_hir::{Expr, ExprKind};
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::{declare_lint_pass, declare_tool_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.
+    ///
+    /// **Why is this bad?** This is in all probability not the intended outcome. At
+    /// the least it hurts readability of the code.
+    ///
+    /// **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!(SingleCharPushStrPass => [SINGLE_CHAR_PUSH_STR]);
+
+impl<'tcx> LateLintPass<'tcx> for SingleCharPushStrPass {
+    fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
+        if_chain! {
+            if let ExprKind::MethodCall(_, _, ref args, _) = expr.kind;
+            if let [base_string, extension_string] = args;
+            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);
+            if let ExprKind::Lit(ref lit) = extension_string.kind;
+            if let LitKind::Str(symbol,_) = lit.node;
+            let extension_string_val = symbol.as_str().to_string();
+            if extension_string_val.len() == 1;
+            then {
+                let mut applicability = Applicability::MachineApplicable;
+                let base_string_snippet = snippet_with_applicability(cx, base_string.span, "_", &mut applicability);
+                let sugg = format!("{}.push({:?})", base_string_snippet, extension_string_val.chars().next().unwrap());
+                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
+                );
+            }
+        }
+    }
+}

clippy_lints/src/utils/paths.rs

@@ -84,6 +84,7 @@ pub const POLL: [&str; 4] = ["core", "task", "poll", "Poll"];
 pub const PTR_EQ: [&str; 3] = ["core", "ptr", "eq"];
 pub const PTR_NULL: [&str; 2] = ["ptr", "null"];
 pub const PTR_NULL_MUT: [&str; 2] = ["ptr", "null_mut"];
+pub const PUSH_STR: [&str; 4] = ["alloc", "string", "String", "push_str"];
 pub const RANGE: [&str; 3] = ["core", "ops", "Range"];
 pub const RANGE_ARGUMENT_TRAIT: [&str; 3] = ["core", "ops", "RangeBounds"];
 pub const RANGE_FROM: [&str; 3] = ["core", "ops", "RangeFrom"];

src/lintlist/mod.rs

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

tests/ui/single_char_push_str.fixed

@@ -0,0 +1,10 @@
+// run-rustfix
+#![warn(clippy::single_char_push_str)]
+
+fn main() {
+    let mut string = String::new();
+    string.push('R');
+    string.push('\'');
+
+    string.push('u');
+}

tests/ui/single_char_push_str.rs

@@ -0,0 +1,10 @@
+// run-rustfix
+#![warn(clippy::single_char_push_str)]
+
+fn main() {
+    let mut string = String::new();
+    string.push_str("R");
+    string.push_str("'");
+
+    string.push('u');
+}

tests/ui/single_char_push_str.stderr

@@ -0,0 +1,16 @@
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_push_str.rs:6:5
+   |
+LL |     string.push_str("R");
+   |     ^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('R')`
+   |
+   = note: `-D clippy::single-char-push-str` implied by `-D warnings`
+
+error: calling `push_str()` using a single-character string literal
+  --> $DIR/single_char_push_str.rs:7:5
+   |
+LL |     string.push_str("'");
+   |     ^^^^^^^^^^^^^^^^^^^^ help: consider using `push` with a character literal: `string.push('/'')`
+
+error: aborting due to 2 previous errors
+