Lint function with too many arguments

Author: mcarton

README.md

@@ -14,7 +14,7 @@ Table of contents:
 * [License](#license)
 
 ##Lints
-There are 135 lints included in this crate:
+There are 136 lints included in this crate:
 
 name                                                                                                                 | default | meaning
 ---------------------------------------------------------------------------------------------------------------------|---------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
@@ -130,6 +130,7 @@ name
 [suspicious_assignment_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_assignment_formatting) | warn    | suspicious formatting of `*=`, `-=` or `!=`
 [suspicious_else_formatting](https://github.com/Manishearth/rust-clippy/wiki#suspicious_else_formatting)             | warn    | suspicious formatting of `else if`
 [temporary_assignment](https://github.com/Manishearth/rust-clippy/wiki#temporary_assignment)                         | warn    | assignments to temporaries
+[too_many_arguments](https://github.com/Manishearth/rust-clippy/wiki#too_many_arguments)                             | warn    | functions with too many arguments
 [toplevel_ref_arg](https://github.com/Manishearth/rust-clippy/wiki#toplevel_ref_arg)                                 | warn    | An entire binding was declared as `ref`, in a function argument (`fn foo(ref x: Bar)`), or a `let` statement (`let ref x = foo()`). In such cases, it is preferred to take references with `&`.
 [trivial_regex](https://github.com/Manishearth/rust-clippy/wiki#trivial_regex)                                       | warn    | finds trivial regular expressions in `Regex::new(_)` invocations
 [type_complexity](https://github.com/Manishearth/rust-clippy/wiki#type_complexity)                                   | warn    | usage of very complex types; recommends factoring out parts into `type` definitions

src/functions.rs

@@ -0,0 +1,75 @@
+use rustc::lint::*;
+use rustc_front::hir;
+use rustc_front::intravisit;
+use syntax::ast;
+use syntax::codemap::Span;
+use utils::span_lint;
+
+/// **What it does:** Check for functions with too many parameters.
+///
+/// **Why is this bad?** Functions with lots of parameters are considered bad style and reduce
+/// readability (“what does the 5th parameter means?”). Consider grouping some parameters into a
+/// new type.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+///
+/// ```
+/// fn foo(x: u32, y: u32, name: &str, c: Color, w: f32, h: f32, a: f32, b: f32) { .. }
+/// ```
+declare_lint! {
+    pub TOO_MANY_ARGUMENTS,
+    Warn,
+    "functions with too many arguments"
+}
+
+#[derive(Copy,Clone)]
+pub struct Functions {
+    threshold: u64,
+}
+
+impl Functions {
+    pub fn new(threshold: u64) -> Functions {
+        Functions {
+            threshold: threshold
+        }
+    }
+}
+
+impl LintPass for Functions {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(TOO_MANY_ARGUMENTS)
+    }
+}
+
+impl LateLintPass for Functions {
+    fn check_fn(&mut self, cx: &LateContext, _: intravisit::FnKind, decl: &hir::FnDecl, _: &hir::Block, span: Span, nodeid: ast::NodeId) {
+        use rustc::front::map::Node::*;
+
+        if let Some(NodeItem(ref item)) = cx.tcx.map.find(cx.tcx.map.get_parent_node(nodeid)) {
+            match item.node {
+                hir::ItemImpl(_, _, _, Some(_), _, _) | hir::ItemDefaultImpl(..) => return,
+                _ => (),
+            }
+        }
+
+        self.check_arg_number(cx, decl, span);
+    }
+
+    fn check_trait_item(&mut self, cx: &LateContext, item: &hir::TraitItem) {
+        if let hir::MethodTraitItem(ref sig, _) = item.node {
+            self.check_arg_number(cx, &sig.decl, item.span);
+        }
+    }
+}
+
+impl Functions {
+    fn check_arg_number(&self, cx: &LateContext, decl: &hir::FnDecl, span: Span) {
+        let args = decl.inputs.len() as u64;
+        if args > self.threshold {
+            span_lint(cx, TOO_MANY_ARGUMENTS, span,
+                      &format!("this function has to many arguments ({}/{})", args, self.threshold));
+        }
+    }
+}

src/lib.rs

@@ -63,6 +63,7 @@ pub mod escape;
 pub mod eta_reduction;
 pub mod format;
 pub mod formatting;
+pub mod functions;
 pub mod identity_op;
 pub mod if_not_else;
 pub mod items_after_statements;
@@ -211,6 +212,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
     reg.register_late_lint_pass(box unused_label::UnusedLabel);
     reg.register_late_lint_pass(box new_without_default::NewWithoutDefault);
     reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(conf.blacklisted_names));
+    reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold));
 
     reg.register_lint_group("clippy_pedantic", vec![
         array_indexing::INDEXING_SLICING,
@@ -263,6 +265,7 @@ pub fn plugin_registrar(reg: &mut Registry) {
         format::USELESS_FORMAT,
         formatting::SUSPICIOUS_ASSIGNMENT_FORMATTING,
         formatting::SUSPICIOUS_ELSE_FORMATTING,
+        functions::TOO_MANY_ARGUMENTS,
         identity_op::IDENTITY_OP,
         if_not_else::IF_NOT_ELSE,
         items_after_statements::ITEMS_AFTER_STATEMENTS,

src/utils/conf.rs

@@ -150,7 +150,7 @@ define_Conf! {
     /// Lint: CYCLOMATIC_COMPLEXITY. The maximum cyclomatic complexity a function can have
     ("cyclomatic-complexity-threshold", cyclomatic_complexity_threshold, 25 => u64),
     /// Lint: TOO_MANY_ARGUMENTS. The maximum number of argument a function or method can have
-    ("too-many-arguments-threshold", too_many_arguments_threshold, 6 => u64),
+    ("too-many-arguments-threshold", too_many_arguments_threshold, 7 => u64),
     /// Lint: TYPE_COMPLEXITY. The maximum complexity a type can have
     ("type-complexity-threshold", type_complexity_threshold, 250 => u64),
 }

tests/compile-fail/functions.rs

@@ -0,0 +1,33 @@
+#![feature(plugin)]
+#![plugin(clippy)]
+
+#![deny(clippy)]
+#![allow(dead_code)]
+
+fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {}
+
+fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {
+    //~^ ERROR: this function has to many arguments (8/7)
+}
+
+trait Foo {
+    fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool);
+    fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ());
+    //~^ ERROR: this function has to many arguments (8/7)
+}
+
+struct Bar;
+
+impl Bar {
+    fn good_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {}
+    fn bad_method(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {}
+    //~^ ERROR: this function has to many arguments (8/7)
+}
+
+// ok, we don’t want to warn implementations
+impl Foo for Bar {
+    fn good(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool) {}
+    fn bad(_one: u32, _two: u32, _three: &str, _four: bool, _five: f32, _six: f32, _seven: bool, _eight: ()) {}
+}
+
+fn main() {}