[fn_params_excessive_bools] Make it possible to allow the lint at the method level

clippy_lints/src/excessive_bools.rs

@@ -1,8 +1,11 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use rustc_ast::ast::{AssocItemKind, Extern, Fn, FnSig, Impl, Item, ItemKind, Trait, Ty, TyKind};
-use rustc_lint::{EarlyContext, EarlyLintPass};
+use clippy_utils::{get_parent_as_impl, has_repr_attr, is_bool};
+use rustc_hir::intravisit::FnKind;
+use rustc_hir::{Body, FnDecl, HirId, Item, ItemKind, TraitFn, TraitItem, TraitItemKind, Ty};
+use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
-use rustc_span::{sym, Span};
+use rustc_span::Span;
+use rustc_target::spec::abi::Abi;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -83,6 +86,12 @@ 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 {
@@ -92,21 +101,20 @@ impl ExcessiveBools {
         }
     }
 
-    fn check_fn_sig(&self, cx: &EarlyContext<'_>, fn_sig: &FnSig, span: Span) {
-        match fn_sig.header.ext {
-            Extern::Implicit(_) | Extern::Explicit(_, _) => return,
-            Extern::None => (),
+    fn too_many_bools<'tcx>(&self, tys: impl Iterator<Item = &'tcx Ty<'tcx>>, 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
         }
+    }
 
-        let fn_sig_bools = fn_sig
-            .decl
-            .inputs
-            .iter()
-            .filter(|param| is_bool_ty(&param.ty))
-            .count()
-            .try_into()
-            .unwrap();
-        if self.max_fn_params_bools < fn_sig_bools {
+    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,
@@ -121,56 +129,55 @@ impl ExcessiveBools {
 
 impl_lint_pass!(ExcessiveBools => [STRUCT_EXCESSIVE_BOOLS, FN_PARAMS_EXCESSIVE_BOOLS]);
 
-fn is_bool_ty(ty: &Ty) -> bool {
-    if let TyKind::Path(None, path) = &ty.kind {
-        if let [name] = path.segments.as_slice() {
-            return name.ident.name == sym::bool;
+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",
+                );
+            }
         }
     }
-    false
-}
 
-impl EarlyLintPass for ExcessiveBools {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
-        if item.span.from_expansion() {
-            return;
+    fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx TraitItem<'tcx>) {
+        // 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
+            {
+            self.check_fn_sig(cx, fn_sig.decl, fn_sig.span);
         }
-        match &item.kind {
-            ItemKind::Struct(variant_data, _) => {
-                if item.attrs.iter().any(|attr| attr.has_name(sym::repr)) {
-                    return;
-                }
+    }
 
-                let struct_bools = variant_data
-                    .fields()
-                    .iter()
-                    .filter(|field| is_bool_ty(&field.ty))
-                    .count()
-                    .try_into()
-                    .unwrap();
-                if self.max_struct_bools < struct_bools {
-                    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",
-                    );
-                }
-            },
-            ItemKind::Impl(box Impl {
-                of_trait: None, items, ..
-            })
-            | ItemKind::Trait(box Trait { items, .. }) => {
-                for item in items {
-                    if let AssocItemKind::Fn(box Fn { sig, .. }) = &item.kind {
-                        self.check_fn_sig(cx, sig, item.span);
-                    }
-                }
-            },
-            ItemKind::Fn(box Fn { sig, .. }) => self.check_fn_sig(cx, sig, item.span),
-            _ => (),
+    fn check_fn(
+        &mut self,
+        cx: &LateContext<'tcx>,
+        fn_kind: FnKind<'tcx>,
+        fn_decl: &'tcx FnDecl<'tcx>,
+        _: &'tcx Body<'tcx>,
+        span: Span,
+        hir_id: HirId,
+    ) {
+        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()
+                )
+            {
+            self.check_fn_sig(cx, fn_decl, span);
         }
     }
 }

clippy_lints/src/lib.rs

@@ -793,7 +793,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     store.register_early_pass(|| Box::new(single_component_path_imports::SingleComponentPathImports));
     let max_fn_params_bools = conf.max_fn_params_bools;
     let max_struct_bools = conf.max_struct_bools;
-    store.register_early_pass(move || {
+    store.register_late_pass(move |_| {
         Box::new(excessive_bools::ExcessiveBools::new(
             max_struct_bools,
             max_fn_params_bools,

clippy_lints/src/methods/mod.rs

@@ -104,11 +104,10 @@ use bind_instead_of_map::BindInsteadOfMap;
 use clippy_utils::consts::{constant, Constant};
 use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
 use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
-use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
+use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
 use if_chain::if_chain;
 use rustc_hir as hir;
-use rustc_hir::def::Res;
-use rustc_hir::{Expr, ExprKind, PrimTy, QPath, TraitItem, TraitItemKind};
+use rustc_hir::{Expr, ExprKind, TraitItem, TraitItemKind};
 use rustc_hir_analysis::hir_ty_to_ty;
 use rustc_lint::{LateContext, LateLintPass, LintContext};
 use rustc_middle::lint::in_external_macro;
@@ -3966,14 +3965,6 @@ impl OutType {
     }
 }
 
-fn is_bool(ty: &hir::Ty<'_>) -> bool {
-    if let hir::TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
-        matches!(path.res, Res::PrimTy(PrimTy::Bool))
-    } else {
-        false
-    }
-}
-
 fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool {
     expected.constness == actual.constness
         && expected.unsafety == actual.unsafety

clippy_lints/src/trailing_empty_array.rs

@@ -1,9 +1,9 @@
 use clippy_utils::diagnostics::span_lint_and_help;
-use rustc_hir::{HirId, Item, ItemKind};
+use clippy_utils::has_repr_attr;
+use rustc_hir::{Item, ItemKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty::Const;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::sym;
 
 declare_clippy_lint! {
     /// ### What it does
@@ -72,7 +72,3 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'_>, item: &Item<'_
         }
     }
 }
-
-fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool {
-    cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr))
-}

clippy_utils/src/hir_utils.rs

@@ -8,7 +8,7 @@ use rustc_hir::HirIdMap;
 use rustc_hir::{
     ArrayLen, BinOpKind, BindingAnnotation, Block, BodyId, Closure, Expr, ExprField, ExprKind, FnRetTy, GenericArg,
     GenericArgs, Guard, HirId, InlineAsmOperand, Let, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path,
-    PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding,
+    PathSegment, PrimTy, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding,
 };
 use rustc_lexer::{tokenize, TokenKind};
 use rustc_lint::LateContext;
@@ -1030,6 +1030,14 @@ pub fn hash_stmt(cx: &LateContext<'_>, s: &Stmt<'_>) -> u64 {
     h.finish()
 }
 
+pub fn is_bool(ty: &Ty<'_>) -> bool {
+    if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
+        matches!(path.res, Res::PrimTy(PrimTy::Bool))
+    } else {
+        false
+    }
+}
+
 pub fn hash_expr(cx: &LateContext<'_>, e: &Expr<'_>) -> u64 {
     let mut h = SpanlessHash::new(cx);
     h.hash_expr(e);

clippy_utils/src/lib.rs

@@ -66,7 +66,7 @@ pub mod visitors;
 pub use self::attrs::*;
 pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match};
 pub use self::hir_utils::{
-    both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash,
+    both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over, HirEqInterExpr, SpanlessEq, SpanlessHash,
 };
 
 use core::ops::ControlFlow;
@@ -1780,6 +1780,10 @@ pub fn has_attr(attrs: &[ast::Attribute], symbol: Symbol) -> bool {
     attrs.iter().any(|attr| attr.has_name(symbol))
 }
 
+pub fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool {
+    has_attr(cx.tcx.hir().attrs(hir_id), sym::repr)
+}
+
 pub fn any_parent_has_attr(tcx: TyCtxt<'_>, node: HirId, symbol: Symbol) -> bool {
     let map = &tcx.hir();
     let mut prev_enclosing_node = None;

tests/ui/fn_params_excessive_bools.rs

@@ -2,6 +2,7 @@
 #![allow(clippy::too_many_arguments)]
 
 extern "C" {
+    // Should not lint, most of the time users have no control over extern function signatures
     fn f(_: bool, _: bool, _: bool, _: bool);
 }
 
@@ -22,8 +23,12 @@ fn t(_: S, _: S, _: Box<S>, _: Vec<u32>, _: bool, _: bool, _: bool, _: bool) {}
 
 struct S;
 trait Trait {
+    // should warn for trait functions with and without body
     fn f(_: bool, _: bool, _: bool, _: bool);
     fn g(_: bool, _: bool, _: bool, _: Vec<u32>);
+    #[allow(clippy::fn_params_excessive_bools)]
+    fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool);
+    fn i(_: bool, _: bool, _: bool, _: bool) {}
 }
 
 impl S {
@@ -34,8 +39,11 @@ impl S {
 }
 
 impl Trait for S {
+    // Should not lint because the trait might not be changeable by the user
+    // We only lint in the trait definition
     fn f(_: bool, _: bool, _: bool, _: bool) {}
     fn g(_: bool, _: bool, _: bool, _: Vec<u32>) {}
+    fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool) {}
 }
 
 fn main() {

tests/ui/fn_params_excessive_bools.stderr

@@ -1,5 +1,5 @@
 error: more than 3 bools in function parameters
-  --> $DIR/fn_params_excessive_bools.rs:18:1
+  --> $DIR/fn_params_excessive_bools.rs:19:1
    |
 LL | fn g(_: bool, _: bool, _: bool, _: bool) {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -8,31 +8,39 @@ LL | fn g(_: bool, _: bool, _: bool, _: bool) {}
    = note: `-D clippy::fn-params-excessive-bools` implied by `-D warnings`
 
 error: more than 3 bools in function parameters
-  --> $DIR/fn_params_excessive_bools.rs:21:1
+  --> $DIR/fn_params_excessive_bools.rs:22:1
    |
 LL | fn t(_: S, _: S, _: Box<S>, _: Vec<u32>, _: bool, _: bool, _: bool, _: bool) {}
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider refactoring bools into two-variant enums
 
 error: more than 3 bools in function parameters
-  --> $DIR/fn_params_excessive_bools.rs:25:5
+  --> $DIR/fn_params_excessive_bools.rs:27:5
    |
 LL |     fn f(_: bool, _: bool, _: bool, _: bool);
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider refactoring bools into two-variant enums
 
 error: more than 3 bools in function parameters
-  --> $DIR/fn_params_excessive_bools.rs:30:5
+  --> $DIR/fn_params_excessive_bools.rs:31:5
+   |
+LL |     fn i(_: bool, _: bool, _: bool, _: bool) {}
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: consider refactoring bools into two-variant enums
+
+error: more than 3 bools in function parameters
+  --> $DIR/fn_params_excessive_bools.rs:35:5
    |
 LL |     fn f(&self, _: bool, _: bool, _: bool, _: bool) {}
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider refactoring bools into two-variant enums
 
 error: more than 3 bools in function parameters
-  --> $DIR/fn_params_excessive_bools.rs:42:5
+  --> $DIR/fn_params_excessive_bools.rs:50:5
    |
 LL | /     fn n(_: bool, _: u32, _: bool, _: Box<u32>, _: bool, _: bool) {
 LL | |         fn nn(_: bool, _: bool, _: bool, _: bool) {}
@@ -42,12 +50,12 @@ LL | |     }
    = help: consider refactoring bools into two-variant enums
 
 error: more than 3 bools in function parameters
-  --> $DIR/fn_params_excessive_bools.rs:43:9
+  --> $DIR/fn_params_excessive_bools.rs:51:9
    |
 LL |         fn nn(_: bool, _: bool, _: bool, _: bool) {}
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = help: consider refactoring bools into two-variant enums
 
-error: aborting due to 6 previous errors
+error: aborting due to 7 previous errors