Use `utils::sugg` in `ASSIGN_OPS`

diff --git a/README.md b/README.md
index 2f54835d5387c6a327fc896c5bacb7c36195a4ac..20c6c6a38a8ae0763b56685fb223900151c60dfb 100644 (file)
--- a/README.md
+++ b/README.md
@@ -25,7 +25,7 @@ name
 [almost_swapped](https://github.com/Manishearth/rust-clippy/wiki#almost_swapped)                                     | warn    | `foo = bar; bar = foo` sequence
 [approx_constant](https://github.com/Manishearth/rust-clippy/wiki#approx_constant)                                   | warn    | the approximate of a known float constant (in `std::f64::consts` or `std::f32::consts`) is found; suggests to use the constant
 [assign_op_pattern](https://github.com/Manishearth/rust-clippy/wiki#assign_op_pattern)                               | warn    | assigning the result of an operation on a variable to that same variable
-[assign_ops](https://github.com/Manishearth/rust-clippy/wiki#assign_ops)                                             | allow   | Any assignment operation
+[assign_ops](https://github.com/Manishearth/rust-clippy/wiki#assign_ops)                                             | allow   | any assignment operation
 [bad_bit_mask](https://github.com/Manishearth/rust-clippy/wiki#bad_bit_mask)                                         | warn    | expressions of the form `_ & mask == select` that will only ever return `true` or `false` (because in the example `select` containing bits that `mask` doesn't have)
 [blacklisted_name](https://github.com/Manishearth/rust-clippy/wiki#blacklisted_name)                                 | warn    | usage of a blacklisted/placeholder name
 [block_in_if_condition_expr](https://github.com/Manishearth/rust-clippy/wiki#block_in_if_condition_expr)             | warn    | braces can be eliminated in conditions that are expressions, e.g `if { true } ...`

diff --git a/clippy_lints/src/assign_ops.rs b/clippy_lints/src/assign_ops.rs
index 1a5ca16b9c574561bdfd7ca8413c94267eab44c6..6ae76f9c3eff93607f0c04ca48f50bf3af3db9f6 100644 (file)
--- a/clippy_lints/src/assign_ops.rs
+++ b/clippy_lints/src/assign_ops.rs
@@ -1,13 +1,14 @@
 use rustc::hir;
 use rustc::lint::*;
 use utils::{span_lint_and_then, span_lint, snippet_opt, SpanlessEq, get_trait_def_id, implements_trait};
+use utils::{higher, sugg};
 
-/// **What it does:** This lint checks for `+=` operations and similar
+/// **What it does:** This lint checks for `+=` 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
+/// **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`
+/// **Known problems:** Types implementing `OpAssign` don't necessarily implement `Op`.
 ///
 /// **Example:**
 /// ```
@@ -15,14 +16,14 @@
 /// ```
 declare_restriction_lint! {
     pub ASSIGN_OPS,
-    "Any assignment operation"
+    "any assignment operation"
 }
 
-/// **What it does:** Check for `a = a op b` or `a = b commutative_op a` patterns
+/// **What it does:** Check for `a = a op b` or `a = b commutative_op a` patterns.
 ///
-/// **Why is this bad?** These can be written as the shorter `a op= b`
+/// **Why is this bad?** These can be written as the shorter `a op= b`.
 ///
-/// **Known problems:** While forbidden by the spec, `OpAssign` traits may have implementations that differ from the regular `Op` impl
+/// **Known problems:** While forbidden by the spec, `OpAssign` traits may have implementations that differ from the regular `Op` impl.
 ///
 /// **Example:**
 ///
@@ -50,24 +51,14 @@ impl LateLintPass for AssignOps {
     fn check_expr(&mut self, cx: &LateContext, expr: &hir::Expr) {
         match expr.node {
             hir::ExprAssignOp(op, ref lhs, ref rhs) => {
-                if let (Some(l), Some(r)) = (snippet_opt(cx, lhs.span), snippet_opt(cx, rhs.span)) {
-                    span_lint_and_then(cx, ASSIGN_OPS, expr.span, "assign operation detected", |db| {
-                        match rhs.node {
-                            hir::ExprBinary(op2, _, _) if op2 != op => {
-                                db.span_suggestion(expr.span,
-                                                   "replace it with",
-                                                   format!("{} = {} {} ({})", l, l, op.node.as_str(), r));
-                            }
-                            _ => {
-                                db.span_suggestion(expr.span,
-                                                   "replace it with",
-                                                   format!("{} = {} {} {}", l, l, op.node.as_str(), r));
-                            }
-                        }
-                    });
-                } else {
-                    span_lint(cx, ASSIGN_OPS, expr.span, "assign operation detected");
-                }
+                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)));
+                });
             }
             hir::ExprAssign(ref assignee, ref e) => {
                 if let hir::ExprBinary(op, ref l, ref r) = e.node {

diff --git a/tests/compile-fail/assign_ops.rs b/tests/compile-fail/assign_ops.rs
index 84d868ecfcc4e6a749a537f3f753d0bbf69e1e2b..2b69e110f43d7749dc9e3d93d5c4053051428c81 100644 (file)
--- a/tests/compile-fail/assign_ops.rs
+++ b/tests/compile-fail/assign_ops.rs
@@ -8,15 +8,31 @@ fn main() {
     i += 2; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i + 2
+    i += 2 + 17; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i + 2 + 17
     i -= 6; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i - 6
+    i -= 2 - 1;
+    //~^ ERROR assign operation detected
+    //~| HELP replace it with
+    //~| SUGGESTION i = i - (2 - 1)
     i *= 5; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i * 5
+    i *= 1+5; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i * (1+5)
     i /= 32; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i / 32
+    i /= 32 | 5; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i / (32 | 5)
+    i /= 32 / 5; //~ ERROR assign operation detected
+    //~^ HELP replace it with
+    //~| SUGGESTION i = i / (32 / 5)
     i %= 42; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i % 42
@@ -26,6 +42,10 @@ fn main() {
     i <<= 9 + 6 - 7; //~ ERROR assign operation detected
     //~^ HELP replace it with
     //~| SUGGESTION i = i << (9 + 6 - 7)
+    i += 1 << 5;
+    //~^ ERROR assign operation detected
+    //~| HELP replace it with
+    //~| SUGGESTION i = i + (1 << 5)
 }
 
 #[allow(dead_code, unused_assignments)]