diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs index 1ce690abcfe..7d4f4e7cbc3 100644 --- a/clippy_lints/src/assign_ops.rs +++ b/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| { diff --git a/clippy_lints/src/deprecated_lints.rs b/clippy_lints/src/deprecated_lints.rs index 1edeb30560c..983f347c56f 100644 --- a/clippy_lints/src/deprecated_lints.rs +++ b/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" +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index ee2f0ca5406..da6db4cfe45 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/tests/ui/assign_ops.rs b/tests/ui/assign_ops.rs index 2b49f2146ba..7332b41fa0b 100644 --- a/tests/ui/assign_ops.rs +++ b/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; diff --git a/tests/ui/assign_ops.stderr b/tests/ui/assign_ops.stderr index 2123507e2ef..826dacc53a9 100644 --- a/tests/ui/assign_ops.stderr +++ b/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