Skip to content

Commit bd421cb

Browse files
flip1995flip1995
committed
Additionally suggest the semantic equal variant
1 parent b7cb075 commit bd421cb

File tree

3 files changed

+96
-20
lines changed

3 files changed

+96
-20
lines changed

‎clippy_lints/src/assign_ops.rs

+56-19
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use rustc::hir;
2+
use rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
23
use rustc::lint::*;
34
use syntax::ast;
45
use utils::{get_trait_def_id, implements_trait, snippet_opt, span_lint_and_then, SpanlessEq};
@@ -98,13 +99,19 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
9899
{
99100
let a = &sugg::Sugg::hir(cx, assignee, "..");
100101
let r = &sugg::Sugg::hir(cx, rhs, "..");
102+
let long = format!("{} = {}", snip_a, sugg::make_binop(higher::binop(op.node), a, r));
101103
db.span_suggestion(
102104
expr.span,
103-
&format!("Did you mean {} = {} {} {} or {} = {}? Consider replacing it with",
105+
&format!("Did you mean {} = {} {} {} or {}? Consider replacing it with",
104106
snip_a, snip_a, op.node.as_str(), snip_r,
105-
snip_a, sugg::make_binop(higher::binop(op.node), a, r)),
107+
long),
106108
format!("{} {}= {}", snip_a, op.node.as_str(), snip_r)
107109
);
110+
db.span_suggestion(
111+
expr.span,
112+
"or",
113+
long
114+
);
108115
},
109116
);
110117
};
@@ -193,23 +200,34 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
193200
);
194201
}
195202
};
196-
// a = a op b
197-
if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, l) {
198-
lint(assignee, r);
199-
}
200-
// a = b commutative_op a
201-
if SpanlessEq::new(cx).ignore_fn().eq_expr(assignee, r) {
202-
match op.node {
203-
hir::BiAdd |
204-
hir::BiMul |
205-
hir::BiAnd |
206-
hir::BiOr |
207-
hir::BiBitXor |
208-
hir::BiBitAnd |
209-
hir::BiBitOr => {
210-
lint(assignee, l);
211-
},
212-
_ => {},
203+
204+
let mut visitor = ExprVisitor {
205+
assignee: assignee,
206+
counter: 0,
207+
cx: 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::BiAdd |
221+
hir::BiMul |
222+
hir::BiAnd |
223+
hir::BiOr |
224+
hir::BiBitXor |
225+
hir::BiBitAnd |
226+
hir::BiBitOr => {
227+
lint(assignee, l);
228+
},
229+
_ => {},
230+
}
213231
}
214232
}
215233
}
@@ -226,3 +244,22 @@ fn is_commutative(op: hir::BinOp_) -> bool {
226244
BiSub | BiDiv | BiRem | BiShl | BiShr | BiLt | BiLe | BiGe | BiGt => false,
227245
}
228246
}
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+
}

‎tests/ui/assign_ops2.rs

+4-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33

44
#[allow(unused_assignments)]
5-
#[warn(misrefactored_assign_op)]
5+
#[warn(misrefactored_assign_op, assign_op_pattern)]
66
fn main() {
77
let mut a = 5;
88
a += a + 1;
@@ -14,6 +14,9 @@ fn main() {
1414
a %= a % 5;
1515
a &= a & 1;
1616
a *= a * a;
17+
a = a * a * a;
18+
a = a * 42 * a;
19+
a = a * 2 + a;
1720
a -= 1 - a;
1821
a /= 5 / a;
1922
a %= 42 % a;

‎tests/ui/assign_ops2.stderr

+36
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,10 @@ help: Did you mean a = a + 1 or a = a + a + 1? Consider replacing it with
99
|
1010
8 | a += 1;
1111
| ^^^^^^
12+
help: or
13+
|
14+
8 | a = a + a + 1;
15+
| ^^^^^^^^^^^^^
1216

1317
error: variable appears on both sides of an assignment operation
1418
--> $DIR/assign_ops2.rs:9:5
@@ -19,6 +23,10 @@ help: Did you mean a = a + 1 or a = a + 1 + a? Consider replacing it with
1923
|
2024
9 | a += 1;
2125
| ^^^^^^
26+
help: or
27+
|
28+
9 | a = a + 1 + a;
29+
| ^^^^^^^^^^^^^
2230

2331
error: variable appears on both sides of an assignment operation
2432
--> $DIR/assign_ops2.rs:10:5
@@ -29,6 +37,10 @@ help: Did you mean a = a - 1 or a = a - (a - 1)? Consider replacing it with
2937
|
3038
10 | a -= 1;
3139
| ^^^^^^
40+
help: or
41+
|
42+
10 | a = a - (a - 1);
43+
| ^^^^^^^^^^^^^^^
3244

3345
error: variable appears on both sides of an assignment operation
3446
--> $DIR/assign_ops2.rs:11:5
@@ -39,6 +51,10 @@ help: Did you mean a = a * 99 or a = a * a * 99? Consider replacing it with
3951
|
4052
11 | a *= 99;
4153
| ^^^^^^^
54+
help: or
55+
|
56+
11 | a = a * a * 99;
57+
| ^^^^^^^^^^^^^^
4258

4359
error: variable appears on both sides of an assignment operation
4460
--> $DIR/assign_ops2.rs:12:5
@@ -49,6 +65,10 @@ help: Did you mean a = a * 42 or a = a * 42 * a? Consider replacing it with
4965
|
5066
12 | a *= 42;
5167
| ^^^^^^^
68+
help: or
69+
|
70+
12 | a = a * 42 * a;
71+
| ^^^^^^^^^^^^^^
5272

5373
error: variable appears on both sides of an assignment operation
5474
--> $DIR/assign_ops2.rs:13:5
@@ -59,6 +79,10 @@ help: Did you mean a = a / 2 or a = a / (a / 2)? Consider replacing it with
5979
|
6080
13 | a /= 2;
6181
| ^^^^^^
82+
help: or
83+
|
84+
13 | a = a / (a / 2);
85+
| ^^^^^^^^^^^^^^^
6286

6387
error: variable appears on both sides of an assignment operation
6488
--> $DIR/assign_ops2.rs:14:5
@@ -69,6 +93,10 @@ help: Did you mean a = a % 5 or a = a % (a % 5)? Consider replacing it with
6993
|
7094
14 | a %= 5;
7195
| ^^^^^^
96+
help: or
97+
|
98+
14 | a = a % (a % 5);
99+
| ^^^^^^^^^^^^^^^
72100

73101
error: variable appears on both sides of an assignment operation
74102
--> $DIR/assign_ops2.rs:15:5
@@ -79,6 +107,10 @@ help: Did you mean a = a & 1 or a = a & a & 1? Consider replacing it with
79107
|
80108
15 | a &= 1;
81109
| ^^^^^^
110+
help: or
111+
|
112+
15 | a = a & a & 1;
113+
| ^^^^^^^^^^^^^
82114

83115
error: variable appears on both sides of an assignment operation
84116
--> $DIR/assign_ops2.rs:16:5
@@ -89,6 +121,10 @@ help: Did you mean a = a * a or a = a * a * a? Consider replacing it with
89121
|
90122
16 | a *= a;
91123
| ^^^^^^
124+
help: or
125+
|
126+
16 | a = a * a * a;
127+
| ^^^^^^^^^^^^^
92128

93129
error: aborting due to 9 previous errors
94130

0 commit comments

Comments
 (0)