Skip to content

Commit

Permalink
New lint useless-nonzero-new_unchecked (#13993)
Browse files Browse the repository at this point in the history
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();
```
  • Loading branch information
Manishearth authored Jan 15, 2025
2 parents 98761e4 + 35dbaf8 commit 25d319d
Show file tree
Hide file tree
Showing 8 changed files with 233 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 30 additions & 0 deletions clippy_lints/src/methods/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
59 changes: 59 additions & 0 deletions clippy_lints/src/methods/useless_nonzero_new_unchecked.rs
Original file line number Diff line number Diff line change
@@ -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");
});
}
}
}
2 changes: 1 addition & 1 deletion clippy_utils/src/msrvs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
52 changes: 52 additions & 0 deletions tests/ui/useless_nonzero_new_unchecked.fixed
Original file line number Diff line number Diff line change
@@ -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!() };
}
52 changes: 52 additions & 0 deletions tests/ui/useless_nonzero_new_unchecked.rs
Original file line number Diff line number Diff line change
@@ -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!() };
}
37 changes: 37 additions & 0 deletions tests/ui/useless_nonzero_new_unchecked.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 25d319d

Please sign in to comment.