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