Auto merge of #13167 - Samarth1696:tests, r=llogiq
Added new `non_zero_suggestions` lint Created lint based on the suggestions in #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
This commit is contained in:
commit
d9c4523e6c
@ -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
|
||||
|
@ -557,6 +557,7 @@
|
||||
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,
|
||||
|
@ -273,6 +273,7 @@
|
||||
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`
|
||||
}
|
||||
|
143
clippy_lints/src/non_zero_suggestions.rs
Normal file
143
clippy_lints/src/non_zero_suggestions.rs
Normal file
@ -0,0 +1,143 @@
|
||||
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};
|
||||
use rustc_middle::ty::{self, Ty};
|
||||
use rustc_session::declare_lint_pass;
|
||||
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
|
||||
/// 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
|
||||
/// 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,
|
||||
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::Binary(op, _, rhs) = expr.kind
|
||||
&& matches!(op.node, BinOpKind::Div | BinOpKind::Rem)
|
||||
{
|
||||
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 !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
|
||||
&& 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);
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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,
|
||||
applicability: Applicability,
|
||||
) {
|
||||
let suggestion = format!("{target_non_zero_type}::{fn_name}({arg_snippet})");
|
||||
span_lint_and_sugg(
|
||||
cx,
|
||||
NON_ZERO_SUGGESTIONS,
|
||||
expr.span,
|
||||
format!("consider using `{target_non_zero_type}::{fn_name}()` for more efficient and type-safe conversion"),
|
||||
"replace with",
|
||||
suggestion,
|
||||
applicability,
|
||||
);
|
||||
}
|
||||
|
||||
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,
|
||||
}
|
||||
}
|
65
tests/ui/non_zero_suggestions.fixed
Normal file
65
tests/ui/non_zero_suggestions.fixed
Normal file
@ -0,0 +1,65 @@
|
||||
#![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);
|
||||
//~^ 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
|
||||
|
||||
let x = NonZeroU64::from(NonZeroU32::new(5).unwrap());
|
||||
//~^ 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;
|
||||
|
||||
// Should not trigger for any other operand other than `/` and `%`
|
||||
let r5 = a + u32::from(b.get());
|
||||
let r6 = a - u32::from(b.get());
|
||||
|
||||
// Same size types
|
||||
let e: u32 = 200;
|
||||
let f = NonZeroU32::new(20).unwrap();
|
||||
let r7 = e / f.get();
|
||||
|
||||
// Smaller to larger, but not NonZero
|
||||
let g: u64 = 1000;
|
||||
let h: u32 = 50;
|
||||
let r8 = g / u64::from(h);
|
||||
|
||||
// Using From correctly
|
||||
let k: u64 = 300;
|
||||
let l = NonZeroU32::new(15).unwrap();
|
||||
let r9 = 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)
|
||||
//~^ ERROR: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
|
||||
}
|
||||
|
||||
struct Calculator {
|
||||
value: u64,
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
}
|
65
tests/ui/non_zero_suggestions.rs
Normal file
65
tests/ui/non_zero_suggestions.rs
Normal file
@ -0,0 +1,65 @@
|
||||
#![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 / 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
|
||||
|
||||
let x = u64::from(NonZeroU32::new(5).unwrap().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;
|
||||
|
||||
// Should not trigger for any other operand other than `/` and `%`
|
||||
let r5 = a + u32::from(b.get());
|
||||
let r6 = a - u32::from(b.get());
|
||||
|
||||
// Same size types
|
||||
let e: u32 = 200;
|
||||
let f = NonZeroU32::new(20).unwrap();
|
||||
let r7 = e / f.get();
|
||||
|
||||
// Smaller to larger, but not NonZero
|
||||
let g: u64 = 1000;
|
||||
let h: u32 = 50;
|
||||
let r8 = g / u64::from(h);
|
||||
|
||||
// Using From correctly
|
||||
let k: u64 = 300;
|
||||
let l = NonZeroU32::new(15).unwrap();
|
||||
let r9 = k / NonZeroU64::from(l);
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
|
||||
struct Calculator {
|
||||
value: u64,
|
||||
}
|
||||
|
||||
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
|
||||
}
|
||||
}
|
41
tests/ui/non_zero_suggestions.stderr
Normal file
41
tests/ui/non_zero_suggestions.stderr
Normal file
@ -0,0 +1,41 @@
|
||||
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
|
||||
--> tests/ui/non_zero_suggestions.rs:9: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:18:18
|
||||
|
|
||||
LL | let r3 = a / u32::from(b.get());
|
||||
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU32::from(b)`
|
||||
|
||||
error: consider using `NonZeroU64::from()` for more efficient and type-safe conversion
|
||||
--> tests/ui/non_zero_suggestions.rs:21:13
|
||||
|
|
||||
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: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:62:22
|
||||
|
|
||||
LL | self.value / u64::from(divisor.get())
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(divisor)`
|
||||
|
||||
error: aborting due to 6 previous errors
|
||||
|
23
tests/ui/non_zero_suggestions_unfixable.rs
Normal file
23
tests/ui/non_zero_suggestions_unfixable.rs
Normal file
@ -0,0 +1,23 @@
|
||||
#![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);
|
||||
|
||||
let m = NonZeroU32::try_from(1).unwrap();
|
||||
let _z: NonZeroU64 = m.into();
|
||||
}
|
||||
|
||||
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) {}
|
23
tests/ui/non_zero_suggestions_unfixable.stderr
Normal file
23
tests/ui/non_zero_suggestions_unfixable.stderr
Normal file
@ -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:19:5
|
||||
|
|
||||
LL | u64::from(y.get())
|
||||
| ^^^^^^^^^^^^^^^^^^ help: replace with: `NonZeroU64::from(y)`
|
||||
|
||||
error: aborting due to 3 previous errors
|
||||
|
Loading…
Reference in New Issue
Block a user