From fa9429aac0ffed505f3f02e8fc75f646c69f5c61 Mon Sep 17 00:00:00 2001
From: Milos Djermanovic <milos.djermanovic@gmail.com>
Date: Fri, 9 Oct 2020 15:15:01 +0200
Subject: [PATCH] Fix: don't count line after EOF in max-lines (#13735)

* Fix: don't count line after EOF in max-lines

* Update docs
---
 docs/rules/max-lines.md      |   1 +
 lib/rules/max-lines.js       |   8 ++
 tests/lib/rules/max-lines.js | 150 ++++++++++++++++++++++++++++++++++-
 3 files changed, 157 insertions(+), 2 deletions(-)

diff --git a/docs/rules/max-lines.md b/docs/rules/max-lines.md
index 0019e983650..de4ea0d94f2 100644
--- a/docs/rules/max-lines.md
+++ b/docs/rules/max-lines.md
@@ -6,6 +6,7 @@ Some people consider large files a code smell. Large files tend to do a lot of t
 
 This rule enforces a maximum number of lines per file, in order to aid in maintainability and reduce complexity.
 
+Please note that most editors show an additional empty line at the end if the file ends with a line break. This rule does not count that extra line.
 
 ## Options
 
diff --git a/lib/rules/max-lines.js b/lib/rules/max-lines.js
index f33adbdb5b6..0c7e761fa61 100644
--- a/lib/rules/max-lines.js
+++ b/lib/rules/max-lines.js
@@ -131,6 +131,14 @@ module.exports = {
                     text
                 }));
 
+                /*
+                 * If file ends with a linebreak, `sourceCode.lines` will have one extra empty line at the end.
+                 * That isn't a real line, so we shouldn't count it.
+                 */
+                if (lines.length > 1 && lodash.last(lines).text === "") {
+                    lines.pop();
+                }
+
                 if (skipBlankLines) {
                     lines = lines.filter(l => l.text.trim() !== "");
                 }
diff --git a/tests/lib/rules/max-lines.js b/tests/lib/rules/max-lines.js
index b7c255e9322..24c811928de 100644
--- a/tests/lib/rules/max-lines.js
+++ b/tests/lib/rules/max-lines.js
@@ -21,8 +21,15 @@ ruleTester.run("max-lines", rule, {
     valid: [
         "var x;",
         "var xy;\nvar xy;",
+        { code: "A", options: [1] },
+        { code: "A\n", options: [1] },
+        { code: "A\r", options: [1] },
+        { code: "A\r\n", options: [1] },
         { code: "var xy;\nvar xy;", options: [2] },
+        { code: "var xy;\nvar xy;\n", options: [2] },
         { code: "var xy;\nvar xy;", options: [{ max: 2 }] },
+        { code: "// comment\n", options: [{ max: 0, skipComments: true }] },
+        { code: "foo;\n /* comment */\n", options: [{ max: 1, skipComments: true }] },
         {
             code: [
                 "//a single line comment",
@@ -233,6 +240,50 @@ ruleTester.run("max-lines", rule, {
                 }
             ]
         },
+        {
+
+            // Questionable. Makes sense to report this, and makes sense to not report this.
+            code: "",
+            options: [{ max: 0 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 0, actual: 1 },
+                    line: 1,
+                    column: 1,
+                    endLine: 1,
+                    endColumn: 1
+                }
+            ]
+        },
+        {
+            code: " ",
+            options: [{ max: 0 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 0, actual: 1 },
+                    line: 1,
+                    column: 1,
+                    endLine: 1,
+                    endColumn: 2
+                }
+            ]
+        },
+        {
+            code: "\n",
+            options: [{ max: 0 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 0, actual: 1 },
+                    line: 1,
+                    column: 1,
+                    endLine: 2,
+                    endColumn: 1
+                }
+            ]
+        },
         {
             code: "A",
             options: [{ max: 0 }],
@@ -247,6 +298,62 @@ ruleTester.run("max-lines", rule, {
                 }
             ]
         },
+        {
+            code: "A\n",
+            options: [{ max: 0 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 0, actual: 1 },
+                    line: 1,
+                    column: 1,
+                    endLine: 2,
+                    endColumn: 1
+                }
+            ]
+        },
+        {
+            code: "A\n ",
+            options: [{ max: 0 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 0, actual: 2 },
+                    line: 1,
+                    column: 1,
+                    endLine: 2,
+                    endColumn: 2
+                }
+            ]
+        },
+        {
+            code: "A\n ",
+            options: [{ max: 1 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 1, actual: 2 },
+                    line: 2,
+                    column: 1,
+                    endLine: 2,
+                    endColumn: 2
+                }
+            ]
+        },
+        {
+            code: "A\n\n",
+            options: [{ max: 1 }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 1, actual: 2 },
+                    line: 2,
+                    column: 1,
+                    endLine: 3,
+                    endColumn: 1
+                }
+            ]
+        },
         {
             code: ["var a = 'a'; ", "var x", "var c;", "console.log"].join(
                 "\n"
@@ -269,7 +376,7 @@ ruleTester.run("max-lines", rule, {
             errors: [
                 {
                     messageId: "exceed",
-                    data: { max: 2, actual: 4 },
+                    data: { max: 2, actual: 3 },
                     line: 3,
                     column: 1,
                     endLine: 4,
@@ -283,7 +390,7 @@ ruleTester.run("max-lines", rule, {
             errors: [
                 {
                     messageId: "exceed",
-                    data: { max: 2, actual: 4 },
+                    data: { max: 2, actual: 3 },
                     line: 3,
                     column: 1,
                     endLine: 4,
@@ -346,6 +453,26 @@ ruleTester.run("max-lines", rule, {
                 }
             ]
         },
+        {
+            code: [
+                "var a = 'a'; ",
+                "var x",
+                "var c;",
+                "console.log",
+                "/* block comments */\n"
+            ].join("\n"),
+            options: [{ max: 2, skipComments: true }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 2, actual: 4 },
+                    line: 3,
+                    column: 1,
+                    endLine: 6,
+                    endColumn: 1
+                }
+            ]
+        },
         {
             code: [
                 "var a = 'a'; ",
@@ -366,6 +493,25 @@ ruleTester.run("max-lines", rule, {
                 }
             ]
         },
+        {
+            code: [
+                "var a = 'a'; ",
+                "",
+                "",
+                "// comment"
+            ].join("\n"),
+            options: [{ max: 2, skipComments: true }],
+            errors: [
+                {
+                    messageId: "exceed",
+                    data: { max: 2, actual: 3 },
+                    line: 3,
+                    column: 1,
+                    endLine: 4,
+                    endColumn: 11
+                }
+            ]
+        },
         {
             code: [
                 "var a = 'a'; ",