From 46f8d360de99afe38dacc1dd6e29133d856e9fd3 Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Fri, 26 Jul 2024 00:51:30 +0530 Subject: [PATCH 1/8] Lint ready --- CHANGELOG.md | 1 + clippy_lints/src/declared_lints.rs | 1 + clippy_lints/src/lib.rs | 2 + clippy_lints/src/non_zero_suggestions.rs | 107 +++++++++++++++++++++++ tests/ui/non_zero_suggestions.rs | 52 +++++++++++ 5 files changed, 163 insertions(+) create mode 100644 clippy_lints/src/non_zero_suggestions.rs create mode 100644 tests/ui/non_zero_suggestions.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f1de51c936ea..0e5d1688e4a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5773,6 +5773,7 @@ Released 2018-09-13 [`non_minimal_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg [`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions [`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty +[`non_zero_suggestions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_zero_suggestions [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool [`nonsensical_open_options`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonsensical_open_options [`nonstandard_macro_braces`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonstandard_macro_braces diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 6f468f01b2fd..16c64830e70d 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -557,6 +557,7 @@ pub static LINTS: &[&crate::LintInfo] = &[ crate::non_expressive_names::SIMILAR_NAMES_INFO, crate::non_octal_unix_permissions::NON_OCTAL_UNIX_PERMISSIONS_INFO, crate::non_send_fields_in_send_ty::NON_SEND_FIELDS_IN_SEND_TY_INFO, + crate::non_zero_suggestions::NON_ZERO_SUGGESTIONS_INFO, crate::nonstandard_macro_braces::NONSTANDARD_MACRO_BRACES_INFO, crate::octal_escapes::OCTAL_ESCAPES_INFO, crate::only_used_in_recursion::ONLY_USED_IN_RECURSION_INFO, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index bc16a3b0c014..3604090b68cc 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -273,6 +273,7 @@ mod non_copy_const; mod non_expressive_names; mod non_octal_unix_permissions; mod non_send_fields_in_send_ty; +mod non_zero_suggestions; mod nonstandard_macro_braces; mod octal_escapes; mod only_used_in_recursion; @@ -940,5 +941,6 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) { store.register_late_pass(|_| Box::new(pointers_in_nomem_asm_block::PointersInNomemAsmBlock)); store.register_late_pass(move |_| Box::new(manual_div_ceil::ManualDivCeil::new(conf))); store.register_late_pass(|_| Box::new(manual_is_power_of_two::ManualIsPowerOfTwo)); + store.register_late_pass(|_| Box::new(non_zero_suggestions::NonZeroSuggestions)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs new file mode 100644 index 000000000000..b9e7053db900 --- /dev/null +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -0,0 +1,107 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; +use rustc_errors::Applicability; +use rustc_hir::{Expr, ExprKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{self, Ty}; +use rustc_session::declare_lint_pass; +use rustc_span::symbol::sym; + +declare_clippy_lint! { + /// ### What it does + /// + /// ### Why is this bad? + /// + /// ### Example + /// ```no_run + /// // example code where clippy issues a warning + /// ``` + /// Use instead: + /// ```no_run + /// // example code which does not raise clippy warning + /// ``` + #[clippy::version = "1.81.0"] + pub NON_ZERO_SUGGESTIONS, + restriction, + "suggests using `NonZero#` from `u#` or `i#` for more efficient and type-safe conversions" +} + +declare_lint_pass!(NonZeroSuggestions => [NON_ZERO_SUGGESTIONS]); + +impl<'tcx> LateLintPass<'tcx> for NonZeroSuggestions { + fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { + if let ExprKind::Call(func, [arg]) = expr.kind { + if let ExprKind::Path(qpath) = &func.kind { + if let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() { + let fn_name = cx.tcx.item_name(def_id); + let target_ty = cx.typeck_results().expr_ty(expr); + + if let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind { + let receiver_ty = cx.typeck_results().expr_ty(receiver); + if let ty::Adt(adt_def, _) = receiver_ty.kind() { + if adt_def.is_struct() && cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero) { + if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) { + let arg_snippet = get_arg_snippet(cx, arg, rcv_path); + suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet); + } + } + } + } + } + } + } + } +} + +fn get_arg_snippet(cx: &LateContext<'_>, arg: &Expr<'_>, rcv_path: &rustc_hir::PathSegment<'_>) -> String { + let arg_snippet = snippet(cx, arg.span, ".."); + if let Some(index) = arg_snippet.rfind(&format!(".{}", rcv_path.ident.name)) { + arg_snippet[..index].trim().to_string() + } else { + arg_snippet.to_string() + } +} + +fn suggest_non_zero_conversion( + cx: &LateContext<'_>, + expr: &Expr<'_>, + fn_name: rustc_span::Symbol, + target_non_zero_type: &str, + arg_snippet: &str, +) { + let suggestion = format!("{}::{}({})", target_non_zero_type, fn_name, arg_snippet); + span_lint_and_sugg( + cx, + NON_ZERO_SUGGESTIONS, + expr.span, + format!( + "Consider using `{}::{}()` for more efficient and type-safe conversion", + target_non_zero_type, fn_name + ), + "Replace with", + suggestion, + Applicability::MachineApplicable, + ); +} + +fn get_target_non_zero_type(ty: Ty<'_>) -> Option<&'static str> { + match ty.kind() { + ty::Uint(uint_ty) => Some(match uint_ty { + ty::UintTy::U8 => "NonZeroU8", + ty::UintTy::U16 => "NonZeroU16", + ty::UintTy::U32 => "NonZeroU32", + ty::UintTy::U64 => "NonZeroU64", + ty::UintTy::U128 => "NonZeroU128", + ty::UintTy::Usize => "NonZeroUsize", + }), + ty::Int(int_ty) => Some(match int_ty { + ty::IntTy::I8 => "NonZeroI8", + ty::IntTy::I16 => "NonZeroI16", + ty::IntTy::I32 => "NonZeroI32", + ty::IntTy::I64 => "NonZeroI64", + ty::IntTy::I128 => "NonZeroI128", + ty::IntTy::Isize => "NonZeroIsize", + }), + _ => None, + } +} diff --git a/tests/ui/non_zero_suggestions.rs b/tests/ui/non_zero_suggestions.rs new file mode 100644 index 000000000000..1a5ee40dc3da --- /dev/null +++ b/tests/ui/non_zero_suggestions.rs @@ -0,0 +1,52 @@ +#![warn(clippy::non_zero_suggestions)] + +use std::num::{ + NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize, NonZeroU128, NonZeroU16, NonZeroU32, + NonZeroU64, NonZeroU8, NonZeroUsize, +}; + +fn main() { + // Basic cases + let _ = u8::try_from(NonZeroU8::new(5).unwrap().get()); + + let _ = u16::from(NonZeroU16::new(10).unwrap().get()); + + // Different integer types + let _ = u32::from(NonZeroU32::new(15).unwrap().get()); + + let _ = u64::from(NonZeroU64::new(20).unwrap().get()); + + let _ = u128::from(NonZeroU128::new(25).unwrap().get()); + + let _ = usize::from(NonZeroUsize::new(30).unwrap().get()); + + // Signed integer types + let _ = i8::try_from(NonZeroI8::new(-5).unwrap().get()); + + let _ = i16::from(NonZeroI16::new(-10).unwrap().get()); + + let _ = i32::from(NonZeroI32::new(-15).unwrap().get()); + + // Edge cases + + // Complex expression + let _ = u8::from(NonZeroU8::new(5).unwrap().get() + 1); + + // Function call + fn get_non_zero() -> NonZeroU8 { + NonZeroU8::new(42).unwrap() + } + let _ = u8::from(get_non_zero().get()); + + // Method chaining + let _ = u16::from(NonZeroU16::new(100).unwrap().get().checked_add(1).unwrap()); + // This should not trigger the lint + + // Different conversion methods + let _ = u32::try_from(NonZeroU32::new(200).unwrap().get()).unwrap(); + + // Cases that should not trigger the lint + let _ = u8::from(5); + let _ = u16::from(10u8); + let _ = i32::try_from(40u32).unwrap(); +} From 73039f654e2efc3a7384462dcd46289851b80e62 Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Fri, 26 Jul 2024 17:11:51 +0530 Subject: [PATCH 2/8] test cases added including some edge cases --- clippy_lints/src/non_zero_suggestions.rs | 22 ++++++- tests/ui/non_zero_suggestions.fixed | 69 ++++++++++++++++++++ tests/ui/non_zero_suggestions.rs | 83 ++++++++++++++---------- tests/ui/non_zero_suggestions.stderr | 59 +++++++++++++++++ 4 files changed, 198 insertions(+), 35 deletions(-) create mode 100644 tests/ui/non_zero_suggestions.fixed create mode 100644 tests/ui/non_zero_suggestions.stderr diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs index b9e7053db900..715d9eda6eeb 100644 --- a/clippy_lints/src/non_zero_suggestions.rs +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -9,16 +9,34 @@ use rustc_span::symbol::sym; declare_clippy_lint! { /// ### What it does + /// Checks for conversions from `NonZero` types to regular integer types, + /// and suggests using `NonZero` types for the target as well. /// /// ### Why is this bad? + /// Converting from `NonZero` types to regular integer types and then back to `NonZero` + /// types is less efficient and loses the type-safety guarantees provided by `NonZero` types. + /// Using `NonZero` types consistently can lead to more optimized code and prevent + /// certain classes of errors related to zero values. /// /// ### Example /// ```no_run - /// // example code where clippy issues a warning + /// use std::num::{NonZeroU32, NonZeroU64}; + /// + /// fn example(x: u64, y: NonZeroU32) { + /// // Bad: Converting NonZeroU32 to u64 unnecessarily + /// let r1 = x / u64::from(y.get()); + /// let r2 = x % u64::from(y.get()); + /// } /// ``` /// Use instead: /// ```no_run - /// // example code which does not raise clippy warning + /// use std::num::{NonZeroU32, NonZeroU64}; + /// + /// fn example(x: u64, y: NonZeroU32) { + /// // Good: Preserving the NonZero property + /// let r1 = x / NonZeroU64::from(y); + /// let r2 = x % NonZeroU64::from(y); + /// } /// ``` #[clippy::version = "1.81.0"] pub NON_ZERO_SUGGESTIONS, diff --git a/tests/ui/non_zero_suggestions.fixed b/tests/ui/non_zero_suggestions.fixed new file mode 100644 index 000000000000..b33de1ef03f5 --- /dev/null +++ b/tests/ui/non_zero_suggestions.fixed @@ -0,0 +1,69 @@ +#![warn(clippy::non_zero_suggestions)] + +use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; + +fn main() { + // Positive test cases (lint should trigger) + + // U32 -> U64 + let x: u64 = 100; + let y = NonZeroU32::new(10).unwrap(); + let r1 = x / NonZeroU64::from(y); + let r2 = x % NonZeroU64::from(y); + + // U16 -> U32 + let a: u32 = 50; + let b = NonZeroU16::new(5).unwrap(); + let r3 = a / NonZeroU32::from(b); + + // I8 -> I16 + let c: i16 = 25; + let d = NonZeroI8::new(3).unwrap(); + let r4 = NonZeroI16::from(d); + + // Different operations + let m: u64 = 400; + let n = NonZeroU32::new(20).unwrap(); + let r5 = m / NonZeroU64::from(n); + + // Edge cases + + // Using the max value of a type + let max_u32 = NonZeroU32::new(u32::MAX).unwrap(); + let r6 = NonZeroU64::from(max_u32); + + // Chained method calls + let _ = NonZeroU64::from(NonZeroU32::new(10).unwrap()); + + // Negative test cases (lint should not trigger) + + // Same size types + let e: u32 = 200; + let f = NonZeroU32::new(20).unwrap(); + let r10 = e / f.get(); + + // Smaller to larger, but not NonZero + let g: u64 = 1000; + let h: u32 = 50; + let r11 = g / u64::from(h); + + // Using From correctly + let k: u64 = 300; + let l = NonZeroU32::new(15).unwrap(); + let r12 = k / NonZeroU64::from(l); +} + +// Additional function to test the lint in a different context +fn divide_numbers(x: u64, y: NonZeroU32) -> u64 { + x / NonZeroU64::from(y) +} + +struct Calculator { + value: u64, +} + +impl Calculator { + fn divide(&self, divisor: NonZeroU32) -> u64 { + self.value / NonZeroU64::from(divisor) + } +} diff --git a/tests/ui/non_zero_suggestions.rs b/tests/ui/non_zero_suggestions.rs index 1a5ee40dc3da..27089eaf86ef 100644 --- a/tests/ui/non_zero_suggestions.rs +++ b/tests/ui/non_zero_suggestions.rs @@ -1,52 +1,69 @@ #![warn(clippy::non_zero_suggestions)] -use std::num::{ - NonZeroI128, NonZeroI16, NonZeroI32, NonZeroI64, NonZeroI8, NonZeroIsize, NonZeroU128, NonZeroU16, NonZeroU32, - NonZeroU64, NonZeroU8, NonZeroUsize, -}; +use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; fn main() { - // Basic cases - let _ = u8::try_from(NonZeroU8::new(5).unwrap().get()); + // Positive test cases (lint should trigger) - let _ = u16::from(NonZeroU16::new(10).unwrap().get()); + // U32 -> U64 + let x: u64 = 100; + let y = NonZeroU32::new(10).unwrap(); + let r1 = x / u64::from(y.get()); + let r2 = x % u64::from(y.get()); - // Different integer types - let _ = u32::from(NonZeroU32::new(15).unwrap().get()); + // U16 -> U32 + let a: u32 = 50; + let b = NonZeroU16::new(5).unwrap(); + let r3 = a / u32::from(b.get()); - let _ = u64::from(NonZeroU64::new(20).unwrap().get()); + // I8 -> I16 + let c: i16 = 25; + let d = NonZeroI8::new(3).unwrap(); + let r4 = i16::from(d.get()); - let _ = u128::from(NonZeroU128::new(25).unwrap().get()); + // Different operations + let m: u64 = 400; + let n = NonZeroU32::new(20).unwrap(); + let r5 = m / u64::from(n.get()); - let _ = usize::from(NonZeroUsize::new(30).unwrap().get()); + // Edge cases - // Signed integer types - let _ = i8::try_from(NonZeroI8::new(-5).unwrap().get()); + // Using the max value of a type + let max_u32 = NonZeroU32::new(u32::MAX).unwrap(); + let r6 = u64::from(max_u32.get()); - let _ = i16::from(NonZeroI16::new(-10).unwrap().get()); + // Chained method calls + let _ = u64::from(NonZeroU32::new(10).unwrap().get()); - let _ = i32::from(NonZeroI32::new(-15).unwrap().get()); + // Negative test cases (lint should not trigger) - // Edge cases + // Same size types + let e: u32 = 200; + let f = NonZeroU32::new(20).unwrap(); + let r10 = e / f.get(); - // Complex expression - let _ = u8::from(NonZeroU8::new(5).unwrap().get() + 1); + // Smaller to larger, but not NonZero + let g: u64 = 1000; + let h: u32 = 50; + let r11 = g / u64::from(h); - // Function call - fn get_non_zero() -> NonZeroU8 { - NonZeroU8::new(42).unwrap() - } - let _ = u8::from(get_non_zero().get()); + // Using From correctly + let k: u64 = 300; + let l = NonZeroU32::new(15).unwrap(); + let r12 = k / NonZeroU64::from(l); +} - // Method chaining - let _ = u16::from(NonZeroU16::new(100).unwrap().get().checked_add(1).unwrap()); - // This should not trigger the lint +// Additional function to test the lint in a different context +fn divide_numbers(x: u64, y: NonZeroU32) -> u64 { + x / u64::from(y.get()) +} - // Different conversion methods - let _ = u32::try_from(NonZeroU32::new(200).unwrap().get()).unwrap(); +struct Calculator { + value: u64, +} - // Cases that should not trigger the lint - let _ = u8::from(5); - let _ = u16::from(10u8); - let _ = i32::try_from(40u32).unwrap(); +impl Calculator { + fn divide(&self, divisor: NonZeroU32) -> u64 { + self.value / u64::from(divisor.get()) + } } diff --git a/tests/ui/non_zero_suggestions.stderr b/tests/ui/non_zero_suggestions.stderr new file mode 100644 index 000000000000..b30f9dd1e5ce --- /dev/null +++ b/tests/ui/non_zero_suggestions.stderr @@ -0,0 +1,59 @@ +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:11:18 + | +LL | let r1 = x / u64::from(y.get()); + | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(y)` + | + = note: `-D clippy::non-zero-suggestions` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]` + +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:12:18 + | +LL | let r2 = x % u64::from(y.get()); + | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(y)` + +error: Consider using `NonZeroU32::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:17:18 + | +LL | let r3 = a / u32::from(b.get()); + | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU32::from(b)` + +error: Consider using `NonZeroI16::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:22:14 + | +LL | let r4 = i16::from(d.get()); + | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroI16::from(d)` + +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:27:18 + | +LL | let r5 = m / u64::from(n.get()); + | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(n)` + +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:33:14 + | +LL | let r6 = u64::from(max_u32.get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(max_u32)` + +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:36:13 + | +LL | let _ = u64::from(NonZeroU32::new(10).unwrap().get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(NonZeroU32::new(10).unwrap())` + +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:58:9 + | +LL | x / u64::from(y.get()) + | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(y)` + +error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:67:22 + | +LL | self.value / u64::from(divisor.get()) + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(divisor)` + +error: aborting due to 9 previous errors + From 6f01273d5d2acec2a726f2115fbf557967c2336a Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Fri, 26 Jul 2024 17:46:41 +0530 Subject: [PATCH 3/8] dogfood test passed --- clippy_lints/src/non_zero_suggestions.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs index 715d9eda6eeb..e652f6f51080 100644 --- a/clippy_lints/src/non_zero_suggestions.rs +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -87,15 +87,12 @@ fn suggest_non_zero_conversion( target_non_zero_type: &str, arg_snippet: &str, ) { - let suggestion = format!("{}::{}({})", target_non_zero_type, fn_name, arg_snippet); + let suggestion = format!("{target_non_zero_type}::{fn_name}({arg_snippet})"); span_lint_and_sugg( cx, NON_ZERO_SUGGESTIONS, expr.span, - format!( - "Consider using `{}::{}()` for more efficient and type-safe conversion", - target_non_zero_type, fn_name - ), + format!("Consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"), "Replace with", suggestion, Applicability::MachineApplicable, From 0f99aa992ea19b0148767c137ff1a936d58b65e0 Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Fri, 26 Jul 2024 18:16:08 +0530 Subject: [PATCH 4/8] all tests passed --- clippy_lints/src/non_zero_suggestions.rs | 4 +-- tests/ui/non_zero_suggestions.stderr | 36 ++++++++++++------------ 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs index e652f6f51080..fc2920868d49 100644 --- a/clippy_lints/src/non_zero_suggestions.rs +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -92,8 +92,8 @@ fn suggest_non_zero_conversion( cx, NON_ZERO_SUGGESTIONS, expr.span, - format!("Consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"), - "Replace with", + format!("consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"), + "replace with", suggestion, Applicability::MachineApplicable, ); diff --git a/tests/ui/non_zero_suggestions.stderr b/tests/ui/non_zero_suggestions.stderr index b30f9dd1e5ce..4c86720c3086 100644 --- a/tests/ui/non_zero_suggestions.stderr +++ b/tests/ui/non_zero_suggestions.stderr @@ -1,59 +1,59 @@ -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:11:18 | LL | let r1 = x / u64::from(y.get()); - | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(y)` + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` | = note: `-D clippy::non-zero-suggestions` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]` -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:12:18 | LL | let r2 = x % u64::from(y.get()); - | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(y)` + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` -error: Consider using `NonZeroU32::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU32::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:17:18 | LL | let r3 = a / u32::from(b.get()); - | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU32::from(b)` + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU32::from(b)` -error: Consider using `NonZeroI16::from()` for more efficient and type-safe conversion +error: consider using `NonZeroI16::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:22:14 | LL | let r4 = i16::from(d.get()); - | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroI16::from(d)` + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroI16::from(d)` -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:27:18 | LL | let r5 = m / u64::from(n.get()); - | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(n)` + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)` -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:33:14 | LL | let r6 = u64::from(max_u32.get()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(max_u32)` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(max_u32)` -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:36:13 | LL | let _ = u64::from(NonZeroU32::new(10).unwrap().get()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(NonZeroU32::new(10).unwrap())` + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(10).unwrap())` -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:58:9 | LL | x / u64::from(y.get()) - | ^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(y)` + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` -error: Consider using `NonZeroU64::from()` for more efficient and type-safe conversion +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion --> tests/ui/non_zero_suggestions.rs:67:22 | LL | self.value / u64::from(divisor.get()) - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: Replace with: `NonZeroU64::from(divisor)` + | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(divisor)` error: aborting due to 9 previous errors From bed44418eca84e81c7efa7a3ee45f450b8a834f4 Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Sat, 27 Jul 2024 02:35:16 +0530 Subject: [PATCH 5/8] refactored the code --- clippy_lints/src/non_zero_suggestions.rs | 31 ++++++++++++------------ 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs index fc2920868d49..1624c365ee94 100644 --- a/clippy_lints/src/non_zero_suggestions.rs +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -48,23 +48,22 @@ declare_lint_pass!(NonZeroSuggestions => [NON_ZERO_SUGGESTIONS]); impl<'tcx> LateLintPass<'tcx> for NonZeroSuggestions { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if let ExprKind::Call(func, [arg]) = expr.kind { - if let ExprKind::Path(qpath) = &func.kind { - if let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() { - let fn_name = cx.tcx.item_name(def_id); - let target_ty = cx.typeck_results().expr_ty(expr); + if let ExprKind::Call(func, [arg]) = expr.kind + && let ExprKind::Path(qpath) = &func.kind + && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() + && let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind + { + let fn_name = cx.tcx.item_name(def_id); + let target_ty = cx.typeck_results().expr_ty(expr); + let receiver_ty = cx.typeck_results().expr_ty(receiver); - if let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind { - let receiver_ty = cx.typeck_results().expr_ty(receiver); - if let ty::Adt(adt_def, _) = receiver_ty.kind() { - if adt_def.is_struct() && cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero) { - if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) { - let arg_snippet = get_arg_snippet(cx, arg, rcv_path); - suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet); - } - } - } - } + if let ty::Adt(adt_def, _) = receiver_ty.kind() + && adt_def.is_struct() + && cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero) + { + if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) { + let arg_snippet = get_arg_snippet(cx, arg, rcv_path); + suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet); } } } From c6c74083a803f32ffe2447fdcd0afaafaa03179c Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Sat, 27 Jul 2024 03:15:40 +0530 Subject: [PATCH 6/8] error notations added --- tests/ui/non_zero_suggestions.fixed | 25 +++++++++++++++++++------ tests/ui/non_zero_suggestions.rs | 25 +++++++++++++++++++------ tests/ui/non_zero_suggestions.stderr | 26 ++++++++++++++++---------- 3 files changed, 54 insertions(+), 22 deletions(-) diff --git a/tests/ui/non_zero_suggestions.fixed b/tests/ui/non_zero_suggestions.fixed index b33de1ef03f5..d7e6b19edc1f 100644 --- a/tests/ui/non_zero_suggestions.fixed +++ b/tests/ui/non_zero_suggestions.fixed @@ -3,40 +3,45 @@ use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; fn main() { - // Positive test cases (lint should trigger) - + /// Positive test cases (lint should trigger) // U32 -> U64 let x: u64 = 100; let y = NonZeroU32::new(10).unwrap(); let r1 = x / NonZeroU64::from(y); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + let r2 = x % NonZeroU64::from(y); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion // U16 -> U32 let a: u32 = 50; let b = NonZeroU16::new(5).unwrap(); let r3 = a / NonZeroU32::from(b); + //~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion // I8 -> I16 let c: i16 = 25; let d = NonZeroI8::new(3).unwrap(); let r4 = NonZeroI16::from(d); + //~^ ERROR: consider using `NonZeroI16::from()` for more efficient and type-safe conversion // Different operations let m: u64 = 400; let n = NonZeroU32::new(20).unwrap(); let r5 = m / NonZeroU64::from(n); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - // Edge cases - + /// Edge cases // Using the max value of a type let max_u32 = NonZeroU32::new(u32::MAX).unwrap(); let r6 = NonZeroU64::from(max_u32); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion // Chained method calls let _ = NonZeroU64::from(NonZeroU32::new(10).unwrap()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - // Negative test cases (lint should not trigger) - + /// Negative test cases (lint should not trigger) // Same size types let e: u32 = 200; let f = NonZeroU32::new(20).unwrap(); @@ -56,8 +61,16 @@ fn main() { // Additional function to test the lint in a different context fn divide_numbers(x: u64, y: NonZeroU32) -> u64 { x / NonZeroU64::from(y) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion } +fn no_bin_exp(x: u64, y: NonZeroU32) -> u64 { + NonZeroU64::from(y) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion +} + +fn some_fn_that_only_takes_u64(_: u64) {} + struct Calculator { value: u64, } diff --git a/tests/ui/non_zero_suggestions.rs b/tests/ui/non_zero_suggestions.rs index 27089eaf86ef..8f256dabcb84 100644 --- a/tests/ui/non_zero_suggestions.rs +++ b/tests/ui/non_zero_suggestions.rs @@ -3,40 +3,45 @@ use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; fn main() { - // Positive test cases (lint should trigger) - + /// Positive test cases (lint should trigger) // U32 -> U64 let x: u64 = 100; let y = NonZeroU32::new(10).unwrap(); let r1 = x / u64::from(y.get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + let r2 = x % u64::from(y.get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion // U16 -> U32 let a: u32 = 50; let b = NonZeroU16::new(5).unwrap(); let r3 = a / u32::from(b.get()); + //~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion // I8 -> I16 let c: i16 = 25; let d = NonZeroI8::new(3).unwrap(); let r4 = i16::from(d.get()); + //~^ ERROR: consider using `NonZeroI16::from()` for more efficient and type-safe conversion // Different operations let m: u64 = 400; let n = NonZeroU32::new(20).unwrap(); let r5 = m / u64::from(n.get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - // Edge cases - + /// Edge cases // Using the max value of a type let max_u32 = NonZeroU32::new(u32::MAX).unwrap(); let r6 = u64::from(max_u32.get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion // Chained method calls let _ = u64::from(NonZeroU32::new(10).unwrap().get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - // Negative test cases (lint should not trigger) - + /// Negative test cases (lint should not trigger) // Same size types let e: u32 = 200; let f = NonZeroU32::new(20).unwrap(); @@ -56,8 +61,16 @@ fn main() { // Additional function to test the lint in a different context fn divide_numbers(x: u64, y: NonZeroU32) -> u64 { x / u64::from(y.get()) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion } +fn no_bin_exp(x: u64, y: NonZeroU32) -> u64 { + u64::from(y.get()) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion +} + +fn some_fn_that_only_takes_u64(_: u64) {} + struct Calculator { value: u64, } diff --git a/tests/ui/non_zero_suggestions.stderr b/tests/ui/non_zero_suggestions.stderr index 4c86720c3086..b231c8d216ed 100644 --- a/tests/ui/non_zero_suggestions.stderr +++ b/tests/ui/non_zero_suggestions.stderr @@ -1,5 +1,5 @@ error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:11:18 + --> tests/ui/non_zero_suggestions.rs:10:18 | LL | let r1 = x / u64::from(y.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` @@ -8,52 +8,58 @@ LL | let r1 = x / u64::from(y.get()); = help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:12:18 + --> tests/ui/non_zero_suggestions.rs:13:18 | LL | let r2 = x % u64::from(y.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` error: consider using `NonZeroU32::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:17:18 + --> tests/ui/non_zero_suggestions.rs:19:18 | LL | let r3 = a / u32::from(b.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU32::from(b)` error: consider using `NonZeroI16::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:22:14 + --> tests/ui/non_zero_suggestions.rs:25:14 | LL | let r4 = i16::from(d.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroI16::from(d)` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:27:18 + --> tests/ui/non_zero_suggestions.rs:31:18 | LL | let r5 = m / u64::from(n.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:33:14 + --> tests/ui/non_zero_suggestions.rs:37:14 | LL | let r6 = u64::from(max_u32.get()); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(max_u32)` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:36:13 + --> tests/ui/non_zero_suggestions.rs:41:13 | LL | let _ = u64::from(NonZeroU32::new(10).unwrap().get()); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(10).unwrap())` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:58:9 + --> tests/ui/non_zero_suggestions.rs:63:9 | LL | x / u64::from(y.get()) | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:67:22 + --> tests/ui/non_zero_suggestions.rs:68:5 + | +LL | u64::from(y.get()) + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` + +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions.rs:80:22 | LL | self.value / u64::from(divisor.get()) | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(divisor)` -error: aborting due to 9 previous errors +error: aborting due to 10 previous errors From d43acb803f1bdabfd3761e54bd9509f8b19fb16a Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Thu, 1 Aug 2024 17:29:07 +0530 Subject: [PATCH 7/8] Added checks for binary expr and added different test cases for unfixable cases --- clippy_lints/src/non_zero_suggestions.rs | 53 +++++++++++++------ tests/ui/non_zero_suggestions.fixed | 43 +++++---------- tests/ui/non_zero_suggestions.rs | 43 +++++---------- tests/ui/non_zero_suggestions.stderr | 42 ++++----------- tests/ui/non_zero_suggestions_unfixable.rs | 20 +++++++ .../ui/non_zero_suggestions_unfixable.stderr | 23 ++++++++ 6 files changed, 115 insertions(+), 109 deletions(-) create mode 100644 tests/ui/non_zero_suggestions_unfixable.rs create mode 100644 tests/ui/non_zero_suggestions_unfixable.stderr diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs index 1624c365ee94..808de147d722 100644 --- a/clippy_lints/src/non_zero_suggestions.rs +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -1,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; +use rustc_ast::ast::BinOpKind; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -48,23 +49,42 @@ declare_lint_pass!(NonZeroSuggestions => [NON_ZERO_SUGGESTIONS]); impl<'tcx> LateLintPass<'tcx> for NonZeroSuggestions { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { - if let ExprKind::Call(func, [arg]) = expr.kind - && let ExprKind::Path(qpath) = &func.kind - && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() - && let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind + if let ExprKind::Binary(op, _, rhs) = expr.kind + && matches!(op.node, BinOpKind::Div | BinOpKind::Rem) { - let fn_name = cx.tcx.item_name(def_id); - let target_ty = cx.typeck_results().expr_ty(expr); - let receiver_ty = cx.typeck_results().expr_ty(receiver); + check_non_zero_conversion(cx, rhs, Applicability::MachineApplicable); + } else { + // Check if the parent expression is a binary operation + let parent_is_binary = cx.tcx.hir().parent_iter(expr.hir_id).any(|(_, node)| { + matches!(node, rustc_hir::Node::Expr(parent_expr) if matches!(parent_expr.kind, ExprKind::Binary(..))) + }); - if let ty::Adt(adt_def, _) = receiver_ty.kind() - && adt_def.is_struct() - && cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero) - { - if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) { - let arg_snippet = get_arg_snippet(cx, arg, rcv_path); - suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet); - } + if !parent_is_binary { + check_non_zero_conversion(cx, expr, Applicability::MaybeIncorrect); + } + } + } +} + +fn check_non_zero_conversion(cx: &LateContext<'_>, expr: &Expr<'_>, applicability: Applicability) { + // Check if the expression is a function call with one argument + if let ExprKind::Call(func, [arg]) = expr.kind + && let ExprKind::Path(qpath) = &func.kind + && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() + && let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind + { + let fn_name = cx.tcx.item_name(def_id); + let target_ty = cx.typeck_results().expr_ty(expr); + let receiver_ty = cx.typeck_results().expr_ty(receiver); + + // Check if the receiver type is a NonZero type + if let ty::Adt(adt_def, _) = receiver_ty.kind() + && adt_def.is_struct() + && cx.tcx.get_diagnostic_name(adt_def.did()) == Some(sym::NonZero) + { + if let Some(target_non_zero_type) = get_target_non_zero_type(target_ty) { + let arg_snippet = get_arg_snippet(cx, arg, rcv_path); + suggest_non_zero_conversion(cx, expr, fn_name, target_non_zero_type, &arg_snippet, applicability); } } } @@ -85,6 +105,7 @@ fn suggest_non_zero_conversion( fn_name: rustc_span::Symbol, target_non_zero_type: &str, arg_snippet: &str, + applicability: Applicability, ) { let suggestion = format!("{target_non_zero_type}::{fn_name}({arg_snippet})"); span_lint_and_sugg( @@ -94,7 +115,7 @@ fn suggest_non_zero_conversion( format!("consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"), "replace with", suggestion, - Applicability::MachineApplicable, + applicability, ); } diff --git a/tests/ui/non_zero_suggestions.fixed b/tests/ui/non_zero_suggestions.fixed index d7e6b19edc1f..9851063782e7 100644 --- a/tests/ui/non_zero_suggestions.fixed +++ b/tests/ui/non_zero_suggestions.fixed @@ -1,5 +1,4 @@ #![warn(clippy::non_zero_suggestions)] - use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; fn main() { @@ -19,43 +18,33 @@ fn main() { let r3 = a / NonZeroU32::from(b); //~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion - // I8 -> I16 - let c: i16 = 25; - let d = NonZeroI8::new(3).unwrap(); - let r4 = NonZeroI16::from(d); - //~^ ERROR: consider using `NonZeroI16::from()` for more efficient and type-safe conversion - - // Different operations - let m: u64 = 400; - let n = NonZeroU32::new(20).unwrap(); - let r5 = m / NonZeroU64::from(n); + let x = NonZeroU64::from(NonZeroU32::new(5).unwrap()); //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - /// Edge cases - // Using the max value of a type - let max_u32 = NonZeroU32::new(u32::MAX).unwrap(); - let r6 = NonZeroU64::from(max_u32); - //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + /// Negative test cases (lint should not trigger) + // Left hand side expressions should not be triggered + let c: u32 = 50; + let d = NonZeroU16::new(5).unwrap(); + let r4 = u32::from(b.get()) / a; - // Chained method calls - let _ = NonZeroU64::from(NonZeroU32::new(10).unwrap()); - //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + // Should not trigger for any other operand other than `/` and `%` + let r5 = a + u32::from(b.get()); + let r6 = a - u32::from(b.get()); - /// Negative test cases (lint should not trigger) // Same size types let e: u32 = 200; let f = NonZeroU32::new(20).unwrap(); - let r10 = e / f.get(); + let r7 = e / f.get(); // Smaller to larger, but not NonZero let g: u64 = 1000; let h: u32 = 50; - let r11 = g / u64::from(h); + let r8 = g / u64::from(h); // Using From correctly let k: u64 = 300; let l = NonZeroU32::new(15).unwrap(); - let r12 = k / NonZeroU64::from(l); + let r9 = k / NonZeroU64::from(l); } // Additional function to test the lint in a different context @@ -64,13 +53,6 @@ fn divide_numbers(x: u64, y: NonZeroU32) -> u64 { //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion } -fn no_bin_exp(x: u64, y: NonZeroU32) -> u64 { - NonZeroU64::from(y) - //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion -} - -fn some_fn_that_only_takes_u64(_: u64) {} - struct Calculator { value: u64, } @@ -78,5 +60,6 @@ struct Calculator { impl Calculator { fn divide(&self, divisor: NonZeroU32) -> u64 { self.value / NonZeroU64::from(divisor) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion } } diff --git a/tests/ui/non_zero_suggestions.rs b/tests/ui/non_zero_suggestions.rs index 8f256dabcb84..1605c459248c 100644 --- a/tests/ui/non_zero_suggestions.rs +++ b/tests/ui/non_zero_suggestions.rs @@ -1,5 +1,4 @@ #![warn(clippy::non_zero_suggestions)] - use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; fn main() { @@ -19,43 +18,33 @@ fn main() { let r3 = a / u32::from(b.get()); //~^ ERROR: consider using `NonZeroU32::from()` for more efficient and type-safe conversion - // I8 -> I16 - let c: i16 = 25; - let d = NonZeroI8::new(3).unwrap(); - let r4 = i16::from(d.get()); - //~^ ERROR: consider using `NonZeroI16::from()` for more efficient and type-safe conversion - - // Different operations - let m: u64 = 400; - let n = NonZeroU32::new(20).unwrap(); - let r5 = m / u64::from(n.get()); + let x = u64::from(NonZeroU32::new(5).unwrap().get()); //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - /// Edge cases - // Using the max value of a type - let max_u32 = NonZeroU32::new(u32::MAX).unwrap(); - let r6 = u64::from(max_u32.get()); - //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + /// Negative test cases (lint should not trigger) + // Left hand side expressions should not be triggered + let c: u32 = 50; + let d = NonZeroU16::new(5).unwrap(); + let r4 = u32::from(b.get()) / a; - // Chained method calls - let _ = u64::from(NonZeroU32::new(10).unwrap().get()); - //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + // Should not trigger for any other operand other than `/` and `%` + let r5 = a + u32::from(b.get()); + let r6 = a - u32::from(b.get()); - /// Negative test cases (lint should not trigger) // Same size types let e: u32 = 200; let f = NonZeroU32::new(20).unwrap(); - let r10 = e / f.get(); + let r7 = e / f.get(); // Smaller to larger, but not NonZero let g: u64 = 1000; let h: u32 = 50; - let r11 = g / u64::from(h); + let r8 = g / u64::from(h); // Using From correctly let k: u64 = 300; let l = NonZeroU32::new(15).unwrap(); - let r12 = k / NonZeroU64::from(l); + let r9 = k / NonZeroU64::from(l); } // Additional function to test the lint in a different context @@ -64,13 +53,6 @@ fn divide_numbers(x: u64, y: NonZeroU32) -> u64 { //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion } -fn no_bin_exp(x: u64, y: NonZeroU32) -> u64 { - u64::from(y.get()) - //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion -} - -fn some_fn_that_only_takes_u64(_: u64) {} - struct Calculator { value: u64, } @@ -78,5 +60,6 @@ struct Calculator { impl Calculator { fn divide(&self, divisor: NonZeroU32) -> u64 { self.value / u64::from(divisor.get()) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion } } diff --git a/tests/ui/non_zero_suggestions.stderr b/tests/ui/non_zero_suggestions.stderr index b231c8d216ed..7a57f7983be7 100644 --- a/tests/ui/non_zero_suggestions.stderr +++ b/tests/ui/non_zero_suggestions.stderr @@ -1,5 +1,5 @@ error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:10:18 + --> tests/ui/non_zero_suggestions.rs:9:18 | LL | let r1 = x / u64::from(y.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` @@ -8,58 +8,34 @@ LL | let r1 = x / u64::from(y.get()); = help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:13:18 + --> tests/ui/non_zero_suggestions.rs:12:18 | LL | let r2 = x % u64::from(y.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` error: consider using `NonZeroU32::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:19:18 + --> tests/ui/non_zero_suggestions.rs:18:18 | LL | let r3 = a / u32::from(b.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU32::from(b)` -error: consider using `NonZeroI16::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:25:14 - | -LL | let r4 = i16::from(d.get()); - | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroI16::from(d)` - -error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:31:18 - | -LL | let r5 = m / u64::from(n.get()); - | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)` - error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:37:14 + --> tests/ui/non_zero_suggestions.rs:21:13 | -LL | let r6 = u64::from(max_u32.get()); - | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(max_u32)` +LL | let x = u64::from(NonZeroU32::new(5).unwrap().get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(5).unwrap())` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:41:13 - | -LL | let _ = u64::from(NonZeroU32::new(10).unwrap().get()); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(10).unwrap())` - -error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:63:9 + --> tests/ui/non_zero_suggestions.rs:52:9 | LL | x / u64::from(y.get()) | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:68:5 - | -LL | u64::from(y.get()) - | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` - -error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions.rs:80:22 + --> tests/ui/non_zero_suggestions.rs:62:22 | LL | self.value / u64::from(divisor.get()) | ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(divisor)` -error: aborting due to 10 previous errors +error: aborting due to 6 previous errors diff --git a/tests/ui/non_zero_suggestions_unfixable.rs b/tests/ui/non_zero_suggestions_unfixable.rs new file mode 100644 index 000000000000..71f5f94dcc7b --- /dev/null +++ b/tests/ui/non_zero_suggestions_unfixable.rs @@ -0,0 +1,20 @@ +#![warn(clippy::non_zero_suggestions)] +//@no-rustfix +use std::num::{NonZeroI16, NonZeroI8, NonZeroU16, NonZeroU32, NonZeroU64, NonZeroU8, NonZeroUsize}; + +fn main() { + let x: u64 = u64::from(NonZeroU32::new(5).unwrap().get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + + let n = NonZeroU32::new(20).unwrap(); + let y = u64::from(n.get()); + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + some_fn_that_only_takes_u64(y); +} + +fn return_non_zero(x: u64, y: NonZeroU32) -> u64 { + u64::from(y.get()) + //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion +} + +fn some_fn_that_only_takes_u64(_: u64) {} diff --git a/tests/ui/non_zero_suggestions_unfixable.stderr b/tests/ui/non_zero_suggestions_unfixable.stderr new file mode 100644 index 000000000000..5e22fddbb1a4 --- /dev/null +++ b/tests/ui/non_zero_suggestions_unfixable.stderr @@ -0,0 +1,23 @@ +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions_unfixable.rs:6:18 + | +LL | let x: u64 = u64::from(NonZeroU32::new(5).unwrap().get()); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(NonZeroU32::new(5).unwrap())` + | + = note: `-D clippy::non-zero-suggestions` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::non_zero_suggestions)]` + +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions_unfixable.rs:10:13 + | +LL | let y = u64::from(n.get()); + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)` + +error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion + --> tests/ui/non_zero_suggestions_unfixable.rs:16:5 + | +LL | u64::from(y.get()) + | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)` + +error: aborting due to 3 previous errors + From af3346a85ffd0952f11581d83c6d6d3d57ae205b Mon Sep 17 00:00:00 2001 From: Samarth1696 Date: Sat, 31 Aug 2024 11:53:16 +0530 Subject: [PATCH 8/8] Check for get method and new test case in unfixable --- clippy_lints/src/non_zero_suggestions.rs | 1 + tests/ui/non_zero_suggestions_unfixable.rs | 3 +++ tests/ui/non_zero_suggestions_unfixable.stderr | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/non_zero_suggestions.rs b/clippy_lints/src/non_zero_suggestions.rs index 808de147d722..90a9f2e994b8 100644 --- a/clippy_lints/src/non_zero_suggestions.rs +++ b/clippy_lints/src/non_zero_suggestions.rs @@ -72,6 +72,7 @@ fn check_non_zero_conversion(cx: &LateContext<'_>, expr: &Expr<'_>, applicabilit && let ExprKind::Path(qpath) = &func.kind && let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() && let ExprKind::MethodCall(rcv_path, receiver, _, _) = &arg.kind + && rcv_path.ident.name.as_str() == "get" { let fn_name = cx.tcx.item_name(def_id); let target_ty = cx.typeck_results().expr_ty(expr); diff --git a/tests/ui/non_zero_suggestions_unfixable.rs b/tests/ui/non_zero_suggestions_unfixable.rs index 71f5f94dcc7b..4eb22a8d4c71 100644 --- a/tests/ui/non_zero_suggestions_unfixable.rs +++ b/tests/ui/non_zero_suggestions_unfixable.rs @@ -10,6 +10,9 @@ fn main() { let y = u64::from(n.get()); //~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion some_fn_that_only_takes_u64(y); + + let m = NonZeroU32::try_from(1).unwrap(); + let _z: NonZeroU64 = m.into(); } fn return_non_zero(x: u64, y: NonZeroU32) -> u64 { diff --git a/tests/ui/non_zero_suggestions_unfixable.stderr b/tests/ui/non_zero_suggestions_unfixable.stderr index 5e22fddbb1a4..787179f2a2d6 100644 --- a/tests/ui/non_zero_suggestions_unfixable.stderr +++ b/tests/ui/non_zero_suggestions_unfixable.stderr @@ -14,7 +14,7 @@ LL | let y = u64::from(n.get()); | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(n)` error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion - --> tests/ui/non_zero_suggestions_unfixable.rs:16:5 + --> tests/ui/non_zero_suggestions_unfixable.rs:19:5 | LL | u64::from(y.get()) | ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`