Skip to content

Commit d43acb8

Browse files
Samarth1696Samarth1696
committed
Added checks for binary expr and added different test cases for unfixable cases
1 parent c6c7408 commit d43acb8

6 files changed

+115
-109
lines changed

‎clippy_lints/src/non_zero_suggestions.rs

Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
22
use clippy_utils::source::snippet;
3+
use rustc_ast::ast::BinOpKind;
34
use rustc_errors::Applicability;
45
use rustc_hir::{Expr, ExprKind};
56
use rustc_lint::{LateContext, LateLintPass};
@@ -48,23 +49,42 @@ declare_lint_pass!(NonZeroSuggestions => [NON_ZERO_SUGGESTIONS]);
4849

4950
impl<'tcx> LateLintPass<'tcx> for NonZeroSuggestions {
5051
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
51-
if let ExprKind::Call(func, [arg]) = expr.kind
52-
&& let ExprKind::Path(qpath) = &func.kind
53-
&& let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()
54-
&& let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind
52+
if let ExprKind::Binary(op, _, rhs) = expr.kind
53+
&& matches!(op.node, BinOpKind::Div | BinOpKind::Rem)
5554
{
56-
let fn_name = cx.tcx.item_name(def_id);
57-
let target_ty = cx.typeck_results().expr_ty(expr);
58-
let receiver_ty = cx.typeck_results().expr_ty(receiver);
55+
check_non_zero_conversion(cx, rhs, Applicability::MachineApplicable);
56+
} else {
57+
// Check if the parent expression is a binary operation
58+
let parent_is_binary = cx.tcx.hir().parent_iter(expr.hir_id).any(|(_, node)| {
59+
matches!(node, rustc_hir::Node::Expr(parent_expr) if matches!(parent_expr.kind, ExprKind::Binary(..)))
60+
});
5961

60-
if let ty::Adt(adt_def, _) = receiver_ty.kind()
61-
&& adt_def.is_struct()
62-
&& cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero)
63-
{
64-
if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) {
65-
let arg_snippet = get_arg_snippet(cx, arg, rcv_path);
66-
suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet);
67-
}
62+
if !parent_is_binary {
63+
check_non_zero_conversion(cx, expr, Applicability::MaybeIncorrect);
64+
}
65+
}
66+
}
67+
}
68+
69+
fn check_non_zero_conversion(cx: &LateContext<'_>, expr: &Expr<'_>, applicability: Applicability) {
70+
// Check if the expression is a function call with one argument
71+
if let ExprKind::Call(func, [arg]) = expr.kind
72+
&& let ExprKind::Path(qpath) = &func.kind
73+
&& let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id()
74+
&& let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind
75+
{
76+
let fn_name = cx.tcx.item_name(def_id);
77+
let target_ty = cx.typeck_results().expr_ty(expr);
78+
let receiver_ty = cx.typeck_results().expr_ty(receiver);
79+
80+
// Check if the receiver type is a NonZero type
81+
if let ty::Adt(adt_def, _) = receiver_ty.kind()
82+
&& adt_def.is_struct()
83+
&& cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero)
84+
{
85+
if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) {
86+
let arg_snippet = get_arg_snippet(cx, arg, rcv_path);
87+
suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet, applicability);
6888
}
6989
}
7090
}
@@ -85,6 +105,7 @@ fn suggest_non_zero_conversion(
85105
fn_name: rustc_span::Symbol,
86106
target_non_zero_type: &str,
87107
arg_snippet: &str,
108+
applicability: Applicability,
88109
) {
89110
let suggestion = format!("{target_non_zero_type}::{fn_name}({arg_snippet})");
90111
span_lint_and_sugg(
@@ -94,7 +115,7 @@ fn suggest_non_zero_conversion(
94115
format!("consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"),
95116
"replace with",
96117
suggestion,
97-
Applicability::MachineApplicable,
118+
applicability,
98119
);
99120
}
100121

‎tests/ui/non_zero_suggestions.fixed

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(clippy::non_zero_suggestions)]
2-
32
use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
43

54
fn main() {
@@ -19,43 +18,33 @@ fn main() {
1918
let r3 = a / NonZeroU32::from(b);
2019
//~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion
2120

22-
// I8 -> I16
23-
let c: i16 = 25;
24-
let d = NonZeroI8::new(3).unwrap();
25-
let r4 = NonZeroI16::from(d);
26-
//~^ ERROR: consider using `NonZeroI16::from()` for more efficient and type-safe conversion
27-
28-
// Different operations
29-
let m: u64 = 400;
30-
let n = NonZeroU32::new(20).unwrap();
31-
let r5 = m / NonZeroU64::from(n);
21+
let x = NonZeroU64::from(NonZeroU32::new(5).unwrap());
3222
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
3323

34-
/// Edge cases
35-
// Using the max value of a type
36-
let max_u32 = NonZeroU32::new(u32::MAX).unwrap();
37-
let r6 = NonZeroU64::from(max_u32);
38-
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
24+
/// Negative test cases (lint should not trigger)
25+
// Left hand side expressions should not be triggered
26+
let c: u32 = 50;
27+
let d = NonZeroU16::new(5).unwrap();
28+
let r4 = u32::from(b.get()) / a;
3929

40-
// Chained method calls
41-
let _ = NonZeroU64::from(NonZeroU32::new(10).unwrap());
42-
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
30+
// Should not trigger for any other operand other than `/` and `%`
31+
let r5 = a + u32::from(b.get());
32+
let r6 = a - u32::from(b.get());
4333

44-
/// Negative test cases (lint should not trigger)
4534
// Same size types
4635
let e: u32 = 200;
4736
let f = NonZeroU32::new(20).unwrap();
48-
let r10 = e / f.get();
37+
let r7 = e / f.get();
4938

5039
// Smaller to larger, but not NonZero
5140
let g: u64 = 1000;
5241
let h: u32 = 50;
53-
let r11 = g / u64::from(h);
42+
let r8 = g / u64::from(h);
5443

5544
// Using From correctly
5645
let k: u64 = 300;
5746
let l = NonZeroU32::new(15).unwrap();
58-
let r12 = k / NonZeroU64::from(l);
47+
let r9 = k / NonZeroU64::from(l);
5948
}
6049

6150
// Additional function to test the lint in a different context
@@ -64,19 +53,13 @@ fn divide_numbers(x: u64, y: NonZeroU32) -> u64 {
6453
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
6554
}
6655

67-
fn no_bin_exp(x: u64, y: NonZeroU32) -> u64 {
68-
NonZeroU64::from(y)
69-
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
70-
}
71-
72-
fn some_fn_that_only_takes_u64(_: u64) {}
73-
7456
struct Calculator {
7557
value: u64,
7658
}
7759

7860
impl Calculator {
7961
fn divide(&self, divisor: NonZeroU32) -> u64 {
8062
self.value / NonZeroU64::from(divisor)
63+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
8164
}
8265
}

‎tests/ui/non_zero_suggestions.rs

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
#![warn(clippy::non_zero_suggestions)]
2-
32
use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
43

54
fn main() {
@@ -19,43 +18,33 @@ fn main() {
1918
let r3 = a / u32::from(b.get());
2019
//~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion
2120

22-
// I8 -> I16
23-
let c: i16 = 25;
24-
let d = NonZeroI8::new(3).unwrap();
25-
let r4 = i16::from(d.get());
26-
//~^ ERROR: consider using `NonZeroI16::from()` for more efficient and type-safe conversion
27-
28-
// Different operations
29-
let m: u64 = 400;
30-
let n = NonZeroU32::new(20).unwrap();
31-
let r5 = m / u64::from(n.get());
21+
let x = u64::from(NonZeroU32::new(5).unwrap().get());
3222
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
3323

34-
/// Edge cases
35-
// Using the max value of a type
36-
let max_u32 = NonZeroU32::new(u32::MAX).unwrap();
37-
let r6 = u64::from(max_u32.get());
38-
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
24+
/// Negative test cases (lint should not trigger)
25+
// Left hand side expressions should not be triggered
26+
let c: u32 = 50;
27+
let d = NonZeroU16::new(5).unwrap();
28+
let r4 = u32::from(b.get()) / a;
3929

40-
// Chained method calls
41-
let _ = u64::from(NonZeroU32::new(10).unwrap().get());
42-
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
30+
// Should not trigger for any other operand other than `/` and `%`
31+
let r5 = a + u32::from(b.get());
32+
let r6 = a - u32::from(b.get());
4333

44-
/// Negative test cases (lint should not trigger)
4534
// Same size types
4635
let e: u32 = 200;
4736
let f = NonZeroU32::new(20).unwrap();
48-
let r10 = e / f.get();
37+
let r7 = e / f.get();
4938

5039
// Smaller to larger, but not NonZero
5140
let g: u64 = 1000;
5241
let h: u32 = 50;
53-
let r11 = g / u64::from(h);
42+
let r8 = g / u64::from(h);
5443

5544
// Using From correctly
5645
let k: u64 = 300;
5746
let l = NonZeroU32::new(15).unwrap();
58-
let r12 = k / NonZeroU64::from(l);
47+
let r9 = k / NonZeroU64::from(l);
5948
}
6049

6150
// Additional function to test the lint in a different context
@@ -64,19 +53,13 @@ fn divide_numbers(x: u64, y: NonZeroU32) -> u64 {
6453
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
6554
}
6655

67-
fn no_bin_exp(x: u64, y: NonZeroU32) -> u64 {
68-
u64::from(y.get())
69-
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
70-
}
71-
72-
fn some_fn_that_only_takes_u64(_: u64) {}
73-
7456
struct Calculator {
7557
value: u64,
7658
}
7759

7860
impl Calculator {
7961
fn divide(&self, divisor: NonZeroU32) -> u64 {
8062
self.value / u64::from(divisor.get())
63+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
8164
}
8265
}

‎tests/ui/non_zero_suggestions.stderr

Lines changed: 9 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
2-
--> tests/ui/non_zero_suggestions.rs:10:18
2+
--> tests/ui/non_zero_suggestions.rs:9:18
33
|
44
LL | let r1 = x / u64::from(y.get());
55
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
@@ -8,58 +8,34 @@ LL | let r1 = x / u64::from(y.get());
88
= help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]`
99

1010
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
11-
--> tests/ui/non_zero_suggestions.rs:13:18
11+
--> tests/ui/non_zero_suggestions.rs:12:18
1212
|
1313
LL | let r2 = x % u64::from(y.get());
1414
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
1515

1616
error: consider using `NonZeroU32::from()` for more efficient and type-safe conversion
17-
--> tests/ui/non_zero_suggestions.rs:19:18
17+
--> tests/ui/non_zero_suggestions.rs:18:18
1818
|
1919
LL | let r3 = a / u32::from(b.get());
2020
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU32::from(b)`
2121

22-
error: consider using `NonZeroI16::from()` for more efficient and type-safe conversion
23-
--> tests/ui/non_zero_suggestions.rs:25:14
24-
|
25-
LL | let r4 = i16::from(d.get());
26-
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroI16::from(d)`
27-
28-
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
29-
--> tests/ui/non_zero_suggestions.rs:31:18
30-
|
31-
LL | let r5 = m / u64::from(n.get());
32-
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)`
33-
3422
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
35-
--> tests/ui/non_zero_suggestions.rs:37:14
23+
--> tests/ui/non_zero_suggestions.rs:21:13
3624
|
37-
LL | let r6 = u64::from(max_u32.get());
38-
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(max_u32)`
25+
LL | let x = u64::from(NonZeroU32::new(5).unwrap().get());
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(5).unwrap())`
3927

4028
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
41-
--> tests/ui/non_zero_suggestions.rs:41:13
42-
|
43-
LL | let _ = u64::from(NonZeroU32::new(10).unwrap().get());
44-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(10).unwrap())`
45-
46-
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
47-
--> tests/ui/non_zero_suggestions.rs:63:9
29+
--> tests/ui/non_zero_suggestions.rs:52:9
4830
|
4931
LL | x / u64::from(y.get())
5032
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
5133

5234
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
53-
--> tests/ui/non_zero_suggestions.rs:68:5
54-
|
55-
LL | u64::from(y.get())
56-
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
57-
58-
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
59-
--> tests/ui/non_zero_suggestions.rs:80:22
35+
--> tests/ui/non_zero_suggestions.rs:62:22
6036
|
6137
LL | self.value / u64::from(divisor.get())
6238
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(divisor)`
6339

64-
error: aborting due to 10 previous errors
40+
error: aborting due to 6 previous errors
6541

‎tests/ui/non_zero_suggestions_unfixable.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
#![warn(clippy::non_zero_suggestions)]
2+
//@no-rustfix
3+
use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
4+
5+
fn main() {
6+
let x: u64 = u64::from(NonZeroU32::new(5).unwrap().get());
7+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
8+
9+
let n = NonZeroU32::new(20).unwrap();
10+
let y = u64::from(n.get());
11+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
12+
some_fn_that_only_takes_u64(y);
13+
}
14+
15+
fn return_non_zero(x: u64, y: NonZeroU32) -> u64 {
16+
u64::from(y.get())
17+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
18+
}
19+
20+
fn some_fn_that_only_takes_u64(_: u64) {}

‎tests/ui/non_zero_suggestions_unfixable.stderr

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
2+
--> tests/ui/non_zero_suggestions_unfixable.rs:6:18
3+
|
4+
LL | let x: u64 = u64::from(NonZeroU32::new(5).unwrap().get());
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(5).unwrap())`
6+
|
7+
= note: `-D clippy::non-zero-suggestions` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]`
9+
10+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
11+
--> tests/ui/non_zero_suggestions_unfixable.rs:10:13
12+
|
13+
LL | let y = u64::from(n.get());
14+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)`
15+
16+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
17+
--> tests/ui/non_zero_suggestions_unfixable.rs:16:5
18+
|
19+
LL | u64::from(y.get())
20+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
21+
22+
error: aborting due to 3 previous errors
23+

0 commit comments

Comments
 (0)