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