Adding lint test for excessive LOC.

CHANGELOG.md

@@ -981,6 +981,7 @@ All notable changes to this project will be documented in this file.
 [`temporary_assignment`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_assignment
 [`temporary_cstring_as_ptr`]: https://rust-lang.github.io/rust-clippy/master/index.html#temporary_cstring_as_ptr
 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments
+[`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines
 [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg
 [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str
 [`transmute_int_to_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_int_to_bool

README.md

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

@@ -64,6 +64,7 @@ impl LintPass for AssignOps {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for AssignOps {
+    #[allow(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx hir::Expr) {
         match &expr.node {
             hir::ExprKind::AssignOp(op, lhs, rhs) => {

clippy_lints/src/bit_mask.rs

@@ -177,6 +177,7 @@ fn check_compare(cx: &LateContext<'_, '_>, bit_op: &Expr, cmp_op: BinOpKind, cmp
     }
 }
 
+#[allow(clippy::too_many_lines)]
 fn check_bit_mask(
     cx: &LateContext<'_, '_>,
     bit_op: BinOpKind,

clippy_lints/src/eq_op.rs

@@ -59,7 +59,7 @@ impl LintPass for EqOp {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for EqOp {
-    #[allow(clippy::similar_names)]
+    #[allow(clippy::similar_names, clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
         if let ExprKind::Binary(op, ref left, ref right) = e.node {
             if in_macro(e.span) {

clippy_lints/src/functions.rs

@@ -1,9 +1,9 @@
-use crate::utils::{iter_input_pats, span_lint, type_is_unsafe_function};
+use crate::utils::{iter_input_pats, snippet, span_lint, type_is_unsafe_function};
 use matches::matches;
 use rustc::hir;
 use rustc::hir::def::Def;
 use rustc::hir::intravisit;
-use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
+use rustc::lint::{in_external_macro, LateContext, LateLintPass, LintArray, LintContext, LintPass};
 use rustc::ty;
 use rustc::{declare_tool_lint, lint_array};
 use rustc_data_structures::fx::FxHashSet;
@@ -31,6 +31,29 @@ declare_clippy_lint! {
     "functions with too many arguments"
 }
 
+/// **What it does:** Checks for functions with a large amount of lines.
+///
+/// **Why is this bad?** Functions with a lot of lines are harder to understand
+/// due to having to look at a larger amount of code to understand what the
+/// function is doing. Consider splitting the body of the function into
+/// multiple functions.
+///
+/// **Known problems:** None.
+///
+/// **Example:**
+/// ``` rust
+/// fn im_too_long() {
+/// println!("");
+/// // ... 100 more LoC
+/// println!("");
+/// }
+/// ```
+declare_clippy_lint! {
+    pub TOO_MANY_LINES,
+    pedantic,
+    "functions with too many lines"
+}
+
 /// **What it does:** Checks for public functions that dereferences raw pointer
 /// arguments but are not marked unsafe.
 ///
@@ -62,17 +85,18 @@ declare_clippy_lint! {
 #[derive(Copy, Clone)]
 pub struct Functions {
     threshold: u64,
+    max_lines: u64,
 }
 
 impl Functions {
-    pub fn new(threshold: u64) -> Self {
-        Self { threshold }
+    pub fn new(threshold: u64, max_lines: u64) -> Self {
+        Self { threshold, max_lines }
     }
 }
 
 impl LintPass for Functions {
     fn get_lints(&self) -> LintArray {
-        lint_array!(TOO_MANY_ARGUMENTS, NOT_UNSAFE_PTR_ARG_DEREF)
+        lint_array!(TOO_MANY_ARGUMENTS, TOO_MANY_LINES, NOT_UNSAFE_PTR_ARG_DEREF)
     }
 
     fn name(&self) -> &'static str {
@@ -123,6 +147,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Functions {
         }
 
         self.check_raw_ptr(cx, unsafety, decl, body, nodeid);
+        self.check_line_number(cx, span);
     }
 
     fn check_trait_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx hir::TraitItem) {
@@ -153,6 +178,72 @@ impl<'a, 'tcx> Functions {
         }
     }
 
+    fn check_line_number(self, cx: &LateContext<'_, '_>, span: Span) {
+        if in_external_macro(cx.sess(), span) {
+            return;
+        }
+
+        let code_snippet = snippet(cx, span, "..");
+        let mut line_count: u64 = 0;
+        let mut in_comment = false;
+        let mut code_in_line;
+
+        // Skip the surrounding function decl.
+        let start_brace_idx = match code_snippet.find('{') {
+            Some(i) => i + 1,
+            None => 0,
+        };
+        let end_brace_idx = match code_snippet.find('}') {
+            Some(i) => i,
+            None => code_snippet.len(),
+        };
+        let function_lines = code_snippet[start_brace_idx..end_brace_idx].lines();
+
+        for mut line in function_lines {
+            code_in_line = false;
+            loop {
+                line = line.trim_start();
+                if line.is_empty() {
+                    break;
+                }
+                if in_comment {
+                    match line.find("*/") {
+                        Some(i) => {
+                            line = &line[i + 2..];
+                            in_comment = false;
+                            continue;
+                        },
+                        None => break,
+                    }
+                } else {
+                    let multi_idx = match line.find("/*") {
+                        Some(i) => i,
+                        None => line.len(),
+                    };
+                    let single_idx = match line.find("//") {
+                        Some(i) => i,
+                        None => line.len(),
+                    };
+                    code_in_line |= multi_idx > 0 && single_idx > 0;
+                    // Implies multi_idx is below line.len()
+                    if multi_idx < single_idx {
+                        line = &line[multi_idx + 2..];
+                        in_comment = true;
+                        continue;
+                    }
+                    break;
+                }
+            }
+            if code_in_line {
+                line_count += 1;
+            }
+        }
+
+        if line_count > self.max_lines {
+            span_lint(cx, TOO_MANY_LINES, span, "This function has a large number of lines.")
+        }
+    }
+
     fn check_raw_ptr(
         self,
         cx: &LateContext<'a, 'tcx>,

clippy_lints/src/lib.rs

@@ -290,6 +290,7 @@ pub fn read_conf(reg: &rustc_plugin::Registry<'_>) -> Conf {
     }
 }
 
+#[allow(clippy::too_many_lines)]
 #[rustfmt::skip]
 pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     let mut store = reg.sess.lint_store.borrow_mut();
@@ -427,7 +428,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
     reg.register_late_lint_pass(box blacklisted_name::BlackListedName::new(
             conf.blacklisted_names.iter().cloned().collect()
     ));
-    reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold));
+    reg.register_late_lint_pass(box functions::Functions::new(conf.too_many_arguments_threshold, conf.too_many_lines_threshold));
     reg.register_early_lint_pass(box doc::Doc::new(conf.doc_valid_idents.iter().cloned().collect()));
     reg.register_late_lint_pass(box neg_multiply::NegMultiply);
     reg.register_early_lint_pass(box unsafe_removed_from_name::UnsafeNameRemoval);
@@ -527,6 +528,7 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
         enum_glob_use::ENUM_GLOB_USE,
         enum_variants::MODULE_NAME_REPETITIONS,
         enum_variants::PUB_ENUM_VARIANT_NAMES,
+        functions::TOO_MANY_LINES,
         if_not_else::IF_NOT_ELSE,
         infinite_iter::MAYBE_INFINITE_ITER,
         items_after_statements::ITEMS_AFTER_STATEMENTS,

clippy_lints/src/loops.rs

@@ -472,6 +472,7 @@ impl LintPass for Pass {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
+    #[allow(clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, expr: &'tcx Expr) {
         // we don't want to check expanded macros
         if in_macro(expr.span) {
@@ -1066,6 +1067,7 @@ fn detect_manual_memcpy<'a, 'tcx>(
 
 /// Check for looping over a range and then indexing a sequence with it.
 /// The iteratee must be a range literal.
+#[allow(clippy::too_many_lines)]
 fn check_for_loop_range<'a, 'tcx>(
     cx: &LateContext<'a, 'tcx>,
     pat: &'tcx Pat,

clippy_lints/src/methods/mod.rs

@@ -1005,6 +1005,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
 }
 
 /// Checks for the `OR_FUN_CALL` lint.
+#[allow(clippy::too_many_lines)]
 fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) {
     /// Check for `unwrap_or(T::new())` or `unwrap_or(T::default())`.
     fn check_unwrap_or_default(
@@ -1151,6 +1152,7 @@ fn lint_or_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Spa
 }
 
 /// Checks for the `EXPECT_FUN_CALL` lint.
+#[allow(clippy::too_many_lines)]
 fn lint_expect_fun_call(cx: &LateContext<'_, '_>, expr: &hir::Expr, method_span: Span, name: &str, args: &[hir::Expr]) {
     // Strip `&`, `as_ref()` and `as_str()` off `arg` until we're left with either a `String` or
     // `&str`

clippy_lints/src/needless_pass_by_value.rs

@@ -73,6 +73,7 @@ macro_rules! need {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for NeedlessPassByValue {
+    #[allow(clippy::too_many_lines)]
     fn check_fn(
         &mut self,
         cx: &LateContext<'a, 'tcx>,

clippy_lints/src/non_expressive_names.rs

@@ -158,6 +158,7 @@ impl<'a, 'tcx, 'b> SimilarNamesNameVisitor<'a, 'tcx, 'b> {
             );
         }
     }
+    #[allow(clippy::too_many_lines)]
     fn check_name(&mut self, span: Span, name: Name) {
         let interned_name = name.as_str();
         if interned_name.chars().any(char::is_uppercase) {

clippy_lints/src/ptr.rs

@@ -151,6 +151,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for PointerPass {
     }
 }
 
+#[allow(clippy::too_many_lines)]
 fn check_fn(cx: &LateContext<'_, '_>, decl: &FnDecl, fn_id: NodeId, opt_body_id: Option<BodyId>) {
     let fn_def_id = cx.tcx.hir().local_def_id(fn_id);
     let sig = cx.tcx.fn_sig(fn_def_id);

clippy_lints/src/redundant_clone.rs

@@ -80,6 +80,7 @@ impl LintPass for RedundantClone {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantClone {
+    #[allow(clippy::too_many_lines)]
     fn check_fn(
         &mut self,
         cx: &LateContext<'a, 'tcx>,

clippy_lints/src/transmute.rs

@@ -226,7 +226,7 @@ impl LintPass for Transmute {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Transmute {
-    #[allow(clippy::similar_names)]
+    #[allow(clippy::similar_names, clippy::too_many_lines)]
     fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) {
         if let ExprKind::Call(ref path_expr, ref args) = e.node {
             if let ExprKind::Path(ref qpath) = path_expr.node {

clippy_lints/src/types.rs

@@ -240,6 +240,7 @@ fn match_type_parameter(cx: &LateContext<'_, '_>, qpath: &QPath, path: &[&str])
 ///
 /// The parameter `is_local` distinguishes the context of the type; types from
 /// local bindings should only be checked for the `BORROWED_BOX` lint.
+#[allow(clippy::too_many_lines)]
 fn check_ty(cx: &LateContext<'_, '_>, hir_ty: &hir::Ty, is_local: bool) {
     if in_macro(hir_ty.span) {
         return;
@@ -1968,7 +1969,7 @@ impl LintPass for ImplicitHasher {
 }
 
 impl<'a, 'tcx> LateLintPass<'a, 'tcx> for ImplicitHasher {
-    #[allow(clippy::cast_possible_truncation)]
+    #[allow(clippy::cast_possible_truncation, clippy::too_many_lines)]
     fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
         use syntax_pos::BytePos;
 

clippy_lints/src/utils/author.rs

@@ -194,6 +194,7 @@ struct PrintVisitor {
 }
 
 impl<'tcx> Visitor<'tcx> for PrintVisitor {
+    #[allow(clippy::too_many_lines)]
     fn visit_expr(&mut self, expr: &Expr) {
         print!("    if let ExprKind::");
         let current = format!("{}.node", self.current);
@@ -506,6 +507,7 @@ impl<'tcx> Visitor<'tcx> for PrintVisitor {
         }
     }
 
+    #[allow(clippy::too_many_lines)]
     fn visit_pat(&mut self, pat: &Pat) {
         print!("    if let PatKind::");
         let current = format!("{}.node", self.current);

clippy_lints/src/utils/conf.rs

@@ -148,6 +148,8 @@ define_Conf! {
     (literal_representation_threshold, "literal_representation_threshold", 16384 => u64),
     /// Lint: TRIVIALLY_COPY_PASS_BY_REF. The maximum size (in bytes) to consider a `Copy` type for passing by value instead of by reference.
     (trivial_copy_size_limit, "trivial_copy_size_limit", None => Option<u64>),
+    /// Lint: TOO_MANY_LINES. The maximum number of lines a function or method can have
+    (too_many_lines_threshold, "too_many_lines_threshold", 100 => u64),
 }
 
 impl Default for Conf {

clippy_lints/src/utils/hir_utils.rs

@@ -389,7 +389,7 @@ impl<'a, 'tcx: 'a> SpanlessHash<'a, 'tcx> {
         .hash(&mut self.s);
     }
 
-    #[allow(clippy::many_single_char_names)]
+    #[allow(clippy::many_single_char_names, clippy::too_many_lines)]
     pub fn hash_expr(&mut self, e: &Expr) {
         if let Some(e) = constant_simple(self.cx, self.tables, e) {
             return e.hash(&mut self.s);

src/driver.rs

@@ -20,6 +20,7 @@ fn show_version() {
     println!(env!("CARGO_PKG_VERSION"));
 }
 
+#[allow(clippy::too_many_lines)]
 pub fn main() {
     rustc_driver::init_rustc_env_logger();
     exit(

tests/ui-toml/functions_maxlines/clippy.toml

@@ -0,0 +1 @@
+too-many-lines-threshold = 1

tests/ui-toml/functions_maxlines/test.rs

@@ -0,0 +1,45 @@
+#![warn(clippy::too_many_lines)]
+
+// This function should be considered one line.
+fn many_comments_but_one_line_of_code() {
+    /* println!("This is good."); */
+    // println!("This is good.");
+    /* */ // println!("This is good.");
+    /* */ // println!("This is good.");
+    /* */ // println!("This is good.");
+    /* */ // println!("This is good.");
+    /* println!("This is good.");
+    println!("This is good.");
+    println!("This is good."); */
+    println!("This is good.");
+}
+
+// This should be considered two and a fail.
+fn too_many_lines() {
+    println!("This is bad.");
+    println!("This is bad.");
+}
+
+// This should be considered one line.
+#[rustfmt::skip]
+fn comment_starts_after_code() {
+    let _ = 5; /* closing comment. */ /*
+    this line shouldn't be counted theoretically.
+    */
+}
+
+// This should be considered one line.
+fn comment_after_code() {
+    let _ = 5; /* this line should get counted once. */
+}
+
+// This should fail since it is technically two lines.
+#[rustfmt::skip]
+fn comment_before_code() {
+    let _ = "test";
+    /* This comment extends to the front of
+    teh code but this line should still count. */ let _ = 5;
+}
+
+// This should be considered one line.
+fn main() {}