Skip to content

Commit dd0fd99

Browse files
pmk21pmk21
committed
Handle symmetric if conditions
1 parent d0f6d54 commit dd0fd99

File tree

3 files changed

+126
-32
lines changed

3 files changed

+126
-32
lines changed

‎clippy_lints/src/implicit_saturating_sub.rs

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,10 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitSaturatingSub {
5252
if let Some((ref cond, ref then, None)) = higher::if_block(&expr);
5353

5454
// Check if the conditional expression is a binary operation
55-
if let ExprKind::Binary(ref op, ref cond_left, ref cond_right) = cond.kind;
55+
if let ExprKind::Binary(ref cond_op, ref cond_left, ref cond_right) = cond.kind;
5656

57-
// Check if the variable in the condition statement is an integer
58-
if cx.tables.expr_ty(cond_left).is_integral();
59-
60-
// Ensure that the binary operator is > or !=
61-
if BinOpKind::Ne == op.node || BinOpKind::Gt == op.node;
57+
// Ensure that the binary operator is >, != and <
58+
if BinOpKind::Ne == cond_op.node || BinOpKind::Gt == cond_op.node || BinOpKind::Lt == cond_op.node;
6259

6360
// Check if the true condition block has only one statement
6461
if let ExprKind::Block(ref block, _) = then.kind;
@@ -70,9 +67,6 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitSaturatingSub {
7067
if BinOpKind::Sub == op1.node;
7168
if let ExprKind::Path(ref assign_path) = target.kind;
7269

73-
// Check if the variable in the condition and assignment statement are the same
74-
if SpanlessEq::new(cx).eq_expr(cond_left, target);
75-
7670
// Extracting out the variable name
7771
if let QPath::Resolved(_, ref ares_path) = assign_path;
7872

@@ -81,10 +75,32 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitSaturatingSub {
8175
if let LitKind::Int(1, _) = lit1.node;
8276

8377
then {
78+
// Handle symmetric conditions in the if statement
79+
let (cond_var, cond_num_val) = if SpanlessEq::new(cx).eq_expr(cond_left, target) {
80+
if BinOpKind::Gt == cond_op.node || BinOpKind::Ne == cond_op.node {
81+
(cond_left, cond_right)
82+
} else {
83+
return;
84+
}
85+
} else if SpanlessEq::new(cx).eq_expr(cond_right, target) {
86+
if BinOpKind::Lt == cond_op.node || BinOpKind::Ne == cond_op.node {
87+
(cond_right, cond_left)
88+
} else {
89+
return;
90+
}
91+
} else {
92+
return;
93+
};
94+
95+
// Check if the variable in the condition statement is an integer
96+
if !cx.tables.expr_ty(cond_var).is_integral() {
97+
return;
98+
}
99+
84100
// Get the variable name
85101
let var_name = ares_path.segments[0].ident.name.as_str();
86102

87-
match cond_right.kind {
103+
match cond_num_val.kind {
88104
ExprKind::Lit(ref cond_lit) => {
89105
// Check if the constant is zero
90106
if let LitKind::Int(0, _) = cond_lit.node {
@@ -97,16 +113,16 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitSaturatingSub {
97113
return;
98114
}
99115
},
100-
ExprKind::Path(ref cond_right_path) => {
101-
if match_qpath(cond_right_path, &["i8", "MIN"]) || match_qpath(cond_right_path, &["i16", "MIN"]) || match_qpath(cond_right_path, &["i32", "MIN"]) || match_qpath(cond_right_path, &["i64", "MIN"]) {
116+
ExprKind::Path(ref cond_num_path) => {
117+
if match_qpath(cond_num_path, &["i8", "MIN"]) || match_qpath(cond_num_path, &["i16", "MIN"]) || match_qpath(cond_num_path, &["i32", "MIN"]) || match_qpath(cond_num_path, &["i64", "MIN"]) {
102118
print_lint_and_sugg(cx, &var_name, expr);
103119
} else {
104120
return;
105121
}
106122
},
107123
ExprKind::Call(ref func, _) => {
108-
if let ExprKind::Path(ref cond_right_path) = func.kind {
109-
if match_qpath(cond_right_path, &["i8", "min_value"]) || match_qpath(cond_right_path, &["i16", "min_value"]) || match_qpath(cond_right_path, &["i32", "min_value"]) || match_qpath(cond_right_path, &["i64", "min_value"]) {
124+
if let ExprKind::Path(ref cond_num_path) = func.kind {
125+
if match_qpath(cond_num_path, &["i8", "min_value"]) || match_qpath(cond_num_path, &["i16", "min_value"]) || match_qpath(cond_num_path, &["i32", "min_value"]) || match_qpath(cond_num_path, &["i64", "min_value"]) {
110126
print_lint_and_sugg(cx, &var_name, expr);
111127
} else {
112128
return;

‎tests/ui/implicit_saturating_sub.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,16 @@ fn main() {
6464
u_64 -= 1;
6565
}
6666

67+
// Lint
68+
if 0 < u_64 {
69+
u_64 -= 1;
70+
}
71+
72+
// Lint
73+
if 0 != u_64 {
74+
u_64 -= 1;
75+
}
76+
6777
// No Lint
6878
if u_64 >= 1 {
6979
u_64 -= 1;
@@ -187,6 +197,26 @@ fn main() {
187197
i_64 -= 1;
188198
}
189199

200+
// Lint
201+
if i64::min_value() != i_64 {
202+
i_64 -= 1;
203+
}
204+
205+
// Lint
206+
if i64::min_value() < i_64 {
207+
i_64 -= 1;
208+
}
209+
210+
// Lint
211+
if i64::MIN != i_64 {
212+
i_64 -= 1;
213+
}
214+
215+
// Lint
216+
if i64::MIN < i_64 {
217+
i_64 -= 1;
218+
}
219+
190220
// No Lint
191221
if i_64 > 0 {
192222
i_64 -= 1;

‎tests/ui/implicit_saturating_sub.stderr

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,140 +41,188 @@ LL | | }
4141
| |_____^ help: try: `u_64 = u_64.saturating_sub(1);`
4242

4343
error: Implicitly performing saturating subtraction
44-
--> $DIR/implicit_saturating_sub.rs:84:5
44+
--> $DIR/implicit_saturating_sub.rs:68:5
45+
|
46+
LL | / if 0 < u_64 {
47+
LL | | u_64 -= 1;
48+
LL | | }
49+
| |_____^ help: try: `u_64 = u_64.saturating_sub(1);`
50+
51+
error: Implicitly performing saturating subtraction
52+
--> $DIR/implicit_saturating_sub.rs:73:5
53+
|
54+
LL | / if 0 != u_64 {
55+
LL | | u_64 -= 1;
56+
LL | | }
57+
| |_____^ help: try: `u_64 = u_64.saturating_sub(1);`
58+
59+
error: Implicitly performing saturating subtraction
60+
--> $DIR/implicit_saturating_sub.rs:94:5
4561
|
4662
LL | / if u_usize > 0 {
4763
LL | | u_usize -= 1;
4864
LL | | }
4965
| |_____^ help: try: `u_usize = u_usize.saturating_sub(1);`
5066

5167
error: Implicitly performing saturating subtraction
52-
--> $DIR/implicit_saturating_sub.rs:96:5
68+
--> $DIR/implicit_saturating_sub.rs:106:5
5369
|
5470
LL | / if i_8 > i8::MIN {
5571
LL | | i_8 -= 1;
5672
LL | | }
5773
| |_____^ help: try: `i_8 = i_8.saturating_sub(1);`
5874

5975
error: Implicitly performing saturating subtraction
60-
--> $DIR/implicit_saturating_sub.rs:101:5
76+
--> $DIR/implicit_saturating_sub.rs:111:5
6177
|
6278
LL | / if i_8 > i8::min_value() {
6379
LL | | i_8 -= 1;
6480
LL | | }
6581
| |_____^ help: try: `i_8 = i_8.saturating_sub(1);`
6682

6783
error: Implicitly performing saturating subtraction
68-
--> $DIR/implicit_saturating_sub.rs:106:5
84+
--> $DIR/implicit_saturating_sub.rs:116:5
6985
|
7086
LL | / if i_8 != i8::MIN {
7187
LL | | i_8 -= 1;
7288
LL | | }
7389
| |_____^ help: try: `i_8 = i_8.saturating_sub(1);`
7490

7591
error: Implicitly performing saturating subtraction
76-
--> $DIR/implicit_saturating_sub.rs:111:5
92+
--> $DIR/implicit_saturating_sub.rs:121:5
7793
|
7894
LL | / if i_8 != i8::min_value() {
7995
LL | | i_8 -= 1;
8096
LL | | }
8197
| |_____^ help: try: `i_8 = i_8.saturating_sub(1);`
8298

8399
error: Implicitly performing saturating subtraction
84-
--> $DIR/implicit_saturating_sub.rs:121:5
100+
--> $DIR/implicit_saturating_sub.rs:131:5
85101
|
86102
LL | / if i_16 > i16::MIN {
87103
LL | | i_16 -= 1;
88104
LL | | }
89105
| |_____^ help: try: `i_16 = i_16.saturating_sub(1);`
90106

91107
error: Implicitly performing saturating subtraction
92-
--> $DIR/implicit_saturating_sub.rs:126:5
108+
--> $DIR/implicit_saturating_sub.rs:136:5
93109
|
94110
LL | / if i_16 > i16::min_value() {
95111
LL | | i_16 -= 1;
96112
LL | | }
97113
| |_____^ help: try: `i_16 = i_16.saturating_sub(1);`
98114

99115
error: Implicitly performing saturating subtraction
100-
--> $DIR/implicit_saturating_sub.rs:131:5
116+
--> $DIR/implicit_saturating_sub.rs:141:5
101117
|
102118
LL | / if i_16 != i16::MIN {
103119
LL | | i_16 -= 1;
104120
LL | | }
105121
| |_____^ help: try: `i_16 = i_16.saturating_sub(1);`
106122

107123
error: Implicitly performing saturating subtraction
108-
--> $DIR/implicit_saturating_sub.rs:136:5
124+
--> $DIR/implicit_saturating_sub.rs:146:5
109125
|
110126
LL | / if i_16 != i16::min_value() {
111127
LL | | i_16 -= 1;
112128
LL | | }
113129
| |_____^ help: try: `i_16 = i_16.saturating_sub(1);`
114130

115131
error: Implicitly performing saturating subtraction
116-
--> $DIR/implicit_saturating_sub.rs:146:5
132+
--> $DIR/implicit_saturating_sub.rs:156:5
117133
|
118134
LL | / if i_32 > i32::MIN {
119135
LL | | i_32 -= 1;
120136
LL | | }
121137
| |_____^ help: try: `i_32 = i_32.saturating_sub(1);`
122138

123139
error: Implicitly performing saturating subtraction
124-
--> $DIR/implicit_saturating_sub.rs:151:5
140+
--> $DIR/implicit_saturating_sub.rs:161:5
125141
|
126142
LL | / if i_32 > i32::min_value() {
127143
LL | | i_32 -= 1;
128144
LL | | }
129145
| |_____^ help: try: `i_32 = i_32.saturating_sub(1);`
130146

131147
error: Implicitly performing saturating subtraction
132-
--> $DIR/implicit_saturating_sub.rs:156:5
148+
--> $DIR/implicit_saturating_sub.rs:166:5
133149
|
134150
LL | / if i_32 != i32::MIN {
135151
LL | | i_32 -= 1;
136152
LL | | }
137153
| |_____^ help: try: `i_32 = i_32.saturating_sub(1);`
138154

139155
error: Implicitly performing saturating subtraction
140-
--> $DIR/implicit_saturating_sub.rs:161:5
156+
--> $DIR/implicit_saturating_sub.rs:171:5
141157
|
142158
LL | / if i_32 != i32::min_value() {
143159
LL | | i_32 -= 1;
144160
LL | | }
145161
| |_____^ help: try: `i_32 = i_32.saturating_sub(1);`
146162

147163
error: Implicitly performing saturating subtraction
148-
--> $DIR/implicit_saturating_sub.rs:171:5
164+
--> $DIR/implicit_saturating_sub.rs:181:5
149165
|
150166
LL | / if i_64 > i64::MIN {
151167
LL | | i_64 -= 1;
152168
LL | | }
153169
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
154170

155171
error: Implicitly performing saturating subtraction
156-
--> $DIR/implicit_saturating_sub.rs:176:5
172+
--> $DIR/implicit_saturating_sub.rs:186:5
157173
|
158174
LL | / if i_64 > i64::min_value() {
159175
LL | | i_64 -= 1;
160176
LL | | }
161177
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
162178

163179
error: Implicitly performing saturating subtraction
164-
--> $DIR/implicit_saturating_sub.rs:181:5
180+
--> $DIR/implicit_saturating_sub.rs:191:5
165181
|
166182
LL | / if i_64 != i64::MIN {
167183
LL | | i_64 -= 1;
168184
LL | | }
169185
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
170186

171187
error: Implicitly performing saturating subtraction
172-
--> $DIR/implicit_saturating_sub.rs:186:5
188+
--> $DIR/implicit_saturating_sub.rs:196:5
173189
|
174190
LL | / if i_64 != i64::min_value() {
175191
LL | | i_64 -= 1;
176192
LL | | }
177193
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
178194

179-
error: aborting due to 22 previous errors
195+
error: Implicitly performing saturating subtraction
196+
--> $DIR/implicit_saturating_sub.rs:201:5
197+
|
198+
LL | / if i64::min_value() != i_64 {
199+
LL | | i_64 -= 1;
200+
LL | | }
201+
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
202+
203+
error: Implicitly performing saturating subtraction
204+
--> $DIR/implicit_saturating_sub.rs:206:5
205+
|
206+
LL | / if i64::min_value() < i_64 {
207+
LL | | i_64 -= 1;
208+
LL | | }
209+
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
210+
211+
error: Implicitly performing saturating subtraction
212+
--> $DIR/implicit_saturating_sub.rs:211:5
213+
|
214+
LL | / if i64::MIN != i_64 {
215+
LL | | i_64 -= 1;
216+
LL | | }
217+
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
218+
219+
error: Implicitly performing saturating subtraction
220+
--> $DIR/implicit_saturating_sub.rs:216:5
221+
|
222+
LL | / if i64::MIN < i_64 {
223+
LL | | i_64 -= 1;
224+
LL | | }
225+
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
226+
227+
error: aborting due to 28 previous errors
180228

0 commit comments

Comments
 (0)