Skip to content

Commit

Permalink
Merge pull request #2986 from bgw/max-len-enhancements
Browse files Browse the repository at this point in the history 
New: Add ignorePattern, ignoreComments, and ignoreUrls options to max-len (fixes #2934, fixes #2221, fixes #1661)
  • Loading branch information
@nzakas
nzakas committed Jul 23, 2015
2 parents 8fa8ce9 + a5bef02 commit 95b1907
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 9 deletions.
9 changes: 9 additions & 0 deletions docs/rules/max-len.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,15 @@ For example, to specify a maximum line length of 80 characters with each tab cou
"max-len": [2, 80, 4]
```

There are additional optional arguments to ignore comments, lines with URLs, or lines matching a regular expression.

```
"max-len": [2, 80, 4, {ignoreComments: true, ignoreUrls: true, ignorePattern: "^\\s*var\\s.+=\\s*require\\s*\\("}]
```

The `ignoreComments` option only ignores trailing comments and comments on their own line. For example, `function foo(/*string*/ bar) { /* ... */ }` isn't collapsed.

Be aware that regular expressions can only match a single line and need to be doubly escaped when written in YAML or JSON.

## Related Rules

Expand Down
97 changes: 90 additions & 7 deletions lib/rules/max-len.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,12 @@
//------------------------------------------------------------------------------

module.exports = function(context) {
// takes some ideas from http://tools.ietf.org/html/rfc3986#appendix-B, however:
// - They're matching an entire string that we know is a URI
// - We're matching part of a string where we think there *might* be a URL
// - We're only concerned about URLs, as picking out any URI would cause too many false positives
// - We don't care about matching the entire URL, any small segment is fine
var URL_REGEXP = /[^:/?#]:\/\/[^?#]/;

/**
* Creates a string that is made up of repeating a given string a certain
Expand All @@ -32,23 +38,85 @@ module.exports = function(context) {
return result;
}

var tabWidth = context.options[1] || 4;

var maxLength = context.options[0] || 80,
tabWidth = context.options[1] || 4,
ignoreOptions = context.options[2] || {},
ignorePattern = ignoreOptions.ignorePattern || null,
ignoreComments = ignoreOptions.ignoreComments || false,
ignoreUrls = ignoreOptions.ignoreUrls || false,
tabString = stringRepeat(" ", tabWidth);

if (ignorePattern) {
ignorePattern = new RegExp(ignorePattern);
}

//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------

/**
* Tells if a given comment is trailing: it starts on the current line and
* extends to or past the end of the current line.
* @param {string} line The source line we want to check for a trailing comment on
* @param {number} lineNumber The one-indexed line number for line
* @param {ASTNode} comment The comment to inspect
* @returns {boolean} If the comment is trailing on the given line
*/
function isTrailingComment(line, lineNumber, comment) {
return comment &&
(comment.loc.start.line <= lineNumber && lineNumber <= comment.loc.end.line) &&
(comment.loc.end.line > lineNumber || comment.loc.end.column === line.length);
}

/**
* Gets the line after the comment and any remaining trailing whitespace is
* stripped.
* @param {string} line The source line with a trailing comment
* @param {number} lineNumber The one-indexed line number this is on
* @param {ASTNode} comment The comment to remove
* @returns {string} Line without comment and trailing whitepace
*/
function stripTrailingComment(line, lineNumber, comment) {
if (comment.loc.start.line < lineNumber) {
// this entire line is a comment
return "";
} else {
// loc.column is zero-indexed
return line.slice(0, comment.loc.start.column).replace(/\s+$/, "");
}
}

function checkProgramForMaxLength(node) {
var lines = context.getSourceLines();
// split (honors line-ending)
var lines = context.getSourceLines(),
// list of comments to ignore
comments = ignoreComments ? context.getAllComments() : [],
// we iterate over comments in parallel with the lines
commentsIndex = 0;

// Replace the tabs
// Split (honors line-ending)
// Iterate
lines.forEach(function(line, i) {
// i is zero-indexed, line numbers are one-indexed
var lineNumber = i + 1;
// we can short-circuit the comment checks if we're already out of comments to check
if (commentsIndex < comments.length) {
// iterate over comments until we find one past the current line
do {
var comment = comments[++commentsIndex];
} while (comment && comment.loc.start.line <= lineNumber);
// and step back by one
comment = comments[--commentsIndex];
if (isTrailingComment(line, lineNumber, comment)) {
line = stripTrailingComment(line, lineNumber, comment);
}
}
if (ignorePattern && ignorePattern.test(line) ||
ignoreUrls && URL_REGEXP.test(line)) {
// ignore this line
return;
}
// replace the tabs
if (line.replace(/\t/g, tabString).length > maxLength) {
context.report(node, { line: i + 1, column: 0 }, "Line " + (i + 1) + " exceeds the maximum line length of " + maxLength + ".");
context.report(node, { line: lineNumber, column: 0 }, "Line " + (i + 1) + " exceeds the maximum line length of " + maxLength + ".");
}
});
}
Expand All @@ -72,5 +140,20 @@ module.exports.schema = [
{
"type": "integer",
"minimum": 0
},
{
"type": "object",
"properties": {
"ignorePattern": {
"type": "string"
},
"ignoreComments": {
"type": "boolean"
},
"ignoreUrls": {
"type": "boolean"
}
},
"additionalProperties": false
}
];
105 changes: 103 additions & 2 deletions tests/lib/rules/max-len.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,40 @@ eslintTester.addRuleTest("lib/rules/max-len", {
}, {
code: "var i = 1;\r\nvar i = 1;\n",
options: [10, 4]
},
{
}, {
code: "\n// Blank line on top\nvar foo = module.exports = {};\n",
options: [80, 4]
},
"\n// Blank line on top\nvar foo = module.exports = {};\n",
{
code: "var foo = module.exports = {}; // really long trailing comment",
options: [40, 4, {ignoreComments: true}]
}, {
code: "foo(); \t// strips entire comment *and* trailing whitespace",
options: [6, 4, {ignoreComments: true}]
}, {
code: "// really long comment on its own line sitting here",
options: [40, 4, {ignoreComments: true}]
},
"var /*inline-comment*/ i = 1;",
{
code: "var /*inline-comment*/ i = 1; // with really long trailing comment",
options: [40, 4, {ignoreComments: true}]
}, {
code: "foo('http://example.com/this/is/?a=longish&url=in#here');",
options: [40, 4, {ignoreUrls: true}]
}, {
code: "foo(bar(bazz('this is a long'), 'line of'), 'stuff');",
options: [40, 4, {ignorePattern: "foo.+bazz\\("}]
}, {
code:
"/* hey there! this is a multiline\n" +
" comment with longish lines in various places\n" +
" but\n" +
" with a short line-length */",
options: [10, 4, {ignoreComments: true}]
},
// blank line
""
],

Expand Down Expand Up @@ -90,6 +118,79 @@ eslintTester.addRuleTest("lib/rules/max-len", {
column: 1
}
]
},
{
code: "var /*this is a long non-removed inline comment*/ i = 1;",
options: [20, 4, {ignoreComments: true}],
errors: [
{
message: "Line 1 exceeds the maximum line length of 20.",
type: "Program",
line: 1,
column: 1
}
]
},
{
code:
"var foobar = 'this line isn\\'t matched by the regexp';\n" +
"var fizzbuzz = 'but this one is matched by the regexp';\n",
options: [20, 4, {ignorePattern: "fizzbuzz"}],
errors: [
{
message: "Line 1 exceeds the maximum line length of 20.",
type: "Program",
line: 1,
column: 1
}
]
},
{
code: "var longLine = 'will trigger'; // even with a comment",
options: [10, 4, {ignoreComments: true}],
errors: [
{
message: "Line 1 exceeds the maximum line length of 10.",
type: "Program",
line: 1,
column: 1
}
]
},
{
code: "var foo = module.exports = {}; // really long trailing comment",
options: [40, 4], // ignoreComments is disabled
errors: [
{
message: "Line 1 exceeds the maximum line length of 40.",
type: "Program",
line: 1,
column: 1
}
]
},
{
code: "foo('http://example.com/this/is/?a=longish&url=in#here');",
options: [40, 4], // ignoreUrls is disabled
errors: [
{
message: "Line 1 exceeds the maximum line length of 40.",
type: "Program",
line: 1,
column: 1
}
]
}, {
code: "foo(bar(bazz('this is a long'), 'line of'), 'stuff');",
options: [40, 4], // ignorePattern is disabled
errors: [
{
message: "Line 1 exceeds the maximum line length of 40.",
type: "Program",
line: 1,
column: 1
}
]
}
]
});

0 comments on commit 95b1907

Please sign in to comment.