Skip to content

Commit fb09156

Browse files
committed
Added check for next_multiple_of in manual_div_ceil
1 parent 9e3e964 commit fb09156

File tree

8 files changed

+229
-51
lines changed

8 files changed

+229
-51
lines changed

clippy_lints/src/operators/manual_div_ceil.rs

Lines changed: 74 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::msrvs::{self, Msrv};
3+
use clippy_utils::res::{MaybeDef, MaybeQPath};
34
use clippy_utils::sugg::{Sugg, has_enclosing_paren};
45
use clippy_utils::{SpanlessEq, sym};
56
use rustc_ast::{BinOpKind, LitIntType, LitKind, UnOp};
67
use rustc_data_structures::packed::Pu128;
78
use rustc_errors::Applicability;
89
use rustc_hir::{Expr, ExprKind};
910
use rustc_lint::LateContext;
10-
use rustc_middle::ty::{self};
11+
use rustc_middle::ty::{self, Ty};
1112
use rustc_span::source_map::Spanned;
1213

1314
use super::MANUAL_DIV_CEIL;
@@ -16,59 +17,84 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, op: BinOpKind, lhs: &
1617
let mut applicability = Applicability::MachineApplicable;
1718

1819
if op == BinOpKind::Div
19-
&& check_int_ty_and_feature(cx, lhs)
20-
&& check_int_ty_and_feature(cx, rhs)
21-
&& let ExprKind::Binary(inner_op, inner_lhs, inner_rhs) = lhs.kind
20+
&& check_int_ty_and_feature(cx, cx.typeck_results().expr_ty(lhs))
21+
&& check_int_ty_and_feature(cx, cx.typeck_results().expr_ty(rhs))
2222
&& msrv.meets(cx, msrvs::DIV_CEIL)
2323
{
24-
// (x + (y - 1)) / y
25-
if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = inner_rhs.kind
26-
&& inner_op.node == BinOpKind::Add
27-
&& sub_op.node == BinOpKind::Sub
28-
&& check_literal(sub_rhs)
29-
&& check_eq_expr(cx, sub_lhs, rhs)
30-
{
31-
build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability);
32-
return;
33-
}
24+
match lhs.kind {
25+
ExprKind::Binary(inner_op, inner_lhs, inner_rhs) => {
26+
// (x + (y - 1)) / y
27+
if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = inner_rhs.kind
28+
&& inner_op.node == BinOpKind::Add
29+
&& sub_op.node == BinOpKind::Sub
30+
&& check_literal(sub_rhs)
31+
&& check_eq_expr(cx, sub_lhs, rhs)
32+
{
33+
build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability);
34+
return;
35+
}
3436

35-
// ((y - 1) + x) / y
36-
if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = inner_lhs.kind
37-
&& inner_op.node == BinOpKind::Add
38-
&& sub_op.node == BinOpKind::Sub
39-
&& check_literal(sub_rhs)
40-
&& check_eq_expr(cx, sub_lhs, rhs)
41-
{
42-
build_suggestion(cx, expr, inner_rhs, rhs, &mut applicability);
43-
return;
44-
}
37+
// ((y - 1) + x) / y
38+
if let ExprKind::Binary(sub_op, sub_lhs, sub_rhs) = inner_lhs.kind
39+
&& inner_op.node == BinOpKind::Add
40+
&& sub_op.node == BinOpKind::Sub
41+
&& check_literal(sub_rhs)
42+
&& check_eq_expr(cx, sub_lhs, rhs)
43+
{
44+
build_suggestion(cx, expr, inner_rhs, rhs, &mut applicability);
45+
return;
46+
}
4547

46-
// (x + y - 1) / y
47-
if let ExprKind::Binary(add_op, add_lhs, add_rhs) = inner_lhs.kind
48-
&& inner_op.node == BinOpKind::Sub
49-
&& add_op.node == BinOpKind::Add
50-
&& check_literal(inner_rhs)
51-
&& check_eq_expr(cx, add_rhs, rhs)
52-
{
53-
build_suggestion(cx, expr, add_lhs, rhs, &mut applicability);
54-
}
48+
// (x + y - 1) / y
49+
if let ExprKind::Binary(add_op, add_lhs, add_rhs) = inner_lhs.kind
50+
&& inner_op.node == BinOpKind::Sub
51+
&& add_op.node == BinOpKind::Add
52+
&& check_literal(inner_rhs)
53+
&& check_eq_expr(cx, add_rhs, rhs)
54+
{
55+
build_suggestion(cx, expr, add_lhs, rhs, &mut applicability);
56+
}
5557

56-
// (x + (Y - 1)) / Y
57-
if inner_op.node == BinOpKind::Add && differ_by_one(inner_rhs, rhs) {
58-
build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability);
59-
}
58+
// (x + (Y - 1)) / Y
59+
if inner_op.node == BinOpKind::Add && differ_by_one(inner_rhs, rhs) {
60+
build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability);
61+
}
6062

61-
// ((Y - 1) + x) / Y
62-
if inner_op.node == BinOpKind::Add && differ_by_one(inner_lhs, rhs) {
63-
build_suggestion(cx, expr, inner_rhs, rhs, &mut applicability);
64-
}
63+
// ((Y - 1) + x) / Y
64+
if inner_op.node == BinOpKind::Add && differ_by_one(inner_lhs, rhs) {
65+
build_suggestion(cx, expr, inner_rhs, rhs, &mut applicability);
66+
}
6567

66-
// (x - (-Y - 1)) / Y
67-
if inner_op.node == BinOpKind::Sub
68-
&& let ExprKind::Unary(UnOp::Neg, abs_div_rhs) = rhs.kind
69-
&& differ_by_one(abs_div_rhs, inner_rhs)
70-
{
71-
build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability);
68+
// (x - (-Y - 1)) / Y
69+
if inner_op.node == BinOpKind::Sub
70+
&& let ExprKind::Unary(UnOp::Neg, abs_div_rhs) = rhs.kind
71+
&& differ_by_one(abs_div_rhs, inner_rhs)
72+
{
73+
build_suggestion(cx, expr, inner_lhs, rhs, &mut applicability);
74+
}
75+
},
76+
ExprKind::MethodCall(method, receiver, [next_multiple_of_arg], _) => {
77+
// x.next_multiple_of(Y) / Y
78+
if method.ident.name == sym::next_multiple_of
79+
&& check_int_ty_and_feature(cx, cx.typeck_results().expr_ty(receiver))
80+
&& check_eq_expr(cx, next_multiple_of_arg, rhs)
81+
{
82+
build_suggestion(cx, expr, receiver, rhs, &mut applicability);
83+
}
84+
},
85+
ExprKind::Call(callee, [receiver, next_multiple_of_arg]) => {
86+
// int_type::next_multiple_of(x, Y)
87+
if let Some(impl_ty_binder) = callee
88+
.ty_rel_def_if_named(cx, sym::next_multiple_of)
89+
.assoc_fn_parent(cx)
90+
.opt_impl_ty(cx)
91+
&& check_int_ty_and_feature(cx, impl_ty_binder.skip_binder())
92+
&& check_eq_expr(cx, next_multiple_of_arg, rhs)
93+
{
94+
build_suggestion(cx, expr, receiver, rhs, &mut applicability);
95+
}
96+
},
97+
_ => (),
7298
}
7399
}
74100
}
@@ -91,8 +117,7 @@ fn differ_by_one(small_expr: &Expr<'_>, large_expr: &Expr<'_>) -> bool {
91117
}
92118
}
93119

94-
fn check_int_ty_and_feature(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
95-
let expr_ty = cx.typeck_results().expr_ty(expr);
120+
fn check_int_ty_and_feature(cx: &LateContext<'_>, expr_ty: Ty<'_>) -> bool {
96121
match expr_ty.peel_refs().kind() {
97122
ty::Uint(_) => true,
98123
ty::Int(_) => cx.tcx.features().enabled(sym::int_roundings),

clippy_utils/src/sym.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ generate! {
244244
next_back,
245245
next_if,
246246
next_if_eq,
247+
next_multiple_of,
247248
next_tuple,
248249
nth,
249250
ok,

tests/ui/manual_div_ceil.fixed

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,32 @@ fn issue_15705(size: u64, c: &u64) {
105105
let _ = size.div_ceil(*c);
106106
//~^ manual_div_ceil
107107
}
108+
109+
struct MyStruct(u32);
110+
impl MyStruct {
111+
// Method matching name on different type should not trigger lint
112+
fn next_multiple_of(self, y: u32) -> u32 {
113+
self.0.next_multiple_of(y)
114+
}
115+
}
116+
117+
fn issue_16219() {
118+
let x = 33u32;
119+
120+
// Lint.
121+
let _ = x.div_ceil(8);
122+
//~^ manual_div_ceil
123+
let _ = x.div_ceil(8);
124+
//~^ manual_div_ceil
125+
126+
let y = &x;
127+
let _ = y.div_ceil(8);
128+
//~^ manual_div_ceil
129+
130+
// No lint.
131+
let _ = x.next_multiple_of(8) / 7;
132+
let _ = x.next_multiple_of(7) / 8;
133+
134+
let z = MyStruct(x);
135+
let _ = z.next_multiple_of(8) / 8;
136+
}

tests/ui/manual_div_ceil.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,3 +105,32 @@ fn issue_15705(size: u64, c: &u64) {
105105
let _ = (size + c - 1) / c;
106106
//~^ manual_div_ceil
107107
}
108+
109+
struct MyStruct(u32);
110+
impl MyStruct {
111+
// Method matching name on different type should not trigger lint
112+
fn next_multiple_of(self, y: u32) -> u32 {
113+
self.0.next_multiple_of(y)
114+
}
115+
}
116+
117+
fn issue_16219() {
118+
let x = 33u32;
119+
120+
// Lint.
121+
let _ = x.next_multiple_of(8) / 8;
122+
//~^ manual_div_ceil
123+
let _ = u32::next_multiple_of(x, 8) / 8;
124+
//~^ manual_div_ceil
125+
126+
let y = &x;
127+
let _ = y.next_multiple_of(8) / 8;
128+
//~^ manual_div_ceil
129+
130+
// No lint.
131+
let _ = x.next_multiple_of(8) / 7;
132+
let _ = x.next_multiple_of(7) / 8;
133+
134+
let z = MyStruct(x);
135+
let _ = z.next_multiple_of(8) / 8;
136+
}

tests/ui/manual_div_ceil.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,5 +131,23 @@ error: manually reimplementing `div_ceil`
131131
LL | let _ = (size + c - 1) / c;
132132
| ^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `size.div_ceil(*c)`
133133

134-
error: aborting due to 20 previous errors
134+
error: manually reimplementing `div_ceil`
135+
--> tests/ui/manual_div_ceil.rs:121:13
136+
|
137+
LL | let _ = x.next_multiple_of(8) / 8;
138+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`
139+
140+
error: manually reimplementing `div_ceil`
141+
--> tests/ui/manual_div_ceil.rs:123:13
142+
|
143+
LL | let _ = u32::next_multiple_of(x, 8) / 8;
144+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`
145+
146+
error: manually reimplementing `div_ceil`
147+
--> tests/ui/manual_div_ceil.rs:127:13
148+
|
149+
LL | let _ = y.next_multiple_of(8) / 8;
150+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `y.div_ceil(8)`
151+
152+
error: aborting due to 23 previous errors
135153

tests/ui/manual_div_ceil_with_feature.fixed

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,32 @@ fn issue_13950() {
8484
let _ = y.div_ceil(-7);
8585
//~^ manual_div_ceil
8686
}
87+
88+
struct MyStruct(i32);
89+
impl MyStruct {
90+
// Method matching name on different type should not trigger lint
91+
fn next_multiple_of(self, y: i32) -> i32 {
92+
self.0.next_multiple_of(y)
93+
}
94+
}
95+
96+
fn issue_16219() {
97+
let x = 33i32;
98+
99+
// Lint.
100+
let _ = x.div_ceil(8);
101+
//~^ manual_div_ceil
102+
let _ = x.div_ceil(8);
103+
//~^ manual_div_ceil
104+
105+
let y = &x;
106+
let _ = y.div_ceil(8);
107+
//~^ manual_div_ceil
108+
109+
// No lint.
110+
let _ = x.next_multiple_of(8) / 7;
111+
let _ = x.next_multiple_of(7) / 8;
112+
113+
let z = MyStruct(x);
114+
let _ = z.next_multiple_of(8) / 8;
115+
}

tests/ui/manual_div_ceil_with_feature.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,3 +84,32 @@ fn issue_13950() {
8484
let _ = (y - 8) / -7;
8585
//~^ manual_div_ceil
8686
}
87+
88+
struct MyStruct(i32);
89+
impl MyStruct {
90+
// Method matching name on different type should not trigger lint
91+
fn next_multiple_of(self, y: i32) -> i32 {
92+
self.0.next_multiple_of(y)
93+
}
94+
}
95+
96+
fn issue_16219() {
97+
let x = 33i32;
98+
99+
// Lint.
100+
let _ = x.next_multiple_of(8) / 8;
101+
//~^ manual_div_ceil
102+
let _ = i32::next_multiple_of(x, 8) / 8;
103+
//~^ manual_div_ceil
104+
105+
let y = &x;
106+
let _ = y.next_multiple_of(8) / 8;
107+
//~^ manual_div_ceil
108+
109+
// No lint.
110+
let _ = x.next_multiple_of(8) / 7;
111+
let _ = x.next_multiple_of(7) / 8;
112+
113+
let z = MyStruct(x);
114+
let _ = z.next_multiple_of(8) / 8;
115+
}

tests/ui/manual_div_ceil_with_feature.stderr

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,5 +139,23 @@ error: manually reimplementing `div_ceil`
139139
LL | let _ = (y - 8) / -7;
140140
| ^^^^^^^^^^^^ help: consider using `.div_ceil()`: `y.div_ceil(-7)`
141141

142-
error: aborting due to 23 previous errors
142+
error: manually reimplementing `div_ceil`
143+
--> tests/ui/manual_div_ceil_with_feature.rs:100:13
144+
|
145+
LL | let _ = x.next_multiple_of(8) / 8;
146+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`
147+
148+
error: manually reimplementing `div_ceil`
149+
--> tests/ui/manual_div_ceil_with_feature.rs:102:13
150+
|
151+
LL | let _ = i32::next_multiple_of(x, 8) / 8;
152+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `x.div_ceil(8)`
153+
154+
error: manually reimplementing `div_ceil`
155+
--> tests/ui/manual_div_ceil_with_feature.rs:106:13
156+
|
157+
LL | let _ = y.next_multiple_of(8) / 8;
158+
| ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using `.div_ceil()`: `y.div_ceil(8)`
159+
160+
error: aborting due to 26 previous errors
143161

0 commit comments

Comments
 (0)