From e683e3eeac5603296c328ff45c9e5bb911327756 Mon Sep 17 00:00:00 2001 From: Victor Song Date: Sat, 30 Sep 2023 22:57:54 -0500 Subject: [PATCH 1/2] Don't lint `manual_non_exhaustive` when enum explicitly marked as `non_exhaustive` There are cases where users create a unit variant for the purposes of tracking the number of variants for an nonexhaustive enum. We should check if an enum is explicitly marked as nonexhaustive before reporting `manual_non_exhaustive` in these cases. Fixes #11583 --- clippy_lints/src/manual_non_exhaustive.rs | 4 +++- tests/ui/manual_non_exhaustive_enum.rs | 3 +-- tests/ui/manual_non_exhaustive_enum.stderr | 20 +------------------- 3 files changed, 5 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs index 0e22485db2c5..9fe55d819da7 100644 --- a/clippy_lints/src/manual_non_exhaustive.rs +++ b/clippy_lints/src/manual_non_exhaustive.rs @@ -3,6 +3,7 @@ use clippy_utils::is_doc_hidden; use clippy_utils::msrvs::{self, Msrv}; use clippy_utils::source::snippet_opt; use rustc_ast::ast::{self, VisibilityKind}; +use rustc_ast::attr; use rustc_data_structures::fx::FxHashSet; use rustc_errors::Applicability; use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res}; @@ -158,7 +159,8 @@ impl<'tcx> LateLintPass<'tcx> for ManualNonExhaustiveEnum { let mut iter = def.variants.iter().filter_map(|v| { (matches!(v.data, hir::VariantData::Unit(_, _)) && v.ident.as_str().starts_with('_') - && is_doc_hidden(cx.tcx.hir().attrs(v.hir_id))) + && is_doc_hidden(cx.tcx.hir().attrs(v.hir_id)) + && !attr::contains_name(cx.tcx.hir().attrs(item.hir_id()), sym::non_exhaustive)) .then_some((v.def_id, v.span)) }); if let Some((id, span)) = iter.next() diff --git a/tests/ui/manual_non_exhaustive_enum.rs b/tests/ui/manual_non_exhaustive_enum.rs index 0e439dabfd65..e32ba8631761 100644 --- a/tests/ui/manual_non_exhaustive_enum.rs +++ b/tests/ui/manual_non_exhaustive_enum.rs @@ -10,10 +10,9 @@ enum E { _C, } -// user forgot to remove the marker +// if the user explicitly marks as nonexhaustive we shouldn't warn them #[non_exhaustive] enum Ep { - //~^ ERROR: this seems like a manual implementation of the non-exhaustive pattern A, B, #[doc(hidden)] diff --git a/tests/ui/manual_non_exhaustive_enum.stderr b/tests/ui/manual_non_exhaustive_enum.stderr index ce7e21c94bbd..7361a4a2cbbe 100644 --- a/tests/ui/manual_non_exhaustive_enum.stderr +++ b/tests/ui/manual_non_exhaustive_enum.stderr @@ -22,23 +22,5 @@ LL | _C, = note: `-D clippy::manual-non-exhaustive` implied by `-D warnings` = help: to override `-D warnings` add `#[allow(clippy::manual_non_exhaustive)]` -error: this seems like a manual implementation of the non-exhaustive pattern - --> $DIR/manual_non_exhaustive_enum.rs:15:1 - | -LL | / enum Ep { -LL | | -LL | | A, -LL | | B, -LL | | #[doc(hidden)] -LL | | _C, -LL | | } - | |_^ - | -help: remove this variant - --> $DIR/manual_non_exhaustive_enum.rs:20:5 - | -LL | _C, - | ^^ - -error: aborting due to 2 previous errors +error: aborting due to previous error From 9dfd60cf4f33c9af8e37d4a4956f56626bac2b3c Mon Sep 17 00:00:00 2001 From: Victor Song Date: Sun, 1 Oct 2023 09:54:45 -0500 Subject: [PATCH 2/2] Remove extraneous `#[non_exhaustive]` check in lint --- clippy_lints/src/manual_non_exhaustive.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/manual_non_exhaustive.rs b/clippy_lints/src/manual_non_exhaustive.rs index 9fe55d819da7..c12727c4a28c 100644 --- a/clippy_lints/src/manual_non_exhaustive.rs +++ b/clippy_lints/src/manual_non_exhaustive.rs @@ -200,16 +200,14 @@ impl<'tcx> LateLintPass<'tcx> for ManualNonExhaustiveEnum { enum_span, "this seems like a manual implementation of the non-exhaustive pattern", |diag| { - if !cx.tcx.adt_def(enum_id).is_variant_list_non_exhaustive() - && let header_span = cx.sess().source_map().span_until_char(enum_span, '{') - && let Some(snippet) = snippet_opt(cx, header_span) - { - diag.span_suggestion( - header_span, - "add the attribute", - format!("#[non_exhaustive] {snippet}"), - Applicability::Unspecified, - ); + let header_span = cx.sess().source_map().span_until_char(enum_span, '{'); + if let Some(snippet) = snippet_opt(cx, header_span) { + diag.span_suggestion( + header_span, + "add the attribute", + format!("#[non_exhaustive] {snippet}"), + Applicability::Unspecified, + ); } diag.span_help(variant_span, "remove this variant"); },