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