Skip to content

Commit 9c69e93

Browse files
kraktuskraktus
committed
Rewrite ExcessiveBools to be a LateLintPass lint
changelog: [`fn_params_excessive_bools`] Make it possible to allow the lint at the method level
1 parent 586bd3f commit 9c69e93

File tree

7 files changed

+90
-84
lines changed

7 files changed

+90
-84
lines changed

‎clippy_lints/src/excessive_bools.rs‎

Lines changed: 66 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2-
use rustc_ast::ast::{AssocItemKind, Extern, Fn, FnSig, Impl, Item, ItemKind, Trait, Ty, TyKind};
3-
use rustc_lint::{EarlyContext, EarlyLintPass};
2+
use clippy_utils::{get_parent_node, is_bool};
3+
use rustc_hir::intravisit::FnKind;
4+
use rustc_hir::{Body, FnDecl, HirId, Item, ItemKind, Node, Ty};
5+
use rustc_lint::{LateContext, LateLintPass};
46
use rustc_session::{declare_tool_lint, impl_lint_pass};
57
use rustc_span::{sym, Span};
8+
use rustc_target::spec::abi::Abi;
69

710
declare_clippy_lint! {
811
/// ### What it does
@@ -83,6 +86,12 @@ pub struct ExcessiveBools {
8386
max_fn_params_bools: u64,
8487
}
8588

89+
#[derive(Eq, PartialEq, Debug)]
90+
enum Kind {
91+
Struct,
92+
Fn,
93+
}
94+
8695
impl ExcessiveBools {
8796
#[must_use]
8897
pub fn new(max_struct_bools: u64, max_fn_params_bools: u64) -> Self {
@@ -92,21 +101,20 @@ impl ExcessiveBools {
92101
}
93102
}
94103

95-
fn check_fn_sig(&self, cx: &EarlyContext<'_>, fn_sig: &FnSig, span: Span) {
96-
match fn_sig.header.ext {
97-
Extern::Implicit(_) | Extern::Explicit(_, _) => return,
98-
Extern::None => (),
104+
fn too_many_bools<'tcx>(&self, tys: impl Iterator<Item = &'tcx Ty<'tcx>>, kind: Kind) -> bool {
105+
if let Ok(bools) = tys.filter(|ty| is_bool(ty)).count().try_into() {
106+
(if Kind::Fn == kind {
107+
self.max_fn_params_bools
108+
} else {
109+
self.max_struct_bools
110+
}) < bools
111+
} else {
112+
false
99113
}
114+
}
100115

101-
let fn_sig_bools = fn_sig
102-
.decl
103-
.inputs
104-
.iter()
105-
.filter(|param| is_bool_ty(&param.ty))
106-
.count()
107-
.try_into()
108-
.unwrap();
109-
if self.max_fn_params_bools < fn_sig_bools {
116+
fn check_fn_sig(&self, cx: &LateContext<'_>, fn_decl: &FnDecl<'_>, span: Span) {
117+
if self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) {
110118
span_lint_and_help(
111119
cx,
112120
FN_PARAMS_EXCESSIVE_BOOLS,
@@ -121,55 +129,53 @@ impl ExcessiveBools {
121129

122130
impl_lint_pass!(ExcessiveBools => [STRUCT_EXCESSIVE_BOOLS, FN_PARAMS_EXCESSIVE_BOOLS]);
123131

124-
fn is_bool_ty(ty: &Ty) -> bool {
125-
if let TyKind::Path(None, path) = &ty.kind {
126-
if let [name] = path.segments.as_slice() {
127-
return name.ident.name == sym::bool;
128-
}
129-
}
130-
false
131-
}
132-
133-
impl EarlyLintPass for ExcessiveBools {
134-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
132+
impl<'tcx> LateLintPass<'tcx> for ExcessiveBools {
133+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
135134
if item.span.from_expansion() {
136135
return;
137136
}
138-
match &item.kind {
139-
ItemKind::Struct(variant_data, _) => {
140-
if item.attrs.iter().any(|attr| attr.has_name(sym::repr)) {
141-
return;
142-
}
137+
if let ItemKind::Struct(variant_data, _) = &item.kind {
138+
if cx
139+
.tcx
140+
.hir()
141+
.attrs(item.hir_id())
142+
.iter()
143+
.any(|attr| attr.has_name(sym::repr))
144+
{
145+
return;
146+
}
147+
148+
if self.too_many_bools(variant_data.fields().iter().map(|field| field.ty), Kind::Struct) {
149+
span_lint_and_help(
150+
cx,
151+
STRUCT_EXCESSIVE_BOOLS,
152+
item.span,
153+
&format!("more than {} bools in a struct", self.max_struct_bools),
154+
None,
155+
"consider using a state machine or refactoring bools into two-variant enums",
156+
)
157+
}
158+
}
159+
}
143160

144-
if let Ok(struct_bools) = variant_data
145-
.fields()
146-
.iter()
147-
.filter(|field| is_bool_ty(&field.ty))
148-
.count()
149-
.try_into() && self.max_struct_bools < struct_bools
150-
{
151-
span_lint_and_help(
152-
cx,
153-
STRUCT_EXCESSIVE_BOOLS,
154-
item.span,
155-
&format!("more than {} bools in a struct", self.max_struct_bools),
156-
None,
157-
"consider using a state machine or refactoring bools into two-variant enums",
158-
)
159-
}
160-
},
161-
ItemKind::Impl(box Impl {
162-
of_trait: None, items, ..
163-
})
164-
| ItemKind::Trait(box Trait { items, .. }) => {
165-
for item in items {
166-
if let AssocItemKind::Fn(box Fn { sig, .. }) = &item.kind {
167-
self.check_fn_sig(cx, sig, item.span);
168-
}
169-
}
170-
},
171-
ItemKind::Fn(box Fn { sig, .. }) => self.check_fn_sig(cx, sig, item.span),
172-
_ => (),
161+
fn check_fn(
162+
&mut self,
163+
cx: &LateContext<'tcx>,
164+
fn_kind: FnKind<'tcx>,
165+
fn_decl: &'tcx FnDecl<'tcx>,
166+
_: &'tcx Body<'tcx>,
167+
span: Span,
168+
hir_id: HirId,
169+
) {
170+
if let Some(fn_header) = fn_kind.header()
171+
&& fn_header.abi == Abi::Rust
172+
&& if let Some(Node::Item(item)) = get_parent_node(cx.tcx, hir_id) {
173+
!matches!(item.kind, ItemKind::ExternCrate(..))
174+
} else {
175+
true
176+
}
177+
&& !span.from_expansion() {
178+
self.check_fn_sig(cx, fn_decl, span)
173179
}
174180
}
175181
}

‎clippy_lints/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -793,7 +793,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
793793
store.register_early_pass(|| Box::new(single_component_path_imports::SingleComponentPathImports));
794794
let max_fn_params_bools = conf.max_fn_params_bools;
795795
let max_struct_bools = conf.max_struct_bools;
796-
store.register_early_pass(move || {
796+
store.register_late_pass(move |_| {
797797
Box::new(excessive_bools::ExcessiveBools::new(
798798
max_struct_bools,
799799
max_fn_params_bools,

‎clippy_lints/src/methods/mod.rs‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -104,11 +104,10 @@ use bind_instead_of_map::BindInsteadOfMap;
104104
use clippy_utils::consts::{constant, Constant};
105105
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
106106
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
107-
use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
107+
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
108108
use if_chain::if_chain;
109109
use rustc_hir as hir;
110-
use rustc_hir::def::Res;
111-
use rustc_hir::{Expr, ExprKind, PrimTy, QPath, TraitItem, TraitItemKind};
110+
use rustc_hir::{Expr, ExprKind, TraitItem, TraitItemKind};
112111
use rustc_hir_analysis::hir_ty_to_ty;
113112
use rustc_lint::{LateContext, LateLintPass, LintContext};
114113
use rustc_middle::lint::in_external_macro;
@@ -3966,14 +3965,6 @@ impl OutType {
39663965
}
39673966
}
39683967

3969-
fn is_bool(ty: &hir::Ty<'_>) -> bool {
3970-
if let hir::TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
3971-
matches!(path.res, Res::PrimTy(PrimTy::Bool))
3972-
} else {
3973-
false
3974-
}
3975-
}
3976-
39773968
fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool {
39783969
expected.constness == actual.constness
39793970
&& expected.unsafety == actual.unsafety

‎clippy_utils/src/hir_utils.rs‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use rustc_hir::HirIdMap;
88
use rustc_hir::{
99
ArrayLen, BinOpKind, BindingAnnotation, Block, BodyId, Closure, Expr, ExprField, ExprKind, FnRetTy, GenericArg,
1010
GenericArgs, Guard, HirId, InlineAsmOperand, Let, Lifetime, LifetimeName, ParamName, Pat, PatField, PatKind, Path,
11-
PathSegment, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding,
11+
PathSegment, PrimTy, QPath, Stmt, StmtKind, Ty, TyKind, TypeBinding,
1212
};
1313
use rustc_lexer::{tokenize, TokenKind};
1414
use rustc_lint::LateContext;
@@ -1030,6 +1030,14 @@ pub fn hash_stmt(cx: &LateContext<'_>, s: &Stmt<'_>) -> u64 {
10301030
h.finish()
10311031
}
10321032

1033+
pub fn is_bool(ty: &Ty<'_>) -> bool {
1034+
if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
1035+
matches!(path.res, Res::PrimTy(PrimTy::Bool))
1036+
} else {
1037+
false
1038+
}
1039+
}
1040+
10331041
pub fn hash_expr(cx: &LateContext<'_>, e: &Expr<'_>) -> u64 {
10341042
let mut h = SpanlessHash::new(cx);
10351043
h.hash_expr(e);

‎clippy_utils/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ pub mod visitors;
6666
pub use self::attrs::*;
6767
pub use self::check_proc_macro::{is_from_proc_macro, is_span_if, is_span_match};
6868
pub use self::hir_utils::{
69-
both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash,
69+
both, count_eq, eq_expr_value, hash_expr, hash_stmt, is_bool, over, HirEqInterExpr, SpanlessEq, SpanlessHash,
7070
};
7171

7272
use core::ops::ControlFlow;

‎tests/ui/fn_params_excessive_bools.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#![allow(clippy::too_many_arguments)]
33

44
extern "C" {
5+
// Should not lint, most of the time users have no control over extern function signatures
56
fn f(_: bool, _: bool, _: bool, _: bool);
67
}
78

‎tests/ui/fn_params_excessive_bools.stderr‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: more than 3 bools in function parameters
2-
--> $DIR/fn_params_excessive_bools.rs:18:1
2+
--> $DIR/fn_params_excessive_bools.rs:19:1
33
|
44
LL | fn g(_: bool, _: bool, _: bool, _: bool) {}
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -8,31 +8,31 @@ LL | fn g(_: bool, _: bool, _: bool, _: bool) {}
88
= note: `-D clippy::fn-params-excessive-bools` implied by `-D warnings`
99

1010
error: more than 3 bools in function parameters
11-
--> $DIR/fn_params_excessive_bools.rs:21:1
11+
--> $DIR/fn_params_excessive_bools.rs:22:1
1212
|
1313
LL | fn t(_: S, _: S, _: Box<S>, _: Vec<u32>, _: bool, _: bool, _: bool, _: bool) {}
1414
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1515
|
1616
= help: consider refactoring bools into two-variant enums
1717

1818
error: more than 3 bools in function parameters
19-
--> $DIR/fn_params_excessive_bools.rs:25:5
19+
--> $DIR/fn_params_excessive_bools.rs:31:5
2020
|
21-
LL | fn f(_: bool, _: bool, _: bool, _: bool);
22-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
21+
LL | fn f(&self, _: bool, _: bool, _: bool, _: bool) {}
22+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2323
|
2424
= help: consider refactoring bools into two-variant enums
2525

2626
error: more than 3 bools in function parameters
27-
--> $DIR/fn_params_excessive_bools.rs:30:5
27+
--> $DIR/fn_params_excessive_bools.rs:38:5
2828
|
29-
LL | fn f(&self, _: bool, _: bool, _: bool, _: bool) {}
30-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
29+
LL | fn f(_: bool, _: bool, _: bool, _: bool) {}
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3131
|
3232
= help: consider refactoring bools into two-variant enums
3333

3434
error: more than 3 bools in function parameters
35-
--> $DIR/fn_params_excessive_bools.rs:42:5
35+
--> $DIR/fn_params_excessive_bools.rs:43:5
3636
|
3737
LL | / fn n(_: bool, _: u32, _: bool, _: Box<u32>, _: bool, _: bool) {
3838
LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {}
@@ -42,7 +42,7 @@ LL | | }
4242
= help: consider refactoring bools into two-variant enums
4343

4444
error: more than 3 bools in function parameters
45-
--> $DIR/fn_params_excessive_bools.rs:43:9
45+
--> $DIR/fn_params_excessive_bools.rs:44:9
4646
|
4747
LL | fn nn(_: bool, _: bool, _: bool, _: bool) {}
4848
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

0 commit comments

Comments
 (0)