clippy_lints/src/assign_ops.rs

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