From 586bd3f735a7ec8c0ee03ab3515dfd8a89a31ff4 Mon Sep 17 00:00:00 2001 From: kraktus Date: Sun, 23 Oct 2022 14:00:44 +0200 Subject: [PATCH 1/7] refactor to avoid potential ICE --- clippy_lints/src/excessive_bools.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 453471c8cdda..e1911449242d 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -141,14 +141,13 @@ impl EarlyLintPass for ExcessiveBools { return; } - let struct_bools = variant_data + if let Ok(struct_bools) = variant_data .fields() .iter() .filter(|field| is_bool_ty(&field.ty)) .count() - .try_into() - .unwrap(); - if self.max_struct_bools < struct_bools { + .try_into() && self.max_struct_bools < struct_bools + { span_lint_and_help( cx, STRUCT_EXCESSIVE_BOOLS, @@ -156,8 +155,8 @@ impl EarlyLintPass for ExcessiveBools { &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, .. From 9c69e93595e581a92c8385b633e687393cb07471 Mon Sep 17 00:00:00 2001 From: kraktus Date: Fri, 28 Oct 2022 14:54:59 +0200 Subject: [PATCH 2/7] Rewrite `ExcessiveBools` to be a `LateLintPass` lint changelog: [`fn_params_excessive_bools`] Make it possible to allow the lint at the method level --- clippy_lints/src/excessive_bools.rs | 126 +++++++++++----------- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/methods/mod.rs | 13 +-- clippy_utils/src/hir_utils.rs | 10 +- clippy_utils/src/lib.rs | 2 +- tests/ui/fn_params_excessive_bools.rs | 1 + tests/ui/fn_params_excessive_bools.stderr | 20 ++-- 7 files changed, 90 insertions(+), 84 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index e1911449242d..08164955c6a3 100644 --- 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_node, is_bool}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, FnDecl, HirId, Item, ItemKind, Node, Ty}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::{sym, 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)] +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>, 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(¶m.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 self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) { span_lint_and_help( cx, FN_PARAMS_EXCESSIVE_BOOLS, @@ -121,55 +129,53 @@ 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 cx + .tcx + .hir() + .attrs(item.hir_id()) + .iter() + .any(|attr| attr.has_name(sym::repr)) + { + 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 Ok(struct_bools) = variant_data - .fields() - .iter() - .filter(|field| is_bool_ty(&field.ty)) - .count() - .try_into() && 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 + && if let Some(Node::Item(item)) = get_parent_node(cx.tcx, hir_id) { + !matches!(item.kind, ItemKind::ExternCrate(..)) + } else { + true + } + && !span.from_expansion() { + self.check_fn_sig(cx, fn_decl, span) } } } diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 01da11958e2a..e96075a2673f 100644 --- a/clippy_lints/src/lib.rs +++ b/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, diff --git a/clippy_lints/src/methods/mod.rs b/clippy_lints/src/methods/mod.rs index 6665329d1148..996d4cb41234 100644 --- a/clippy_lints/src/methods/mod.rs +++ b/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 diff --git a/clippy_utils/src/hir_utils.rs b/clippy_utils/src/hir_utils.rs index 02b973e5b278..da8c976c952c 100644 --- 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); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index dda4660fb7a3..0000896fdb65 100644 --- 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; diff --git a/tests/ui/fn_params_excessive_bools.rs b/tests/ui/fn_params_excessive_bools.rs index f805bcc9ba8a..4fcd4d54ce62 100644 --- 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); } diff --git a/tests/ui/fn_params_excessive_bools.stderr b/tests/ui/fn_params_excessive_bools.stderr index 11627105691b..9d22d3851d2f 100644 --- 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, _: Vec, _: bool, _: bool, _: bool, _: bool) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -16,23 +16,23 @@ LL | fn t(_: S, _: S, _: Box, _: Vec, _: 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:31:5 | -LL | fn f(_: bool, _: bool, _: bool, _: bool); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +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:30:5 + --> $DIR/fn_params_excessive_bools.rs:38:5 | -LL | fn f(&self, _: bool, _: bool, _: bool, _: bool) {} - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +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:42:5 + --> $DIR/fn_params_excessive_bools.rs:43:5 | LL | / fn n(_: bool, _: u32, _: bool, _: Box, _: bool, _: bool) { LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {} @@ -42,7 +42,7 @@ 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:44:9 | LL | fn nn(_: bool, _: bool, _: bool, _: bool) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From e8c2c3d1c6ac27d0f1fa9125b79f1f927a59cfcf Mon Sep 17 00:00:00 2001 From: kraktus Date: Fri, 28 Oct 2022 14:57:51 +0200 Subject: [PATCH 3/7] refactor `has_repr_attr` --- clippy_lints/src/excessive_bools.rs | 21 +++++---------------- clippy_lints/src/trailing_empty_array.rs | 8 ++------ clippy_utils/src/lib.rs | 4 ++++ 3 files changed, 11 insertions(+), 22 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 08164955c6a3..08fcf304958f 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -1,10 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{get_parent_node, is_bool}; +use clippy_utils::{has_repr_attr, is_bool}; use rustc_hir::intravisit::FnKind; -use rustc_hir::{Body, FnDecl, HirId, Item, ItemKind, Node, Ty}; +use rustc_hir::{Body, FnDecl, HirId, Item, ItemKind, 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! { @@ -135,13 +135,7 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { return; } if let ItemKind::Struct(variant_data, _) = &item.kind { - if cx - .tcx - .hir() - .attrs(item.hir_id()) - .iter() - .any(|attr| attr.has_name(sym::repr)) - { + if has_repr_attr(cx, item.hir_id()) { return; } @@ -165,15 +159,10 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { fn_decl: &'tcx FnDecl<'tcx>, _: &'tcx Body<'tcx>, span: Span, - hir_id: HirId, + _: HirId, ) { if let Some(fn_header) = fn_kind.header() && fn_header.abi == Abi::Rust - && if let Some(Node::Item(item)) = get_parent_node(cx.tcx, hir_id) { - !matches!(item.kind, ItemKind::ExternCrate(..)) - } else { - true - } && !span.from_expansion() { self.check_fn_sig(cx, fn_decl, span) } diff --git a/clippy_lints/src/trailing_empty_array.rs b/clippy_lints/src/trailing_empty_array.rs index 58cc057a39ed..712a7a6601ac 100644 --- 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)) -} diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 0000896fdb65..7ce8bbc1b7b3 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -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; From d9b940e2c36eee3172c0f41f570de9e2d8ed95fd Mon Sep 17 00:00:00 2001 From: kraktus Date: Sun, 23 Oct 2022 16:03:05 +0200 Subject: [PATCH 4/7] dogfood --- clippy_lints/src/excessive_bools.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 08fcf304958f..281d0631d06a 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -86,7 +86,7 @@ pub struct ExcessiveBools { max_fn_params_bools: u64, } -#[derive(Eq, PartialEq, Debug)] +#[derive(Eq, PartialEq, Debug, Copy, Clone)] enum Kind { Struct, Fn, @@ -147,7 +147,7 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { &format!("more than {} bools in a struct", self.max_struct_bools), None, "consider using a state machine or refactoring bools into two-variant enums", - ) + ); } } } @@ -164,7 +164,7 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { if let Some(fn_header) = fn_kind.header() && fn_header.abi == Abi::Rust && !span.from_expansion() { - self.check_fn_sig(cx, fn_decl, span) + self.check_fn_sig(cx, fn_decl, span); } } } From 9e1c7febe261b834d2c787aca2fcd9de577d76d8 Mon Sep 17 00:00:00 2001 From: kraktus Date: Sat, 29 Oct 2022 16:12:41 +0200 Subject: [PATCH 5/7] `excessive_bools`, do not lint in trait impls Should not lint because the trait might not be changeable by the user We only lint in the trait definition --- clippy_lints/src/excessive_bools.rs | 11 ++++++++--- tests/ui/fn_params_excessive_bools.rs | 2 ++ tests/ui/fn_params_excessive_bools.stderr | 14 +++----------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 281d0631d06a..68c5e5ab1f1e 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::{has_repr_attr, is_bool}; +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, Ty}; use rustc_lint::{LateContext, LateLintPass}; @@ -159,11 +159,16 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { fn_decl: &'tcx FnDecl<'tcx>, _: &'tcx Body<'tcx>, span: Span, - _: HirId, + hir_id: HirId, ) { if let Some(fn_header) = fn_kind.header() && fn_header.abi == Abi::Rust - && !span.from_expansion() { + && !span.from_expansion() + && 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); } } diff --git a/tests/ui/fn_params_excessive_bools.rs b/tests/ui/fn_params_excessive_bools.rs index 4fcd4d54ce62..2635e433a319 100644 --- a/tests/ui/fn_params_excessive_bools.rs +++ b/tests/ui/fn_params_excessive_bools.rs @@ -35,6 +35,8 @@ 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) {} } diff --git a/tests/ui/fn_params_excessive_bools.stderr b/tests/ui/fn_params_excessive_bools.stderr index 9d22d3851d2f..7edfd07b9db3 100644 --- a/tests/ui/fn_params_excessive_bools.stderr +++ b/tests/ui/fn_params_excessive_bools.stderr @@ -24,15 +24,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:38: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:43:5 + --> $DIR/fn_params_excessive_bools.rs:45:5 | LL | / fn n(_: bool, _: u32, _: bool, _: Box, _: bool, _: bool) { LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {} @@ -42,12 +34,12 @@ LL | | } = help: consider refactoring bools into two-variant enums error: more than 3 bools in function parameters - --> $DIR/fn_params_excessive_bools.rs:44:9 + --> $DIR/fn_params_excessive_bools.rs:46: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 5 previous errors From 13b737d5b89c374d7d0c9b2f3968654b9c5ba370 Mon Sep 17 00:00:00 2001 From: kraktus Date: Sat, 29 Oct 2022 17:43:43 +0200 Subject: [PATCH 6/7] [`fn_params_excessive_bools`] whitelist in signle function in test --- tests/ui/fn_params_excessive_bools.rs | 3 +++ tests/ui/fn_params_excessive_bools.stderr | 6 +++--- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/ui/fn_params_excessive_bools.rs b/tests/ui/fn_params_excessive_bools.rs index 2635e433a319..7995912110d6 100644 --- a/tests/ui/fn_params_excessive_bools.rs +++ b/tests/ui/fn_params_excessive_bools.rs @@ -25,6 +25,8 @@ struct S; trait Trait { fn f(_: bool, _: bool, _: bool, _: bool); fn g(_: bool, _: bool, _: bool, _: Vec); + #[allow(clippy::fn_params_excessive_bools)] + fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool); } impl S { @@ -39,6 +41,7 @@ impl Trait for S { // We only lint in the trait definition fn f(_: bool, _: bool, _: bool, _: bool) {} fn g(_: bool, _: bool, _: bool, _: Vec) {} + fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool) {} } fn main() { diff --git a/tests/ui/fn_params_excessive_bools.stderr b/tests/ui/fn_params_excessive_bools.stderr index 7edfd07b9db3..8c2383873fe9 100644 --- a/tests/ui/fn_params_excessive_bools.stderr +++ b/tests/ui/fn_params_excessive_bools.stderr @@ -16,7 +16,7 @@ LL | fn t(_: S, _: S, _: Box, _: Vec, _: 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:31:5 + --> $DIR/fn_params_excessive_bools.rs:33:5 | LL | fn f(&self, _: bool, _: bool, _: bool, _: bool) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,7 +24,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:45:5 + --> $DIR/fn_params_excessive_bools.rs:48:5 | LL | / fn n(_: bool, _: u32, _: bool, _: Box, _: bool, _: bool) { LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {} @@ -34,7 +34,7 @@ LL | | } = help: consider refactoring bools into two-variant enums error: more than 3 bools in function parameters - --> $DIR/fn_params_excessive_bools.rs:46:9 + --> $DIR/fn_params_excessive_bools.rs:49:9 | LL | fn nn(_: bool, _: bool, _: bool, _: bool) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ From 3d4b73c26da1630318e76f43a7dedfc4dbc716cd Mon Sep 17 00:00:00 2001 From: kraktus Date: Mon, 7 Nov 2022 21:00:37 +0100 Subject: [PATCH 7/7] [`excessive_bools`] lint trait functions even without bodies --- clippy_lints/src/excessive_bools.rs | 14 ++++++++++--- tests/ui/fn_params_excessive_bools.rs | 2 ++ tests/ui/fn_params_excessive_bools.stderr | 24 +++++++++++++++++++---- 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/clippy_lints/src/excessive_bools.rs b/clippy_lints/src/excessive_bools.rs index 68c5e5ab1f1e..fc2912f696e0 100644 --- a/clippy_lints/src/excessive_bools.rs +++ b/clippy_lints/src/excessive_bools.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_help; 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, Ty}; +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::Span; @@ -114,7 +114,7 @@ impl ExcessiveBools { } fn check_fn_sig(&self, cx: &LateContext<'_>, fn_decl: &FnDecl<'_>, span: Span) { - if self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) { + if !span.from_expansion() && self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) { span_lint_and_help( cx, FN_PARAMS_EXCESSIVE_BOOLS, @@ -152,6 +152,15 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { } } + 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>, @@ -163,7 +172,6 @@ impl<'tcx> LateLintPass<'tcx> for ExcessiveBools { ) { if let Some(fn_header) = fn_kind.header() && fn_header.abi == Abi::Rust - && !span.from_expansion() && get_parent_as_impl(cx.tcx, hir_id) .map_or(true, |impl_item| impl_item.of_trait.is_none() diff --git a/tests/ui/fn_params_excessive_bools.rs b/tests/ui/fn_params_excessive_bools.rs index 7995912110d6..f53e531629aa 100644 --- a/tests/ui/fn_params_excessive_bools.rs +++ b/tests/ui/fn_params_excessive_bools.rs @@ -23,10 +23,12 @@ fn t(_: S, _: S, _: Box, _: Vec, _: 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); #[allow(clippy::fn_params_excessive_bools)] fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool); + fn i(_: bool, _: bool, _: bool, _: bool) {} } impl S { diff --git a/tests/ui/fn_params_excessive_bools.stderr b/tests/ui/fn_params_excessive_bools.stderr index 8c2383873fe9..43363b46972c 100644 --- a/tests/ui/fn_params_excessive_bools.stderr +++ b/tests/ui/fn_params_excessive_bools.stderr @@ -16,7 +16,23 @@ LL | fn t(_: S, _: S, _: Box, _: Vec, _: 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:33: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: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) {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -24,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:48:5 + --> $DIR/fn_params_excessive_bools.rs:50:5 | LL | / fn n(_: bool, _: u32, _: bool, _: Box, _: bool, _: bool) { LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {} @@ -34,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:49: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 5 previous errors +error: aborting due to 7 previous errors