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