Skip to content

Commit 14c05c9

Browse files
borsbors
committed
Auto merge of #13533 - blyxyas:fix-implicit_saturating_sub, r=dswij
Fix span issue on `implicit_saturating_sub` Fixes #13524 changelog: [`implicit_saturating_sub`]: Fix span issue on else blocks
2 parents 5840f78 + 140a127 commit 14c05c9

6 files changed

+51
-60
lines changed

‎clippy_lints/src/implicit_saturating_sub.rs

Lines changed: 10 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -107,9 +107,7 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
107107
}) = higher::If::hir(expr)
108108
&& let ExprKind::Binary(ref cond_op, cond_left, cond_right) = cond.kind
109109
{
110-
check_manual_check(
111-
cx, expr, cond_op, cond_left, cond_right, if_block, else_block, &self.msrv,
112-
);
110+
check_manual_check(cx, cond_op, cond_left, cond_right, if_block, else_block, &self.msrv);
113111
}
114112
}
115113

@@ -119,7 +117,6 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub {
119117
#[allow(clippy::too_many_arguments)]
120118
fn check_manual_check<'tcx>(
121119
cx: &LateContext<'tcx>,
122-
expr: &Expr<'tcx>,
123120
condition: &BinOp,
124121
left_hand: &Expr<'tcx>,
125122
right_hand: &Expr<'tcx>,
@@ -130,26 +127,12 @@ fn check_manual_check<'tcx>(
130127
let ty = cx.typeck_results().expr_ty(left_hand);
131128
if ty.is_numeric() && !ty.is_signed() {
132129
match condition.node {
133-
BinOpKind::Gt | BinOpKind::Ge => check_gt(
134-
cx,
135-
condition.span,
136-
expr.span,
137-
left_hand,
138-
right_hand,
139-
if_block,
140-
else_block,
141-
msrv,
142-
),
143-
BinOpKind::Lt | BinOpKind::Le => check_gt(
144-
cx,
145-
condition.span,
146-
expr.span,
147-
right_hand,
148-
left_hand,
149-
if_block,
150-
else_block,
151-
msrv,
152-
),
130+
BinOpKind::Gt | BinOpKind::Ge => {
131+
check_gt(cx, condition.span, left_hand, right_hand, if_block, else_block, msrv);
132+
},
133+
BinOpKind::Lt | BinOpKind::Le => {
134+
check_gt(cx, condition.span, right_hand, left_hand, if_block, else_block, msrv);
135+
},
153136
_ => {},
154137
}
155138
}
@@ -159,7 +142,6 @@ fn check_manual_check<'tcx>(
159142
fn check_gt(
160143
cx: &LateContext<'_>,
161144
condition_span: Span,
162-
expr_span: Span,
163145
big_var: &Expr<'_>,
164146
little_var: &Expr<'_>,
165147
if_block: &Expr<'_>,
@@ -169,16 +151,7 @@ fn check_gt(
169151
if let Some(big_var) = Var::new(big_var)
170152
&& let Some(little_var) = Var::new(little_var)
171153
{
172-
check_subtraction(
173-
cx,
174-
condition_span,
175-
expr_span,
176-
big_var,
177-
little_var,
178-
if_block,
179-
else_block,
180-
msrv,
181-
);
154+
check_subtraction(cx, condition_span, big_var, little_var, if_block, else_block, msrv);
182155
}
183156
}
184157

@@ -200,7 +173,6 @@ impl Var {
200173
fn check_subtraction(
201174
cx: &LateContext<'_>,
202175
condition_span: Span,
203-
expr_span: Span,
204176
big_var: Var,
205177
little_var: Var,
206178
if_block: &Expr<'_>,
@@ -217,16 +189,7 @@ fn check_subtraction(
217189
}
218190
// If the subtraction is done in the `else` block, then we need to also revert the two
219191
// variables as it means that the check was reverted too.
220-
check_subtraction(
221-
cx,
222-
condition_span,
223-
expr_span,
224-
little_var,
225-
big_var,
226-
else_block,
227-
if_block,
228-
msrv,
229-
);
192+
check_subtraction(cx, condition_span, little_var, big_var, else_block, if_block, msrv);
230193
return;
231194
}
232195
if is_integer_literal(else_block, 0)
@@ -245,7 +208,7 @@ fn check_subtraction(
245208
span_lint_and_sugg(
246209
cx,
247210
IMPLICIT_SATURATING_SUB,
248-
expr_span,
211+
else_block.span,
249212
"manual arithmetic check found",
250213
"replace it with",
251214
format!("{big_var_snippet}.saturating_sub({little_var_snippet})"),

‎tests/ui/implicit_saturating_sub.fixed

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,3 +222,14 @@ fn main() {
222222
a - b
223223
};
224224
}
225+
226+
// https://github.com/rust-lang/rust-clippy/issues/13524
227+
fn regression_13524(a: usize, b: usize, c: bool) -> usize {
228+
if c {
229+
123
230+
} else if a >= b {
231+
b.saturating_sub(a)
232+
} else {
233+
b - a
234+
}
235+
}

‎tests/ui/implicit_saturating_sub.rs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,3 +268,14 @@ fn main() {
268268
a - b
269269
};
270270
}
271+
272+
// https://github.com/rust-lang/rust-clippy/issues/13524
273+
fn regression_13524(a: usize, b: usize, c: bool) -> usize {
274+
if c {
275+
123
276+
} else if a >= b {
277+
0
278+
} else {
279+
b - a
280+
}
281+
}

‎tests/ui/implicit_saturating_sub.stderr

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,5 +185,11 @@ LL | | i_64 -= 1;
185185
LL | | }
186186
| |_____^ help: try: `i_64 = i_64.saturating_sub(1);`
187187

188-
error: aborting due to 23 previous errors
188+
error: manual arithmetic check found
189+
--> tests/ui/implicit_saturating_sub.rs:277:9
190+
|
191+
LL | 0
192+
| ^ help: replace it with: `b.saturating_sub(a)`
193+
194+
error: aborting due to 24 previous errors
189195

‎tests/ui/manual_arithmetic_check.fixed

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ fn main() {
66
let b = 13u32;
77
let c = 8u32;
88

9-
let result = a.saturating_sub(b);
9+
let result = if a > b { a - b } else { a.saturating_sub(b) };
1010
//~^ ERROR: manual arithmetic check found
11-
let result = a.saturating_sub(b);
11+
let result = if b < a { a - b } else { a.saturating_sub(b) };
1212
//~^ ERROR: manual arithmetic check found
1313

14-
let result = a.saturating_sub(b);
14+
let result = if a < b { a.saturating_sub(b) } else { a - b };
1515
//~^ ERROR: manual arithmetic check found
16-
let result = a.saturating_sub(b);
16+
let result = if b > a { a.saturating_sub(b) } else { a - b };
1717
//~^ ERROR: manual arithmetic check found
1818

1919
// Should not warn!

‎tests/ui/manual_arithmetic_check.stderr

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,29 @@
11
error: manual arithmetic check found
2-
--> tests/ui/manual_arithmetic_check.rs:9:18
2+
--> tests/ui/manual_arithmetic_check.rs:9:44
33
|
44
LL | let result = if a > b { a - b } else { 0 };
5-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
5+
| ^ help: replace it with: `a.saturating_sub(b)`
66
|
77
= note: `-D clippy::implicit-saturating-sub` implied by `-D warnings`
88
= help: to override `-D warnings` add `#[allow(clippy::implicit_saturating_sub)]`
99

1010
error: manual arithmetic check found
11-
--> tests/ui/manual_arithmetic_check.rs:11:18
11+
--> tests/ui/manual_arithmetic_check.rs:11:44
1212
|
1313
LL | let result = if b < a { a - b } else { 0 };
14-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
14+
| ^ help: replace it with: `a.saturating_sub(b)`
1515

1616
error: manual arithmetic check found
17-
--> tests/ui/manual_arithmetic_check.rs:14:18
17+
--> tests/ui/manual_arithmetic_check.rs:14:29
1818
|
1919
LL | let result = if a < b { 0 } else { a - b };
20-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
20+
| ^ help: replace it with: `a.saturating_sub(b)`
2121

2222
error: manual arithmetic check found
23-
--> tests/ui/manual_arithmetic_check.rs:16:18
23+
--> tests/ui/manual_arithmetic_check.rs:16:29
2424
|
2525
LL | let result = if b > a { 0 } else { a - b };
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `a.saturating_sub(b)`
26+
| ^ help: replace it with: `a.saturating_sub(b)`
2727

2828
error: aborting due to 4 previous errors
2929

0 commit comments

Comments
 (0)