clippy_lints/src/assign_ops.rs

   1 use rustc::hir;
   2 use rustc::lint::*;
   3 use syntax::ast;
   4 use utils::{span_lint_and_then, snippet_opt, SpanlessEq, get_trait_def_id, implements_trait};
   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(cx,
  92                                                MISREFACTORED_ASSIGN_OP,
  93                                                expr.span,
  94                                                "variable appears on both sides of an assignment operation",
  95                                                |db| if let (Some(snip_a), Some(snip_r)) =
  96                                                    (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span)) {
  97                                                    db.span_suggestion(expr.span,
  98                                                                       "replace it with",
  99                                                                       format!("{} {}= {}",
 100                                                                               snip_a,
 101                                                                               op.node.as_str(),
 102                                                                               snip_r));
 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                     #[allow(cyclomatic_complexity)]
 119                     let lint = |assignee: &hir::Expr, rhs: &hir::Expr| {
 120                         let ty = cx.tables.expr_ty(assignee);
 121                         let rty = cx.tables.expr_ty(rhs);
 122                         macro_rules! ops {
 123                             ($op:expr,
 124                              $cx:expr,
 125                              $ty:expr,
 126                              $rty:expr,
 127                              $($trait_name:ident:$full_trait_name:ident),+) => {
 128                                 match $op {
 129                                     $(hir::$full_trait_name => {
 130                                         let [krate, module] = ::utils::paths::OPS_MODULE;
 131                                         let path = [krate, module, concat!(stringify!($trait_name), "Assign")];
 132                                         let trait_id = if let Some(trait_id) = get_trait_def_id($cx, &path) {
 133                                             trait_id
 134                                         } else {
 135                                             return; // useless if the trait doesn't exist
 136                                         };
 137                                         // check that we are not inside an `impl AssignOp` of this exact operation
 138                                         let parent_fn = cx.tcx.hir.get_parent(e.id);
 139                                         let parent_impl = cx.tcx.hir.get_parent(parent_fn);
 140                                         // the crate node is the only one that is not in the map
 141                                         if_let_chain!{[
 142                                             parent_impl != ast::CRATE_NODE_ID,
 143                                             let hir::map::Node::NodeItem(item) = cx.tcx.hir.get(parent_impl),
 144                                             let hir::Item_::ItemImpl(_, _, _, _, Some(ref trait_ref), _, _) = item.node,
 145                                             trait_ref.path.def.def_id() == trait_id
 146                                         ], { return; }}
 147                                         implements_trait($cx, $ty, trait_id, &[$rty])
 148                                     },)*
 149                                     _ => false,
 150                                 }
 151                             }
 152                         }
 153                         if ops!(
 154                             op.node,
 155                             cx,
 156                             ty,
 157                             rty,
 158                             Add: BiAdd,
 159                             Sub: BiSub,
 160                             Mul: BiMul,
 161                             Div: BiDiv,
 162                             Rem: BiRem,
 163                             And: BiAnd,
 164                             Or: BiOr,
 165                             BitAnd: BiBitAnd,
 166                             BitOr: BiBitOr,
 167                             BitXor: BiBitXor,
 168                             Shr: BiShr,
 169                             Shl: BiShl
 170                         )
 171                         {
 172                             span_lint_and_then(
 173                                 cx,
 174                                 ASSIGN_OP_PATTERN,
 175                                 expr.span,
 176                                 "manual implementation of an assign operation",
 177                                 |db| if let (Some(snip_a), Some(snip_r)) =
 178                                     (snippet_opt(cx, assignee.span), snippet_opt(cx, rhs.span))
 179                                 {
 180                                     db.span_suggestion(
 181                                         expr.span,
 182                                         "replace it with",
 183                                         format!("{} {}= {}", snip_a, op.node.as_str(), snip_r),
 184                                     );
 185                                 },
 186                             );
 187                         }
 188                     };
 189                     // a = a op b
 190                     if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, l) {
 191                         lint(assignee, r);
 192                     }
 193                     // a = b commutative_op a
 194                     if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, r) {
 195                         match op.node {
 196                             hir::BiAdd | hir::BiMul | hir::BiAnd | hir::BiOr | hir::BiBitXor | hir::BiBitAnd |
 197                             hir::BiBitOr => {
 198                                 lint(assignee, l);
 199                             },
 200                             _ => {},
 201                         }
 202                     }
 203                 }
 204             },
 205             _ => {},
 206         }
 207     }
 208 }
 209
 210 fn is_commutative(op: hir::BinOp_) -> bool {
 211     use rustc::hir::BinOp_::*;
 212     match op {
 213         BiAdd | BiMul | BiAnd | BiOr | BiBitXor | BiBitAnd | BiBitOr | BiEq | BiNe => true,
 214         BiSub | BiDiv | BiRem | BiShl | BiShr | BiLt | BiLe | BiGe | BiGt => false,
 215     }
 216 }