Auto merge of #9698 - kraktus:xc_bool, r=xFrednet

[`fn_params_excessive_bools`] Make it possible to allow the lint at the method level changelog: FP: [`fn_params_excessive_bools`]: `#[allow]` now works on methods fix https://github.com/rust-lang/rust-clippy/issues/9687 Tested without committing but `#[allow]`ing now works. Also rewrote the lint to be a late lint while at it :) r? `@xFrednet`
2025-01-31 17:12:53 +00:00 · 2022-11-13 09:57:36 +00:00 · 2022-11-13 09:57:36 +00:00 · a8151409a0
commit a8151409a0
parent 9f283c97e1 3d4b73c26d
8 changed files with 111 additions and 89 deletions
--- a/clippy_lints/src/excessive_bools.rs
+++ b/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;
-        }
-    }
-    false
-}
-
-impl EarlyLintPass for ExcessiveBools {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+impl<'tcx> LateLintPass<'tcx> for ExcessiveBools {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
        if item.span.from_expansion() {
            return;
        }
-        match &item.kind {
-            ItemKind::Struct(variant_data, _) => {
-                if item.attrs.iter().any(|attr| attr.has_name(sym::repr)) {
-                    return;
-                }
+        if let ItemKind::Struct(variant_data, _) = &item.kind {
+            if has_repr_attr(cx, item.hir_id()) {
+                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),
-            _ => (),
+            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",
+                );
+            }
+        }
+    }
+
+    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);
+        }
+    }
+
+    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);
        }
    }
 }
--- a/clippy_lints/src/lib.rs
+++ b/clippy_lints/src/lib.rs
@ -795,7 +795,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,
--- a/clippy_lints/src/methods/mod.rs
+++ b/clippy_lints/src/methods/mod.rs
@ -105,11 +105,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;
@ -3992,14 +3991,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
--- a/clippy_lints/src/trailing_empty_array.rs
+++ b/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))
-}
--- a/clippy_utils/src/hir_utils.rs
+++ b/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);
--- a/clippy_utils/src/lib.rs
+++ b/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;
--- a/tests/ui/fn_params_excessive_bools.rs
+++ b/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() {
--- a/tests/ui/fn_params_excessive_bools.stderr
+++ b/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,7 +8,7 @@ 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) {}
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -16,7 +16,7 @@ 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);
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -24,7 +24,15 @@ 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) {}
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@ -32,7 +40,7 @@ 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