]> git.lizzy.rs Git - rust.git/blob - clippy_lints/src/suspicious_trait_impl.rs
Auto merge of #3646 - matthiaskrgr:travis, r=phansch
[rust.git] / clippy_lints / src / suspicious_trait_impl.rs
1 use crate::utils::{get_trait_def_id, span_lint};
2 use if_chain::if_chain;
3 use rustc::hir;
4 use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
5 use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
6 use rustc::{declare_tool_lint, lint_array};
7 use syntax::ast;
8
9 /// **What it does:** Lints for suspicious operations in impls of arithmetic operators, e.g.
10 /// subtracting elements in an Add impl.
11 ///
12 /// **Why this is bad?** This is probably a typo or copy-and-paste error and not intended.
13 ///
14 /// **Known problems:** None.
15 ///
16 /// **Example:**
17 /// ```rust
18 /// impl Add for Foo {
19 ///     type Output = Foo;
20 ///
21 ///     fn add(self, other: Foo) -> Foo {
22 ///         Foo(self.0 - other.0)
23 ///     }
24 /// }
25 /// ```
26 declare_clippy_lint! {
27     pub SUSPICIOUS_ARITHMETIC_IMPL,
28     correctness,
29     "suspicious use of operators in impl of arithmetic trait"
30 }
31
32 /// **What it does:** Lints for suspicious operations in impls of OpAssign, e.g.
33 /// subtracting elements in an AddAssign impl.
34 ///
35 /// **Why this is bad?** This is probably a typo or copy-and-paste error and not intended.
36 ///
37 /// **Known problems:** None.
38 ///
39 /// **Example:**
40 /// ```rust
41 /// impl AddAssign for Foo {
42 ///     fn add_assign(&mut self, other: Foo) {
43 ///         *self = *self - other;
44 ///     }
45 /// }
46 /// ```
47 declare_clippy_lint! {
48     pub SUSPICIOUS_OP_ASSIGN_IMPL,
49     correctness,
50     "suspicious use of operators in impl of OpAssign trait"
51 }
52
53 #[derive(Copy, Clone)]
54 pub struct SuspiciousImpl;
55
56 impl LintPass for SuspiciousImpl {
57     fn get_lints(&self) -> LintArray {
58         lint_array![SUSPICIOUS_ARITHMETIC_IMPL, SUSPICIOUS_OP_ASSIGN_IMPL]
59     }
60 }
61
62 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for SuspiciousImpl {
63     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
64         if let hir::ExprKind::Binary(binop, _, _) = expr.node {
65             match binop.node {
66                 hir::BinOpKind::Eq
67                 | hir::BinOpKind::Lt
68                 | hir::BinOpKind::Le
69                 | hir::BinOpKind::Ne
70                 | hir::BinOpKind::Ge
71                 | hir::BinOpKind::Gt => return,
72                 _ => {},
73             }
74             // Check if the binary expression is part of another bi/unary expression
75             // as a child node
76             let mut parent_expr = cx.tcx.hir().get_parent_node(expr.id);
77             while parent_expr != ast::CRATE_NODE_ID {
78                 if let hir::Node::Expr(e) = cx.tcx.hir().get(parent_expr) {
79                     match e.node {
80                         hir::ExprKind::Binary(..)
81                         | hir::ExprKind::Unary(hir::UnOp::UnNot, _)
82                         | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) => return,
83                         _ => {},
84                     }
85                 }
86                 parent_expr = cx.tcx.hir().get_parent_node(parent_expr);
87             }
88             // as a parent node
89             let mut visitor = BinaryExprVisitor { in_binary_expr: false };
90             walk_expr(&mut visitor, expr);
91
92             if visitor.in_binary_expr {
93                 return;
94             }
95
96             if let Some(impl_trait) = check_binop(
97                 cx,
98                 expr,
99                 binop.node,
100                 &["Add", "Sub", "Mul", "Div"],
101                 &[
102                     hir::BinOpKind::Add,
103                     hir::BinOpKind::Sub,
104                     hir::BinOpKind::Mul,
105                     hir::BinOpKind::Div,
106                 ],
107             ) {
108                 span_lint(
109                     cx,
110                     SUSPICIOUS_ARITHMETIC_IMPL,
111                     binop.span,
112                     &format!(r#"Suspicious use of binary operator in `{}` impl"#, impl_trait),
113                 );
114             }
115
116             if let Some(impl_trait) = check_binop(
117                 cx,
118                 expr,
119                 binop.node,
120                 &[
121                     "AddAssign",
122                     "SubAssign",
123                     "MulAssign",
124                     "DivAssign",
125                     "BitAndAssign",
126                     "BitOrAssign",
127                     "BitXorAssign",
128                     "RemAssign",
129                     "ShlAssign",
130                     "ShrAssign",
131                 ],
132                 &[
133                     hir::BinOpKind::Add,
134                     hir::BinOpKind::Sub,
135                     hir::BinOpKind::Mul,
136                     hir::BinOpKind::Div,
137                     hir::BinOpKind::BitAnd,
138                     hir::BinOpKind::BitOr,
139                     hir::BinOpKind::BitXor,
140                     hir::BinOpKind::Rem,
141                     hir::BinOpKind::Shl,
142                     hir::BinOpKind::Shr,
143                 ],
144             ) {
145                 span_lint(
146                     cx,
147                     SUSPICIOUS_OP_ASSIGN_IMPL,
148                     binop.span,
149                     &format!(r#"Suspicious use of binary operator in `{}` impl"#, impl_trait),
150                 );
151             }
152         }
153     }
154 }
155
156 fn check_binop<'a>(
157     cx: &LateContext<'_, '_>,
158     expr: &hir::Expr,
159     binop: hir::BinOpKind,
160     traits: &[&'a str],
161     expected_ops: &[hir::BinOpKind],
162 ) -> Option<&'a str> {
163     let mut trait_ids = vec![];
164     let [krate, module] = crate::utils::paths::OPS_MODULE;
165
166     for t in traits {
167         let path = [krate, module, t];
168         if let Some(trait_id) = get_trait_def_id(cx, &path) {
169             trait_ids.push(trait_id);
170         } else {
171             return None;
172         }
173     }
174
175     // Get the actually implemented trait
176     let parent_fn = cx.tcx.hir().get_parent(expr.id);
177     let parent_impl = cx.tcx.hir().get_parent(parent_fn);
178
179     if_chain! {
180         if parent_impl != ast::CRATE_NODE_ID;
181         if let hir::Node::Item(item) = cx.tcx.hir().get(parent_impl);
182         if let hir::ItemKind::Impl(_, _, _, _, Some(ref trait_ref), _, _) = item.node;
183         if let Some(idx) = trait_ids.iter().position(|&tid| tid == trait_ref.path.def.def_id());
184         if binop != expected_ops[idx];
185         then{
186             return Some(traits[idx])
187         }
188     }
189
190     None
191 }
192
193 struct BinaryExprVisitor {
194     in_binary_expr: bool,
195 }
196
197 impl<'a, 'tcx: 'a> Visitor<'tcx> for BinaryExprVisitor {
198     fn visit_expr(&mut self, expr: &'tcx hir::Expr) {
199         match expr.node {
200             hir::ExprKind::Binary(..)
201             | hir::ExprKind::Unary(hir::UnOp::UnNot, _)
202             | hir::ExprKind::Unary(hir::UnOp::UnNeg, _) => self.in_binary_expr = true,
203             _ => {},
204         }
205
206         walk_expr(self, expr);
207     }
208     fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
209         NestedVisitorMap::None
210     }
211 }