Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add empty_drop #8571

Merged
merged 1 commit into from
Apr 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3365,6 +3365,7 @@ Released 2018-09-13
[`duplicate_underscore_argument`]: https://rust-lang.github.io/rust-clippy/master/index.html#duplicate_underscore_argument
[`duration_subsec`]: https://rust-lang.github.io/rust-clippy/master/index.html#duration_subsec
[`else_if_without_else`]: https://rust-lang.github.io/rust-clippy/master/index.html#else_if_without_else
[`empty_drop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_drop
[`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
[`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr
[`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
Expand Down
65 changes: 65 additions & 0 deletions clippy_lints/src/empty_drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, peel_blocks};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{Body, ExprKind, Impl, ImplItemKind, Item, ItemKind, Node};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

declare_clippy_lint! {
/// ### What it does
/// Checks for empty `Drop` implementations.
///
/// ### Why is this bad?
/// Empty `Drop` implementations have no effect when dropping an instance of the type. They are
/// most likely useless. However, an empty `Drop` implementation prevents a type from being
/// destructured, which might be the intention behind adding the implementation as a marker.
///
/// ### Example
/// ```rust
/// struct S;
///
/// impl Drop for S {
/// fn drop(&mut self) {}
/// }
/// ```
/// Use instead:
/// ```rust
/// struct S;
/// ```
#[clippy::version = "1.61.0"]
pub EMPTY_DROP,
restriction,
"empty `Drop` implementations"
}
declare_lint_pass!(EmptyDrop => [EMPTY_DROP]);

impl LateLintPass<'_> for EmptyDrop {
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
if_chain! {
if let ItemKind::Impl(Impl {
of_trait: Some(ref trait_ref),
items: [child],
..
}) = item.kind;
if trait_ref.trait_def_id() == cx.tcx.lang_items().drop_trait();
if let impl_item_hir = child.id.hir_id();
if let Some(Node::ImplItem(impl_item)) = cx.tcx.hir().find(impl_item_hir);
if let ImplItemKind::Fn(_, b) = &impl_item.kind;
if let Body { value: func_expr, .. } = cx.tcx.hir().body(*b);
let func_expr = peel_blocks(func_expr);
if let ExprKind::Block(block, _) = func_expr.kind;
if block.stmts.is_empty() && block.expr.is_none();
flip1995 marked this conversation as resolved.
Show resolved Hide resolved
then {
span_lint_and_sugg(
cx,
EMPTY_DROP,
item.span,
"empty drop implementation",
"try removing this impl",
String::new(),
Applicability::MaybeIncorrect
);
}
}
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ store.register_lints(&[
drop_forget_ref::UNDROPPED_MANUALLY_DROPS,
duration_subsec::DURATION_SUBSEC,
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
empty_drop::EMPTY_DROP,
empty_enum::EMPTY_ENUM,
empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS,
entry::MAP_ENTRY,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_restriction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(default_union_representation::DEFAULT_UNION_REPRESENTATION),
LintId::of(disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS),
LintId::of(else_if_without_else::ELSE_IF_WITHOUT_ELSE),
LintId::of(empty_drop::EMPTY_DROP),
LintId::of(empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS),
LintId::of(exhaustive_items::EXHAUSTIVE_ENUMS),
LintId::of(exhaustive_items::EXHAUSTIVE_STRUCTS),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ mod double_parens;
mod drop_forget_ref;
mod duration_subsec;
mod else_if_without_else;
mod empty_drop;
mod empty_enum;
mod empty_structs_with_brackets;
mod entry;
Expand Down Expand Up @@ -822,6 +823,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(move || Box::new(disallowed_methods::DisallowedMethods::new(disallowed_methods.clone())));
store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86AttSyntax));
store.register_early_pass(|| Box::new(asm_syntax::InlineAsmX86IntelSyntax));
store.register_late_pass(|| Box::new(empty_drop::EmptyDrop));
store.register_late_pass(|| Box::new(strings::StrToString));
store.register_late_pass(|| Box::new(strings::StringToString));
store.register_late_pass(|| Box::new(zero_sized_map_values::ZeroSizedMapValues));
Expand Down
24 changes: 24 additions & 0 deletions tests/ui/empty_drop.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// run-rustfix
#![warn(clippy::empty_drop)]
#![allow(unused)]

// should cause an error
struct Foo;



// shouldn't cause an error
struct Bar;

impl Drop for Bar {
fn drop(&mut self) {
println!("dropping bar!");
}
}

// should error
struct Baz;



fn main() {}
30 changes: 30 additions & 0 deletions tests/ui/empty_drop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// run-rustfix
#![warn(clippy::empty_drop)]
PyroTechniac marked this conversation as resolved.
Show resolved Hide resolved
#![allow(unused)]

// should cause an error
struct Foo;

impl Drop for Foo {
fn drop(&mut self) {}
}

// shouldn't cause an error
struct Bar;

impl Drop for Bar {
fn drop(&mut self) {
println!("dropping bar!");
}
}

// should error
struct Baz;

impl Drop for Baz {
fn drop(&mut self) {
{}
}
}

fn main() {}
22 changes: 22 additions & 0 deletions tests/ui/empty_drop.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error: empty drop implementation
--> $DIR/empty_drop.rs:8:1
|
LL | / impl Drop for Foo {
LL | | fn drop(&mut self) {}
LL | | }
| |_^ help: try removing this impl
|
= note: `-D clippy::empty-drop` implied by `-D warnings`

error: empty drop implementation
--> $DIR/empty_drop.rs:24:1
|
LL | / impl Drop for Baz {
LL | | fn drop(&mut self) {
LL | | {}
LL | | }
LL | | }
| |_^ help: try removing this impl

error: aborting due to 2 previous errors