Skip to content

Commit d9c4523

Browse files
committed
Auto merge of rust-lang#13167 - Samarth1696:tests, r=llogiq
Added new `non_zero_suggestions` lint Created lint based on the suggestions in rust-lang#7291 - \[x] Followed [lint naming conventions][lint_naming] - \[x] Added passing UI tests (including committed `.stderr` file) - \[x] `cargo test` passes locally - \[x] Executed `cargo dev update_lints` - \[x] Added lint documentation - \[x] Run `cargo dev fmt` ---- changelog: new [`non_zero_suggestions`] lint
2 parents fb9913e + af3346a commit d9c4523

9 files changed

+364
-0
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5773,6 +5773,7 @@ Released 2018-09-13
57735773
[`non_minimal_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg
57745774
[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
57755775
[`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty
5776+
[`non_zero_suggestions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_zero_suggestions
57765777
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool
57775778
[`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options
57785779
[`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
557557
crate::non_expressive_names::SIMILAR_NAMES_INFO,
558558
crate::non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS_INFO,
559559
crate::non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY_INFO,
560+
crate::non_zero_suggestions::NON_ZERO_SUGGESTIONS_INFO,
560561
crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO,
561562
crate::octal_escapes::OCTAL_ESCAPES_INFO,
562563
crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO,

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -273,6 +273,7 @@ mod non_copy_const;
273273
mod non_expressive_names;
274274
mod non_octal_unix_permissions;
275275
mod non_send_fields_in_send_ty;
276+
mod non_zero_suggestions;
276277
mod nonstandard_macro_braces;
277278
mod octal_escapes;
278279
mod only_used_in_recursion;
@@ -940,5 +941,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
940941
store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock));
941942
store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf)));
942943
store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo));
944+
store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions));
943945
// add lints here, do not remove this comment, it's used in `new_lint`
944946
}
+143
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
use clippy_utils::diagnostics::span_lint_and_sugg;
2+
use clippy_utils::source::snippet;
3+
use rustc_ast::ast::BinOpKind;
4+
use rustc_errors::Applicability;
5+
use rustc_hir::{Expr, ExprKind};
6+
use rustc_lint::{LateContext, LateLintPass};
7+
use rustc_middle::ty::{self, Ty};
8+
use rustc_session::declare_lint_pass;
9+
use rustc_span::symbol::sym;
10+
11+
declare_clippy_lint! {
12+
/// ### What it does
13+
/// Checks for conversions from `NonZero` types to regular integer types,
14+
/// and suggests using `NonZero` types for the target as well.
15+
///
16+
/// ### Why is this bad?
17+
/// Converting from `NonZero` types to regular integer types and then back to `NonZero`
18+
/// types is less efficient and loses the type-safety guarantees provided by `NonZero` types.
19+
/// Using `NonZero` types consistently can lead to more optimized code and prevent
20+
/// certain classes of errors related to zero values.
21+
///
22+
/// ### Example
23+
/// ```no_run
24+
/// use std::num::{NonZeroU32, NonZeroU64};
25+
///
26+
/// fn example(x: u64, y: NonZeroU32) {
27+
/// // Bad: Converting NonZeroU32 to u64 unnecessarily
28+
/// let r1 = x / u64::from(y.get());
29+
/// let r2 = x % u64::from(y.get());
30+
/// }
31+
/// ```
32+
/// Use instead:
33+
/// ```no_run
34+
/// use std::num::{NonZeroU32, NonZeroU64};
35+
///
36+
/// fn example(x: u64, y: NonZeroU32) {
37+
/// // Good: Preserving the NonZero property
38+
/// let r1 = x / NonZeroU64::from(y);
39+
/// let r2 = x % NonZeroU64::from(y);
40+
/// }
41+
/// ```
42+
#[clippy::version = "1.81.0"]
43+
pub NON_ZERO_SUGGESTIONS,
44+
restriction,
45+
"suggests using `NonZero#` from `u#` or `i#` for more efficient and type-safe conversions"
46+
}
47+
48+
declare_lint_pass!(NonZeroSuggestions => [NON_ZERO_SUGGESTIONS]);
49+
50+
impl<'tcx> LateLintPass<'tcx> for NonZeroSuggestions {
51+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
52+
if let ExprKind::Binary(op, _, rhs) = expr.kind
53+
&& matches!(op.node, BinOpKind::Div | BinOpKind::Rem)
54+
{
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+
});
61+
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+
&& rcv_path.ident.name.as_str() == "get"
76+
{
77+
let fn_name = cx.tcx.item_name(def_id);
78+
let target_ty = cx.typeck_results().expr_ty(expr);
79+
let receiver_ty = cx.typeck_results().expr_ty(receiver);
80+
81+
// Check if the receiver type is a NonZero type
82+
if let ty::Adt(adt_def, _) = receiver_ty.kind()
83+
&& adt_def.is_struct()
84+
&& cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero)
85+
{
86+
if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) {
87+
let arg_snippet = get_arg_snippet(cx, arg, rcv_path);
88+
suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet, applicability);
89+
}
90+
}
91+
}
92+
}
93+
94+
fn get_arg_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, rcv_path: &rustc_hir::PathSegment<'_>) -> String {
95+
let arg_snippet = snippet(cx, arg.span, "..");
96+
if let Some(index) = arg_snippet.rfind(&format!(".{}", rcv_path.ident.name)) {
97+
arg_snippet[..index].trim().to_string()
98+
} else {
99+
arg_snippet.to_string()
100+
}
101+
}
102+
103+
fn suggest_non_zero_conversion(
104+
cx: &LateContext<'_>,
105+
expr: &Expr<'_>,
106+
fn_name: rustc_span::Symbol,
107+
target_non_zero_type: &str,
108+
arg_snippet: &str,
109+
applicability: Applicability,
110+
) {
111+
let suggestion = format!("{target_non_zero_type}::{fn_name}({arg_snippet})");
112+
span_lint_and_sugg(
113+
cx,
114+
NON_ZERO_SUGGESTIONS,
115+
expr.span,
116+
format!("consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"),
117+
"replace with",
118+
suggestion,
119+
applicability,
120+
);
121+
}
122+
123+
fn get_target_non_zero_type(ty: Ty<'_>) -> Option<&'static str> {
124+
match ty.kind() {
125+
ty::Uint(uint_ty) => Some(match uint_ty {
126+
ty::UintTy::U8 => "NonZeroU8",
127+
ty::UintTy::U16 => "NonZeroU16",
128+
ty::UintTy::U32 => "NonZeroU32",
129+
ty::UintTy::U64 => "NonZeroU64",
130+
ty::UintTy::U128 => "NonZeroU128",
131+
ty::UintTy::Usize => "NonZeroUsize",
132+
}),
133+
ty::Int(int_ty) => Some(match int_ty {
134+
ty::IntTy::I8 => "NonZeroI8",
135+
ty::IntTy::I16 => "NonZeroI16",
136+
ty::IntTy::I32 => "NonZeroI32",
137+
ty::IntTy::I64 => "NonZeroI64",
138+
ty::IntTy::I128 => "NonZeroI128",
139+
ty::IntTy::Isize => "NonZeroIsize",
140+
}),
141+
_ => None,
142+
}
143+
}

tests/ui/non_zero_suggestions.fixed

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#![warn(clippy::non_zero_suggestions)]
2+
use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
3+
4+
fn main() {
5+
/// Positive test cases (lint should trigger)
6+
// U32 -> U64
7+
let x: u64 = 100;
8+
let y = NonZeroU32::new(10).unwrap();
9+
let r1 = x / NonZeroU64::from(y);
10+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
11+
12+
let r2 = x % NonZeroU64::from(y);
13+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
14+
15+
// U16 -> U32
16+
let a: u32 = 50;
17+
let b = NonZeroU16::new(5).unwrap();
18+
let r3 = a / NonZeroU32::from(b);
19+
//~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion
20+
21+
let x = NonZeroU64::from(NonZeroU32::new(5).unwrap());
22+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
23+
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;
29+
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());
33+
34+
// Same size types
35+
let e: u32 = 200;
36+
let f = NonZeroU32::new(20).unwrap();
37+
let r7 = e / f.get();
38+
39+
// Smaller to larger, but not NonZero
40+
let g: u64 = 1000;
41+
let h: u32 = 50;
42+
let r8 = g / u64::from(h);
43+
44+
// Using From correctly
45+
let k: u64 = 300;
46+
let l = NonZeroU32::new(15).unwrap();
47+
let r9 = k / NonZeroU64::from(l);
48+
}
49+
50+
// Additional function to test the lint in a different context
51+
fn divide_numbers(x: u64, y: NonZeroU32) -> u64 {
52+
x / NonZeroU64::from(y)
53+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
54+
}
55+
56+
struct Calculator {
57+
value: u64,
58+
}
59+
60+
impl Calculator {
61+
fn divide(&self, divisor: NonZeroU32) -> u64 {
62+
self.value / NonZeroU64::from(divisor)
63+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
64+
}
65+
}

tests/ui/non_zero_suggestions.rs

+65
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
#![warn(clippy::non_zero_suggestions)]
2+
use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize};
3+
4+
fn main() {
5+
/// Positive test cases (lint should trigger)
6+
// U32 -> U64
7+
let x: u64 = 100;
8+
let y = NonZeroU32::new(10).unwrap();
9+
let r1 = x / u64::from(y.get());
10+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
11+
12+
let r2 = x % u64::from(y.get());
13+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
14+
15+
// U16 -> U32
16+
let a: u32 = 50;
17+
let b = NonZeroU16::new(5).unwrap();
18+
let r3 = a / u32::from(b.get());
19+
//~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion
20+
21+
let x = u64::from(NonZeroU32::new(5).unwrap().get());
22+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
23+
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;
29+
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());
33+
34+
// Same size types
35+
let e: u32 = 200;
36+
let f = NonZeroU32::new(20).unwrap();
37+
let r7 = e / f.get();
38+
39+
// Smaller to larger, but not NonZero
40+
let g: u64 = 1000;
41+
let h: u32 = 50;
42+
let r8 = g / u64::from(h);
43+
44+
// Using From correctly
45+
let k: u64 = 300;
46+
let l = NonZeroU32::new(15).unwrap();
47+
let r9 = k / NonZeroU64::from(l);
48+
}
49+
50+
// Additional function to test the lint in a different context
51+
fn divide_numbers(x: u64, y: NonZeroU32) -> u64 {
52+
x / u64::from(y.get())
53+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
54+
}
55+
56+
struct Calculator {
57+
value: u64,
58+
}
59+
60+
impl Calculator {
61+
fn divide(&self, divisor: NonZeroU32) -> u64 {
62+
self.value / u64::from(divisor.get())
63+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
64+
}
65+
}

tests/ui/non_zero_suggestions.stderr

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
2+
--> tests/ui/non_zero_suggestions.rs:9:18
3+
|
4+
LL | let r1 = x / u64::from(y.get());
5+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
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.rs:12:18
12+
|
13+
LL | let r2 = x % u64::from(y.get());
14+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
15+
16+
error: consider using `NonZeroU32::from()` for more efficient and type-safe conversion
17+
--> tests/ui/non_zero_suggestions.rs:18:18
18+
|
19+
LL | let r3 = a / u32::from(b.get());
20+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU32::from(b)`
21+
22+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
23+
--> tests/ui/non_zero_suggestions.rs:21:13
24+
|
25+
LL | let x = u64::from(NonZeroU32::new(5).unwrap().get());
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(5).unwrap())`
27+
28+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
29+
--> tests/ui/non_zero_suggestions.rs:52:9
30+
|
31+
LL | x / u64::from(y.get())
32+
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
33+
34+
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
35+
--> tests/ui/non_zero_suggestions.rs:62:22
36+
|
37+
LL | self.value / u64::from(divisor.get())
38+
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(divisor)`
39+
40+
error: aborting due to 6 previous errors
41+
+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
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+
let m = NonZeroU32::try_from(1).unwrap();
15+
let _z: NonZeroU64 = m.into();
16+
}
17+
18+
fn return_non_zero(x: u64, y: NonZeroU32) -> u64 {
19+
u64::from(y.get())
20+
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
21+
}
22+
23+
fn some_fn_that_only_takes_u64(_: u64) {}
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:19: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)