clippy_lints/src/assign_ops.rs

   1 use rustc::hir;
   2 use rustc::lint::*;
   3 use utils::{span_lint_and_then, snippet_opt, SpanlessEq, get_trait_def_id, implements_trait};
   4 use utils::{higher, sugg};
   5
   6 /// **What it does:** Checks for compound assignment operations (`+=` and similar).
   7 ///
   8 /// **Why is this bad?** Projects with many developers from languages without
   9 /// those operations may find them unreadable and not worth their weight.
  10 ///
  11 /// **Known problems:** Types implementing `OpAssign` don't necessarily implement `Op`.
  12 ///
  13 /// **Example:**
  14 /// ```rust
  15 /// a += 1;
  16 /// ```
  17 declare_restriction_lint! {
  18     pub ASSIGN_OPS,
  19     "any assignment operation"
  20 }
  21
  22 /// **What it does:** Checks for `a = a op b` or `a = b commutative_op a` patterns.
  23 ///
  24 /// **Why is this bad?** These can be written as the shorter `a op= b`.
  25 ///
  26 /// **Known problems:** While forbidden by the spec, `OpAssign` traits may have
  27 /// implementations that differ from the regular `Op` impl.
  28 ///
  29 /// **Example:**
  30 /// ```rust
  31 /// let mut a = 5;
  32 /// ...
  33 /// a = a + b;
  34 /// ```
  35 declare_lint! {
  36     pub ASSIGN_OP_PATTERN,
  37     Warn,
  38     "assigning the result of an operation on a variable to that same variable"
  39 }
  40
  41 /// **What it does:** Checks for `a op= a op b` or `a op= b op a` patterns.
  42 ///
  43 /// **Why is this bad?** Most likely these are bugs where one meant to write `a op= b`.
  44 ///
  45 /// **Known problems:** Someone might actually mean `a op= a op b`, but that
  46 /// should rather be written as `a = (2 * a) op b` where applicable.
  47 ///
  48 /// **Example:**
  49 /// ```rust
  50 /// let mut a = 5;
  51 /// ...
  52 /// a += a + b;
  53 /// ```
  54 declare_lint! {
  55     pub MISREFACTORED_ASSIGN_OP,
  56     Warn,
  57     "having a variable on both sides of an assign op"
  58 }
  59
  60 #[derive(Copy, Clone, Default)]
  61 pub struct AssignOps;
  62
  63 impl LintPass for AssignOps {
  64     fn get_lints(&self) -> LintArray {
  65         lint_array!(ASSIGN_OPS, ASSIGN_OP_PATTERN, MISREFACTORED_ASSIGN_OP)
  66     }
  67 }
  68
  69 impl LateLintPass for AssignOps {
  70     fn check_expr(&mut self, cx: &LateContext, expr: &hir::Expr) {
  71         match expr.node {
  72             hir::ExprAssignOp(op, ref lhs, ref rhs) => {
  73                 span_lint_and_then(cx, ASSIGN_OPS, expr.span, "assign operation detected", |db| {
  74                     let lhs = &sugg::Sugg::hir(cx, lhs, "..");
  75                     let rhs = &sugg::Sugg::hir(cx, rhs, "..");
  76
  77                     db.span_suggestion(expr.span,
  78                                        "replace it with",
  79                                        format!("{} = {}", lhs, sugg::make_binop(higher::binop(op.node), lhs, rhs)));
  80                 });
  81                 if let hir::ExprBinary(binop, ref l, ref r) = rhs.node {
  82                     if op.node == binop.node {
  83                         let lint = |assignee: &hir::Expr, rhs: &hir::Expr| {
  84                             let ty = cx.tcx.expr_ty(assignee);
  85                             if ty.walk_shallow().next().is_some() {
  86                                 return; // implements_trait does not work with generics
  87                             }
  88                             let rty = cx.tcx.expr_ty(rhs);
  89                             if rty.walk_shallow().next().is_some() {
  90                                 return; // implements_trait does not work with generics
  91                             }
  92                             span_lint_and_then(cx,
  93                                                MISREFACTORED_ASSIGN_OP,
  94                                                expr.span,
  95                                                "variable appears on both sides of an assignment operation",
  96                                                |db| {
  97                                                    if let (Some(snip_a), Some(snip_r)) = (snippet_opt(cx, assignee.span),
  98                                                                                           snippet_opt(cx, rhs.span)) {
  99                                                        db.span_suggestion(expr.span,
 100                                                                           "replace it with",
 101                                                                           format!("{} {}= {}", snip_a, op.node.as_str(), snip_r));
 102                                                    }
 103                                                });
 104                         };
 105                         // lhs op= l op r
 106                         if SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, l) {
 107                             lint(lhs, r);
 108                         }
 109                         // lhs op= l commutative_op r
 110                         if is_commutative(op.node) && SpanlessEq::new(cx).ignore_fn().eq_expr(lhs, r) {
 111                             lint(lhs, l);
 112                         }
 113                     }
 114                 }
 115             }
 116             hir::ExprAssign(ref assignee, ref e) => {
 117                 if let hir::ExprBinary(op, ref l, ref r) = e.node {
 118                     let lint = |assignee: &hir::Expr, rhs: &hir::Expr| {
 119                         let ty = cx.tcx.expr_ty(assignee);
 120                         if ty.walk_shallow().next().is_some() {
 121                             return; // implements_trait does not work with generics
 122                         }
 123                         let rty = cx.tcx.expr_ty(rhs);
 124                         if rty.walk_shallow().next().is_some() {
 125                             return; // implements_trait does not work with generics
 126                         }
 127                         macro_rules! ops {
 128                             ($op:expr, $cx:expr, $ty:expr, $rty:expr, $($trait_name:ident:$full_trait_name:ident),+) => {
 129                                 match $op {
 130                                     $(hir::$full_trait_name => {
 131                                         let [krate, module] = ::utils::paths::OPS_MODULE;
 132                                         let path = [krate, module, concat!(stringify!($trait_name), "Assign")];
 133                                         let trait_id = if let Some(trait_id) = get_trait_def_id($cx, &path) {
 134                                             trait_id
 135                                         } else {
 136                                             return; // useless if the trait doesn't exist
 137                                         };
 138                                         implements_trait($cx, $ty, trait_id, vec![$rty])
 139                                     },)*
 140                                     _ => false,
 141                                 }
 142                             }
 143                         }
 144                         if ops!(op.node,
 145                                 cx,
 146                                 ty,
 147                                 rty,
 148                                 Add: BiAdd,
 149                                 Sub: BiSub,
 150                                 Mul: BiMul,
 151                                 Div: BiDiv,
 152                                 Rem: BiRem,
 153                                 And: BiAnd,
 154                                 Or: BiOr,
 155                                 BitAnd: BiBitAnd,
 156                                 BitOr: BiBitOr,
 157                                 BitXor: BiBitXor,
 158                                 Shr: BiShr,
 159                                 Shl: BiShl) {
 160                             span_lint_and_then(cx,
 161                                                ASSIGN_OP_PATTERN,
 162                                                expr.span,
 163                                                "manual implementation of an assign operation",
 164                                                |db| {
 165                                                    if let (Some(snip_a), Some(snip_r)) = (snippet_opt(cx, assignee.span),
 166                                                                                           snippet_opt(cx, rhs.span)) {
 167                                                        db.span_suggestion(expr.span,
 168                                                                           "replace it with",
 169                                                                           format!("{} {}= {}", snip_a, op.node.as_str(), snip_r));
 170                                                    }
 171                                                });
 172                         }
 173                     };
 174                     // a = a op b
 175                     if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, l) {
 176                         lint(assignee, r);
 177                     }
 178                     // a = b commutative_op a
 179                     if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, r) {
 180                         match op.node {
 181                             hir::BiAdd | hir::BiMul | hir::BiAnd | hir::BiOr | hir::BiBitXor | hir::BiBitAnd |
 182                             hir::BiBitOr => {
 183                                 lint(assignee, l);
 184                             }
 185                             _ => {}
 186                         }
 187                     }
 188                 }
 189             }
 190             _ => {}
 191         }
 192     }
 193 }
 194
 195 fn is_commutative(op: hir::BinOp_) -> bool {
 196     use rustc::hir::BinOp_::*;
 197     match op {
 198         BiAdd |
 199         BiMul |
 200         BiAnd |
 201         BiOr |
 202         BiBitXor |
 203         BiBitAnd |
 204         BiBitOr |
 205         BiEq |
 206         BiNe => true,
 207         BiSub |
 208         BiDiv |
 209         BiRem |
 210         BiShl |
 211         BiShr |
 212         BiLt |
 213         BiLe |
 214         BiGe |
 215         BiGt => false,
 216     }
 217 }