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

[mdjermanovic]

Prerequisites checklist

• I have read the contributing guidelines.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[x] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Tell us about your environment

• ESLint Version: v7.10.0
• Node Version: v12.18.4
• npm Version: v6.14.6

What parser (default, @babel/eslint-parser, @typescript-eslint/parser, etc.) are you using?

default

Please show your full configuration:

Configuration

module.exports = {
  parserOptions: {
    ecmaVersion: 2015
  }
};

What did you do? Please include the actual source code causing the issue.

Online Demo

/* eslint max-lines: ["error", { max: 3 }] */
second_line
third_line

There's a linebreak at the end of third_line, and nothing after that linebreak.

What did you expect to happen?

No errors. Editors may show 4 lines, but in this case there are only 3 real lines.

What actually happened? Please include the actual, raw output from ESLint.

1 error:

  4:1  error  File has too many lines (4). Maximum allowed is 3  max-lines

What changes did you make? (Give an overview)

Fixed the max-lines rule to not count line after EOF.

Is there anything you'd like reviewers to focus on?

[nzakas]

Did this behavior exist before your change? I'd be in favor of assuming max: 0 is a mistake and normalizing to max: 1. I can't think of a reason we'd ever want 0 max lines in a file.

[mdjermanovic]

Yes, that's behavior in the actual version as well, I thought it would be better not to change that behavior in this PR and just added this test to make sure that this change doesn't affect max: 0 on an empty file, since that isn't technically an extra empty line after a linebreak.

I'm not sure why is it allowed to set max: 0, but it seems to be working like that from the start, and I couldn't find any concerns regarding that in the initial PR. There is also an existing test with max: 0.

The only use case for max: 0 I could think of is using ESLint to disallow any .js files in some directories, and for that cause it might make sense to treat an empty file as a file with 1 line.

[nzakas]

Okay, I agree with not changing what it’s already doing. 👍

[nzakas]

Code looks fine. Can you add some documentation about this?

[mdjermanovic]

Code looks fine. Can you add some documentation about this?

Done!

[nzakas]

LGTM