From 2df14c370133781f4b5aa31b0168138cba0009b7 Mon Sep 17 00:00:00 2001 From: Michael Howell Date: Sat, 16 Feb 2019 21:28:16 -0700 Subject: [PATCH] Add a lint to warn on `T: Drop` bounds **What it does:** Checks for generics with `std::ops::Drop` as bounds. **Why is this bad?** `Drop` bounds do not really accomplish anything. A type may have compiler-generated drop glue without implementing the `Drop` trait itself. The `Drop` trait also only has one method, `Drop::drop`, and that function is by fiat not callable in user code. So there is really no use case for using `Drop` in trait bounds. **Known problems:** None. **Example:** ```rust fn foo() {} ``` --- clippy_lints/src/drop_bounds.rs | 71 +++++++++++++++++++++++++++++++++ clippy_lints/src/lib.rs | 4 ++ clippy_lints/src/utils/paths.rs | 1 + tests/ui/drop_bounds.rs | 4 ++ tests/ui/drop_bounds.stderr | 16 ++++++++ 5 files changed, 96 insertions(+) create mode 100644 clippy_lints/src/drop_bounds.rs create mode 100644 tests/ui/drop_bounds.rs create mode 100644 tests/ui/drop_bounds.stderr diff --git a/clippy_lints/src/drop_bounds.rs b/clippy_lints/src/drop_bounds.rs new file mode 100644 index 00000000000..fea53f39438 --- /dev/null +++ b/clippy_lints/src/drop_bounds.rs @@ -0,0 +1,71 @@ +use crate::utils::{match_def_path, paths, span_lint}; +use if_chain::if_chain; +use rustc::hir::*; +use rustc::lint::{LateLintPass, LintArray, LintPass}; +use rustc::{declare_tool_lint, lint_array}; + +/// **What it does:** Checks for generics with `std::ops::Drop` as bounds. +/// +/// **Why is this bad?** `Drop` bounds do not really accomplish anything. +/// A type may have compiler-generated drop glue without implementing the +/// `Drop` trait itself. The `Drop` trait also only has one method, +/// `Drop::drop`, and that function is by fiat not callable in user code. +/// So there is really no use case for using `Drop` in trait bounds. +/// +/// **Known problems:** None. +/// +/// **Example:** +/// ```rust +/// fn foo() {} +/// ``` +declare_clippy_lint! { + pub DROP_BOUNDS, + correctness, + "Bounds of the form `T: Drop` are useless" +} + +const DROP_BOUNDS_SUMMARY: &str = "Bounds of the form `T: Drop` are useless. \ + Use `std::mem::needs_drop` to detect if a type has drop glue."; + +pub struct Pass; + +impl LintPass for Pass { + fn get_lints(&self) -> LintArray { + lint_array!(DROP_BOUNDS) + } + + fn name(&self) -> &'static str { + "DropBounds" + } +} + +impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass { + fn check_generic_param(&mut self, cx: &rustc::lint::LateContext<'a, 'tcx>, p: &'tcx GenericParam) { + for bound in &p.bounds { + lint_bound(cx, bound); + } + } + fn check_where_predicate(&mut self, cx: &rustc::lint::LateContext<'a, 'tcx>, p: &'tcx WherePredicate) { + if let WherePredicate::BoundPredicate(WhereBoundPredicate { bounds, .. }) = p { + for bound in bounds { + lint_bound(cx, bound); + } + } + } +} + +fn lint_bound<'a, 'tcx>(cx: &rustc::lint::LateContext<'a, 'tcx>, bound: &'tcx GenericBound) { + if_chain! { + if let GenericBound::Trait(t, _) = bound; + if let Some(def_id) = t.trait_ref.path.def.opt_def_id(); + if match_def_path(cx.tcx, def_id, &paths::DROP_TRAIT); + then { + span_lint( + cx, + DROP_BOUNDS, + t.span, + DROP_BOUNDS_SUMMARY + ); + } + } +} diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d0032fe78fc..1e2e861ea27 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -144,6 +144,7 @@ pub mod derive; pub mod doc; pub mod double_comparison; pub mod double_parens; +pub mod drop_bounds; pub mod drop_forget_ref; pub mod duration_subsec; pub mod else_if_without_else; @@ -467,6 +468,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { reg.register_late_lint_pass(box derive::Derive); reg.register_late_lint_pass(box types::CharLitAsU8); reg.register_late_lint_pass(box vec::Pass); + reg.register_late_lint_pass(box drop_bounds::Pass); reg.register_late_lint_pass(box drop_forget_ref::Pass); reg.register_late_lint_pass(box empty_enum::EmptyEnum); reg.register_late_lint_pass(box types::AbsurdExtremeComparisons); @@ -653,6 +655,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { derive::DERIVE_HASH_XOR_EQ, double_comparison::DOUBLE_COMPARISONS, double_parens::DOUBLE_PARENS, + drop_bounds::DROP_BOUNDS, drop_forget_ref::DROP_COPY, drop_forget_ref::DROP_REF, drop_forget_ref::FORGET_COPY, @@ -1017,6 +1020,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) { copies::IFS_SAME_COND, copies::IF_SAME_THEN_ELSE, derive::DERIVE_HASH_XOR_EQ, + drop_bounds::DROP_BOUNDS, drop_forget_ref::DROP_COPY, drop_forget_ref::DROP_REF, drop_forget_ref::FORGET_COPY, diff --git a/clippy_lints/src/utils/paths.rs b/clippy_lints/src/utils/paths.rs index 0a1b53c32ac..4108732653f 100644 --- a/clippy_lints/src/utils/paths.rs +++ b/clippy_lints/src/utils/paths.rs @@ -24,6 +24,7 @@ pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "der pub const DISPLAY_FMT_METHOD: [&str; 4] = ["core", "fmt", "Display", "fmt"]; pub const DOUBLE_ENDED_ITERATOR: [&str; 4] = ["core", "iter", "traits", "DoubleEndedIterator"]; pub const DROP: [&str; 3] = ["core", "mem", "drop"]; +pub const DROP_TRAIT: [&str; 4] = ["core", "ops", "drop", "Drop"]; pub const DURATION: [&str; 3] = ["core", "time", "Duration"]; pub const EARLY_CONTEXT: [&str; 4] = ["rustc", "lint", "context", "EarlyContext"]; pub const FMT_ARGUMENTS_NEWV1: [&str; 4] = ["core", "fmt", "Arguments", "new_v1"]; diff --git a/tests/ui/drop_bounds.rs b/tests/ui/drop_bounds.rs new file mode 100644 index 00000000000..baa74037c1a --- /dev/null +++ b/tests/ui/drop_bounds.rs @@ -0,0 +1,4 @@ +#![allow(unused)] +fn foo() {} +fn bar() where T: Drop {} +fn main() {} diff --git a/tests/ui/drop_bounds.stderr b/tests/ui/drop_bounds.stderr new file mode 100644 index 00000000000..15b6ea6a795 --- /dev/null +++ b/tests/ui/drop_bounds.stderr @@ -0,0 +1,16 @@ +error: Bounds of the form `T: Drop` are useless. Use `std::mem::needs_drop` to detect if a type has drop glue. + --> $DIR/drop_bounds.rs:2:11 + | +LL | fn foo() {} + | ^^^^ + | + = note: #[deny(clippy::drop_bounds)] on by default + +error: Bounds of the form `T: Drop` are useless. Use `std::mem::needs_drop` to detect if a type has drop glue. + --> $DIR/drop_bounds.rs:3:22 + | +LL | fn bar() where T: Drop {} + | ^^^^ + +error: aborting due to 2 previous errors +