Port mitsuhiko's 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
@@ -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_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 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:
 

clippy_lints/src/excessive_bools.rs

@@ -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),
+            _ => (),
+        }
+    }
+}

clippy_lints/src/functions.rs

@@ -1,9 +1,8 @@
 use crate::utils::{
-    attr_by_name, attrs::is_proc_macro, is_must_use_ty, iter_input_pats, match_def_path, must_use_attr, qpath_res,
-    return_ty, snippet, snippet_opt, span_lint, span_lint_and_help, span_lint_and_then, trait_ref_of_method,
-    type_is_unsafe_function,
+    attr_by_name, attrs::is_proc_macro, is_must_use_ty, is_trait_impl_item, iter_input_pats, match_def_path,
+    must_use_attr, qpath_res, return_ty, snippet, snippet_opt, span_lint, span_lint_and_help, span_lint_and_then,
+    trait_ref_of_method, type_is_unsafe_function,
 };
-use matches::matches;
 use rustc::hir::map::Map;
 use rustc::lint::in_external_macro;
 use rustc::ty::{self, Ty};
@@ -195,20 +194,14 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
         span: Span,
         hir_id: hir::HirId,
     ) {
-        let is_impl = if let Some(hir::Node::Item(item)) = cx.tcx.hir().find(cx.tcx.hir().get_parent_node(hir_id)) {
-            matches!(item.kind, hir::ItemKind::Impl{ of_trait: Some(_), .. })
-        } else {
-            false
-        };
-
         let unsafety = match kind {
             intravisit::FnKind::ItemFn(_, _, hir::FnHeader { unsafety, .. }, _, _) => unsafety,
             intravisit::FnKind::Method(_, sig, _, _) => sig.header.unsafety,
             intravisit::FnKind::Closure(_) => return,
         };
 
         // don't warn for implementations, it's not their fault
-        if !is_impl {
+        if !is_trait_impl_item(cx, hir_id) {
             // don't lint extern functions decls, it's not their fault either
             match kind {
                 intravisit::FnKind::Method(

clippy_lints/src/lib.rs

@@ -202,6 +202,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;
@@ -528,6 +529,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,
@@ -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 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),
@@ -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::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": u64, 512_000),
     /// 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),
+    /// 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 {

clippy_lints/src/utils/mod.rs

@@ -1354,3 +1354,18 @@ 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 {
+    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(_), .. })
+    } else {
+        false
+    }
+}

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; 352] = [
+pub const ALL_LINTS: [Lint; 354] = [
     Lint {
         name: "absurd_extreme_comparisons",
         group: "correctness",
@@ -623,6 +623,13 @@ pub const ALL_LINTS: [Lint; 352] = [
         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",
@@ -1932,6 +1939,13 @@ pub const ALL_LINTS: [Lint; 352] = [
         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