-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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
- Loading branch information
Showing
9 changed files
with
364 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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, | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) {} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 | ||
|