Merge string_add_assign lint into string_add lint

Author: ThibsG

CHANGELOG.md

@@ -1555,7 +1555,6 @@ Released 2018-09-13
 [`slow_vector_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#slow_vector_initialization
 [`str_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#str_to_string
 [`string_add`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add
-[`string_add_assign`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_add_assign
 [`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
 [`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
 [`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string

clippy_lints/src/lib.rs

@@ -780,7 +780,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &single_component_path_imports::SINGLE_COMPONENT_PATH_IMPORTS,
         &slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
         &strings::STRING_ADD,
-        &strings::STRING_ADD_ASSIGN,
         &strings::STRING_LIT_AS_BYTES,
         &suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
         &suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
@@ -1147,7 +1146,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&non_expressive_names::SIMILAR_NAMES),
         LintId::of(&ranges::RANGE_PLUS_ONE),
         LintId::of(&shadow::SHADOW_UNRELATED),
-        LintId::of(&strings::STRING_ADD_ASSIGN),
         LintId::of(&trait_bounds::TYPE_REPETITION_IN_BOUNDS),
         LintId::of(&trivially_copy_pass_by_ref::TRIVIALLY_COPY_PASS_BY_REF),
         LintId::of(&types::CAST_LOSSLESS),

clippy_lints/src/strings.rs

@@ -8,35 +8,14 @@ use rustc_span::source_map::Spanned;
 use if_chain::if_chain;
 
 use crate::utils::SpanlessEq;
-use crate::utils::{get_parent_expr, is_allowed, is_type_diagnostic_item, span_lint, span_lint_and_sugg, walk_ptrs_ty};
+use crate::utils::{get_parent_expr, is_type_diagnostic_item, span_lint, span_lint_and_sugg, walk_ptrs_ty};
 
 declare_clippy_lint! {
-    /// **What it does:** Checks for string appends of the form `x = x + y` (without
-    /// `let`!).
+    /// **What it does:** Checks for all instances of `x + _` or `x = x + y` (without `let`)
+    /// where `x` is of type `String`.
     ///
-    /// **Why is this bad?** It's not really bad, but some people think that the
-    /// `.push_str(_)` method is more readable.
-    ///
-    /// **Known problems:** None.
-    ///
-    /// **Example:**
-    ///
-    /// ```rust
-    /// let mut x = "Hello".to_owned();
-    /// x = x + ", World";
-    /// ```
-    pub STRING_ADD_ASSIGN,
-    pedantic,
-    "using `x = x + ..` where x is a `String` instead of `push_str()`"
-}
-
-declare_clippy_lint! {
-    /// **What it does:** Checks for all instances of `x + _` where `x` is of type
-    /// `String`, but only if [`string_add_assign`](#string_add_assign) does *not*
-    /// match.
-    ///
-    /// **Why is this bad?** It's not bad in and of itself. However, this particular
-    /// `Add` implementation is asymmetric (the other operand need not be `String`,
+    /// **Why is this bad?** It's not bad in and of itself, but `String::push_str` is more readable.
+    /// However, this particular `Add` implementation is asymmetric (the other operand need not be `String`,
     /// but `x` does), while addition as mathematically defined is symmetric, also
     /// the `String::push_str(_)` function is a perfectly good replacement.
     /// Therefore, some dislike it and wish not to have it in their code.
@@ -47,11 +26,27 @@ declare_clippy_lint! {
     ///
     /// **Known problems:** None.
     ///
-    /// **Example:**
+    /// **Examples:**
+    /// ```rust,ignore
+    /// # let x = "Hello".to_owned();
     ///
-    /// ```rust
-    /// let x = "Hello".to_owned();
+    /// // Bad
     /// x + ", World";
+    ///
+    /// // Good
+    /// x.push_str(", World");
+    /// ```
+    ///
+    /// // or
+    ///
+    /// ```rust,ignore
+    /// # let mut x = "Hello".to_owned();
+    ///
+    /// // Bad
+    /// x = x + ", World";
+    ///
+    /// // Good
+    /// x += ", World";
     /// ```
     pub STRING_ADD,
     restriction,
@@ -69,14 +64,19 @@ declare_clippy_lint! {
     ///
     /// **Example:**
     /// ```rust
+    ///
+    /// // Bad
     /// let bs = "a byte string".as_bytes();
+    ///
+    /// // Good
+    /// let bs = b"a byte string";
     /// ```
     pub STRING_LIT_AS_BYTES,
     style,
     "calling `as_bytes` on a string literal instead of using a byte string literal"
 }
 
-declare_lint_pass!(StringAdd => [STRING_ADD, STRING_ADD_ASSIGN]);
+declare_lint_pass!(StringAdd => [STRING_ADD]);
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) {
@@ -93,14 +93,11 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
         ) = e.kind
         {
             if is_string(cx, left) {
-                if !is_allowed(cx, STRING_ADD_ASSIGN, e.hir_id) {
-                    let parent = get_parent_expr(cx, e);
-                    if let Some(p) = parent {
-                        if let ExprKind::Assign(ref target, _, _) = p.kind {
-                            // avoid duplicate matches
-                            if SpanlessEq::new(cx).eq_expr(target, left) {
-                                return;
-                            }
+                if let Some(parent) = get_parent_expr(cx, e) {
+                    if let ExprKind::Assign(ref target, _, _) = parent.kind {
+                        // avoid duplicate matches
+                        if SpanlessEq::new(cx).eq_expr(target, left) {
+                            return;
                         }
                     }
                 }
@@ -115,10 +112,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for StringAdd {
             if is_string(cx, target) && is_add(cx, src, target) {
                 span_lint(
                     cx,
-                    STRING_ADD_ASSIGN,
+                    STRING_ADD,
                     e.span,
                     "you assigned the result of adding something to this string. Consider using \
-                     `String::push_str()` instead",
+                    `String::push_str()` instead",
                 );
             }
         }

src/lintlist/mod.rs

@@ -1977,13 +1977,6 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         module: "strings",
     },
-    Lint {
-        name: "string_add_assign",
-        group: "pedantic",
-        desc: "using `x = x + ..` where x is a `String` instead of `push_str()`",
-        deprecation: None,
-        module: "strings",
-    },
     Lint {
         name: "string_extend_chars",
         group: "style",

tests/ui/string_add.rs

@@ -4,7 +4,7 @@
 extern crate macro_rules;
 
 #[warn(clippy::string_add)]
-#[allow(clippy::string_add_assign, unused)]
+#[allow(unused)]
 fn main() {
     // ignores assignment distinction
     let mut x = "".to_owned();

tests/ui/string_add.stderr

@@ -1,18 +1,18 @@
-error: manual implementation of an assign operation
+error: you assigned the result of adding something to this string. Consider using `String::push_str()` instead
   --> $DIR/string_add.rs:13:9
    |
 LL |         x = x + ".";
-   |         ^^^^^^^^^^^ help: replace it with: `x += "."`
+   |         ^^^^^^^^^^^
    |
-   = note: `-D clippy::assign-op-pattern` implied by `-D warnings`
+   = note: `-D clippy::string-add` implied by `-D warnings`
 
-error: you added something to a string. Consider using `String::push_str()` instead
-  --> $DIR/string_add.rs:13:13
+error: manual implementation of an assign operation
+  --> $DIR/string_add.rs:13:9
    |
 LL |         x = x + ".";
-   |             ^^^^^^^
+   |         ^^^^^^^^^^^ help: replace it with: `x += "."`
    |
-   = note: `-D clippy::string-add` implied by `-D warnings`
+   = note: `-D clippy::assign-op-pattern` implied by `-D warnings`
 
 error: you added something to a string. Consider using `String::push_str()` instead
   --> $DIR/string_add.rs:17:13