New lint: [`derive_partial_eq_without_eq`]

8 files changed, 77 insertions, 6 deletions

diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs
index e23428e216d..28c77ea40ef 100644
--- a/clippy_lints/src/checked_conversions.rs
+++ b/clippy_lints/src/checked_conversions.rs
@@ -123,7 +123,7 @@ struct Conversion<'a> {
 }
 
 /// The kind of conversion that is checked
-#[derive(Copy, Clone, Debug, PartialEq)]
+#[derive(Copy, Clone, Debug, PartialEq, Eq)]
 enum ConversionType {
     SignedToUnsigned,
     SignedToSigned,
diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs
index 557e101494e..a4757ebd8c7 100644
--- a/clippy_lints/src/derive.rs
+++ b/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,
+    style,
+    "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 {
@@ -179,6 +213,7 @@ impl<'tcx> LateLintPass<'tcx> for Derive {
 
             if is_automatically_derived {
                 check_unsafe_derive_deserialize(cx, item, trait_ref, ty);
+                check_partial_eq_without_eq(cx, item.span, trait_ref, ty);
             } else {
                 check_copy_clone(cx, item, trait_ref, ty);
             }
@@ -419,3 +454,36 @@ 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>) {
+    if_chain! {
+        if let ty::Adt(adt, substs) = ty.kind();
+        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,
+            )
+        }
+    }
+}
diff --git a/clippy_lints/src/lib.register_all.rs b/clippy_lints/src/lib.register_all.rs
index e68718f9fdf..0f43b320f76 100644
--- a/clippy_lints/src/lib.register_all.rs
+++ b/clippy_lints/src/lib.register_all.rs
@@ -46,6 +46,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(derivable_impls::DERIVABLE_IMPLS),
     LintId::of(derive::DERIVE_HASH_XOR_EQ),
     LintId::of(derive::DERIVE_ORD_XOR_PARTIAL_ORD),
+    LintId::of(derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ),
     LintId::of(disallowed_methods::DISALLOWED_METHODS),
     LintId::of(disallowed_types::DISALLOWED_TYPES),
     LintId::of(doc::MISSING_SAFETY_DOC),
diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs
index 5768edc5018..4e302b10a0f 100644
--- a/clippy_lints/src/lib.register_lints.rs
+++ b/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,
diff --git a/clippy_lints/src/lib.register_style.rs b/clippy_lints/src/lib.register_style.rs
index d183c0449cd..62f26d821a0 100644
--- a/clippy_lints/src/lib.register_style.rs
+++ b/clippy_lints/src/lib.register_style.rs
@@ -16,6 +16,7 @@ store.register_group(true, "clippy::style", Some("clippy_style"), vec![
     LintId::of(comparison_chain::COMPARISON_CHAIN),
     LintId::of(default::FIELD_REASSIGN_WITH_DEFAULT),
     LintId::of(dereference::NEEDLESS_BORROW),
+    LintId::of(derive::DERIVE_PARTIAL_EQ_WITHOUT_EQ),
     LintId::of(disallowed_methods::DISALLOWED_METHODS),
     LintId::of(disallowed_types::DISALLOWED_TYPES),
     LintId::of(doc::MISSING_SAFETY_DOC),
diff --git a/clippy_lints/src/loops/utils.rs b/clippy_lints/src/loops/utils.rs
index 772d251b620..4801a84eb92 100644
--- a/clippy_lints/src/loops/utils.rs
+++ b/clippy_lints/src/loops/utils.rs
@@ -13,7 +13,7 @@ use rustc_span::symbol::{sym, Symbol};
 use rustc_typeck::hir_ty_to_ty;
 use std::iter::Iterator;
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Eq)]
 enum IncrementVisitorVarState {
     Initial,  // Not examined yet
     IncrOnce, // Incremented exactly once, may be a loop counter
diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs
index e452614ce17..cfeee4d0c70 100644
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@@ -2835,7 +2835,7 @@ const TRAIT_METHODS: [ShouldImplTraitCase; 30] = [
     ShouldImplTraitCase::new("std::ops::Sub", "sub",  2,  FN_HEADER,  SelfKind::Value,  OutType::Any, true),
 ];
 
-#[derive(Clone, Copy, PartialEq, Debug)]
+#[derive(Clone, Copy, PartialEq, Eq, Debug)]
 enum SelfKind {
     Value,
     Ref,
diff --git a/clippy_lints/src/methods/str_splitn.rs b/clippy_lints/src/methods/str_splitn.rs
index fc375763542..90651a6ba04 100644
--- a/clippy_lints/src/methods/str_splitn.rs
+++ b/clippy_lints/src/methods/str_splitn.rs
@@ -271,7 +271,7 @@ enum IterUsageKind {
     NextTuple,
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Eq)]
 enum UnwrapKind {
     Unwrap,
     QuestionMark,