clippy_lints/src/assign_ops.rs

   1 use crate::utils::{get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, SpanlessEq};
   2 use crate::utils::{higher, sugg};
   3 use rustc::hir;
   4 use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
   5 use rustc::lint::*;
   6 use syntax::ast;
   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::ExprKind::AssignOp(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::ExprKind::Binary(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::ExprKind::Assign(ref assignee, ref e) => {
 135                 if let hir::ExprKind::Binary(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),+) => {
 146                                 match $op {
 147                                     $(hir::BinOpKind::$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::ItemKind::Impl(_, _, _, _, 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,
 179                             Sub,
 180                             Mul,
 181                             Div,
 182                             Rem,
 183                             And,
 184                             Or,
 185                             BitAnd,
 186                             BitOr,
 187                             BitXor,
 188                             Shr,
 189                             Shl
 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::BinOpKind::Add
 228                                 | hir::BinOpKind::Mul
 229                                 | hir::BinOpKind::And
 230                                 | hir::BinOpKind::Or
 231                                 | hir::BinOpKind::BitXor
 232                                 | hir::BinOpKind::BitAnd
 233                                 | hir::BinOpKind::BitOr => {
 234                                     lint(assignee, l);
 235                                 },
 236                                 _ => {},
 237                             }
 238                         }
 239                     }
 240                 }
 241             },
 242             _ => {},
 243         }
 244     }
 245 }
 246
 247 fn is_commutative(op: hir::BinOpKind) -> bool {
 248     use rustc::hir::BinOpKind::*;
 249     match op {
 250         Add | Mul | And | Or | BitXor | BitAnd | BitOr | Eq | Ne => true,
 251         Sub | Div | Rem | Shl | Shr | Lt | Le | Ge | Gt => 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 }