Skip to content

Commit e42d365

Browse files
Centri3Centri3
committed
New lint [unnecessary_unwrap_unchecked]
1 parent 3be3fb7 commit e42d365

File tree

7 files changed

+335
-1
lines changed

7 files changed

+335
-1
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5277,6 +5277,7 @@ Released 2018-09-13
52775277
[`unnecessary_struct_initialization`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_struct_initialization
52785278
[`unnecessary_to_owned`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_to_owned
52795279
[`unnecessary_unwrap`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap
5280+
[`unnecessary_unwrap_unchecked`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_unwrap_unchecked
52805281
[`unnecessary_wraps`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_wraps
52815282
[`unneeded_field_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_field_pattern
52825283
[`unneeded_wildcard_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#unneeded_wildcard_pattern

‎clippy_lints/src/declared_lints.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
419419
crate::methods::UNNECESSARY_LITERAL_UNWRAP_INFO,
420420
crate::methods::UNNECESSARY_SORT_BY_INFO,
421421
crate::methods::UNNECESSARY_TO_OWNED_INFO,
422+
crate::methods::UNNECESSARY_UNWRAP_UNCHECKED_INFO,
422423
crate::methods::UNWRAP_OR_ELSE_DEFAULT_INFO,
423424
crate::methods::UNWRAP_USED_INFO,
424425
crate::methods::USELESS_ASREF_INFO,

‎clippy_lints/src/methods/mod.rs

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ mod unnecessary_lazy_eval;
100100
mod unnecessary_literal_unwrap;
101101
mod unnecessary_sort_by;
102102
mod unnecessary_to_owned;
103+
mod unnecessary_unwrap_unchecked;
103104
mod unwrap_or_else_default;
104105
mod unwrap_used;
105106
mod useless_asref;
@@ -3378,6 +3379,28 @@ declare_clippy_lint! {
33783379
"calling `Stdin::read_line`, then trying to parse it without first trimming"
33793380
}
33803381

3382+
declare_clippy_lint! {
3383+
/// ### What it does
3384+
/// Checks for calls to `unwrap_unchecked` when an `_unchecked` variant of the function exists.
3385+
///
3386+
/// ### Why is this bad?
3387+
/// Calling the `_unchecked` variant instead alleviates a check that's entirely redundant if
3388+
/// `unwrap_unchecked` is called.
3389+
///
3390+
/// ### Example
3391+
/// ```rust
3392+
/// let s = unsafe { std::str::from_utf8(&[]).unwrap_unchecked() };
3393+
/// ```
3394+
/// Use instead:
3395+
/// ```rust
3396+
/// let s = unsafe { std::str::from_utf8_unchecked(&[]) };
3397+
/// ```
3398+
#[clippy::version = "1.72.0"]
3399+
pub UNNECESSARY_UNWRAP_UNCHECKED,
3400+
perf,
3401+
"calling `unwrap_unchecked` when an `_unchecked` variant of the function exists"
3402+
}
3403+
33813404
pub struct Methods {
33823405
avoid_breaking_exported_api: bool,
33833406
msrv: Msrv,
@@ -3512,6 +3535,7 @@ impl_lint_pass!(Methods => [
35123535
UNNECESSARY_LITERAL_UNWRAP,
35133536
DRAIN_COLLECT,
35143537
MANUAL_TRY_FOLD,
3538+
UNNECESSARY_UNWRAP_UNCHECKED,
35153539
]);
35163540

35173541
/// Extracts a method call name, args, and `Span` of the method name.
@@ -3999,6 +4023,9 @@ impl Methods {
39994023
}
40004024
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
40014025
unwrap_used::check(cx, expr, recv, false, self.allow_unwrap_in_tests);
4026+
}
4027+
("unwrap_unchecked", []) => {
4028+
unnecessary_unwrap_unchecked::check(cx, expr, recv, span);
40024029
},
40034030
("unwrap_err", []) => {
40044031
unnecessary_literal_unwrap::check(cx, expr, recv, name, args);
@@ -4204,7 +4231,7 @@ impl SelfKind {
42044231
};
42054232

42064233
let Some(trait_def_id) = cx.tcx.get_diagnostic_item(trait_sym) else {
4207-
return false
4234+
return false;
42084235
};
42094236
implements_trait(cx, ty, trait_def_id, &[parent_ty.into()])
42104237
}

‎clippy_lints/src/methods/unnecessary_unwrap_unchecked.rs

Lines changed: 181 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,181 @@
1+
use clippy_utils::{diagnostics::span_lint_and_help, is_from_proc_macro, last_path_segment, path_res};
2+
use rustc_hir::{def::DefKind, def_id::DefId, Expr, ExprKind};
3+
use rustc_hir_typeck::{FnCtxt, Inherited};
4+
use rustc_lint::LateContext;
5+
use rustc_middle::ty::Ty;
6+
use rustc_span::{symbol::Ident, Span};
7+
8+
use super::UNNECESSARY_UNWRAP_UNCHECKED;
9+
10+
#[derive(Clone, Copy, Debug)]
11+
struct VariantAndIdent {
12+
variant: Variant,
13+
ident: Ident,
14+
}
15+
16+
impl<'tcx> VariantAndIdent {
17+
fn new(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, recv: &Expr<'_>, fcx: &FnCtxt<'_, 'tcx>) -> Option<Self> {
18+
let expected_ret_ty = cx.typeck_results().expr_ty(expr);
19+
match recv.kind {
20+
// Construct `Variant::Fn(_)`, if applicable. This is necessary for us to handle
21+
// functions like `std::str::from_utf8_unchecked`.
22+
ExprKind::Call(path, _) if let ExprKind::Path(qpath) = path.kind
23+
&& let parent = cx.tcx.parent(path_res(cx, path).def_id())
24+
// Don't use `parent_module`. We only want to lint if its first parent is a `Mod`
25+
&& cx.tcx.def_kind(parent) == DefKind::Mod
26+
&& let children = parent.as_local().map_or_else(
27+
|| cx.tcx.module_children(parent),
28+
// We must use a !query for local modules to prevent an ICE.
29+
|parent| cx.tcx.module_children_local(parent),
30+
)
31+
&& !children.is_empty()
32+
&& let Some(unchecked_ident) = unchecked_ident(&last_path_segment(&qpath).ident)
33+
&& let Some(unchecked_def_id) = children
34+
.iter()
35+
.find(|child| child.ident == unchecked_ident)
36+
.map(|func| func.res.def_id())
37+
&& fn_ret_ty(cx, unchecked_def_id) == expected_ret_ty =>
38+
{
39+
Some(Self {
40+
variant: Variant::Fn(unchecked_def_id),
41+
ident: unchecked_ident,
42+
})
43+
},
44+
// We unfortunately must handle `A::a(&a)` and `a.a()` separately, this handles the
45+
// former
46+
ExprKind::Call(path, _) if let ExprKind::Path(qpath) = path.kind
47+
&& let parent = cx.tcx.parent(path_res(cx, path).def_id())
48+
// Don't use `parent_impl`. We only want to lint if its first parent is an `Impl`
49+
&& matches!(cx.tcx.def_kind(parent), DefKind::Impl { .. })
50+
&& let Some(unchecked_ident) = unchecked_ident(&last_path_segment(&qpath).ident)
51+
&& let Some(unchecked) = fcx.associated_value(parent, unchecked_ident)
52+
&& fn_ret_ty(cx, unchecked.def_id) == expected_ret_ty =>
53+
{
54+
Some(Self {
55+
variant: Variant::Assoc(AssocKind::new(unchecked.fn_has_self_parameter), unchecked.def_id),
56+
ident: unchecked_ident,
57+
})
58+
},
59+
// ... And now the latter ^^
60+
ExprKind::MethodCall(segment, _, _, _)
61+
if let Some(def_id) = cx.typeck_results().type_dependent_def_id(recv.hir_id)
62+
&& let parent = cx.tcx.parent(def_id)
63+
// Don't use `parent_impl`. We only want to lint if its first parent is an `Impl`
64+
&& matches!(cx.tcx.def_kind(parent), DefKind::Impl { .. })
65+
&& let ident = segment.ident.to_string()
66+
&& let Some(unchecked_ident) = unchecked_ident(&ident)
67+
&& let Some(unchecked) = fcx.associated_value(parent, unchecked_ident)
68+
&& fn_ret_ty(cx, unchecked.def_id) == expected_ret_ty =>
69+
{
70+
Some(Self {
71+
variant: Variant::Assoc(AssocKind::Method, unchecked.def_id),
72+
ident: unchecked_ident,
73+
})
74+
},
75+
_ => None,
76+
}
77+
}
78+
79+
fn msg(self) -> &'static str {
80+
// Don't use `format!` instead -- it won't be optimized out.
81+
match self.variant {
82+
Variant::Fn(_) => "usage of `unwrap_unchecked` when an `_unchecked` variant of the function exists",
83+
Variant::Assoc(kind, _) => {
84+
if kind.is_assoc_fn() {
85+
"usage of `unwrap_unchecked` when an `_unchecked` variant of the associated function exists"
86+
} else {
87+
"usage of `unwrap_unchecked` when an `_unchecked` variant of the method exists"
88+
}
89+
},
90+
}
91+
}
92+
93+
fn as_str(self) -> &'static str {
94+
match self.variant {
95+
Variant::Fn(_) => "function",
96+
Variant::Assoc(kind, _) => {
97+
if kind.is_assoc_fn() {
98+
"associated function"
99+
} else {
100+
"method"
101+
}
102+
},
103+
}
104+
}
105+
}
106+
107+
#[derive(Clone, Copy, Debug)]
108+
enum Variant {
109+
/// Free `fn` in a module. `DefId` is the `_unchecked` function.
110+
Fn(DefId),
111+
/// Associated item from an `impl`. `DefId` is the `_unchecked` associated item.
112+
Assoc(AssocKind, DefId),
113+
}
114+
115+
fn unchecked_ident(checked_ident: &impl ToString) -> Option<Ident> {
116+
let checked_ident = checked_ident.to_string();
117+
// Only add `_unchecked` if it doesn't already end with `_`
118+
(!checked_ident.ends_with('_')).then(|| Ident::from_str(&(checked_ident + "_unchecked")))
119+
}
120+
121+
fn fn_ret_ty<'tcx>(cx: &LateContext<'tcx>, def_id: DefId) -> Ty<'tcx> {
122+
cx.tcx.fn_sig(def_id).skip_binder().output().skip_binder()
123+
}
124+
125+
/// This only exists so the help message shows `associated function` or `method`, depending on
126+
/// whether it has a `self` parameter.
127+
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
128+
enum AssocKind {
129+
/// No `self`: `fn new() -> Self`
130+
Fn,
131+
/// Has `self`: `fn ty<'tcx>(&self) -> Ty<'tcx>`
132+
Method,
133+
}
134+
135+
impl AssocKind {
136+
fn new(fn_has_self_parameter: bool) -> Self {
137+
if fn_has_self_parameter {
138+
AssocKind::Method
139+
} else {
140+
AssocKind::Fn
141+
}
142+
}
143+
144+
fn is_assoc_fn(self) -> bool {
145+
self == Self::Fn
146+
}
147+
148+
#[expect(dead_code)]
149+
fn is_method(self) -> bool {
150+
self == Self::Method
151+
}
152+
}
153+
154+
pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, recv: &Expr<'_>, span: Span) {
155+
if !expr.span.from_expansion()
156+
&& let Some(variant) = VariantAndIdent::new(
157+
cx,
158+
expr,
159+
recv,
160+
&FnCtxt::new(
161+
&Inherited::new(cx.tcx, expr.hir_id.owner.def_id),
162+
cx.param_env,
163+
expr.hir_id.owner.def_id,
164+
),
165+
)
166+
&& !is_from_proc_macro(cx, expr)
167+
{
168+
span_lint_and_help(
169+
cx,
170+
UNNECESSARY_UNWRAP_UNCHECKED,
171+
span,
172+
variant.msg(),
173+
None,
174+
&format!(
175+
"call the {} `{}` instead, and remove the `unwrap_unchecked` call",
176+
variant.as_str(),
177+
variant.ident,
178+
),
179+
);
180+
}
181+
}

‎tests/ui/auxiliary/unnecessary_unwrap_unchecked_helper.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
#![allow(unused)]
2+
3+
pub fn lol() -> Option<u32> {
4+
Some(0)
5+
}
6+
7+
pub fn lol_unchecked() -> u32 {
8+
0
9+
}

‎tests/ui/unnecessary_unwrap_unchecked.rs

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
//@aux-build:unnecessary_unwrap_unchecked_helper.rs
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![allow(clippy::useless_vec, unused)]
4+
#![warn(clippy::unnecessary_unwrap_unchecked)]
5+
6+
#[macro_use]
7+
extern crate proc_macros;
8+
9+
use unnecessary_unwrap_unchecked_helper::lol;
10+
11+
mod b {
12+
pub fn test_fn() -> Option<u32> {
13+
Some(0)
14+
}
15+
16+
pub fn test_fn_unchecked() -> u32 {
17+
0
18+
}
19+
}
20+
21+
fn test_fn() -> Option<u32> {
22+
Some(0)
23+
}
24+
25+
fn test_fn_unchecked() -> u32 {
26+
0
27+
}
28+
29+
struct A;
30+
31+
impl A {
32+
fn a(&self) -> Option<u32> {
33+
Some(0)
34+
}
35+
36+
fn a_unchecked(&self) -> u32 {
37+
0
38+
}
39+
40+
fn an_assoc_fn() -> Option<u32> {
41+
Some(0)
42+
}
43+
44+
fn an_assoc_fn_unchecked() -> u32 {
45+
0
46+
}
47+
}
48+
49+
fn main() {
50+
let string_slice = unsafe { std::str::from_utf8(&[]).unwrap_unchecked() };
51+
let a = unsafe { A::a(&A).unwrap_unchecked() };
52+
let a = unsafe {
53+
let a = A;
54+
a.a().unwrap_unchecked()
55+
};
56+
let an_assoc_fn = unsafe { A::an_assoc_fn().unwrap_unchecked() };
57+
let extern_fn = unsafe { lol().unwrap_unchecked() };
58+
let local_fn = unsafe { b::test_fn().unwrap_unchecked() };
59+
macro_rules! local {
60+
() => {{
61+
unsafe { ::std::str::from_utf8(&[]).unwrap_unchecked() };
62+
}};
63+
}
64+
local!();
65+
external! {
66+
unsafe { std::str::from_utf8(&[]).unwrap_unchecked() };
67+
}
68+
with_span! {
69+
span
70+
unsafe { std::str::from_utf8(&[]).unwrap_unchecked() };
71+
}
72+
}

‎tests/ui/unnecessary_unwrap_unchecked.stderr

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
error: usage of `unwrap_unchecked` when an `_unchecked` variant of the method exists
2+
--> $DIR/unnecessary_unwrap_unchecked.rs:51:31
3+
|
4+
LL | let a = unsafe { A::a(&A).unwrap_unchecked() };
5+
| ^^^^^^^^^^^^^^^^
6+
|
7+
= help: call the method `a_unchecked` instead, and remove the `unwrap_unchecked` call
8+
= note: `-D clippy::unnecessary-unwrap-unchecked` implied by `-D warnings`
9+
10+
error: usage of `unwrap_unchecked` when an `_unchecked` variant of the method exists
11+
--> $DIR/unnecessary_unwrap_unchecked.rs:54:15
12+
|
13+
LL | a.a().unwrap_unchecked()
14+
| ^^^^^^^^^^^^^^^^
15+
|
16+
= help: call the method `a_unchecked` instead, and remove the `unwrap_unchecked` call
17+
18+
error: usage of `unwrap_unchecked` when an `_unchecked` variant of the associated function exists
19+
--> $DIR/unnecessary_unwrap_unchecked.rs:56:49
20+
|
21+
LL | let an_assoc_fn = unsafe { A::an_assoc_fn().unwrap_unchecked() };
22+
| ^^^^^^^^^^^^^^^^
23+
|
24+
= help: call the associated function `an_assoc_fn_unchecked` instead, and remove the `unwrap_unchecked` call
25+
26+
error: usage of `unwrap_unchecked` when an `_unchecked` variant of the function exists
27+
--> $DIR/unnecessary_unwrap_unchecked.rs:57:36
28+
|
29+
LL | let extern_fn = unsafe { lol().unwrap_unchecked() };
30+
| ^^^^^^^^^^^^^^^^
31+
|
32+
= help: call the function `lol_unchecked` instead, and remove the `unwrap_unchecked` call
33+
34+
error: usage of `unwrap_unchecked` when an `_unchecked` variant of the function exists
35+
--> $DIR/unnecessary_unwrap_unchecked.rs:58:42
36+
|
37+
LL | let local_fn = unsafe { b::test_fn().unwrap_unchecked() };
38+
| ^^^^^^^^^^^^^^^^
39+
|
40+
= help: call the function `test_fn_unchecked` instead, and remove the `unwrap_unchecked` call
41+
42+
error: aborting due to 5 previous errors
43+

0 commit comments

Comments
 (0)