Skip to content

Commit 43f1891

Browse files
authored
Fix manual_abs_diff suggests wrongly behind refs (#15265)
Closes #15254 changelog: [`manual_abs_diff`] fix wrong suggestions behind refs
2 parents 6f2567d + 9d96404 commit 43f1891

File tree

4 files changed

+33
-6
lines changed

4 files changed

+33
-6
lines changed

clippy_lints/src/manual_abs_diff.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use clippy_utils::msrvs::{self, Msrv};
55
use clippy_utils::source::HasSession as _;
66
use clippy_utils::sugg::Sugg;
77
use clippy_utils::ty::is_type_diagnostic_item;
8-
use clippy_utils::{eq_expr_value, peel_blocks, span_contains_comment};
8+
use clippy_utils::{eq_expr_value, peel_blocks, peel_middle_ty_refs, span_contains_comment};
99
use rustc_errors::Applicability;
1010
use rustc_hir::{BinOpKind, Expr, ExprKind};
1111
use rustc_lint::{LateContext, LateLintPass};
@@ -62,7 +62,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualAbsDiff {
6262
&& let ExprKind::Binary(op, rhs, lhs) = if_expr.cond.kind
6363
&& let (BinOpKind::Gt | BinOpKind::Ge, mut a, mut b) | (BinOpKind::Lt | BinOpKind::Le, mut b, mut a) =
6464
(op.node, rhs, lhs)
65-
&& let Some(ty) = self.are_ty_eligible(cx, a, b)
65+
&& let Some((ty, b_n_refs)) = self.are_ty_eligible(cx, a, b)
6666
&& is_sub_expr(cx, if_expr.then, a, b, ty)
6767
&& is_sub_expr(cx, r#else, b, a, ty)
6868
{
@@ -86,8 +86,9 @@ impl<'tcx> LateLintPass<'tcx> for ManualAbsDiff {
8686
}
8787
};
8888
let sugg = format!(
89-
"{}.abs_diff({})",
89+
"{}.abs_diff({}{})",
9090
Sugg::hir(cx, a, "..").maybe_paren(),
91+
"*".repeat(b_n_refs),
9192
Sugg::hir(cx, b, "..")
9293
);
9394
diag.span_suggestion(expr.span, "replace with `abs_diff`", sugg, applicability);
@@ -100,13 +101,15 @@ impl<'tcx> LateLintPass<'tcx> for ManualAbsDiff {
100101
impl ManualAbsDiff {
101102
/// Returns a type if `a` and `b` are both of it, and this lint can be applied to that
102103
/// type (currently, any primitive int, or a `Duration`)
103-
fn are_ty_eligible<'tcx>(&self, cx: &LateContext<'tcx>, a: &Expr<'_>, b: &Expr<'_>) -> Option<Ty<'tcx>> {
104+
fn are_ty_eligible<'tcx>(&self, cx: &LateContext<'tcx>, a: &Expr<'_>, b: &Expr<'_>) -> Option<(Ty<'tcx>, usize)> {
104105
let is_int = |ty: Ty<'_>| matches!(ty.kind(), ty::Uint(_) | ty::Int(_)) && self.msrv.meets(cx, msrvs::ABS_DIFF);
105106
let is_duration =
106107
|ty| is_type_diagnostic_item(cx, ty, sym::Duration) && self.msrv.meets(cx, msrvs::DURATION_ABS_DIFF);
107108

108109
let a_ty = cx.typeck_results().expr_ty(a).peel_refs();
109-
(a_ty == cx.typeck_results().expr_ty(b).peel_refs() && (is_int(a_ty) || is_duration(a_ty))).then_some(a_ty)
110+
let (b_ty, b_n_refs) = peel_middle_ty_refs(cx.typeck_results().expr_ty(b));
111+
112+
(a_ty == b_ty && (is_int(a_ty) || is_duration(a_ty))).then_some((a_ty, b_n_refs))
110113
}
111114
}
112115

tests/ui/manual_abs_diff.fixed

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,7 @@ fn non_primitive_ty() {
104104
let (a, b) = (S(10), S(20));
105105
let _ = if a < b { b - a } else { a - b };
106106
}
107+
108+
fn issue15254(a: &usize, b: &usize) -> usize {
109+
b.abs_diff(*a)
110+
}

tests/ui/manual_abs_diff.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,12 @@ fn non_primitive_ty() {
114114
let (a, b) = (S(10), S(20));
115115
let _ = if a < b { b - a } else { a - b };
116116
}
117+
118+
fn issue15254(a: &usize, b: &usize) -> usize {
119+
if a < b {
120+
//~^ manual_abs_diff
121+
b - a
122+
} else {
123+
a - b
124+
}
125+
}

tests/ui/manual_abs_diff.stderr

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,5 +79,16 @@ error: manual absolute difference pattern without using `abs_diff`
7979
LL | let _ = if a > b { (a - b) as u32 } else { (b - a) as u32 };
8080
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with `abs_diff`: `a.abs_diff(b)`
8181

82-
error: aborting due to 11 previous errors
82+
error: manual absolute difference pattern without using `abs_diff`
83+
--> tests/ui/manual_abs_diff.rs:119:5
84+
|
85+
LL | / if a < b {
86+
LL | |
87+
LL | | b - a
88+
LL | | } else {
89+
LL | | a - b
90+
LL | | }
91+
| |_____^ help: replace with `abs_diff`: `b.abs_diff(*a)`
92+
93+
error: aborting due to 12 previous errors
8394

0 commit comments

Comments
 (0)