add excessive bools lints

This commit is contained in:
Areredify 2020-02-02 02:49:52 +03:00 committed by Mikhail Babenko
parent 8e28b2fdf1
commit 338fb7a3e9
19 changed files with 424 additions and 4 deletions

View File

@ -1147,6 +1147,7 @@ Released 2018-09-13
[`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic [`float_arithmetic`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_arithmetic
[`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp [`float_cmp`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp
[`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const [`float_cmp_const`]: https://rust-lang.github.io/rust-clippy/master/index.html#float_cmp_const
[`fn_params_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_params_excessive_bools
[`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast [`fn_to_numeric_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast
[`fn_to_numeric_cast_with_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation [`fn_to_numeric_cast_with_truncation`]: https://rust-lang.github.io/rust-clippy/master/index.html#fn_to_numeric_cast_with_truncation
[`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map [`for_kv_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#for_kv_map
@ -1342,6 +1343,7 @@ Released 2018-09-13
[`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars [`string_extend_chars`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_extend_chars
[`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes [`string_lit_as_bytes`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_lit_as_bytes
[`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string [`string_to_string`]: https://rust-lang.github.io/rust-clippy/master/index.html#string_to_string
[`struct_excessive_bools`]: https://rust-lang.github.io/rust-clippy/master/index.html#struct_excessive_bools
[`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl [`suspicious_arithmetic_impl`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_arithmetic_impl
[`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting [`suspicious_assignment_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_assignment_formatting
[`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting [`suspicious_else_formatting`]: https://rust-lang.github.io/rust-clippy/master/index.html#suspicious_else_formatting

View File

@ -6,7 +6,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code. A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
[There are 352 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html) [There are 354 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you: We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

View File

@ -0,0 +1,174 @@
use crate::utils::{attr_by_name, in_macro, match_path_ast, span_lint_and_help};
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::Span;
use syntax::ast::{AssocItemKind, Extern, FnSig, Item, ItemKind, Ty, TyKind};
use std::convert::TryInto;
declare_clippy_lint! {
/// **What it does:** Checks for excessive
/// use of bools in structs.
///
/// **Why is this bad?** Excessive bools in a struct
/// is often a sign that it's used as a state machine,
/// which is much better implemented as an enum.
/// If it's not the case, excessive bools usually benefit
/// from refactoring into two-variant enums for better
/// readability and API.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust
/// struct S {
/// is_pending: bool,
/// is_processing: bool,
/// is_finished: bool,
/// }
/// ```
///
/// Good:
/// ```rust
/// enum S {
/// Pending,
/// Processing,
/// Finished,
/// }
/// ```
pub STRUCT_EXCESSIVE_BOOLS,
pedantic,
"using too many bools in a struct"
}
declare_clippy_lint! {
/// **What it does:** Checks for excessive use of
/// bools in function definitions.
///
/// **Why is this bad?** Calls to such functions
/// are confusing and error prone, because it's
/// hard to remember argument order and you have
/// no type system support to back you up. Using
/// two-variant enums instead of bools often makes
/// API easier to use.
///
/// **Known problems:** None.
///
/// **Example:**
/// Bad:
/// ```rust,ignore
/// fn f(is_round: bool, is_hot: bool) { ... }
/// ```
///
/// Good:
/// ```rust,ignore
/// enum Shape {
/// Round,
/// Spiky,
/// }
///
/// enum Temperature {
/// Hot,
/// IceCold,
/// }
///
/// fn f(shape: Shape, temperature: Temperature) { ... }
/// ```
pub FN_PARAMS_EXCESSIVE_BOOLS,
pedantic,
"using too many bools in function parameters"
}
pub struct ExcessiveBools {
max_struct_bools: u64,
max_fn_params_bools: u64,
}
impl ExcessiveBools {
#[must_use]
pub fn new(max_struct_bools: u64, max_fn_params_bools: u64) -> Self {
Self {
max_struct_bools,
max_fn_params_bools,
}
}
fn check_fn_sig(&self, cx: &EarlyContext<'_>, fn_sig: &FnSig, span: Span) {
match fn_sig.header.ext {
Extern::Implicit | Extern::Explicit(_) => return,
Extern::None => (),
}
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 {
span_lint_and_help(
cx,
FN_PARAMS_EXCESSIVE_BOOLS,
span,
&format!("more than {} bools in function parameters", self.max_fn_params_bools),
"consider refactoring bools into two-variant enums",
);
}
}
}
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 {
return match_path_ast(path, &["bool"]);
}
false
}
impl EarlyLintPass for ExcessiveBools {
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
if in_macro(item.span) {
return;
}
match &item.kind {
ItemKind::Struct(variant_data, _) => {
if attr_by_name(&item.attrs, "repr").is_some() {
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),
"consider using a state machine or refactoring bools into two-variant enums",
);
}
},
ItemKind::Impl {
of_trait: None, items, ..
}
| ItemKind::Trait(_, _, _, _, items) => {
for item in items {
if let AssocItemKind::Fn(fn_sig, _) = &item.kind {
self.check_fn_sig(cx, fn_sig, item.span);
}
}
},
ItemKind::Fn(fn_sig, _, _) => self.check_fn_sig(cx, fn_sig, item.span),
_ => (),
}
}
}

View File

@ -202,6 +202,7 @@ pub mod erasing_op;
pub mod escape; pub mod escape;
pub mod eta_reduction; pub mod eta_reduction;
pub mod eval_order_dependence; pub mod eval_order_dependence;
pub mod excessive_bools;
pub mod excessive_precision; pub mod excessive_precision;
pub mod exit; pub mod exit;
pub mod explicit_write; pub mod explicit_write;
@ -528,6 +529,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS, &eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
&eval_order_dependence::DIVERGING_SUB_EXPRESSION, &eval_order_dependence::DIVERGING_SUB_EXPRESSION,
&eval_order_dependence::EVAL_ORDER_DEPENDENCE, &eval_order_dependence::EVAL_ORDER_DEPENDENCE,
&excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS,
&excessive_bools::STRUCT_EXCESSIVE_BOOLS,
&excessive_precision::EXCESSIVE_PRECISION, &excessive_precision::EXCESSIVE_PRECISION,
&exit::EXIT, &exit::EXIT,
&explicit_write::EXPLICIT_WRITE, &explicit_write::EXPLICIT_WRITE,
@ -997,6 +1000,9 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box let_underscore::LetUnderscore); store.register_late_pass(|| box let_underscore::LetUnderscore);
store.register_late_pass(|| box atomic_ordering::AtomicOrdering); store.register_late_pass(|| box atomic_ordering::AtomicOrdering);
store.register_early_pass(|| box single_component_path_imports::SingleComponentPathImports); store.register_early_pass(|| box 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 || box excessive_bools::ExcessiveBools::new(max_struct_bools, max_fn_params_bools));
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC), LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1051,6 +1057,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&enum_variants::MODULE_NAME_REPETITIONS), LintId::of(&enum_variants::MODULE_NAME_REPETITIONS),
LintId::of(&enum_variants::PUB_ENUM_VARIANT_NAMES), LintId::of(&enum_variants::PUB_ENUM_VARIANT_NAMES),
LintId::of(&eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS), LintId::of(&eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS),
LintId::of(&excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
LintId::of(&excessive_bools::STRUCT_EXCESSIVE_BOOLS),
LintId::of(&functions::MUST_USE_CANDIDATE), LintId::of(&functions::MUST_USE_CANDIDATE),
LintId::of(&functions::TOO_MANY_LINES), LintId::of(&functions::TOO_MANY_LINES),
LintId::of(&if_not_else::IF_NOT_ELSE), LintId::of(&if_not_else::IF_NOT_ELSE),

View File

@ -154,6 +154,10 @@ define_Conf! {
(array_size_threshold, "array_size_threshold": u64, 512_000), (array_size_threshold, "array_size_threshold": u64, 512_000),
/// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed /// Lint: VEC_BOX. The size of the boxed type in bytes, where boxing in a `Vec` is allowed
(vec_box_size_threshold, "vec_box_size_threshold": u64, 4096), (vec_box_size_threshold, "vec_box_size_threshold": u64, 4096),
/// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have
(max_struct_bools, "max_struct_bools": u64, 3),
/// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have
(max_fn_params_bools, "max_fn_params_bools": u64, 3),
} }
impl Default for Conf { impl Default for Conf {

View File

@ -1355,6 +1355,13 @@ pub fn is_no_std_crate(krate: &Crate<'_>) -> bool {
}) })
} }
/// Check if parent of a hir node is a trait implementation block.
/// For example, `f` in
/// ```rust,ignore
/// impl Trait for S {
/// fn f() {}
/// }
/// ```
pub fn is_trait_impl_item(cx: &LateContext<'_, '_>, hir_id: HirId) -> bool { pub fn is_trait_impl_item(cx: &LateContext<'_, '_>, hir_id: HirId) -> bool {
if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) { if let Some(Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) {
matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. }) matches!(item.kind, ItemKind::Impl{ of_trait: Some(_), .. })

View File

@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS; pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, its used in `update_lints` // begin lint list, do not remove this comment, its used in `update_lints`
pub const ALL_LINTS: [Lint; 352] = [ pub const ALL_LINTS: [Lint; 354] = [
Lint { Lint {
name: "absurd_extreme_comparisons", name: "absurd_extreme_comparisons",
group: "correctness", group: "correctness",
@ -623,6 +623,13 @@ pub const ALL_LINTS: [Lint; 352] = [
deprecation: None, deprecation: None,
module: "misc", module: "misc",
}, },
Lint {
name: "fn_params_excessive_bools",
group: "pedantic",
desc: "using too many bools in function parameters",
deprecation: None,
module: "excessive_bools",
},
Lint { Lint {
name: "fn_to_numeric_cast", name: "fn_to_numeric_cast",
group: "style", group: "style",
@ -1932,6 +1939,13 @@ pub const ALL_LINTS: [Lint; 352] = [
deprecation: None, deprecation: None,
module: "strings", module: "strings",
}, },
Lint {
name: "struct_excessive_bools",
group: "pedantic",
desc: "using too many bools in a struct",
deprecation: None,
module: "excessive_bools",
},
Lint { Lint {
name: "suspicious_arithmetic_impl", name: "suspicious_arithmetic_impl",
group: "correctness", group: "correctness",

View File

@ -0,0 +1 @@
max-fn-params-bools = 1

View File

@ -0,0 +1,6 @@
#![warn(clippy::fn_params_excessive_bools)]
fn f(_: bool) {}
fn g(_: bool, _: bool) {}
fn main() {}

View File

@ -0,0 +1,11 @@
error: more than 1 bools in function parameters
--> $DIR/test.rs:4:1
|
LL | fn g(_: bool, _: bool) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::fn-params-excessive-bools` implied by `-D warnings`
= help: consider refactoring bools into two-variant enums
error: aborting due to previous error

View File

@ -0,0 +1 @@
max-struct-bools = 0

View File

@ -0,0 +1,9 @@
#![warn(clippy::struct_excessive_bools)]
struct S {
a: bool,
}
struct Foo {}
fn main() {}

View File

@ -0,0 +1,13 @@
error: more than 0 bools in a struct
--> $DIR/test.rs:3:1
|
LL | / struct S {
LL | | a: bool,
LL | | }
| |_^
|
= note: `-D clippy::struct-excessive-bools` implied by `-D warnings`
= help: consider using a state machine or refactoring bools into two-variant enums
error: aborting due to previous error

View File

@ -1,4 +1,4 @@
error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `third-party` at line 5 column 1 error: error reading Clippy's configuration file `$DIR/clippy.toml`: unknown field `foobar`, expected one of `blacklisted-names`, `cognitive-complexity-threshold`, `cyclomatic-complexity-threshold`, `doc-valid-idents`, `too-many-arguments-threshold`, `type-complexity-threshold`, `single-char-binding-names-threshold`, `too-large-for-stack`, `enum-variant-name-threshold`, `enum-variant-size-threshold`, `verbose-bit-mask-threshold`, `literal-representation-threshold`, `trivial-copy-size-limit`, `too-many-lines-threshold`, `array-size-threshold`, `vec-box-size-threshold`, `max-struct-bools`, `max-fn-params-bools`, `third-party` at line 5 column 1
error: aborting due to previous error error: aborting due to previous error

View File

@ -0,0 +1,44 @@
#![warn(clippy::fn_params_excessive_bools)]
extern "C" {
fn f(_: bool, _: bool, _: bool, _: bool);
}
macro_rules! foo {
() => {
fn fff(_: bool, _: bool, _: bool, _: bool) {}
};
}
foo!();
#[no_mangle]
extern "C" fn k(_: bool, _: bool, _: bool, _: bool) {}
fn g(_: bool, _: bool, _: bool, _: bool) {}
fn h(_: bool, _: bool, _: bool) {}
fn e(_: S, _: S, _: Box<S>, _: Vec<u32>) {}
fn t(_: S, _: S, _: Box<S>, _: Vec<u32>, _: bool, _: bool, _: bool, _: bool) {}
struct S {}
trait Trait {
fn f(_: bool, _: bool, _: bool, _: bool);
fn g(_: bool, _: bool, _: bool, _: Vec<u32>);
}
impl S {
fn f(&self, _: bool, _: bool, _: bool, _: bool) {}
fn g(&self, _: bool, _: bool, _: bool) {}
#[no_mangle]
extern "C" fn h(_: bool, _: bool, _: bool, _: bool) {}
}
impl Trait for S {
fn f(_: bool, _: bool, _: bool, _: bool) {}
fn g(_: bool, _: bool, _: bool, _: Vec<u32>) {}
}
fn main() {
fn n(_: bool, _: u32, _: bool, _: Box<u32>, _: bool, _: bool) {
fn nn(_: bool, _: bool, _: bool, _: bool) {}
}
}

View File

@ -0,0 +1,53 @@
error: more than 3 bools in function parameters
--> $DIR/fn_params_excessive_bools.rs:17:1
|
LL | fn g(_: bool, _: bool, _: bool, _: bool) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::fn-params-excessive-bools` implied by `-D warnings`
= help: consider refactoring bools into two-variant enums
error: more than 3 bools in function parameters
--> $DIR/fn_params_excessive_bools.rs:20: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:24: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:29: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:41:5
|
LL | / fn n(_: bool, _: u32, _: bool, _: Box<u32>, _: bool, _: bool) {
LL | | fn nn(_: bool, _: bool, _: bool, _: bool) {}
LL | | }
| |_____^
|
= help: consider refactoring bools into two-variant enums
error: more than 3 bools in function parameters
--> $DIR/fn_params_excessive_bools.rs:42:9
|
LL | fn nn(_: bool, _: bool, _: bool, _: bool) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider refactoring bools into two-variant enums
error: aborting due to 6 previous errors

View File

@ -0,0 +1,44 @@
#![warn(clippy::struct_excessive_bools)]
macro_rules! foo {
() => {
struct MacroFoo {
a: bool,
b: bool,
c: bool,
d: bool,
}
};
}
foo!();
struct Foo {
a: bool,
b: bool,
c: bool,
}
struct BadFoo {
a: bool,
b: bool,
c: bool,
d: bool,
}
#[repr(C)]
struct Bar {
a: bool,
b: bool,
c: bool,
d: bool,
}
fn main() {
struct FooFoo {
a: bool,
b: bool,
c: bool,
d: bool,
}
}

View File

@ -0,0 +1,29 @@
error: more than 3 bools in a struct
--> $DIR/struct_excessive_bools.rs:22:1
|
LL | / struct BadFoo {
LL | | a: bool,
LL | | b: bool,
LL | | c: bool,
LL | | d: bool,
LL | | }
| |_^
|
= note: `-D clippy::struct-excessive-bools` implied by `-D warnings`
= help: consider using a state machine or refactoring bools into two-variant enums
error: more than 3 bools in a struct
--> $DIR/struct_excessive_bools.rs:38:5
|
LL | / struct FooFoo {
LL | | a: bool,
LL | | b: bool,
LL | | c: bool,
LL | | d: bool,
LL | | }
| |_____^
|
= help: consider using a state machine or refactoring bools into two-variant enums
error: aborting due to 2 previous errors

View File

@ -1,5 +1,5 @@
#![warn(clippy::unused_self)] #![warn(clippy::unused_self)]
#![allow(clippy::boxed_local)] #![allow(clippy::boxed_local, clippy::fn_params_excessive_bools)]
mod unused_self { mod unused_self {
use std::pin::Pin; use std::pin::Pin;