From a1989046c195f40f90670318c848bf843152ab74 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Jan 2024 23:26:38 +0100 Subject: [PATCH 1/3] Don't emit `derive_partial_eq_without_eq` lint if the type has the `non_exhaustive` attribute --- clippy_lints/src/derive.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index d8abe411030b..aab2550ba6eb 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -11,8 +11,8 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_middle::traits::Reveal; use rustc_middle::ty::{ - self, ClauseKind, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, ToPredicate, TraitPredicate, Ty, - TyCtxt, + self, AdtDef, ClauseKind, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, ToPredicate, TraitPredicate, + Ty, TyCtxt, }; use rustc_session::declare_lint_pass; use rustc_span::def_id::LocalDefId; @@ -442,6 +442,17 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { } } +fn has_non_exhaustive_attr(cx: &LateContext<'_>, adt: AdtDef<'_>) -> bool { + adt.is_variant_list_non_exhaustive() + || cx.tcx.has_attr(adt.did(), sym::non_exhaustive) + || adt.variants().iter().any(|variant_def| { + variant_def.is_field_list_non_exhaustive() || cx.tcx.has_attr(variant_def.def_id, sym::non_exhaustive) + }) + || adt + .all_fields() + .any(|field_def| cx.tcx.has_attr(field_def.did, sym::non_exhaustive)) +} + /// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint. fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { if let ty::Adt(adt, args) = ty.kind() @@ -449,6 +460,7 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r && let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq) && let Some(def_id) = trait_ref.trait_def_id() && cx.tcx.is_diagnostic_item(sym::PartialEq, def_id) + && !has_non_exhaustive_attr(cx, *adt) && let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id) && !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, adt.did(),&[]) // If all of our fields implement `Eq`, we can implement `Eq` too From 136a582349a5f10ebde381cfd1eb30b03745c5c1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 15 Jan 2024 23:27:47 +0100 Subject: [PATCH 2/3] Add `non_exhaustive` checks in `derive_partial_eq_without_eq.rs` ui test --- tests/ui/derive_partial_eq_without_eq.fixed | 32 +++++++++++++++++++++ tests/ui/derive_partial_eq_without_eq.rs | 32 +++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/tests/ui/derive_partial_eq_without_eq.fixed b/tests/ui/derive_partial_eq_without_eq.fixed index a7f5d3ec7cd0..aa7a95405e02 100644 --- a/tests/ui/derive_partial_eq_without_eq.fixed +++ b/tests/ui/derive_partial_eq_without_eq.fixed @@ -121,4 +121,36 @@ pub fn _from_mod() -> _hidden::InPubFn { #[derive(PartialEq)] struct InternalTy; +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +#[non_exhaustive] +pub struct MissingEqNonExhaustive { + foo: u32, + bar: String, +} + +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +pub struct MissingEqNonExhaustive1 { + foo: u32, + #[non_exhaustive] + bar: String, +} + +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +#[non_exhaustive] +pub enum MissingEqNonExhaustive2 { + Foo, + Bar, +} + +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +pub enum MissingEqNonExhaustive3 { + Foo, + #[non_exhaustive] + Bar, +} + fn main() {} diff --git a/tests/ui/derive_partial_eq_without_eq.rs b/tests/ui/derive_partial_eq_without_eq.rs index 476d2aee23a8..90ac7a7989e0 100644 --- a/tests/ui/derive_partial_eq_without_eq.rs +++ b/tests/ui/derive_partial_eq_without_eq.rs @@ -121,4 +121,36 @@ pub fn _from_mod() -> _hidden::InPubFn { #[derive(PartialEq)] struct InternalTy; +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +#[non_exhaustive] +pub struct MissingEqNonExhaustive { + foo: u32, + bar: String, +} + +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +pub struct MissingEqNonExhaustive1 { + foo: u32, + #[non_exhaustive] + bar: String, +} + +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +#[non_exhaustive] +pub enum MissingEqNonExhaustive2 { + Foo, + Bar, +} + +// This is a `non_exhaustive` type so should not warn. +#[derive(Debug, PartialEq)] +pub enum MissingEqNonExhaustive3 { + Foo, + #[non_exhaustive] + Bar, +} + fn main() {} From fd6e752f4e4fef715144b360fb7e29b4f3d10df9 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 20 Jan 2024 16:57:43 +0100 Subject: [PATCH 3/3] Move `has_non_exhaustive_attr` function into `clippy_utils` --- clippy_lints/src/derive.rs | 19 ++++--------------- clippy_utils/src/attrs.rs | 12 ++++++++++++ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/clippy_lints/src/derive.rs b/clippy_lints/src/derive.rs index aab2550ba6eb..081cbcb770fb 100644 --- a/clippy_lints/src/derive.rs +++ b/clippy_lints/src/derive.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::ty::{implements_trait, implements_trait_with_env, is_copy}; -use clippy_utils::{is_lint_allowed, match_def_path, paths}; +use clippy_utils::{has_non_exhaustive_attr, is_lint_allowed, match_def_path, paths}; use rustc_errors::Applicability; use rustc_hir::def_id::DefId; use rustc_hir::intravisit::{walk_expr, walk_fn, walk_item, FnKind, Visitor}; @@ -11,8 +11,8 @@ use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::hir::nested_filter; use rustc_middle::traits::Reveal; use rustc_middle::ty::{ - self, AdtDef, ClauseKind, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, ToPredicate, TraitPredicate, - Ty, TyCtxt, + self, ClauseKind, GenericArgKind, GenericParamDefKind, ImplPolarity, ParamEnv, ToPredicate, TraitPredicate, Ty, + TyCtxt, }; use rustc_session::declare_lint_pass; use rustc_span::def_id::LocalDefId; @@ -442,17 +442,6 @@ impl<'tcx> Visitor<'tcx> for UnsafeVisitor<'_, 'tcx> { } } -fn has_non_exhaustive_attr(cx: &LateContext<'_>, adt: AdtDef<'_>) -> bool { - adt.is_variant_list_non_exhaustive() - || cx.tcx.has_attr(adt.did(), sym::non_exhaustive) - || adt.variants().iter().any(|variant_def| { - variant_def.is_field_list_non_exhaustive() || cx.tcx.has_attr(variant_def.def_id, sym::non_exhaustive) - }) - || adt - .all_fields() - .any(|field_def| cx.tcx.has_attr(field_def.did, sym::non_exhaustive)) -} - /// Implementation of the `DERIVE_PARTIAL_EQ_WITHOUT_EQ` lint. fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_ref: &hir::TraitRef<'_>, ty: Ty<'tcx>) { if let ty::Adt(adt, args) = ty.kind() @@ -460,7 +449,7 @@ fn check_partial_eq_without_eq<'tcx>(cx: &LateContext<'tcx>, span: Span, trait_r && let Some(eq_trait_def_id) = cx.tcx.get_diagnostic_item(sym::Eq) && let Some(def_id) = trait_ref.trait_def_id() && cx.tcx.is_diagnostic_item(sym::PartialEq, def_id) - && !has_non_exhaustive_attr(cx, *adt) + && !has_non_exhaustive_attr(cx.tcx, *adt) && let param_env = param_env_for_derived_eq(cx.tcx, adt.did(), eq_trait_def_id) && !implements_trait_with_env(cx.tcx, param_env, ty, eq_trait_def_id, adt.did(),&[]) // If all of our fields implement `Eq`, we can implement `Eq` too diff --git a/clippy_utils/src/attrs.rs b/clippy_utils/src/attrs.rs index ad8619f0d3d9..2d0c2cf12536 100644 --- a/clippy_utils/src/attrs.rs +++ b/clippy_utils/src/attrs.rs @@ -1,5 +1,6 @@ use rustc_ast::{ast, attr}; use rustc_errors::Applicability; +use rustc_middle::ty::{AdtDef, TyCtxt}; use rustc_session::Session; use rustc_span::sym; use std::str::FromStr; @@ -159,3 +160,14 @@ pub fn is_doc_hidden(attrs: &[ast::Attribute]) -> bool { .filter_map(ast::Attribute::meta_item_list) .any(|l| attr::list_contains_name(&l, sym::hidden)) } + +pub fn has_non_exhaustive_attr(tcx: TyCtxt<'_>, adt: AdtDef<'_>) -> bool { + adt.is_variant_list_non_exhaustive() + || tcx.has_attr(adt.did(), sym::non_exhaustive) + || adt.variants().iter().any(|variant_def| { + variant_def.is_field_list_non_exhaustive() || tcx.has_attr(variant_def.def_id, sym::non_exhaustive) + }) + || adt + .all_fields() + .any(|field_def| tcx.has_attr(field_def.did, sym::non_exhaustive)) +}