]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 XL |
1 | use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then}; |
2 | use clippy_utils::source::snippet; | |
3 | use clippy_utils::ty::{implements_trait, is_copy}; | |
3c0e092e XL |
4 | use clippy_utils::{ |
5 | ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of, is_in_test_function, | |
6 | }; | |
f20569fa XL |
7 | use if_chain::if_chain; |
8 | use rustc_errors::Applicability; | |
9 | use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind}; | |
10 | use rustc_lint::{LateContext, LateLintPass}; | |
11 | use rustc_session::{declare_lint_pass, declare_tool_lint}; | |
12 | ||
13 | declare_clippy_lint! { | |
94222f64 XL |
14 | /// ### What it does |
15 | /// Checks for equal operands to comparison, logical and | |
f20569fa XL |
16 | /// bitwise, difference and division binary operators (`==`, `>`, etc., `&&`, |
17 | /// `||`, `&`, `|`, `^`, `-` and `/`). | |
18 | /// | |
94222f64 XL |
19 | /// ### Why is this bad? |
20 | /// This is usually just a typo or a copy and paste error. | |
f20569fa | 21 | /// |
94222f64 XL |
22 | /// ### Known problems |
23 | /// False negatives: We had some false positives regarding | |
f20569fa XL |
24 | /// calls (notably [racer](https://github.com/phildawes/racer) had one instance |
25 | /// of `x.pop() && x.pop()`), so we removed matching any function or method | |
26 | /// calls. We may introduce a list of known pure functions in the future. | |
27 | /// | |
94222f64 | 28 | /// ### Example |
f20569fa XL |
29 | /// ```rust |
30 | /// # let x = 1; | |
31 | /// if x + 1 == x + 1 {} | |
32 | /// ``` | |
33 | /// or | |
34 | /// ```rust | |
35 | /// # let a = 3; | |
36 | /// # let b = 4; | |
37 | /// assert_eq!(a, a); | |
38 | /// ``` | |
39 | pub EQ_OP, | |
40 | correctness, | |
41 | "equal operands on both sides of a comparison or bitwise combination (e.g., `x == x`)" | |
42 | } | |
43 | ||
44 | declare_clippy_lint! { | |
94222f64 XL |
45 | /// ### What it does |
46 | /// Checks for arguments to `==` which have their address | |
f20569fa XL |
47 | /// taken to satisfy a bound |
48 | /// and suggests to dereference the other argument instead | |
49 | /// | |
94222f64 XL |
50 | /// ### Why is this bad? |
51 | /// It is more idiomatic to dereference the other argument. | |
f20569fa | 52 | /// |
94222f64 XL |
53 | /// ### Known problems |
54 | /// None | |
f20569fa | 55 | /// |
94222f64 | 56 | /// ### Example |
f20569fa XL |
57 | /// ```ignore |
58 | /// // Bad | |
59 | /// &x == y | |
60 | /// | |
61 | /// // Good | |
62 | /// x == *y | |
63 | /// ``` | |
64 | pub OP_REF, | |
65 | style, | |
66 | "taking a reference to satisfy the type constraints on `==`" | |
67 | } | |
68 | ||
69 | declare_lint_pass!(EqOp => [EQ_OP, OP_REF]); | |
70 | ||
71 | const ASSERT_MACRO_NAMES: [&str; 4] = ["assert_eq", "assert_ne", "debug_assert_eq", "debug_assert_ne"]; | |
72 | ||
73 | impl<'tcx> LateLintPass<'tcx> for EqOp { | |
74 | #[allow(clippy::similar_names, clippy::too_many_lines)] | |
75 | fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) { | |
cdc7bbd5 | 76 | if let ExprKind::Block(block, _) = e.kind { |
f20569fa XL |
77 | for stmt in block.stmts { |
78 | for amn in &ASSERT_MACRO_NAMES { | |
79 | if_chain! { | |
80 | if is_expn_of(stmt.span, amn).is_some(); | |
cdc7bbd5 | 81 | if let StmtKind::Semi(matchexpr) = stmt.kind; |
f20569fa XL |
82 | if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr); |
83 | if macro_args.len() == 2; | |
84 | let (lhs, rhs) = (macro_args[0], macro_args[1]); | |
85 | if eq_expr_value(cx, lhs, rhs); | |
3c0e092e | 86 | if !is_in_test_function(cx.tcx, e.hir_id); |
f20569fa XL |
87 | then { |
88 | span_lint( | |
89 | cx, | |
90 | EQ_OP, | |
91 | lhs.span.to(rhs.span), | |
92 | &format!("identical args used in this `{}!` macro call", amn), | |
93 | ); | |
94 | } | |
95 | } | |
96 | } | |
97 | } | |
98 | } | |
cdc7bbd5 | 99 | if let ExprKind::Binary(op, left, right) = e.kind { |
f20569fa XL |
100 | if e.span.from_expansion() { |
101 | return; | |
102 | } | |
103 | let macro_with_not_op = |expr_kind: &ExprKind<'_>| { | |
cdc7bbd5 | 104 | if let ExprKind::Unary(_, expr) = *expr_kind { |
f20569fa XL |
105 | in_macro(expr.span) |
106 | } else { | |
107 | false | |
108 | } | |
109 | }; | |
110 | if macro_with_not_op(&left.kind) || macro_with_not_op(&right.kind) { | |
111 | return; | |
112 | } | |
3c0e092e XL |
113 | if is_useless_with_eq_exprs(op.node.into()) |
114 | && eq_expr_value(cx, left, right) | |
115 | && !is_in_test_function(cx.tcx, e.hir_id) | |
116 | { | |
f20569fa XL |
117 | span_lint( |
118 | cx, | |
119 | EQ_OP, | |
120 | e.span, | |
121 | &format!("equal expressions as operands to `{}`", op.node.as_str()), | |
122 | ); | |
123 | return; | |
124 | } | |
125 | let (trait_id, requires_ref) = match op.node { | |
126 | BinOpKind::Add => (cx.tcx.lang_items().add_trait(), false), | |
127 | BinOpKind::Sub => (cx.tcx.lang_items().sub_trait(), false), | |
128 | BinOpKind::Mul => (cx.tcx.lang_items().mul_trait(), false), | |
129 | BinOpKind::Div => (cx.tcx.lang_items().div_trait(), false), | |
130 | BinOpKind::Rem => (cx.tcx.lang_items().rem_trait(), false), | |
131 | // don't lint short circuiting ops | |
132 | BinOpKind::And | BinOpKind::Or => return, | |
133 | BinOpKind::BitXor => (cx.tcx.lang_items().bitxor_trait(), false), | |
134 | BinOpKind::BitAnd => (cx.tcx.lang_items().bitand_trait(), false), | |
135 | BinOpKind::BitOr => (cx.tcx.lang_items().bitor_trait(), false), | |
136 | BinOpKind::Shl => (cx.tcx.lang_items().shl_trait(), false), | |
137 | BinOpKind::Shr => (cx.tcx.lang_items().shr_trait(), false), | |
138 | BinOpKind::Ne | BinOpKind::Eq => (cx.tcx.lang_items().eq_trait(), true), | |
139 | BinOpKind::Lt | BinOpKind::Le | BinOpKind::Ge | BinOpKind::Gt => { | |
140 | (cx.tcx.lang_items().partial_ord_trait(), true) | |
141 | }, | |
142 | }; | |
143 | if let Some(trait_id) = trait_id { | |
144 | #[allow(clippy::match_same_arms)] | |
145 | match (&left.kind, &right.kind) { | |
146 | // do not suggest to dereference literals | |
147 | (&ExprKind::Lit(..), _) | (_, &ExprKind::Lit(..)) => {}, | |
148 | // &foo == &bar | |
cdc7bbd5 | 149 | (&ExprKind::AddrOf(BorrowKind::Ref, _, l), &ExprKind::AddrOf(BorrowKind::Ref, _, r)) => { |
f20569fa XL |
150 | let lty = cx.typeck_results().expr_ty(l); |
151 | let rty = cx.typeck_results().expr_ty(r); | |
152 | let lcpy = is_copy(cx, lty); | |
153 | let rcpy = is_copy(cx, rty); | |
154 | // either operator autorefs or both args are copyable | |
155 | if (requires_ref || (lcpy && rcpy)) && implements_trait(cx, lty, trait_id, &[rty.into()]) { | |
156 | span_lint_and_then( | |
157 | cx, | |
158 | OP_REF, | |
159 | e.span, | |
160 | "needlessly taken reference of both operands", | |
161 | |diag| { | |
162 | let lsnip = snippet(cx, l.span, "...").to_string(); | |
163 | let rsnip = snippet(cx, r.span, "...").to_string(); | |
164 | multispan_sugg( | |
165 | diag, | |
166 | "use the values directly", | |
167 | vec![(left.span, lsnip), (right.span, rsnip)], | |
168 | ); | |
169 | }, | |
17df50a5 | 170 | ); |
f20569fa XL |
171 | } else if lcpy |
172 | && !rcpy | |
173 | && implements_trait(cx, lty, trait_id, &[cx.typeck_results().expr_ty(right).into()]) | |
174 | { | |
175 | span_lint_and_then( | |
176 | cx, | |
177 | OP_REF, | |
178 | e.span, | |
179 | "needlessly taken reference of left operand", | |
180 | |diag| { | |
181 | let lsnip = snippet(cx, l.span, "...").to_string(); | |
182 | diag.span_suggestion( | |
183 | left.span, | |
184 | "use the left value directly", | |
185 | lsnip, | |
186 | Applicability::MaybeIncorrect, // FIXME #2597 | |
187 | ); | |
188 | }, | |
17df50a5 | 189 | ); |
f20569fa XL |
190 | } else if !lcpy |
191 | && rcpy | |
192 | && implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()]) | |
193 | { | |
194 | span_lint_and_then( | |
195 | cx, | |
196 | OP_REF, | |
197 | e.span, | |
198 | "needlessly taken reference of right operand", | |
199 | |diag| { | |
200 | let rsnip = snippet(cx, r.span, "...").to_string(); | |
201 | diag.span_suggestion( | |
202 | right.span, | |
203 | "use the right value directly", | |
204 | rsnip, | |
205 | Applicability::MaybeIncorrect, // FIXME #2597 | |
206 | ); | |
207 | }, | |
17df50a5 | 208 | ); |
f20569fa XL |
209 | } |
210 | }, | |
211 | // &foo == bar | |
cdc7bbd5 | 212 | (&ExprKind::AddrOf(BorrowKind::Ref, _, l), _) => { |
f20569fa XL |
213 | let lty = cx.typeck_results().expr_ty(l); |
214 | let lcpy = is_copy(cx, lty); | |
215 | if (requires_ref || lcpy) | |
216 | && implements_trait(cx, lty, trait_id, &[cx.typeck_results().expr_ty(right).into()]) | |
217 | { | |
218 | span_lint_and_then( | |
219 | cx, | |
220 | OP_REF, | |
221 | e.span, | |
222 | "needlessly taken reference of left operand", | |
223 | |diag| { | |
224 | let lsnip = snippet(cx, l.span, "...").to_string(); | |
225 | diag.span_suggestion( | |
226 | left.span, | |
227 | "use the left value directly", | |
228 | lsnip, | |
229 | Applicability::MaybeIncorrect, // FIXME #2597 | |
230 | ); | |
231 | }, | |
17df50a5 | 232 | ); |
f20569fa XL |
233 | } |
234 | }, | |
235 | // foo == &bar | |
cdc7bbd5 | 236 | (_, &ExprKind::AddrOf(BorrowKind::Ref, _, r)) => { |
f20569fa XL |
237 | let rty = cx.typeck_results().expr_ty(r); |
238 | let rcpy = is_copy(cx, rty); | |
239 | if (requires_ref || rcpy) | |
240 | && implements_trait(cx, cx.typeck_results().expr_ty(left), trait_id, &[rty.into()]) | |
241 | { | |
242 | span_lint_and_then(cx, OP_REF, e.span, "taken reference of right operand", |diag| { | |
243 | let rsnip = snippet(cx, r.span, "...").to_string(); | |
244 | diag.span_suggestion( | |
245 | right.span, | |
246 | "use the right value directly", | |
247 | rsnip, | |
248 | Applicability::MaybeIncorrect, // FIXME #2597 | |
249 | ); | |
17df50a5 | 250 | }); |
f20569fa XL |
251 | } |
252 | }, | |
253 | _ => {}, | |
254 | } | |
255 | } | |
256 | } | |
257 | } | |
258 | } |