Skip to content

Commit f22f163

Browse files
ArhanChaudharyArhanChaudhary
committed
apply PR review suggestions
1 parent 4eeadb0 commit f22f163

5 files changed

Lines changed: 102 additions & 50 deletions

File tree

‎clippy_lints/src/nonnull_unchecked_on_box_ptr.rs‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use clippy_config::Conf;
22
use clippy_utils::diagnostics::span_lint_and_sugg;
33
use clippy_utils::msrvs::{self, Msrv};
4-
use clippy_utils::res::{MaybeDef, MaybeResPath};
4+
use clippy_utils::res::{MaybeDef, MaybeQPath};
55
use clippy_utils::source::snippet_with_context;
66
use clippy_utils::sym;
77
use clippy_utils::visitors::is_expr_unsafe;
88
use rustc_errors::Applicability;
9-
use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, LangItem, Node, QPath, UnsafeSource};
9+
use rustc_hir::{Block, BlockCheckMode, Expr, ExprKind, LangItem, Node, UnsafeSource};
1010
use rustc_lint::{LateContext, LateLintPass};
1111
use rustc_session::impl_lint_pass;
1212

@@ -50,40 +50,46 @@ impl NonnullUncheckedOnBoxPtr {
5050
impl<'tcx> LateLintPass<'tcx> for NonnullUncheckedOnBoxPtr {
5151
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
5252
if !expr.span.from_expansion()
53-
// Parse `NonNull::new_unchecked`
5453
&& let ExprKind::Call(nonnull_new_unchecked, [arg]) = expr.kind
55-
&& let ExprKind::Path(QPath::TypeRelative(nonnnull, new_unchecked)) = nonnull_new_unchecked.kind
56-
&& new_unchecked.ident.name == sym::new_unchecked
57-
&& nonnnull.basic_res().is_diag_item(cx, sym::NonNull)
58-
// Parse `Box::into_raw`
5954
&& let ExprKind::Call(box_into_raw, [arg]) = arg.kind
60-
&& let ExprKind::Path(QPath::TypeRelative(r#box, into_raw)) = box_into_raw.kind
61-
&& into_raw.ident.name == sym::into_raw
62-
&& r#box.basic_res().is_lang_item(cx, LangItem::OwnedBox)
63-
&& self.msrv.meets(cx, msrvs::NONNULL_FROM_MUT)
55+
&& nonnull_new_unchecked
56+
.ty_rel_def_if_named(cx, sym::new_unchecked)
57+
.opt_parent(cx)
58+
.opt_impl_ty(cx)
59+
.is_diag_item(cx, sym::NonNull)
60+
&& box_into_raw
61+
.ty_rel_def_if_named(cx, sym::into_raw)
62+
.opt_parent(cx)
63+
.opt_impl_ty(cx)
64+
.is_lang_item(cx, LangItem::OwnedBox)
65+
&& self.msrv.meets(cx, msrvs::BOX_LEAK)
6466
{
6567
let ctxt = expr.span.ctxt();
6668
let span = match cx.tcx.parent_hir_node(expr.hir_id) {
67-
Node::Block(
68-
block @ &Block {
69-
rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
70-
span: unsafe_span,
71-
..
72-
},
73-
) if unsafe_span.ctxt() == ctxt && !is_expr_unsafe(cx, arg) && block.stmts.is_empty() => unsafe_span,
69+
Node::Block(&Block {
70+
rules: BlockCheckMode::UnsafeBlock(UnsafeSource::UserProvided),
71+
span: unsafe_span,
72+
stmts,
73+
..
74+
}) if unsafe_span.ctxt() == ctxt && !is_expr_unsafe(cx, arg) && stmts.is_empty() => unsafe_span,
7475
_ => expr.span,
7576
};
7677

7778
let mut app = Applicability::MachineApplicable;
7879
let arg_name = snippet_with_context(cx, arg.span, ctxt, "_", &mut app).0;
80+
let sugg = if self.msrv.meets(cx, msrvs::NONNULL_FROM_MUT) {
81+
format!("NonNull::from_mut(Box::leak({arg_name}))")
82+
} else {
83+
format!("NonNull::from(Box::leak({arg_name}))")
84+
};
7985

8086
span_lint_and_sugg(
8187
cx,
8288
NONNULL_UNCHECKED_ON_BOX_PTR,
8389
span,
8490
"use of `NonNull::new_unchecked` with `Box::into_raw`",
8591
"try",
86-
format!("NonNull::from_mut(Box::leak({arg_name}))"),
92+
sugg,
8793
app,
8894
);
8995
}

‎clippy_utils/src/msrvs.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ msrv_aliases! {
8181
1,29,0 { ITER_FLATTEN }
8282
1,28,0 { FROM_BOOL, REPEAT_WITH, SLICE_FROM_REF }
8383
1,27,0 { ITERATOR_TRY_FOLD, DOUBLE_ENDED_ITERATOR_RFIND, DURATION_FROM_NANOS_MICROS }
84-
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS }
84+
1,26,0 { RANGE_INCLUSIVE, STRING_RETAIN, POINTER_ADD_SUB_METHODS, BOX_LEAK }
8585
1,24,0 { IS_ASCII_DIGIT, PTR_NULL }
8686
1,18,0 { HASH_MAP_RETAIN, HASH_SET_RETAIN }
8787
1,17,0 { FIELD_INIT_SHORTHAND, STATIC_IN_CONST, EXPECT_ERR }

‎tests/ui/nonnull_unchecked_on_box_ptr.fixed‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,21 @@ fn lint() {
4545
let _ = NonNull::from_mut(Box::leak(one));
4646
//~^ nonnull_unchecked_on_box_ptr
4747

48-
use Box as Box2;
49-
use NonNull as NonNull2;
50-
let one = Box::new(1);
51-
let _ = NonNull::from_mut(Box::leak(one));
52-
//~^ nonnull_unchecked_on_box_ptr
48+
{
49+
use Box as Box2;
50+
use NonNull as NonNull2;
51+
let one = Box2::new(1);
52+
let _ = NonNull::from_mut(Box::leak(one));
53+
//~^ nonnull_unchecked_on_box_ptr
54+
}
55+
56+
{
57+
type Box2<T> = Box<T>;
58+
type NonNull2<T> = NonNull<T>;
59+
let one = Box2::new(1);
60+
let _ = NonNull::from_mut(Box::leak(one));
61+
//~^ nonnull_unchecked_on_box_ptr
62+
}
5363
}
5464

5565
fn macros() {
@@ -136,12 +146,19 @@ fn no_lint() {
136146
}
137147
}
138148

139-
#[clippy::msrv = "1.88"]
140-
fn msrv_1_88() {
149+
#[clippy::msrv = "1.25"]
150+
fn msrv_1_25() {
141151
let one = Box::new(1);
142152
let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
143153
}
144154

155+
#[clippy::msrv = "1.26"]
156+
fn msrv_1_26() {
157+
let one = Box::new(1);
158+
let _ = NonNull::from(Box::leak(one));
159+
//~^ nonnull_unchecked_on_box_ptr
160+
}
161+
145162
#[clippy::msrv = "1.89"]
146163
fn msrv_1_89() {
147164
let one = Box::new(1);

‎tests/ui/nonnull_unchecked_on_box_ptr.rs‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,11 +45,21 @@ fn lint() {
4545
let _ = unsafe { std::ptr::NonNull::new_unchecked(std::boxed::Box::into_raw(one)) };
4646
//~^ nonnull_unchecked_on_box_ptr
4747

48-
use Box as Box2;
49-
use NonNull as NonNull2;
50-
let one = Box::new(1);
51-
let _ = unsafe { NonNull2::new_unchecked(Box2::into_raw(one)) };
52-
//~^ nonnull_unchecked_on_box_ptr
48+
{
49+
use Box as Box2;
50+
use NonNull as NonNull2;
51+
let one = Box2::new(1);
52+
let _ = unsafe { NonNull2::new_unchecked(Box2::into_raw(one)) };
53+
//~^ nonnull_unchecked_on_box_ptr
54+
}
55+
56+
{
57+
type Box2<T> = Box<T>;
58+
type NonNull2<T> = NonNull<T>;
59+
let one = Box2::new(1);
60+
let _ = unsafe { NonNull2::new_unchecked(Box2::into_raw(one)) };
61+
//~^ nonnull_unchecked_on_box_ptr
62+
}
5363
}
5464

5565
fn macros() {
@@ -136,12 +146,19 @@ fn no_lint() {
136146
}
137147
}
138148

139-
#[clippy::msrv = "1.88"]
140-
fn msrv_1_88() {
149+
#[clippy::msrv = "1.25"]
150+
fn msrv_1_25() {
141151
let one = Box::new(1);
142152
let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
143153
}
144154

155+
#[clippy::msrv = "1.26"]
156+
fn msrv_1_26() {
157+
let one = Box::new(1);
158+
let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
159+
//~^ nonnull_unchecked_on_box_ptr
160+
}
161+
145162
#[clippy::msrv = "1.89"]
146163
fn msrv_1_89() {
147164
let one = Box::new(1);

‎tests/ui/nonnull_unchecked_on_box_ptr.stderr‎

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -20,82 +20,94 @@ LL | let _ = unsafe { std::ptr::NonNull::new_unchecked(std::boxed::Box::
2020
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
2121

2222
error: use of `NonNull::new_unchecked` with `Box::into_raw`
23-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:51:17
23+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:52:21
2424
|
25-
LL | let _ = unsafe { NonNull2::new_unchecked(Box2::into_raw(one)) };
26-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
25+
LL | let _ = unsafe { NonNull2::new_unchecked(Box2::into_raw(one)) };
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
2727

2828
error: use of `NonNull::new_unchecked` with `Box::into_raw`
29-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:57:17
29+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:60:21
30+
|
31+
LL | let _ = unsafe { NonNull2::new_unchecked(Box2::into_raw(one)) };
32+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
33+
34+
error: use of `NonNull::new_unchecked` with `Box::into_raw`
35+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:67:17
3036
|
3137
LL | let _ = unsafe { NonNull::new_unchecked(Box::into_raw(identity!(one))) };
3238
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
3339

3440
error: use of `NonNull::new_unchecked` with `Box::into_raw`
35-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:61:27
41+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:71:27
3642
|
3743
LL | let _ = identity!(unsafe { NonNull::new_unchecked(Box::into_raw(one)) });
3844
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
3945

4046
error: use of `NonNull::new_unchecked` with `Box::into_raw`
41-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:65:17
47+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:75:17
4248
|
4349
LL | let _ = unsafe { identity!(NonNull::new_unchecked(Box::into_raw(one))) };
4450
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
4551

4652
error: use of `NonNull::new_unchecked` with `Box::into_raw`
47-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:69:17
53+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:79:17
4854
|
4955
LL | let _ = unsafe { NonNull::new_unchecked(identity!(Box::into_raw(one))) };
5056
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
5157

5258
error: use of `NonNull::new_unchecked` with `Box::into_raw`
53-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:73:17
59+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:83:17
5460
|
5561
LL | let _ = unsafe { NonNull::new_unchecked(identity!(Box::into_raw(identity!(one)))) };
5662
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
5763

5864
error: use of `NonNull::new_unchecked` with `Box::into_raw`
59-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:77:17
65+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:87:17
6066
|
6167
LL | let _ = unsafe { NonNull::new_unchecked(Box::into_raw(weird!(one))) };
6268
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(weird!(one)))`
6369

6470
error: use of `NonNull::new_unchecked` with `Box::into_raw`
65-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:84:13
71+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:94:13
6672
|
6773
LL | NonNull::new_unchecked(Box::into_raw(unsafe_identity(one)))
6874
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(unsafe_identity(one)))`
6975

7076
error: use of `NonNull::new_unchecked` with `Box::into_raw`
71-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:90:22
77+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:100:22
7278
|
7379
LL | identity(NonNull::new_unchecked(Box::into_raw(one)))
7480
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
7581

7682
error: use of `NonNull::new_unchecked` with `Box::into_raw`
77-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:96:29
83+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:106:29
7884
|
7985
LL | unsafe_identity(NonNull::new_unchecked(Box::into_raw(one)))
8086
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
8187

8288
error: use of `NonNull::new_unchecked` with `Box::into_raw`
83-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:101:13
89+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:111:13
8490
|
8591
LL | NonNull::new_unchecked(Box::into_raw(Box::new(std::num::NonZeroI32::new_unchecked(1))))
8692
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(Box::new(std::num::NonZeroI32::new_unchecked(1))))`
8793

8894
error: use of `NonNull::new_unchecked` with `Box::into_raw`
89-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:107:13
95+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:117:13
9096
|
9197
LL | NonNull::new_unchecked(Box::into_raw(one))
9298
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
9399

94100
error: use of `NonNull::new_unchecked` with `Box::into_raw`
95-
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:148:13
101+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:158:13
102+
|
103+
LL | let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
104+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from(Box::leak(one))`
105+
106+
error: use of `NonNull::new_unchecked` with `Box::into_raw`
107+
--> tests/ui/nonnull_unchecked_on_box_ptr.rs:165:13
96108
|
97109
LL | let _ = unsafe { NonNull::new_unchecked(Box::into_raw(one)) };
98110
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `NonNull::from_mut(Box::leak(one))`
99111

100-
error: aborting due to 16 previous errors
112+
error: aborting due to 18 previous errors
101113

0 commit comments

Comments
 (0)