add excessive bools lints

CHANGELOG.md

@@ -1147,6 +1147,7 @@ Released 2018-09-13
 [`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_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_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
@@ -1341,6 +1342,7 @@ Released 2018-09-13
 [`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_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_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

README.md

@@ -6,7 +6,7 @@
 
 A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
 
-[There are 351 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
+[There are 353 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:
 

clippy_lints/src/excessive_bools.rs

@@ -0,0 +1,176 @@
+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 => (),
+        }
+
+        if self.max_fn_params_bools
+            < fn_sig
+                .decl
+                .inputs
+                .iter()
+                .filter(|param| is_bool_ty(&param.ty))
+                .count()
+                .try_into()
+                .unwrap()
+        {
+            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;
+                }
+
+                if self.max_struct_bools
+                    < variant_data
+                        .fields()
+                        .iter()
+                        .filter(|field| is_bool_ty(&field.ty))
+                        .count()
+                        .try_into()
+                        .unwrap()
+                {
+                    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),
+            _ => (),
+        }
+    }
+}

clippy_lints/src/lib.rs

@@ -204,6 +204,7 @@ pub mod erasing_op;
 pub mod escape;
 pub mod eta_reduction;
 pub mod eval_order_dependence;
+pub mod excessive_bools;
 pub mod excessive_precision;
 pub mod exit;
 pub mod explicit_write;
@@ -532,6 +533,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &eta_reduction::REDUNDANT_CLOSURE_FOR_METHOD_CALLS,
         &eval_order_dependence::DIVERGING_SUB_EXPRESSION,
         &eval_order_dependence::EVAL_ORDER_DEPENDENCE,
+        &excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS,
+        &excessive_bools::STRUCT_EXCESSIVE_BOOLS,
         &excessive_precision::EXCESSIVE_PRECISION,
         &exit::EXIT,
         &explicit_write::EXPLICIT_WRITE,
@@ -1001,6 +1004,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 atomic_ordering::AtomicOrdering);
     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![
         LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@@ -1055,6 +1061,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&enum_variants::MODULE_NAME_REPETITIONS),
         LintId::of(&enum_variants::PUB_ENUM_VARIANT_NAMES),
         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::TOO_MANY_LINES),
         LintId::of(&if_not_else::IF_NOT_ELSE),

clippy_lints/src/utils/conf.rs

@@ -154,6 +154,10 @@ define_Conf! {
     (array_size_threshold, "array_size_threshold", 512_000 => u64),
     /// 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", 4096 => u64),
+    /// Lint: STRUCT_EXCESSIVE_BOOLS. The maximum number of bools a struct can have
+    (max_struct_bools, "max_struct_bools", 3 => u64),
+    /// Lint: FN_PARAMS_EXCESSIVE_BOOLS. The maximum number of bools function parameters can have
+    (max_fn_params_bools, "max_fn_params_bools", 3 => u64),
 }
 
 impl Default for Conf {

clippy_lints/src/utils/mod.rs

@@ -1355,6 +1355,14 @@ pub fn is_no_std_crate(krate: &Crate<'_>) -> bool {
     })
 }
 
+// Check if parent of a hir node is an item
+// in 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 {
     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(_), .. })

src/lintlist/mod.rs

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

tests/ui-toml/fn_params_excessive_bools/clippy.toml

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

tests/ui-toml/fn_params_excessive_bools/test.rs

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

tests/ui-toml/fn_params_excessive_bools/test.stderr

@@ -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
+

tests/ui-toml/struct_excessive_bools/clippy.toml

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

tests/ui-toml/struct_excessive_bools/test.rs

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

tests/ui-toml/struct_excessive_bools/test.stderr

@@ -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
+

tests/ui-toml/toml_unknown_key/conf_unknown_key.stderr

@@ -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
 

tests/ui/fn_params_excessive_bools.rs

@@ -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) {}
+    }
+}