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