New lint: [derive_partial_eq_without_eq]

Author: nsunderland1

CHANGELOG.md

@@ -3350,6 +3350,7 @@ Released 2018-09-13
 [`derivable_impls`]: https://rust-lang.github.io/rust-clippy/master/index.html#derivable_impls
 [`derive_hash_xor_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_hash_xor_eq
 [`derive_ord_xor_partial_ord`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_ord_xor_partial_ord
+[`derive_partial_eq_without_eq`]: https://rust-lang.github.io/rust-clippy/master/index.html#derive_partial_eq_without_eq
 [`disallowed_methods`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_methods
 [`disallowed_script_idents`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_script_idents
 [`disallowed_types`]: https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_types

clippy_lints/src/derive.rs

@@ -1,8 +1,9 @@
-use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_then};
+use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then};
 use clippy_utils::paths;
 use clippy_utils::ty::{implements_trait, is_copy};
 use clippy_utils::{is_automatically_derived, is_lint_allowed, match_def_path};
 use if_chain::if_chain;
+use rustc_errors::Applicability;
 use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor};
 use rustc_hir::{
     BlockCheckMode, BodyId, Expr, ExprKind, FnDecl, HirId, Impl, Item, ItemKind, TraitRef, UnsafeSource, Unsafety,
@@ -156,11 +157,44 @@ declare_clippy_lint! {
     "deriving `serde::Deserialize` on a type that has methods using `unsafe`"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for types that derive `PartialEq` and could implement `Eq`.
+    ///
+    /// ### Why is this bad?
+    /// If a type `T` derives `PartialEq` and all of its members implement `Eq`,
+    /// then `T` can always implement `Eq`. Implementing `Eq` allows `T` to be used
+    /// in APIs that require `Eq` types. It also allows structs containing `T` to derive
+    /// `Eq` themselves.
+    ///
+    /// ### Example
+    /// ```rust
+    /// #[derive(PartialEq)]
+    /// struct Foo {
+    ///     i_am_eq: i32,
+    ///     i_am_eq_too: Vec<String>,
+    /// }
+    /// ```
+    /// Use instead:
+    /// ```rust
+    /// #[derive(PartialEq, Eq)]
+    /// struct Foo {
+    ///     i_am_eq: i32,
+    ///     i_am_eq_too: Vec<String>,
+    /// }
+    /// ```
+    #[clippy::version = "1.62.0"]
+    pub DERIVE_PARTIAL_EQ_WITHOUT_EQ,
+    nursery,
+    "deriving `PartialEq` on a type that can implement `Eq`, without implementing `Eq`"
+}
+
 declare_lint_pass!(Derive => [
     EXPL_IMPL_CLONE_ON_COPY,
     DERIVE_HASH_XOR_EQ,
     DERIVE_ORD_XOR_PARTIAL_ORD,
-    UNSAFE_DERIVE_DESERIALIZE
+    UNSAFE_DERIVE_DESERIALIZE,
+    DERIVE_PARTIAL_EQ_WITHOUT_EQ
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Derive {
@@ -176,6 +210,7 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
 
             check_hash_peq(cx, item.span, trait_ref, ty, is_automatically_derived);
             check_ord_partial_ord(cx, item.span, trait_ref, ty, is_automatically_derived);
+            check_partial_eq_without_eq(cx, item.span, trait_ref, ty, is_automatically_derived);
 
             if is_automatically_derived {
                 check_unsafe_derive_deserialize(cx, item, trait_ref, ty);
@@ -419,3 +454,43 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> {
         self.cx.tcx.hir()
     }
 }
+
+/// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint.
+fn check_partial_eq_without_eq<'tcx>(
+    cx: &LateContext<'tcx>,
+    span: Span,
+    trait_ref: &TraitRef<'_>,
+    ty: Ty<'tcx>,
+    peq_is_automatically_derived: bool,
+) {
+    if_chain! {
+        if let ty::Adt(adt, substs) = ty.kind();
+        if peq_is_automatically_derived;
+        if let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq);
+        if let Some(def_id) = trait_ref.trait_def_id();
+        if cx.tcx.is_diagnostic_item(sym::PartialEq, def_id);
+        if !implements_trait(cx, ty, eq_trait_def_id, substs);
+        then {
+            // If all of our fields implement `Eq`, we can implement `Eq` too
+            for variant in adt.variants() {
+                for field in &variant.fields {
+                    let ty = field.ty(cx.tcx, substs);
+
+                    if !implements_trait(cx, ty, eq_trait_def_id, substs) {
+                        return;
+                    }
+                }
+            }
+
+            span_lint_and_sugg(
+                cx,
+                DERIVE_PARTIAL_EQ_WITHOUT_EQ,
+                span.ctxt().outer_expn_data().call_site,
+                "you are deriving `PartialEq` and can implement `Eq`",
+                "consider deriving `Eq` as well",
+                "PartialEq, Eq".to_string(),
+                Applicability::MachineApplicable,
+            )
+        }
+    }
+}

clippy_lints/src/lib.register_lints.rs

@@ -113,6 +113,7 @@ store.register_lints(&[
     derivable_impls::DERIVABLE_IMPLS,
     derive::DERIVE_HASH_XOR_EQ,
     derive::DERIVE_ORD_XOR_PARTIAL_ORD,
+    derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ,
     derive::EXPL_IMPL_CLONE_ON_COPY,
     derive::UNSAFE_DERIVE_DESERIALIZE,
     disallowed_methods::DISALLOWED_METHODS,

clippy_lints/src/lib.register_nursery.rs

@@ -6,6 +6,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![
     LintId::of(attrs::EMPTY_LINE_AFTER_OUTER_ATTR),
     LintId::of(cognitive_complexity::COGNITIVE_COMPLEXITY),
     LintId::of(copies::BRANCHES_SHARING_CODE),
+    LintId::of(derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ),
     LintId::of(equatable_if_let::EQUATABLE_IF_LET),
     LintId::of(fallible_impl_from::FALLIBLE_IMPL_FROM),
     LintId::of(floating_point_arithmetic::IMPRECISE_FLOPS),

tests/ui/derive_partial_eq_without_eq.rs

@@ -0,0 +1,91 @@
+#![warn(clippy::derive_partial_eq_without_eq)]
+
+// Don't warn on structs that aren't PartialEq
+struct NotPartialEq {
+    foo: u32,
+    bar: String,
+}
+
+// Eq can be derived but is missing
+#[derive(Debug, PartialEq)]
+struct MissingEq {
+    foo: u32,
+    bar: String,
+}
+
+// Eq is derived
+#[derive(PartialEq, Eq)]
+struct NotMissingEq {
+    foo: u32,
+    bar: String,
+}
+
+// Eq is manually implemented
+#[derive(PartialEq)]
+struct ManualEqImpl {
+    foo: u32,
+    bar: String,
+}
+
+impl Eq for ManualEqImpl {}
+
+// Cannot be Eq because f32 isn't Eq
+#[derive(PartialEq)]
+struct CannotBeEq {
+    foo: u32,
+    bar: f32,
+}
+
+// Don't warn if PartialEq is manually implemented
+struct ManualPartialEqImpl {
+    foo: u32,
+    bar: String,
+}
+
+impl PartialEq for ManualPartialEqImpl {
+    fn eq(&self, other: &Self) -> bool {
+        self.foo == other.foo && self.bar == other.bar
+    }
+}
+
+// Generic fields should be properly checked for Eq-ness
+#[derive(PartialEq)]
+struct GenericNotEq<T: Eq, U: PartialEq> {
+    foo: T,
+    bar: U,
+}
+
+#[derive(PartialEq)]
+struct GenericEq<T: Eq, U: Eq> {
+    foo: T,
+    bar: U,
+}
+
+#[derive(PartialEq)]
+struct TupleStruct(u32);
+
+#[derive(PartialEq)]
+struct GenericTupleStruct<T: Eq>(T);
+
+#[derive(PartialEq)]
+struct TupleStructNotEq(f32);
+
+#[derive(PartialEq)]
+enum Enum {
+    Foo(u32),
+    Bar { a: String, b: () },
+}
+
+#[derive(PartialEq)]
+enum GenericEnum<T: Eq, U: Eq, V: Eq> {
+    Foo(T),
+    Bar { a: U, b: V },
+}
+
+#[derive(PartialEq)]
+enum EnumNotEq {
+    Foo(u32),
+    Bar { a: String, b: f32 },
+}
+
+fn main() {}

tests/ui/derive_partial_eq_without_eq.stderr

@@ -0,0 +1,40 @@
+error: you are deriving `PartialEq` and can implement `Eq`
+  --> $DIR/derive_partial_eq_without_eq.rs:10:17
+   |
+LL | #[derive(Debug, PartialEq)]
+   |                 ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
+   |
+   = note: `-D clippy::derive-partial-eq-without-eq` implied by `-D warnings`
+
+error: you are deriving `PartialEq` and can implement `Eq`
+  --> $DIR/derive_partial_eq_without_eq.rs:58:10
+   |
+LL | #[derive(PartialEq)]
+   |          ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
+
+error: you are deriving `PartialEq` and can implement `Eq`
+  --> $DIR/derive_partial_eq_without_eq.rs:64:10
+   |
+LL | #[derive(PartialEq)]
+   |          ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
+
+error: you are deriving `PartialEq` and can implement `Eq`
+  --> $DIR/derive_partial_eq_without_eq.rs:67:10
+   |
+LL | #[derive(PartialEq)]
+   |          ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
+
+error: you are deriving `PartialEq` and can implement `Eq`
+  --> $DIR/derive_partial_eq_without_eq.rs:73:10
+   |
+LL | #[derive(PartialEq)]
+   |          ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
+
+error: you are deriving `PartialEq` and can implement `Eq`
+  --> $DIR/derive_partial_eq_without_eq.rs:79:10
+   |
+LL | #[derive(PartialEq)]
+   |          ^^^^^^^^^ help: consider deriving `Eq` as well: `PartialEq, Eq`
+
+error: aborting due to 6 previous errors
+