add excessive bools lints

Author: basil-cow

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,201 @@
+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::{AssocItem, AssocItemKind, Attribute, FnSig, Item, ItemKind, Ty, TyKind},
+    visit::Visitor,
+};
+
+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,
+    style,
+    "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,
+    style,
+    "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,
+        }
+    }
+}
+
+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
+}
+
+fn check_fn(cx: &EarlyContext<'_>, fn_sig: &FnSig, attrs: &[Attribute], span: Span, max_fn_params_bools: u64) {
+    if attr_by_name(attrs, "no_mangle").is_some() {
+        return;
+    }
+
+    if 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,
+            "excessive amount of bools in function parameters",
+            "consider refactoring bools into two-variant enums",
+        );
+    }
+}
+
+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,
+                        "excessive amount of bools in a struct",
+                        "consider using a state machine or refactoring bools into two-variant enums",
+                    );
+                }
+            },
+            ItemKind::Impl {
+                of_trait: None, items, ..
+            }
+            | ItemKind::Trait(_, _, _, _, items) => {
+                let mut visitor = AssocFnVisitor {
+                    cx,
+                    max_fn_params_bools: self.max_fn_params_bools,
+                };
+                for item in items {
+                    visitor.visit_assoc_item(item);
+                }
+            },
+            ItemKind::Fn(fn_sig, _, _) => check_fn(cx, fn_sig, &item.attrs, item.span, self.max_fn_params_bools),
+            _ => (),
+        }
+    }
+}
+
+struct AssocFnVisitor<'a> {
+    cx: &'a EarlyContext<'a>,
+    max_fn_params_bools: u64,
+}
+
+impl<'a> Visitor<'a> for AssocFnVisitor<'a> {
+    fn visit_assoc_item(&mut self, assoc_item: &AssocItem) {
+        if let AssocItemKind::Fn(fn_sig, _) = &assoc_item.kind {
+            check_fn(
+                self.cx,
+                fn_sig,
+                &assoc_item.attrs,
+                assoc_item.span,
+                self.max_fn_params_bools,
+            );
+        }
+
+        // dont go down into children because `check_item` is going to cover them
+    }
+}

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),
@@ -1150,6 +1156,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&eta_reduction::REDUNDANT_CLOSURE),
         LintId::of(&eval_order_dependence::DIVERGING_SUB_EXPRESSION),
         LintId::of(&eval_order_dependence::EVAL_ORDER_DEPENDENCE),
+        LintId::of(&excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
+        LintId::of(&excessive_bools::STRUCT_EXCESSIVE_BOOLS),
         LintId::of(&excessive_precision::EXCESSIVE_PRECISION),
         LintId::of(&explicit_write::EXPLICIT_WRITE),
         LintId::of(&format::USELESS_FORMAT),
@@ -1375,6 +1383,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&enum_variants::MODULE_INCEPTION),
         LintId::of(&eq_op::OP_REF),
         LintId::of(&eta_reduction::REDUNDANT_CLOSURE),
+        LintId::of(&excessive_bools::FN_PARAMS_EXCESSIVE_BOOLS),
+        LintId::of(&excessive_bools::STRUCT_EXCESSIVE_BOOLS),
         LintId::of(&excessive_precision::EXCESSIVE_PRECISION),
         LintId::of(&formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING),
         LintId::of(&formatting::SUSPICIOUS_ELSE_FORMATTING),

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 {

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: "style",
+        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: "style",
+        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,4 @@
+fn f(_: bool) {}
+fn g(_: bool, _: bool) {}
+
+fn main() {}

tests/ui-toml/fn_params_excessive_bools/test.stderr

@@ -0,0 +1,11 @@
+error: excessive amount of bools in function parameters
+  --> $DIR/test.rs:2: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,7 @@
+struct S {
+    a: bool,
+}
+
+struct Foo {}
+
+fn main() {}

tests/ui-toml/struct_excessive_bools/test.stderr

@@ -0,0 +1,13 @@
+error: excessive amount of bools in a struct
+  --> $DIR/test.rs:1: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