Skip to content

Commit a815140

Browse files
borsbors
committed
Auto merge of rust-lang#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 rust-lang/rust-clippy#9687 Tested without committing but `#[allow]`ing now works. Also rewrote the lint to be a late lint while at it :) r? `@xFrednet`
2 parents 9f283c9 + 3d4b73c commit a815140

File tree

8 files changed

+110
-88
lines changed

8 files changed

+110
-88
lines changed

‎clippy_lints/src/excessive_bools.rs

Lines changed: 68 additions & 61 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_as_impl, has_repr_attr, is_bool};
3+
use rustc_hir::intravisit::FnKind;
4+
use rustc_hir::{Body, FnDecl, HirId, Item, ItemKind, TraitFn, TraitItem, TraitItemKind, Ty};
5+
use rustc_lint::{LateContext, LateLintPass};
46
use rustc_session::{declare_tool_lint, impl_lint_pass};
5-
use rustc_span::{sym, Span};
7+
use rustc_span::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, Copy, Clone)]
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 !span.from_expansion() && self.too_many_bools(fn_decl.inputs.iter(), Kind::Fn) {
110118
span_lint_and_help(
111119
cx,
112120
FN_PARAMS_EXCESSIVE_BOOLS,
@@ -121,56 +129,55 @@ 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;
132+
impl<'tcx> LateLintPass<'tcx> for ExcessiveBools {
133+
fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) {
134+
if item.span.from_expansion() {
135+
return;
136+
}
137+
if let ItemKind::Struct(variant_data, _) = &item.kind {
138+
if has_repr_attr(cx, item.hir_id()) {
139+
return;
140+
}
141+
142+
if self.too_many_bools(variant_data.fields().iter().map(|field| field.ty), Kind::Struct) {
143+
span_lint_and_help(
144+
cx,
145+
STRUCT_EXCESSIVE_BOOLS,
146+
item.span,
147+
&format!("more than {} bools in a struct", self.max_struct_bools),
148+
None,
149+
"consider using a state machine or refactoring bools into two-variant enums",
150+
);
151+
}
128152
}
129153
}
130-
false
131-
}
132154

133-
impl EarlyLintPass for ExcessiveBools {
134-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
135-
if item.span.from_expansion() {
136-
return;
155+
fn check_trait_item(&mut self, cx: &LateContext<'tcx>, trait_item: &'tcx TraitItem<'tcx>) {
156+
// functions with a body are already checked by `check_fn`
157+
if let TraitItemKind::Fn(fn_sig, TraitFn::Required(_)) = &trait_item.kind
158+
&& fn_sig.header.abi == Abi::Rust
159+
{
160+
self.check_fn_sig(cx, fn_sig.decl, fn_sig.span);
137161
}
138-
match &item.kind {
139-
ItemKind::Struct(variant_data, _) => {
140-
if item.attrs.iter().any(|attr| attr.has_name(sym::repr)) {
141-
return;
142-
}
162+
}
143163

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

‎clippy_lints/src/lib.rs

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

‎clippy_lints/src/methods/mod.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,10 @@ use bind_instead_of_map::BindInsteadOfMap;
105105
use clippy_utils::consts::{constant, Constant};
106106
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
107107
use clippy_utils::ty::{contains_ty_adt_constructor_opaque, implements_trait, is_copy, is_type_diagnostic_item};
108-
use clippy_utils::{contains_return, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
108+
use clippy_utils::{contains_return, is_bool, is_trait_method, iter_input_pats, meets_msrv, msrvs, return_ty};
109109
use if_chain::if_chain;
110110
use rustc_hir as hir;
111-
use rustc_hir::def::Res;
112-
use rustc_hir::{Expr, ExprKind, PrimTy, QPath, TraitItem, TraitItemKind};
111+
use rustc_hir::{Expr, ExprKind, TraitItem, TraitItemKind};
113112
use rustc_hir_analysis::hir_ty_to_ty;
114113
use rustc_lint::{LateContext, LateLintPass, LintContext};
115114
use rustc_middle::lint::in_external_macro;
@@ -3992,14 +3991,6 @@ impl OutType {
39923991
}
39933992
}
39943993

3995-
fn is_bool(ty: &hir::Ty<'_>) -> bool {
3996-
if let hir::TyKind::Path(QPath::Resolved(_, path)) = ty.kind {
3997-
matches!(path.res, Res::PrimTy(PrimTy::Bool))
3998-
} else {
3999-
false
4000-
}
4001-
}
4002-
40033994
fn fn_header_equals(expected: hir::FnHeader, actual: hir::FnHeader) -> bool {
40043995
expected.constness == actual.constness
40053996
&& expected.unsafety == actual.unsafety

‎clippy_lints/src/trailing_empty_array.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2-
use rustc_hir::{HirId, Item, ItemKind};
2+
use clippy_utils::has_repr_attr;
3+
use rustc_hir::{Item, ItemKind};
34
use rustc_lint::{LateContext, LateLintPass};
45
use rustc_middle::ty::Const;
56
use rustc_session::{declare_lint_pass, declare_tool_lint};
6-
use rustc_span::sym;
77

88
declare_clippy_lint! {
99
/// ### What it does
@@ -72,7 +72,3 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'_>, item: &Item<'_
7272
}
7373
}
7474
}
75-
76-
fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool {
77-
cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr))
78-
}

‎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: 5 additions & 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;
@@ -1780,6 +1780,10 @@ pub fn has_attr(attrs: &[ast::Attribute], symbol: Symbol) -> bool {
17801780
attrs.iter().any(|attr| attr.has_name(symbol))
17811781
}
17821782

1783+
pub fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool {
1784+
has_attr(cx.tcx.hir().attrs(hir_id), sym::repr)
1785+
}
1786+
17831787
pub fn any_parent_has_attr(tcx: TyCtxt<'_>, node: HirId, symbol: Symbol) -> bool {
17841788
let map = &tcx.hir();
17851789
let mut prev_enclosing_node = None;

‎tests/ui/fn_params_excessive_bools.rs

Lines changed: 8 additions & 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

@@ -22,8 +23,12 @@ fn t(_: S, _: S, _: Box<S>, _: Vec<u32>, _: bool, _: bool, _: bool, _: bool) {}
2223

2324
struct S;
2425
trait Trait {
26+
// should warn for trait functions with and without body
2527
fn f(_: bool, _: bool, _: bool, _: bool);
2628
fn g(_: bool, _: bool, _: bool, _: Vec<u32>);
29+
#[allow(clippy::fn_params_excessive_bools)]
30+
fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool);
31+
fn i(_: bool, _: bool, _: bool, _: bool) {}
2732
}
2833

2934
impl S {
@@ -34,8 +39,11 @@ impl S {
3439
}
3540

3641
impl Trait for S {
42+
// Should not lint because the trait might not be changeable by the user
43+
// We only lint in the trait definition
3744
fn f(_: bool, _: bool, _: bool, _: bool) {}
3845
fn g(_: bool, _: bool, _: bool, _: Vec<u32>) {}
46+
fn h(_: bool, _: bool, _: bool, _: bool, _: bool, _: bool) {}
3947
}
4048

4149
fn main() {

‎tests/ui/fn_params_excessive_bools.stderr

Lines changed: 15 additions & 7 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,39 @@ 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:27:5
2020
|
2121
LL | fn f(_: bool, _: bool, _: bool, _: bool);
2222
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
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:31:5
28+
|
29+
LL | fn i(_: bool, _: bool, _: bool, _: bool) {}
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
31+
|
32+
= help: consider refactoring bools into two-variant enums
33+
34+
error: more than 3 bools in function parameters
35+
--> $DIR/fn_params_excessive_bools.rs:35:5
2836
|
2937
LL | fn f(&self, _: bool, _: bool, _: bool, _: bool) {}
3038
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3139
|
3240
= help: consider refactoring bools into two-variant enums
3341

3442
error: more than 3 bools in function parameters
35-
--> $DIR/fn_params_excessive_bools.rs:42:5
43+
--> $DIR/fn_params_excessive_bools.rs:50:5
3644
|
3745
LL | / fn n(_: bool, _: u32, _: bool, _: Box<u32>, _: bool, _: bool) {
3846
LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {}
@@ -42,12 +50,12 @@ LL | | }
4250
= help: consider refactoring bools into two-variant enums
4351

4452
error: more than 3 bools in function parameters
45-
--> $DIR/fn_params_excessive_bools.rs:43:9
53+
--> $DIR/fn_params_excessive_bools.rs:51:9
4654
|
4755
LL | fn nn(_: bool, _: bool, _: bool, _: bool) {}
4856
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
4957
|
5058
= help: consider refactoring bools into two-variant enums
5159

52-
error: aborting due to 6 previous errors
60+
error: aborting due to 7 previous errors
5361

0 commit comments

Comments
 (0)