about summary refs log tree commit diff
diff options
context:
space:
mode:
authorMichael Howell <michael@notriddle.com>2019-02-16 21:28:16 -0700
committerMichael Howell <michael@notriddle.com>2019-02-17 22:53:08 -0700
commit2df14c370133781f4b5aa31b0168138cba0009b7 (patch)
tree97070bbf3da380e6d44bf4ac5abd67a7fc475437
parenta71acac1da7eaf667ab90a1d65d10e5cc4b80191 (diff)
downloadrust-2df14c370133781f4b5aa31b0168138cba0009b7.tar.gz
rust-2df14c370133781f4b5aa31b0168138cba0009b7.zip
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>() {}
```
-rw-r--r--clippy_lints/src/drop_bounds.rs71
-rw-r--r--clippy_lints/src/lib.rs4
-rw-r--r--clippy_lints/src/utils/paths.rs1
-rw-r--r--tests/ui/drop_bounds.rs4
-rw-r--r--tests/ui/drop_bounds.stderr16
5 files changed, 96 insertions, 0 deletions
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<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
+            );
+        }
+    }
+}
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<T: Drop>() {}
+fn bar<T>() 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<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
+