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<T: Drop>() {}
```
This commit is contained in:
Michael Howell 2019-02-16 21:28:16 -07:00
parent a71acac1da
commit 2df14c3701
5 changed files with 96 additions and 0 deletions

View File

@ -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<T: Drop>() {}
/// ```
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
);
}
}
}

View File

@ -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,

View File

@ -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"];

4
tests/ui/drop_bounds.rs Normal file
View File

@ -0,0 +1,4 @@
#![allow(unused)]
fn foo<T: Drop>() {}
fn bar<T>() where T: Drop {}
fn main() {}

View File

@ -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<T: Drop>() {}
| ^^^^
|
= 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<T>() where T: Drop {}
| ^^^^
error: aborting due to 2 previous errors