clippy_lints/src/assign_ops.rs

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