about summary refs log tree commit diff
diff options
context:
space:
mode:
authorManish Goregaokar <manishsmail@gmail.com>2025-01-15 01:00:54 +0000
committerGitHub <noreply@github.com>2025-01-15 01:00:54 +0000
commit25d319d1b2536e4c22b8013b95bd140a23ed78d7 (patch)
tree8d2a815332ef5fbf435d1bdd5c7f4405f12e1d50
parent98761e4812f564d86030bc6596bee3102a3ca6c5 (diff)
parent35dbaf8a618403e4a9ae2da04eb479d6450c7c88 (diff)
downloadrust-25d319d1b2536e4c22b8013b95bd140a23ed78d7.tar.gz
rust-25d319d1b2536e4c22b8013b95bd140a23ed78d7.zip
New lint `useless-nonzero-new_unchecked` (#13993)
changelog: [`useless-nonzero-new_unchecked`]: new lint

Close #13991

### What it does

Checks for `NonZero*::new_unchecked(<literal>)` being used in a `const`
context.

### Why is this bad?

Using `NonZero*::new_unchecked()` is an `unsafe` function and requires
an `unsafe` context. When used with an
integer literal in a `const` context, `NonZero*::new().unwrap()` will
provide the same result with identical
runtime performances while not requiring `unsafe`.

### Example
```no_run
const PLAYERS: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(3) };
```
Use instead:
```no_run
const PLAYERS: NonZeroUsize = NonZeroUsize::new(3).unwrap();
```
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--clippy_lints/src/methods/mod.rs30
-rw-r--r--clippy_lints/src/methods/useless_nonzero_new_unchecked.rs59
-rw-r--r--clippy_utils/src/msrvs.rs2
-rw-r--r--tests/ui/useless_nonzero_new_unchecked.fixed52
-rw-r--r--tests/ui/useless_nonzero_new_unchecked.rs52
-rw-r--r--tests/ui/useless_nonzero_new_unchecked.stderr37
8 files changed, 233 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2c71281acd4..ceb08eeac3f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6218,6 +6218,7 @@ Released 2018-09-13
 [`useless_conversion`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_conversion
 [`useless_format`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_format
 [`useless_let_if_seq`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_let_if_seq
+[`useless_nonzero_new_unchecked`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_nonzero_new_unchecked
 [`useless_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_transmute
 [`useless_vec`]: https://rust-lang.github.io/rust-clippy/master/index.html#useless_vec
 [`vec_box`]: https://rust-lang.github.io/rust-clippy/master/index.html#vec_box
diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs
index ac933d7cd70..5e6cfd94283 100644
--- a/clippy_lints/src/declared_lints.rs
+++ b/clippy_lints/src/declared_lints.rs
@@ -496,6 +496,7 @@ pub static LINTS: &[&crate::LintInfo] = &[
     crate::methods::UNWRAP_OR_DEFAULT_INFO,
     crate::methods::UNWRAP_USED_INFO,
     crate::methods::USELESS_ASREF_INFO,
+    crate::methods::USELESS_NONZERO_NEW_UNCHECKED_INFO,
     crate::methods::VEC_RESIZE_TO_ZERO_INFO,
     crate::methods::VERBOSE_FILE_READS_INFO,
     crate::methods::WAKER_CLONE_WAKE_INFO,
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index f66466896e5..d7e9f65bfa3 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -130,6 +130,7 @@ mod unnecessary_to_owned;
 mod unused_enumerate_index;
 mod unwrap_expect_used;
 mod useless_asref;
+mod useless_nonzero_new_unchecked;
 mod utils;
 mod vec_resize_to_zero;
 mod verbose_file_reads;
@@ -4311,6 +4312,33 @@ declare_clippy_lint! {
     "using `Iterator::last` on a `DoubleEndedIterator`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    ///
+    /// Checks for `NonZero*::new_unchecked()` being used in a `const` context.
+    ///
+    /// ### Why is this bad?
+    ///
+    /// Using `NonZero*::new_unchecked()` is an `unsafe` function and requires an `unsafe` context. When used in a
+    /// context evaluated at compilation time, `NonZero*::new().unwrap()` will provide the same result with identical
+    /// runtime performances while not requiring `unsafe`.
+    ///
+    /// ### Example
+    /// ```no_run
+    /// use std::num::NonZeroUsize;
+    /// const PLAYERS: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(3) };
+    /// ```
+    /// Use instead:
+    /// ```no_run
+    /// use std::num::NonZeroUsize;
+    /// const PLAYERS: NonZeroUsize = NonZeroUsize::new(3).unwrap();
+    /// ```
+    #[clippy::version = "1.86.0"]
+    pub USELESS_NONZERO_NEW_UNCHECKED,
+    complexity,
+    "using `NonZero::new_unchecked()` in a `const` context"
+}
+
 pub struct Methods {
     avoid_breaking_exported_api: bool,
     msrv: Msrv,
@@ -4477,6 +4505,7 @@ impl_lint_pass!(Methods => [
     MAP_WITH_UNUSED_ARGUMENT_OVER_RANGES,
     UNNECESSARY_MAP_OR,
     DOUBLE_ENDED_ITERATOR_LAST,
+    USELESS_NONZERO_NEW_UNCHECKED,
 ]);
 
 /// Extracts a method call name, args, and `Span` of the method name.
@@ -4505,6 +4534,7 @@ impl<'tcx> LateLintPass<'tcx> for Methods {
                 from_iter_instead_of_collect::check(cx, expr, args, func);
                 unnecessary_fallible_conversions::check_function(cx, expr, func);
                 manual_c_str_literals::check(cx, expr, func, args, &self.msrv);
+                useless_nonzero_new_unchecked::check(cx, expr, func, args, &self.msrv);
             },
             ExprKind::MethodCall(method_call, receiver, args, _) => {
                 let method_span = method_call.ident.span;
diff --git a/clippy_lints/src/methods/useless_nonzero_new_unchecked.rs b/clippy_lints/src/methods/useless_nonzero_new_unchecked.rs
new file mode 100644
index 00000000000..0bd50429c09
--- /dev/null
+++ b/clippy_lints/src/methods/useless_nonzero_new_unchecked.rs
@@ -0,0 +1,59 @@
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then};
+use clippy_utils::is_inside_always_const_context;
+use clippy_utils::msrvs::{self, Msrv};
+use clippy_utils::source::snippet_with_applicability;
+use clippy_utils::ty::is_type_diagnostic_item;
+use rustc_errors::Applicability;
+use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, Node, QPath, UnsafeSource};
+use rustc_lint::LateContext;
+use rustc_span::sym;
+
+use super::USELESS_NONZERO_NEW_UNCHECKED;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, func: &Expr<'tcx>, args: &[Expr<'_>], msrv: &Msrv) {
+    if msrv.meets(msrvs::CONST_UNWRAP)
+        && let ExprKind::Path(QPath::TypeRelative(ty, segment)) = func.kind
+        && segment.ident.name == sym::new_unchecked
+        && let [init_arg] = args
+        && is_inside_always_const_context(cx.tcx, expr.hir_id)
+        && is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::NonZero)
+    {
+        let mut app = Applicability::MachineApplicable;
+        let ty_str = snippet_with_applicability(cx, ty.span, "_", &mut app);
+        let msg = format!("`{ty_str}::new()` and `Option::unwrap()` can be safely used in a `const` context");
+        let sugg = format!(
+            "{ty_str}::new({}).unwrap()",
+            snippet_with_applicability(cx, init_arg.span, "_", &mut app)
+        );
+
+        if let Node::Block(Block {
+            stmts: [],
+            span: block_span,
+            rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
+            ..
+        }) = cx.tcx.parent_hir_node(expr.hir_id)
+        {
+            if !block_span.from_expansion() {
+                // The expression is the only component of an `unsafe` block. Propose
+                // to replace the block altogether.
+                span_lint_and_sugg(
+                    cx,
+                    USELESS_NONZERO_NEW_UNCHECKED,
+                    *block_span,
+                    msg,
+                    "use instead",
+                    sugg,
+                    app,
+                );
+            }
+        } else {
+            // The expression is enclosed in a larger `unsafe` context. Indicate that
+            // this may no longer be needed for the fixed expression.
+            span_lint_and_then(cx, USELESS_NONZERO_NEW_UNCHECKED, expr.span, msg, |diagnostic| {
+                diagnostic
+                    .span_suggestion(expr.span, "use instead", sugg, app)
+                    .note("the fixed expression does not require an `unsafe` context");
+            });
+        }
+    }
+}
diff --git a/clippy_utils/src/msrvs.rs b/clippy_utils/src/msrvs.rs
index 62b70ef6af4..eeb96a6ccb8 100644
--- a/clippy_utils/src/msrvs.rs
+++ b/clippy_utils/src/msrvs.rs
@@ -18,7 +18,7 @@ macro_rules! msrv_aliases {
 
 // names may refer to stabilized feature flags or library items
 msrv_aliases! {
-    1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY }
+    1,83,0 { CONST_EXTERN_FN, CONST_FLOAT_BITS_CONV, CONST_FLOAT_CLASSIFY, CONST_UNWRAP }
     1,82,0 { IS_NONE_OR, REPEAT_N, RAW_REF_OP }
     1,81,0 { LINT_REASONS_STABILIZATION, ERROR_IN_CORE, EXPLICIT_SELF_TYPE_ELISION }
     1,80,0 { BOX_INTO_ITER }
diff --git a/tests/ui/useless_nonzero_new_unchecked.fixed b/tests/ui/useless_nonzero_new_unchecked.fixed
new file mode 100644
index 00000000000..03b34afa54e
--- /dev/null
+++ b/tests/ui/useless_nonzero_new_unchecked.fixed
@@ -0,0 +1,52 @@
+#![warn(clippy::useless_nonzero_new_unchecked)]
+
+use std::num::{NonZero, NonZeroUsize};
+
+#[clippy::msrv = "1.83"]
+const fn func() -> NonZeroUsize {
+    const { NonZeroUsize::new(3).unwrap() }
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+}
+
+#[clippy::msrv = "1.82"]
+const fn func_older() -> NonZeroUsize {
+    unsafe { NonZeroUsize::new_unchecked(3) }
+}
+
+const fn func_performance_hit_if_linted() -> NonZeroUsize {
+    unsafe { NonZeroUsize::new_unchecked(3) }
+}
+
+const fn func_may_panic_at_run_time_if_linted(x: usize) -> NonZeroUsize {
+    unsafe { NonZeroUsize::new_unchecked(x) }
+}
+
+macro_rules! uns {
+    ($expr:expr) => {
+        unsafe { $expr }
+    };
+}
+
+macro_rules! nzu {
+    () => {
+        NonZeroUsize::new_unchecked(1)
+    };
+}
+
+fn main() {
+    const _A: NonZeroUsize = NonZeroUsize::new(3).unwrap();
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    static _B: NonZero<u8> = NonZero::<u8>::new(42).unwrap();
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    const _C: usize = unsafe { NonZeroUsize::new(3).unwrap().get() };
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    const AUX: usize = 3;
+    const _D: NonZeroUsize = NonZeroUsize::new(AUX).unwrap();
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    const _X: NonZeroUsize = uns!(NonZeroUsize::new_unchecked(3));
+    const _Y: NonZeroUsize = unsafe { nzu!() };
+}
diff --git a/tests/ui/useless_nonzero_new_unchecked.rs b/tests/ui/useless_nonzero_new_unchecked.rs
new file mode 100644
index 00000000000..d450e3a03ec
--- /dev/null
+++ b/tests/ui/useless_nonzero_new_unchecked.rs
@@ -0,0 +1,52 @@
+#![warn(clippy::useless_nonzero_new_unchecked)]
+
+use std::num::{NonZero, NonZeroUsize};
+
+#[clippy::msrv = "1.83"]
+const fn func() -> NonZeroUsize {
+    const { unsafe { NonZeroUsize::new_unchecked(3) } }
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+}
+
+#[clippy::msrv = "1.82"]
+const fn func_older() -> NonZeroUsize {
+    unsafe { NonZeroUsize::new_unchecked(3) }
+}
+
+const fn func_performance_hit_if_linted() -> NonZeroUsize {
+    unsafe { NonZeroUsize::new_unchecked(3) }
+}
+
+const fn func_may_panic_at_run_time_if_linted(x: usize) -> NonZeroUsize {
+    unsafe { NonZeroUsize::new_unchecked(x) }
+}
+
+macro_rules! uns {
+    ($expr:expr) => {
+        unsafe { $expr }
+    };
+}
+
+macro_rules! nzu {
+    () => {
+        NonZeroUsize::new_unchecked(1)
+    };
+}
+
+fn main() {
+    const _A: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(3) };
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    static _B: NonZero<u8> = unsafe { NonZero::<u8>::new_unchecked(42) };
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    const _C: usize = unsafe { NonZeroUsize::new_unchecked(3).get() };
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    const AUX: usize = 3;
+    const _D: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(AUX) };
+    //~^ ERROR: `Option::unwrap()` can be safely used in a `const` context
+
+    const _X: NonZeroUsize = uns!(NonZeroUsize::new_unchecked(3));
+    const _Y: NonZeroUsize = unsafe { nzu!() };
+}
diff --git a/tests/ui/useless_nonzero_new_unchecked.stderr b/tests/ui/useless_nonzero_new_unchecked.stderr
new file mode 100644
index 00000000000..adb14616763
--- /dev/null
+++ b/tests/ui/useless_nonzero_new_unchecked.stderr
@@ -0,0 +1,37 @@
+error: `NonZeroUsize::new()` and `Option::unwrap()` can be safely used in a `const` context
+  --> tests/ui/useless_nonzero_new_unchecked.rs:7:13
+   |
+LL |     const { unsafe { NonZeroUsize::new_unchecked(3) } }
+   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use instead: `NonZeroUsize::new(3).unwrap()`
+   |
+   = note: `-D clippy::useless-nonzero-new-unchecked` implied by `-D warnings`
+   = help: to override `-D warnings` add `#[allow(clippy::useless_nonzero_new_unchecked)]`
+
+error: `NonZeroUsize::new()` and `Option::unwrap()` can be safely used in a `const` context
+  --> tests/ui/useless_nonzero_new_unchecked.rs:37:30
+   |
+LL |     const _A: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(3) };
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use instead: `NonZeroUsize::new(3).unwrap()`
+
+error: `NonZero::<u8>::new()` and `Option::unwrap()` can be safely used in a `const` context
+  --> tests/ui/useless_nonzero_new_unchecked.rs:40:30
+   |
+LL |     static _B: NonZero<u8> = unsafe { NonZero::<u8>::new_unchecked(42) };
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use instead: `NonZero::<u8>::new(42).unwrap()`
+
+error: `NonZeroUsize::new()` and `Option::unwrap()` can be safely used in a `const` context
+  --> tests/ui/useless_nonzero_new_unchecked.rs:43:32
+   |
+LL |     const _C: usize = unsafe { NonZeroUsize::new_unchecked(3).get() };
+   |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use instead: `NonZeroUsize::new(3).unwrap()`
+   |
+   = note: the fixed expression does not require an `unsafe` context
+
+error: `NonZeroUsize::new()` and `Option::unwrap()` can be safely used in a `const` context
+  --> tests/ui/useless_nonzero_new_unchecked.rs:47:30
+   |
+LL |     const _D: NonZeroUsize = unsafe { NonZeroUsize::new_unchecked(AUX) };
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: use instead: `NonZeroUsize::new(AUX).unwrap()`
+
+error: aborting due to 5 previous errors
+