diff --git a/CHANGELOG.md b/CHANGELOG.md index fd288285532e..c0ff1df00962 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3259,6 +3259,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 diff --git a/clippy_lints/src/empty_drop.rs b/clippy_lints/src/empty_drop.rs new file mode 100644 index 000000000000..325ae2356c14 --- /dev/null +++ b/clippy_lints/src/empty_drop.rs @@ -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(); + then { + span_lint_and_sugg( + cx, + EMPTY_DROP, + item.span, + "empty drop implementation", + "try removing this impl", + String::new(), + Applicability::MaybeIncorrect + ); + } + } + } +} diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 21f1ef562b5a..137e5b7684c3 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -127,6 +127,7 @@ store.register_lints(&[ drop_forget_ref::FORGET_REF, duration_subsec::DURATION_SUBSEC, else_if_without_else::ELSE_IF_WITHOUT_ELSE, + empty_drop::EMPTY_DROP, empty_enum::EMPTY_ENUM, entry::MAP_ENTRY, enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT, diff --git a/clippy_lints/src/lib.register_restriction.rs b/clippy_lints/src/lib.register_restriction.rs index 6ab139b2fb67..c353c350b531 100644 --- a/clippy_lints/src/lib.register_restriction.rs +++ b/clippy_lints/src/lib.register_restriction.rs @@ -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(exhaustive_items::EXHAUSTIVE_ENUMS), LintId::of(exhaustive_items::EXHAUSTIVE_STRUCTS), LintId::of(exit::EXIT), diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index f2a079991444..46d44ed53d4a 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -207,6 +207,7 @@ mod double_parens; mod drop_forget_ref; mod duration_subsec; mod else_if_without_else; +mod empty_drop; mod empty_enum; mod entry; mod enum_clike; @@ -867,6 +868,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: ignore_publish: cargo_ignore_publish, }) }); + store.register_late_pass(|| Box::new(empty_drop::EmptyDrop)); // add lints here, do not remove this comment, it's used in `new_lint` } diff --git a/tests/ui/empty_drop.fixed b/tests/ui/empty_drop.fixed new file mode 100644 index 000000000000..2e1b768461ab --- /dev/null +++ b/tests/ui/empty_drop.fixed @@ -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() {} diff --git a/tests/ui/empty_drop.rs b/tests/ui/empty_drop.rs new file mode 100644 index 000000000000..75232b0334df --- /dev/null +++ b/tests/ui/empty_drop.rs @@ -0,0 +1,30 @@ +// run-rustfix +#![warn(clippy::empty_drop)] +#![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() {} diff --git a/tests/ui/empty_drop.stderr b/tests/ui/empty_drop.stderr new file mode 100644 index 000000000000..70f7880d0360 --- /dev/null +++ b/tests/ui/empty_drop.stderr @@ -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 +