From f178316ae24453cf85d3667fed164895d260a1aa Mon Sep 17 00:00:00 2001 From: Jason Newcomb Date: Tue, 11 Jun 2024 00:25:17 -0400 Subject: [PATCH] Refactor `excessive_bools`: * Check HIR tree before checking for macros. * Check item count before checking for bools. --- clippy_lints/src/excessive_bools.rs | 97 +++++++++++++---------------- 1 file changed, 44 insertions(+), 53 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 62d5ce24d..76e72eefa 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -87,12 +87,6 @@ pub struct ExcessiveBools { max_fn_params_bools: u64, } -#[derive(Eq, PartialEq, Debug, Copy, Clone)] -enum Kind { - Struct, - Fn, -} - impl ExcessiveBools { #[must_use] pub fn new(max_struct_bools: u64, max_fn_params_bools: u64) -> Self { @@ -101,55 +95,50 @@ impl ExcessiveBools { max_fn_params_bools, } } - - fn too_many_bools<'tcx>(&self, tys: impl Iterator>, kind: Kind) -> bool { - if let Ok(bools) = tys.filter(|ty| is_bool(ty)).count().try_into() { - (if Kind::Fn == kind { - self.max_fn_params_bools - } else { - self.max_struct_bools - }) < bools - } else { - false - } - } - - fn check_fn_sig(&self, cx: &LateContext<'_>, fn_decl: &FnDecl<'_>, span: Span) { - if !span.from_expansion() && self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) { - span_lint_and_help( - cx, - FN_PARAMS_EXCESSIVE_BOOLS, - span, - format!("more than {} bools in function parameters", self.max_fn_params_bools), - None, - "consider refactoring bools into two-variant enums", - ); - } - } } impl_lint_pass!(ExcessiveBools => [STRUCT_EXCESSIVE_BOOLS, FN_PARAMS_EXCESSIVE_BOOLS]); +fn has_n_bools<'tcx>(iter: impl Iterator>, mut count: u64) -> bool { + iter.filter(|ty| is_bool(ty)).any(|_| { + let (x, overflow) = count.overflowing_sub(1); + count = x; + overflow + }) +} + +fn check_fn_decl(cx: &LateContext<'_>, decl: &FnDecl<'_>, sp: Span, max: u64) { + if has_n_bools(decl.inputs.iter(), max) && !sp.from_expansion() { + span_lint_and_help( + cx, + FN_PARAMS_EXCESSIVE_BOOLS, + sp, + format!("more than {max} bools in function parameters"), + None, + "consider refactoring bools into two-variant enums", + ); + } +} + impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { - if item.span.from_expansion() { - return; - } - if let ItemKind::Struct(variant_data, _) = &item.kind { - if has_repr_attr(cx, item.hir_id()) { - return; - } - - if self.too_many_bools(variant_data.fields().iter().map(|field| field.ty), Kind::Struct) { - span_lint_and_help( - cx, - STRUCT_EXCESSIVE_BOOLS, - item.span, - format!("more than {} bools in a struct", self.max_struct_bools), - None, - "consider using a state machine or refactoring bools into two-variant enums", - ); - } + if let ItemKind::Struct(variant_data, _) = &item.kind + && variant_data.fields().len() as u64 > self.max_struct_bools + && has_n_bools( + variant_data.fields().iter().map(|field| field.ty), + self.max_struct_bools, + ) + && !has_repr_attr(cx, item.hir_id()) + && !item.span.from_expansion() + { + span_lint_and_help( + cx, + STRUCT_EXCESSIVE_BOOLS, + item.span, + format!("more than {} bools in a struct", self.max_struct_bools), + None, + "consider using a state machine or refactoring bools into two-variant enums", + ); } } @@ -157,8 +146,9 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { // functions with a body are already checked by `check_fn` if let TraitItemKind::Fn(fn_sig, TraitFn::Required(_)) = &trait_item.kind && fn_sig.header.abi == Abi::Rust + && fn_sig.decl.inputs.len() as u64 > self.max_fn_params_bools { - self.check_fn_sig(cx, fn_sig.decl, fn_sig.span); + check_fn_decl(cx, fn_sig.decl, fn_sig.span, self.max_fn_params_bools); } } @@ -171,12 +161,13 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { span: Span, def_id: LocalDefId, ) { - let hir_id = cx.tcx.local_def_id_to_hir_id(def_id); if let Some(fn_header) = fn_kind.header() && fn_header.abi == Abi::Rust - && get_parent_as_impl(cx.tcx, hir_id).map_or(true, |impl_item| impl_item.of_trait.is_none()) + && fn_decl.inputs.len() as u64 > self.max_fn_params_bools + && get_parent_as_impl(cx.tcx, cx.tcx.local_def_id_to_hir_id(def_id)) + .map_or(true, |impl_item| impl_item.of_trait.is_none()) { - self.check_fn_sig(cx, fn_decl, span); + check_fn_decl(cx, fn_decl, span, self.max_fn_params_bools); } } }