New lint: [derive_partial_eq_without_eq]

This commit is contained in:
nsunderland1 2022-05-06 00:10:11 -07:00
parent 77effb7bb2
commit fe84ff3360
29 changed files with 359 additions and 38 deletions

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

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

View file

@ -271,7 +271,7 @@ enum IterUsageKind {
NextTuple,
}
#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Eq)]
enum UnwrapKind {
Unwrap,
QuestionMark,