Fix max-statements-per-line

[nzakas]

While reviewing #7044, I find that max-statements-per-line has an undocumented and confusing behavior. For some reason, we allow an option of max: 0, which seems like a mistake (though intentional because there are tests). It looks like the only time this is triggered is when there's an empty blocks statement and there's a special case for that in the code: https://github.com/eslint/eslint/blob/master/lib/rules/max-statements-per-line.js#L167

This seems very broken because it doesn't make sense to ever have max statements be 0 per line, and we already have no-empty to catch empty blocks.

I think we should fix this by:

1. Ensuring max is 1 or larger
2. Removing the empty block check

This would be semver-patch because we are reducing the number of warnings this rule produces.

@mysticatea @scriptdaemon

[btmills]

@nzakas what is the desired behavior when max is 0? Does it silently ignore that setting?

[sstern6]

@nzakas I would like to make a PR for this, this will be my first PR in the code base if you are ok with me taking this on.

Thank you

[ilyavolodin]

@btmills I think what @nzakas means is we should tweak schema to not allow 0 value at all.

@sstern6 Go for it! We are always glad to accept contributions from new people!

[sstern6]

@ilyavolodin thank you, will hopefully have a PR open in a few days. Will let you know if I have any questions about execution etc.

[nzakas]

@btmills yes, what @ilyavolodin said. The minimum for max should be set to 1 in the schema.

[platinumazure]

@sstern6 Thanks for volunteering to take this on! If you have questions, feel free to either leave a comment in the issue or stop by our Gitter chat.

[btmills]

@ilyavolodin @nzakas got it, thanks! If we're making the schema more restrictive, then this should be a semver-minor change, right?

[ilyavolodin]

@btmills Was just about to say that. I'm actually not sure if even semver-minor would be enough, since it's a breaking change for anyone using max: 0... Granted, max: 0 doesn't make a whole lot of sense to begin with, so there shouldn't be anyone using it, but you never know.

[nzakas]

I can't see a scenario where someone is using max:0, because you can't write code with zero statements per line. (Can someone verify that?)

[platinumazure]

@nzakas Confirmed on online demo:

/* eslint max-statements-per-line: ["error", { "max": 0 }] */

something();

Result:

3:1 - This line has too many statements. Maximum allowed is 0. (max-statements-per-line)

Now that I've seen this behavior, I agree it's useless and buggy and does not need a semver-major release to fix.

[ilyavolodin]

In that case, I'm perfectly fine calling this a bug and doing as a semver-patch