clippy_lints/src/assign_ops.rs

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