Skip to content

Commit 9ee05a0

Browse files
Jarchopitaj
authored andcommitted
Add lints drop_non_drop and forget_non_drop
1 parent 409a936 commit 9ee05a0

17 files changed

+269
-128
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -3256,6 +3256,7 @@ Released 2018-09-13
32563256
[`double_neg`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_neg
32573257
[`double_parens`]: https://rust-lang.github.io/rust-clippy/master/index.html#double_parens
32583258
[`drop_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_copy
3259+
[`drop_non_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_non_drop
32593260
[`drop_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#drop_ref
32603261
[`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument
32613262
[`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
@@ -3308,6 +3309,7 @@ Released 2018-09-13
33083309
[`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map
33093310
[`for_loops_over_fallibles`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_loops_over_fallibles
33103311
[`forget_copy`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_copy
3312+
[`forget_non_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_non_drop
33113313
[`forget_ref`]: https://rust-lang.github.io/rust-clippy/master/index.html#forget_ref
33123314
[`format_in_format_args`]: https://rust-lang.github.io/rust-clippy/master/index.html#format_in_format_args
33133315
[`from_iter_instead_of_collect`]: https://rust-lang.github.io/rust-clippy/master/index.html#from_iter_instead_of_collect

clippy_lints/src/drop_forget_ref.rs

+127-54
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
1-
use clippy_utils::diagnostics::span_lint_and_note;
2-
use clippy_utils::ty::is_copy;
3-
use if_chain::if_chain;
4-
use rustc_hir::{Expr, ExprKind};
1+
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
2+
use clippy_utils::is_must_use_func_call;
3+
use clippy_utils::ty::{is_copy, is_must_use_ty, is_type_lang_item};
4+
use rustc_hir::{Expr, ExprKind, LangItem};
55
use rustc_lint::{LateContext, LateLintPass};
6-
use rustc_middle::ty;
76
use rustc_session::{declare_lint_pass, declare_tool_lint};
87
use rustc_span::sym;
98

@@ -103,6 +102,75 @@ declare_clippy_lint! {
103102
"calls to `std::mem::forget` with a value that implements Copy"
104103
}
105104

105+
declare_clippy_lint! {
106+
/// ### What it does
107+
/// Checks for calls to `std::mem::drop` with a value that does not implement `Drop`.
108+
///
109+
/// ### Why is this bad?
110+
/// Calling `std::mem::drop` is no different than dropping such a type. A different value may
111+
/// have been intended.
112+
///
113+
/// ### Example
114+
/// ```rust
115+
/// struct Foo;
116+
/// let x = Foo;
117+
/// std::mem::drop(x);
118+
/// ```
119+
#[clippy::version = "1.61.0"]
120+
pub DROP_NON_DROP,
121+
suspicious,
122+
"call to `std::mem::drop` with a value which does not implement `Drop`"
123+
}
124+
125+
declare_clippy_lint! {
126+
/// ### What it does
127+
/// Checks for calls to `std::mem::forget` with a value that does not implement `Drop`.
128+
///
129+
/// ### Why is this bad?
130+
/// Calling `std::mem::forget` is no different than dropping such a type. A different value may
131+
/// have been intended.
132+
///
133+
/// ### Example
134+
/// ```rust
135+
/// struct Foo;
136+
/// let x = Foo;
137+
/// std::mem::forget(x);
138+
/// ```
139+
#[clippy::version = "1.61.0"]
140+
pub FORGET_NON_DROP,
141+
suspicious,
142+
"call to `std::mem::forget` with a value which does not implement `Drop`"
143+
}
144+
145+
declare_clippy_lint! {
146+
/// ### What it does
147+
/// Prevents the safe `std::mem::drop` function from being called on `std::mem::ManuallyDrop`.
148+
///
149+
/// ### Why is this bad?
150+
/// The safe `drop` function does not drop the inner value of a `ManuallyDrop`.
151+
///
152+
/// ### Known problems
153+
/// Does not catch cases if the user binds `std::mem::drop`
154+
/// to a different name and calls it that way.
155+
///
156+
/// ### Example
157+
/// ```rust
158+
/// struct S;
159+
/// drop(std::mem::ManuallyDrop::new(S));
160+
/// ```
161+
/// Use instead:
162+
/// ```rust
163+
/// struct S;
164+
/// unsafe {
165+
/// std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S));
166+
/// }
167+
/// ```
168+
#[clippy::version = "1.49.0"]
169+
pub UNDROPPED_MANUALLY_DROPS,
170+
correctness,
171+
"use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value"
172+
}
173+
106174
const DROP_REF_SUMMARY: &str = "calls to `std::mem::drop` with a reference instead of an owned value. \
107175
Dropping a reference does nothing";
108176
const FORGET_REF_SUMMARY: &str = "calls to `std::mem::forget` with a reference instead of an owned value. \
@@ -111,60 +179,65 @@ const DROP_COPY_SUMMARY: &str = "calls to `std::mem::drop` with a value that imp
111179
Dropping a copy leaves the original intact";
112180
const FORGET_COPY_SUMMARY: &str = "calls to `std::mem::forget` with a value that implements `Copy`. \
113181
Forgetting a copy leaves the original intact";
182+
const DROP_NON_DROP_SUMMARY: &str = "call to `std::mem::drop` with a value that does not implement `Drop`. \
183+
Dropping such a type only extends it's contained lifetimes";
184+
const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \
185+
Forgetting such a type is the same as dropping it";
114186

115-
declare_lint_pass!(DropForgetRef => [DROP_REF, FORGET_REF, DROP_COPY, FORGET_COPY]);
187+
declare_lint_pass!(DropForgetRef => [
188+
DROP_REF,
189+
FORGET_REF,
190+
DROP_COPY,
191+
FORGET_COPY,
192+
DROP_NON_DROP,
193+
FORGET_NON_DROP,
194+
UNDROPPED_MANUALLY_DROPS
195+
]);
116196

117197
impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
118198
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
119-
if_chain! {
120-
if let ExprKind::Call(path, args) = expr.kind;
121-
if let ExprKind::Path(ref qpath) = path.kind;
122-
if args.len() == 1;
123-
if let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id();
124-
then {
125-
let lint;
126-
let msg;
127-
let arg = &args[0];
128-
let arg_ty = cx.typeck_results().expr_ty(arg);
129-
130-
if let ty::Ref(..) = arg_ty.kind() {
131-
match cx.tcx.get_diagnostic_name(def_id) {
132-
Some(sym::mem_drop) => {
133-
lint = DROP_REF;
134-
msg = DROP_REF_SUMMARY.to_string();
135-
},
136-
Some(sym::mem_forget) => {
137-
lint = FORGET_REF;
138-
msg = FORGET_REF_SUMMARY.to_string();
139-
},
140-
_ => return,
141-
}
142-
span_lint_and_note(cx,
143-
lint,
144-
expr.span,
145-
&msg,
146-
Some(arg.span),
147-
&format!("argument has type `{}`", arg_ty));
148-
} else if is_copy(cx, arg_ty) {
149-
match cx.tcx.get_diagnostic_name(def_id) {
150-
Some(sym::mem_drop) => {
151-
lint = DROP_COPY;
152-
msg = DROP_COPY_SUMMARY.to_string();
153-
},
154-
Some(sym::mem_forget) => {
155-
lint = FORGET_COPY;
156-
msg = FORGET_COPY_SUMMARY.to_string();
157-
},
158-
_ => return,
159-
}
160-
span_lint_and_note(cx,
161-
lint,
162-
expr.span,
163-
&msg,
164-
Some(arg.span),
165-
&format!("argument has type {}", arg_ty));
199+
if let ExprKind::Call(path, [arg]) = expr.kind
200+
&& let ExprKind::Path(ref qpath) = path.kind
201+
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
202+
&& let Some(fn_name) = cx.tcx.get_diagnostic_name(def_id)
203+
{
204+
let arg_ty = cx.typeck_results().expr_ty(arg);
205+
let (lint, msg) = match fn_name {
206+
sym::mem_drop if arg_ty.is_ref() => (DROP_REF, DROP_REF_SUMMARY),
207+
sym::mem_forget if arg_ty.is_ref() => (FORGET_REF, FORGET_REF_SUMMARY),
208+
sym::mem_drop if is_copy(cx, arg_ty) => (DROP_COPY, DROP_COPY_SUMMARY),
209+
sym::mem_forget if is_copy(cx, arg_ty) => (FORGET_COPY, FORGET_COPY_SUMMARY),
210+
sym::mem_drop if is_type_lang_item(cx, arg_ty, LangItem::ManuallyDrop) => {
211+
span_lint_and_help(
212+
cx,
213+
UNDROPPED_MANUALLY_DROPS,
214+
expr.span,
215+
"the inner value of this ManuallyDrop will not be dropped",
216+
None,
217+
"to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop",
218+
);
219+
return;
166220
}
167-
}
221+
sym::mem_drop
222+
if !(arg_ty.needs_drop(cx.tcx, cx.param_env)
223+
|| is_must_use_func_call(cx, arg)
224+
|| is_must_use_ty(cx, arg_ty)) =>
225+
{
226+
(DROP_NON_DROP, DROP_NON_DROP_SUMMARY)
227+
},
228+
sym::mem_forget if !arg_ty.needs_drop(cx.tcx, cx.param_env) => {
229+
(FORGET_NON_DROP, FORGET_NON_DROP_SUMMARY)
230+
},
231+
_ => return,
232+
};
233+
span_lint_and_note(
234+
cx,
235+
lint,
236+
expr.span,
237+
msg,
238+
Some(arg.span),
239+
&format!("argument has type `{}`", arg_ty),
240+
);
168241
}
169242
}
170243
}

clippy_lints/src/lib.register_all.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,12 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
5050
LintId::of(double_comparison::DOUBLE_COMPARISONS),
5151
LintId::of(double_parens::DOUBLE_PARENS),
5252
LintId::of(drop_forget_ref::DROP_COPY),
53+
LintId::of(drop_forget_ref::DROP_NON_DROP),
5354
LintId::of(drop_forget_ref::DROP_REF),
5455
LintId::of(drop_forget_ref::FORGET_COPY),
56+
LintId::of(drop_forget_ref::FORGET_NON_DROP),
5557
LintId::of(drop_forget_ref::FORGET_REF),
58+
LintId::of(drop_forget_ref::UNDROPPED_MANUALLY_DROPS),
5659
LintId::of(duration_subsec::DURATION_SUBSEC),
5760
LintId::of(entry::MAP_ENTRY),
5861
LintId::of(enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
@@ -297,7 +300,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
297300
LintId::of(types::REDUNDANT_ALLOCATION),
298301
LintId::of(types::TYPE_COMPLEXITY),
299302
LintId::of(types::VEC_BOX),
300-
LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
301303
LintId::of(unicode::INVISIBLE_CHARACTERS),
302304
LintId::of(uninit_vec::UNINIT_VEC),
303305
LintId::of(unit_hash::UNIT_HASH),

clippy_lints/src/lib.register_correctness.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
2222
LintId::of(drop_forget_ref::DROP_REF),
2323
LintId::of(drop_forget_ref::FORGET_COPY),
2424
LintId::of(drop_forget_ref::FORGET_REF),
25+
LintId::of(drop_forget_ref::UNDROPPED_MANUALLY_DROPS),
2526
LintId::of(enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT),
2627
LintId::of(eq_op::EQ_OP),
2728
LintId::of(erasing_op::ERASING_OP),
@@ -62,7 +63,6 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
6263
LintId::of(transmute::UNSOUND_COLLECTION_TRANSMUTE),
6364
LintId::of(transmute::WRONG_TRANSMUTE),
6465
LintId::of(transmuting_null::TRANSMUTING_NULL),
65-
LintId::of(undropped_manually_drops::UNDROPPED_MANUALLY_DROPS),
6666
LintId::of(unicode::INVISIBLE_CHARACTERS),
6767
LintId::of(uninit_vec::UNINIT_VEC),
6868
LintId::of(unit_hash::UNIT_HASH),

clippy_lints/src/lib.register_lints.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -123,9 +123,12 @@ store.register_lints(&[
123123
double_comparison::DOUBLE_COMPARISONS,
124124
double_parens::DOUBLE_PARENS,
125125
drop_forget_ref::DROP_COPY,
126+
drop_forget_ref::DROP_NON_DROP,
126127
drop_forget_ref::DROP_REF,
127128
drop_forget_ref::FORGET_COPY,
129+
drop_forget_ref::FORGET_NON_DROP,
128130
drop_forget_ref::FORGET_REF,
131+
drop_forget_ref::UNDROPPED_MANUALLY_DROPS,
129132
duration_subsec::DURATION_SUBSEC,
130133
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
131134
empty_enum::EMPTY_ENUM,
@@ -507,7 +510,6 @@ store.register_lints(&[
507510
types::TYPE_COMPLEXITY,
508511
types::VEC_BOX,
509512
undocumented_unsafe_blocks::UNDOCUMENTED_UNSAFE_BLOCKS,
510-
undropped_manually_drops::UNDROPPED_MANUALLY_DROPS,
511513
unicode::INVISIBLE_CHARACTERS,
512514
unicode::NON_ASCII_LITERAL,
513515
unicode::UNICODE_NOT_NFC,

clippy_lints/src/lib.register_suspicious.rs

+2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ store.register_group(true, "clippy::suspicious", Some("clippy_suspicious"), vec!
1010
LintId::of(casts::CAST_ENUM_CONSTRUCTOR),
1111
LintId::of(casts::CAST_ENUM_TRUNCATION),
1212
LintId::of(crate_in_macro_def::CRATE_IN_MACRO_DEF),
13+
LintId::of(drop_forget_ref::DROP_NON_DROP),
14+
LintId::of(drop_forget_ref::FORGET_NON_DROP),
1315
LintId::of(eval_order_dependence::EVAL_ORDER_DEPENDENCE),
1416
LintId::of(float_equality_without_abs::FLOAT_EQUALITY_WITHOUT_ABS),
1517
LintId::of(format_impl::PRINT_IN_FORMAT_IMPL),

clippy_lints/src/lib.rs

-2
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,6 @@ mod transmuting_null;
378378
mod try_err;
379379
mod types;
380380
mod undocumented_unsafe_blocks;
381-
mod undropped_manually_drops;
382381
mod unicode;
383382
mod uninit_vec;
384383
mod unit_hash;
@@ -815,7 +814,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
815814
store.register_late_pass(move || Box::new(disallowed_methods::DisallowedMethods::new(disallowed_methods.clone())));
816815
store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86AttSyntax));
817816
store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax));
818-
store.register_late_pass(|| Box::new(undropped_manually_drops::UndroppedManuallyDrops));
819817
store.register_late_pass(|| Box::new(strings::StrToString));
820818
store.register_late_pass(|| Box::new(strings::StringToString));
821819
store.register_late_pass(|| Box::new(zero_sized_map_values::ZeroSizedMapValues));

clippy_lints/src/undropped_manually_drops.rs

-59
This file was deleted.

0 commit comments

Comments
 (0)