Merge pull request #3048 from goodmanjonathan/assign_op

deprecate assign_ops lint

clippy_lints/src/assign_ops.rs

@@ -7,25 +7,6 @@ use rustc::{declare_lint, lint_array};
 use if_chain::if_chain;
 use syntax::ast;
 
-/// **What it does:** Checks for compound assignment operations (`+=` and
-/// similar).
-///
-/// **Why is this bad?** Projects with many developers from languages without
-/// those operations may find them unreadable and not worth their weight.
-///
-/// **Known problems:** Types implementing `OpAssign` don't necessarily
-/// implement `Op`.
-///
-/// **Example:**
-/// ```rust
-/// a += 1;
-/// ```
-declare_clippy_lint! {
-    pub ASSIGN_OPS,
-    restriction,
-    "any compound assignment operation"
-}
-
 /// **What it does:** Checks for `a = a op b` or `a = b commutative_op a`
 /// patterns.
 ///
@@ -73,7 +54,7 @@ pub struct AssignOps;
 
 impl LintPass for AssignOps {
     fn get_lints(&self) -> LintArray {
-        lint_array!(ASSIGN_OPS, ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP)
+        lint_array!(ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP)
     }
 }
 
@@ -81,16 +62,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
         match expr.node {
             hir::ExprKind::AssignOp(op, ref lhs, ref rhs) => {
-                span_lint_and_then(cx, ASSIGN_OPS, expr.span, "assign operation detected", |db| {
-                    let lhs = &sugg::Sugg::hir(cx, lhs, "..");
-                    let rhs = &sugg::Sugg::hir(cx, rhs, "..");
-
-                    db.span_suggestion(
-                        expr.span,
-                        "replace it with",
-                        format!("{} = {}", lhs, sugg::make_binop(higher::binop(op.node), lhs, rhs)),
-                    );
-                });
                 if let hir::ExprKind::Binary(binop, ref l, ref r) = rhs.node {
                     if op.node == binop.node {
                         let lint = |assignee: &hir::Expr, rhs_other: &hir::Expr| {

clippy_lints/src/deprecated_lints.rs

@@ -82,3 +82,13 @@ declare_deprecated_lint! {
     pub MISALIGNED_TRANSMUTE,
     "this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr"
 }
+
+/// **What it does:** Nothing. This lint has been deprecated.
+///
+/// **Deprecation reason:** This lint is too subjective, not having a good reason for being in clippy.
+/// Additionally, compound assignment operators may be overloaded separately from their non-assigning
+/// counterparts, so this lint may suggest a change in behavior or the code may not compile.
+declare_deprecated_lint! {
+    pub ASSIGN_OPS,
+    "using compound assignment operators (e.g. `+=`) is harmless"
+}

clippy_lints/src/lib.rs

@@ -265,6 +265,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         "misaligned_transmute",
         "this lint has been split into cast_ptr_alignment and transmute_ptr_to_ptr",
     );
+    store.register_removed(
+        "assign_ops",
+        "using compound assignment operators (e.g. `+=`) is harmless",
+    );
     // end deprecated lints, do not remove this comment, it’s used in `update_lints`
 
     reg.register_late_lint_pass(box serde_api::Serde);
@@ -409,7 +413,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_lint_group("clippy_restriction", vec![
         arithmetic::FLOAT_ARITHMETIC,
         arithmetic::INTEGER_ARITHMETIC,
-        assign_ops::ASSIGN_OPS,
         else_if_without_else::ELSE_IF_WITHOUT_ELSE,
         indexing_slicing::INDEXING_SLICING,
         inherent_impl::MULTIPLE_INHERENT_IMPL,

tests/ui/assign_ops.rs

@@ -1,28 +1,6 @@
-
-
-
-#[warn(assign_ops)]
-#[allow(unused_assignments)]
-fn main() {
-    let mut i = 1i32;
-    i += 2;
-    i += 2 + 17;
-    i -= 6;
-    i -= 2 - 1;
-    i *= 5;
-    i *= 1+5;
-    i /= 32;
-    i /= 32 | 5;
-    i /= 32 / 5;
-    i %= 42;
-    i >>= i;
-    i <<= 9 + 6 - 7;
-    i += 1 << 5;
-}
-
 #[allow(dead_code, unused_assignments)]
 #[warn(assign_op_pattern)]
-fn bla() {
+fn main() {
     let mut a = 5;
     a = a + 1;
     a = 1 + a;

tests/ui/assign_ops.stderr

@@ -1,138 +1,58 @@
-error: assign operation detected
+error: manual implementation of an assign operation
+ --> $DIR/assign_ops.rs:5:5
+  |
+5 |     a = a + 1;
+  |     ^^^^^^^^^ help: replace it with: `a += 1`
+  |
+  = note: `-D assign-op-pattern` implied by `-D warnings`
+
+error: manual implementation of an assign operation
+ --> $DIR/assign_ops.rs:6:5
+  |
+6 |     a = 1 + a;
+  |     ^^^^^^^^^ help: replace it with: `a += 1`
+
+error: manual implementation of an assign operation
+ --> $DIR/assign_ops.rs:7:5
+  |
+7 |     a = a - 1;
+  |     ^^^^^^^^^ help: replace it with: `a -= 1`
+
+error: manual implementation of an assign operation
  --> $DIR/assign_ops.rs:8:5
   |
-8 |     i += 2;
-  |     ^^^^^^ help: replace it with: `i = i + 2`
-  |
-  = note: `-D assign-ops` implied by `-D warnings`
+8 |     a = a * 99;
+  |     ^^^^^^^^^^ help: replace it with: `a *= 99`
 
-error: assign operation detected
+error: manual implementation of an assign operation
  --> $DIR/assign_ops.rs:9:5
   |
-9 |     i += 2 + 17;
-  |     ^^^^^^^^^^^ help: replace it with: `i = i + 2 + 17`
+9 |     a = 42 * a;
+  |     ^^^^^^^^^^ help: replace it with: `a *= 42`
 
-error: assign operation detected
+error: manual implementation of an assign operation
   --> $DIR/assign_ops.rs:10:5
    |
-10 |     i -= 6;
-   |     ^^^^^^ help: replace it with: `i = i - 6`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:11:5
-   |
-11 |     i -= 2 - 1;
-   |     ^^^^^^^^^^ help: replace it with: `i = i - (2 - 1)`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:12:5
-   |
-12 |     i *= 5;
-   |     ^^^^^^ help: replace it with: `i = i * 5`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:13:5
-   |
-13 |     i *= 1+5;
-   |     ^^^^^^^^ help: replace it with: `i = i * (1+5)`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:14:5
-   |
-14 |     i /= 32;
-   |     ^^^^^^^ help: replace it with: `i = i / 32`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:15:5
-   |
-15 |     i /= 32 | 5;
-   |     ^^^^^^^^^^^ help: replace it with: `i = i / (32 | 5)`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:16:5
-   |
-16 |     i /= 32 / 5;
-   |     ^^^^^^^^^^^ help: replace it with: `i = i / (32 / 5)`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:17:5
-   |
-17 |     i %= 42;
-   |     ^^^^^^^ help: replace it with: `i = i % 42`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:18:5
-   |
-18 |     i >>= i;
-   |     ^^^^^^^ help: replace it with: `i = i >> i`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:19:5
-   |
-19 |     i <<= 9 + 6 - 7;
-   |     ^^^^^^^^^^^^^^^ help: replace it with: `i = i << (9 + 6 - 7)`
-
-error: assign operation detected
-  --> $DIR/assign_ops.rs:20:5
-   |
-20 |     i += 1 << 5;
-   |     ^^^^^^^^^^^ help: replace it with: `i = i + (1 << 5)`
-
-error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:27:5
-   |
-27 |     a = a + 1;
-   |     ^^^^^^^^^ help: replace it with: `a += 1`
-   |
-   = note: `-D assign-op-pattern` implied by `-D warnings`
-
-error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:28:5
-   |
-28 |     a = 1 + a;
-   |     ^^^^^^^^^ help: replace it with: `a += 1`
-
-error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:29:5
-   |
-29 |     a = a - 1;
-   |     ^^^^^^^^^ help: replace it with: `a -= 1`
-
-error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:30:5
-   |
-30 |     a = a * 99;
-   |     ^^^^^^^^^^ help: replace it with: `a *= 99`
-
-error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:31:5
-   |
-31 |     a = 42 * a;
-   |     ^^^^^^^^^^ help: replace it with: `a *= 42`
-
-error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:32:5
-   |
-32 |     a = a / 2;
+10 |     a = a / 2;
    |     ^^^^^^^^^ help: replace it with: `a /= 2`
 
 error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:33:5
+  --> $DIR/assign_ops.rs:11:5
    |
-33 |     a = a % 5;
+11 |     a = a % 5;
    |     ^^^^^^^^^ help: replace it with: `a %= 5`
 
 error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:34:5
+  --> $DIR/assign_ops.rs:12:5
    |
-34 |     a = a & 1;
+12 |     a = a & 1;
    |     ^^^^^^^^^ help: replace it with: `a &= 1`
 
 error: manual implementation of an assign operation
-  --> $DIR/assign_ops.rs:40:5
+  --> $DIR/assign_ops.rs:18:5
    |
-40 |     s = s + "bla";
+18 |     s = s + "bla";
    |     ^^^^^^^^^^^^^ help: replace it with: `s += "bla"`
 
-error: aborting due to 22 previous errors
+error: aborting due to 9 previous errors