From 2520f5a579945fac07349d63e530ae3ce27371b6 Mon Sep 17 00:00:00 2001 From: alberto Date: Fri, 10 Jun 2016 18:14:46 +0200 Subject: [PATCH] New: `max-lines` rule (fixes #6078) (#6321) --- conf/eslint.json | 1 + docs/rules/README.md | 1 + docs/rules/max-lines.md | 125 +++++++++++++++++++++++++ lib/rules/max-lines.js | 148 +++++++++++++++++++++++++++++ tests/lib/rules/max-lines.js | 176 +++++++++++++++++++++++++++++++++++ 5 files changed, 451 insertions(+) create mode 100644 docs/rules/max-lines.md create mode 100644 lib/rules/max-lines.js create mode 100644 tests/lib/rules/max-lines.js diff --git a/conf/eslint.json b/conf/eslint.json index 7aff7058046..cb6fa2e96d4 100755 --- a/conf/eslint.json +++ b/conf/eslint.json @@ -167,6 +167,7 @@ "lines-around-comment": "off", "max-depth": "off", "max-len": "off", + "max-lines": "off", "max-nested-callbacks": "off", "max-params": "off", "max-statements": "off", diff --git a/docs/rules/README.md b/docs/rules/README.md index 0a0aa6c3e93..f97c1bf881f 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -174,6 +174,7 @@ These rules relate to style guidelines, and are therefore quite subjective: * [lines-around-comment](lines-around-comment.md): require empty lines around comments * [max-depth](max-depth.md): enforce a maximum depth that blocks can be nested * [max-len](max-len.md): enforce a maximum line length +* [max-lines](max-lines.md): enforce a maximum file length * [max-nested-callbacks](max-nested-callbacks.md): enforce a maximum depth that callbacks can be nested * [max-params](max-params.md): enforce a maximum number of parameters in `function` definitions * [max-statements](max-statements.md): enforce a maximum number of statements allowed in `function` blocks diff --git a/docs/rules/max-lines.md b/docs/rules/max-lines.md new file mode 100644 index 00000000000..ea810ce20d9 --- /dev/null +++ b/docs/rules/max-lines.md @@ -0,0 +1,125 @@ +# enforce a maximum file length (max-lines) + +Some people consider large files a code smell. Large files tend to do a lot of things and can make it hard following what's going. While there is not an objective maximum number of lines considered acceptable in a file, most people would agree it should not be in the thousands. Recommendations usually range from 100 to 500 lines. + +## Rule Details + +This rule enforces a maximum number of lines per file, in order to aid in maintainability and reduce complexity. + + +## Options + +This rule has a number or object option: + +* `"max"` (default `300`) enforces a maximum number of lines in a file + +* `"skipBlankLines": true` ignore lines made up purely of whitespace. + +* `"skipComment": true` ignore lines containing just comments + +### code + +Examples of **incorrect** code for this rule with a max value of `2`: + +```js +/*eslint max-lines: ["error", 2]*/ +var a, + b, + c; +``` + +```js +/*eslint max-lines: ["error", 2]*/ + +var a, + b,c; +``` + +```js +/*eslint max-lines: ["error", 2]*/ +// a comment +var a, + b,c; +``` + +Examples of **correct** code for this rule with a max value of `2`: + +```js +/*eslint max-lines: ["error", 2]*/ +var a, + b, c; +``` + +```js +/*eslint max-lines: ["error", 2]*/ + +var a, b, c; +``` + +```js +/*eslint max-lines: ["error", 2]*/ +// a comment +var a, b, c; +``` + +### skipBlankLines + +Examples of **incorrect** code for this rule with the `{ "skipBlankLines": true }` option: + +```js +/*eslint max-lines: ["error", 2, {"skipBlankLines": true}]*/ + +var a, + b, + c; +``` + +Examples of **correct** code for this rule with the `{ "skipBlankLines": true }` option: + +```js +/*eslint max-lines: ["error", 2, {"skipBlankLines": true}]*/ + +var a, + b, c; +``` + +### skipComments + +Examples of **incorrect** code for this rule with the `{ "skipComments": true }` option: + +```js +/*eslint max-lines: ["error", 2, {"skipComments": true}]*/ +// a comment +var a, + b, + c; +``` + +Examples of **correct** code for this rule with the `{ "skipComments": true }` option: + +```js +/*eslint max-lines: ["error", 2, {"skipComments": true}]*/ +// a comment +var a, + b, c; +``` + +## When Not To Use It + +You can turn this rule off if you are not concerned with the number of lines in your files. + +## Further reading + +* [Software Module size and file size](http://www.mind2b.com/component/content/article/24-software-module-size-and-file-size) + +## Related Rules + +* [complexity](complexity.md) +* [max-depth](max-depth.md) +* [max-nested-callbacks](max-nested-callbacks.md) +* [max-params](max-params.md) +* [max-statements](max-statements.md) + +## Compatibility + +* **JSCS**: [maximumNumberOfLines](http://jscs.info/rule/maximumNumberOfLines) diff --git a/lib/rules/max-lines.js b/lib/rules/max-lines.js new file mode 100644 index 00000000000..751310e81da --- /dev/null +++ b/lib/rules/max-lines.js @@ -0,0 +1,148 @@ +/** + * @fileoverview enforce a maximum file length + * @author Alberto Rodríguez + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var lodash = require("lodash"); +var astUtils = require("../ast-utils"); + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +module.exports = { + meta: { + docs: { + description: "enforce a maximum number of lines per file", + category: "Stylistic Issues", + recommended: false + }, + + schema: [ + { + oneOf: [ + { + type: "integer", + minimum: 0 + }, + { + type: "object", + properties: { + max: { + type: "integer", + minimum: 0 + }, + skipComments: { + type: "boolean" + }, + skipBlankLines: { + type: "boolean" + } + }, + additionalProperties: false + } + ] + } + ] + }, + + create: function(context) { + var option = context.options[0], + max = 300; + + if (typeof option === "object" && option.hasOwnProperty("max") && typeof option.max === "number") { + max = option.max; + } + + if (typeof option === "number") { + max = option; + } + + var skipComments = option && option.skipComments; + var skipBlankLines = option && option.skipBlankLines; + + var sourceCode = context.getSourceCode(); + + /** + * Returns whether or not a token is a comment node type + * @param {Token} token The token to check + * @returns {boolean} True if the token is a comment node + */ + function isCommentNodeType(token) { + return token && (token.type === "Block" || token.type === "Line"); + } + + /** + * Returns the line numbers of a comment that don't have any code on the same line + * @param {Node} comment The comment node to check + * @returns {int[]} The line numbers + */ + function getLinesWithoutCode(comment) { + var start = comment.loc.start.line; + var end = comment.loc.end.line; + + var token; + + token = comment; + do { + token = sourceCode.getTokenOrCommentBefore(token); + } while (isCommentNodeType(token)); + + if (token && astUtils.isTokenOnSameLine(token, comment)) { + start += 1; + } + + token = comment; + do { + token = sourceCode.getTokenOrCommentAfter(token); + } while (isCommentNodeType(token)); + + if (token && astUtils.isTokenOnSameLine(comment, token)) { + end -= 1; + } + + if (start <= end) { + return lodash.range(start, end + 1); + } + return []; + } + + return { + "Program:exit": function() { + var lines = sourceCode.lines.map(function(text, i) { + return { lineNumber: i + 1, text: text }; + }); + + if (skipBlankLines) { + lines = lines.filter(function(l) { + return l.text.trim() !== ""; + }); + } + + if (skipComments) { + var comments = sourceCode.getAllComments(); + + var commentLines = lodash.flatten(comments.map(function(comment) { + return getLinesWithoutCode(comment); + })); + + lines = lines.filter(function(l) { + return !lodash.includes(commentLines, l.lineNumber); + }); + } + + if (lines.length > max) { + context.report({ + loc: { line: 1, column: 0 }, + message: "File must be at most " + max + " lines long" + }); + } + } + }; + } +}; diff --git a/tests/lib/rules/max-lines.js b/tests/lib/rules/max-lines.js new file mode 100644 index 00000000000..42da05f2269 --- /dev/null +++ b/tests/lib/rules/max-lines.js @@ -0,0 +1,176 @@ +/** + * @fileoverview enforce a maximum file length + * @author Alberto Rodríguez + */ +"use strict"; + +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var rule = require("../../../lib/rules/max-lines"), + + RuleTester = require("../../../lib/testers/rule-tester"); + + +//------------------------------------------------------------------------------ +// Tests +//------------------------------------------------------------------------------ + +var ruleTester = new RuleTester(); + +/** + * Returns the error message with the specified max number of lines + * @param {number} lines Maximum number of lines + * @returns {string} error message + */ +function errorMessage(lines) { + return "File must be at most " + lines + " lines long"; +} + +ruleTester.run("max-lines", rule, { + valid: [ + {code: "var x;" }, + {code: "var xy;\nvar xy;" }, + {code: "var xy;\nvar xy;", options: [2] }, + {code: "var xy;\nvar xy;", options: [{max: 2}] }, + { + code: [ + "//a single line comment", + "var xy;", + "var xy;", + " /* a multiline", + " really really", + " long comment*/ " + ].join("\n"), + options: [{max: 2, skipComments: true} ] + }, + { + code: [ + "var x; /* inline comment", + " spanning multiple lines */ var z;" + ].join("\n"), + options: [{max: 2, skipComments: true} ] + }, + { + code: [ + "var x; /* inline comment", + " spanning multiple lines */", + "var z;" + ].join("\n"), + options: [{max: 2, skipComments: true} ] + }, + { + code: [ + "var x;", + "", + "\t", + "\t ", + "var y;" + ].join("\n"), + options: [{max: 2, skipBlankLines: true} ] + }, + { + code: [ + "//a single line comment", + "var xy;", + " ", + "var xy;", + " ", + " /* a multiline", + " really really", + " long comment*/" + ].join("\n"), + options: [{max: 2, skipComments: true, skipBlankLines: true} ] + } + ], + invalid: [ + { + code: "var xyz;\nvar xyz;\nvar xyz;", + options: [2], + errors: [{message: errorMessage(2)}] + }, + { + code: "/* a multiline comment\n that goes to many lines*/\nvar xy;\nvar xy;", + options: [2], + errors: [{message: errorMessage(2)}] + }, + { + code: "//a single line comment\nvar xy;\nvar xy;", + options: [2], + errors: [{message: errorMessage(2)}] + }, + { + code: [ + "var x;", + "", + "", + "", + "var y;" + ].join("\n"), + options: [{max: 2} ], + errors: [{message: errorMessage(2)}] + }, + { + code: [ + "//a single line comment", + "var xy;", + " ", + "var xy;", + " ", + " /* a multiline", + " really really", + " long comment*/" + ].join("\n"), + options: [{max: 2, skipComments: true } ], + errors: [{message: errorMessage(2)}] + }, + { + code: [ + "var x; // inline comment", + "var y;", + "var z;" + ].join("\n"), + options: [{max: 2, skipComments: true} ], + errors: [{message: errorMessage(2)}] + }, + { + code: [ + "var x; /* inline comment", + " spanning multiple lines */", + "var y;", + "var z;" + ].join("\n"), + options: [{max: 2, skipComments: true} ], + errors: [{message: errorMessage(2)}] + }, + { + code: [ + "//a single line comment", + "var xy;", + " ", + "var xy;", + " ", + " /* a multiline", + " really really", + " long comment*/" + ].join("\n"), + options: [{max: 2, skipBlankLines: true } ], + errors: [{message: errorMessage(2)}] + }, + { + code: [ + "//a single line comment", + "var xy;", + " ", + "var xy;", + " ", + " /* a multiline", + " really really", + " long comment*/" + ].join("\n"), + options: [{max: 2, skipComments: true } ], + errors: [{message: errorMessage(2)}] + } + ] +});