From c6dd11fc598d007f60ec23437efabb4f17bed767 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Sat, 14 Dec 2019 00:20:19 +0900 Subject: [PATCH 1/6] Update: fix counting jsx comment len in max-len (fixes #12213) --- lib/rules/max-len.js | 22 +++++++ tests/lib/rules/max-len.js | 122 +++++++++++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+) diff --git a/lib/rules/max-len.js b/lib/rules/max-len.js index b29099ed4c4..23f572860c7 100644 --- a/lib/rules/max-len.js +++ b/lib/rules/max-len.js @@ -183,6 +183,23 @@ module.exports = { (end.line > lineNumber || (end.line === lineNumber && end.column === line.length)); } + /** + * Check if a comment is in the single line JSXEmptyExpression. + * @param {ASTNode} comment The comment to check. + * @returns {boolean} True if the comment is in the single line JSXEmptyExpression. + */ + function isCommentInSingleLineJSXEmptyExpression(comment) { + const node = sourceCode.getNodeByRangeIndex(comment.range[0]); + + if (!node || !node.parent || node.type !== "JSXEmptyExpression" || node.parent.type !== "JSXExpressionContainer") { + return false; + } + + const parent = node.parent; + + return parent.loc.start.line === parent.loc.end.line; + } + /** * Gets the line after the comment and any remaining trailing whitespace is * stripped. @@ -305,6 +322,11 @@ module.exports = { // and step back by one comment = comments[--commentsIndex]; + // check jsx comment + if (isCommentInSingleLineJSXEmptyExpression(comment)) { + comment = sourceCode.getNodeByRangeIndex(comment.range[0]).parent; + } + if (isFullLineComment(line, lineNumber, comment)) { lineIsComment = true; textToMeasure = line; diff --git a/tests/lib/rules/max-len.js b/tests/lib/rules/max-len.js index 99822ccaef0..026a9f8cf0e 100644 --- a/tests/lib/rules/max-len.js +++ b/tests/lib/rules/max-len.js @@ -195,6 +195,45 @@ ruleTester.run("max-len", rule, { { code: "\tfoo", options: [4, { tabWidth: 0 }] + }, + + // https://github.com/eslint/eslint/issues/12213 + { + code: "var jsx = (<>\n" + + " { /* this line has 38 characters */}\n" + + ")", + options: [15, { comments: 38 }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + "\t\t{ /* this line has 40 characters */}\n" + + ")", + options: [15, 4, { comments: 44 }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " <> text { /* this line has 49 characters */}\n" + + ")", + options: [13, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line has 44 characters */}\n" + + ")", + options: [44, { comments: 37 }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line has 44 characters */}\n" + + ")", + options: [37, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } } ], @@ -650,6 +689,89 @@ ruleTester.run("max-len", rule, { column: 1 } ] + }, + + // https://github.com/eslint/eslint/issues/12213 + { + code: "var jsx = (<>\n" + + " { /* this line has 38 characters */}\n" + + ")", + options: [15, { comments: 37 }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "maxComment", + data: { lineLength: 38, maxCommentLength: 37 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + "\t\t{ /* this line has 40 characters */}\n" + + ")", + options: [15, 4, { comments: 40 }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "maxComment", + data: { lineLength: 44, maxCommentLength: 40 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + "{ 38/* this line has 38 characters */}\n" + + ")", + options: [15, { comments: 37 }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 38, maxLength: 15 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " <> 50 { 50/* this line has 50 characters */}\n" + + ")", + options: [49, { comments: 100 }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 50, maxLength: 49 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 44 characters */}\n" + + " <> {/* this line has 44 characters */}\n" + + ")", + options: [37, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 44, maxLength: 37 }, + type: "Program", + line: 2, + column: 1 + } + ] } ] }); From f6fe2445100c5caab12e923f6fb8c8222122df65 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Tue, 24 Dec 2019 18:22:11 +0900 Subject: [PATCH 2/6] Add test cases --- tests/lib/rules/max-len.js | 95 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 95 insertions(+) diff --git a/tests/lib/rules/max-len.js b/tests/lib/rules/max-len.js index 026a9f8cf0e..c3eb2a761d8 100644 --- a/tests/lib/rules/max-len.js +++ b/tests/lib/rules/max-len.js @@ -234,6 +234,36 @@ ruleTester.run("max-len", rule, { ")", options: [37, { ignoreTrailingComments: true }], parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = \n" + + " attr = {a && b/* this line has 57 characters */}\n" + + ";", + options: [57], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = \n" + + " attr = {/* this line has 57 characters */a && b}\n" + + ";", + options: [57], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = \n" + + " attr = \n" + + " {/* this line has 45 characters */}\n" + + ";", + options: [20, { comments: 45 }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = \n" + + " attr = \n" + + " {/* this line has 45 characters */}\n" + + ";", + options: [20, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } } ], @@ -772,6 +802,71 @@ ruleTester.run("max-len", rule, { column: 1 } ] + }, + { + code: "var jsx = \n" + + " attr = {a && b/* this line has 57 characters */}\n" + + ";", + options: [56], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 57, maxLength: 56 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = \n" + + " attr = {/* this line has 57 characters */a && b}\n" + + ";", + options: [56], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 57, maxLength: 56 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = \n" + + " attr = {a & b/* this line has 56 characters */}\n" + + ";", + options: [55, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 56, maxLength: 55 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = \n" + + " attr = \n" + + " {/* this line has 45 characters */}\n" + + ";", + options: [80, { comments: 44 }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "maxComment", + data: { lineLength: 45, maxCommentLength: 44 }, + type: "Program", + line: 3, + column: 1 + } + ] } ] }); From f486da59faf379398fbd2cbab4e3ab956cf45148 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Mon, 30 Dec 2019 22:31:45 +0900 Subject: [PATCH 3/6] map comments with desired nodes --- lib/rules/max-len.js | 37 ++++++++---- tests/lib/rules/max-len.js | 115 +++++++++++++++++++++++++++++-------- 2 files changed, 115 insertions(+), 37 deletions(-) diff --git a/lib/rules/max-len.js b/lib/rules/max-len.js index 23f572860c7..9d204f6ef0d 100644 --- a/lib/rules/max-len.js +++ b/lib/rules/max-len.js @@ -184,13 +184,11 @@ module.exports = { } /** - * Check if a comment is in the single line JSXEmptyExpression. - * @param {ASTNode} comment The comment to check. - * @returns {boolean} True if the comment is in the single line JSXEmptyExpression. + * Check if a node is a JSXEmptyExpression contained in a single line JSXExpressionContainer. + * @param {ASTNode} node A node to check. + * @returns {boolean} True if the node is a JSXEmptyExpression contained in a single line JSXExpressionContainer. */ - function isCommentInSingleLineJSXEmptyExpression(comment) { - const node = sourceCode.getNodeByRangeIndex(comment.range[0]); - + function isJSXEmptyExpressionInSingleLineContainer(node) { if (!node || !node.parent || node.type !== "JSXEmptyExpression" || node.parent.type !== "JSXExpressionContainer") { return false; } @@ -269,6 +267,26 @@ module.exports = { return acc; } + /** + * Returns an array of all comments in the source code. + * If the element in the array is a JSXEmptyExpression contained with a single line JSXExpressionContainer, + * the element is changed with JSXExpressionContainer node. + * @returns {ASTNode[]} An array of comment nodes + */ + function getAllComments() { + return sourceCode + .getAllComments() + .map(comment => { + const containingNode = sourceCode.getNodeByRangeIndex(comment.range[0]); + + if (isJSXEmptyExpressionInSingleLineContainer(containingNode)) { + return containingNode.parent; + } + + return comment; + }); + } + /** * Check the program for max length * @param {ASTNode} node Node to examine @@ -281,7 +299,7 @@ module.exports = { const lines = sourceCode.lines, // list of comments to ignore - comments = ignoreComments || maxCommentLength || ignoreTrailingComments ? sourceCode.getAllComments() : []; + comments = ignoreComments || maxCommentLength || ignoreTrailingComments ? getAllComments() : []; // we iterate over comments in parallel with the lines let commentsIndex = 0; @@ -322,11 +340,6 @@ module.exports = { // and step back by one comment = comments[--commentsIndex]; - // check jsx comment - if (isCommentInSingleLineJSXEmptyExpression(comment)) { - comment = sourceCode.getNodeByRangeIndex(comment.range[0]).parent; - } - if (isFullLineComment(line, lineNumber, comment)) { lineIsComment = true; textToMeasure = line; diff --git a/tests/lib/rules/max-len.js b/tests/lib/rules/max-len.js index c3eb2a761d8..b335a7742ec 100644 --- a/tests/lib/rules/max-len.js +++ b/tests/lib/rules/max-len.js @@ -236,33 +236,48 @@ ruleTester.run("max-len", rule, { parserOptions: { ecmaFeatures: { jsx: true } } }, { - code: "var jsx = \n" + + code: "var jsx = ;", + ">;", options: [57], parserOptions: { ecmaFeatures: { jsx: true } } }, { - code: "var jsx = \n" + + code: "var jsx = ;", + ">;", options: [57], parserOptions: { ecmaFeatures: { jsx: true } } }, { - code: "var jsx = \n" + + code: "var jsx = ;", - options: [20, { comments: 45 }], + " {a & b/* this line has 50 characters */}\n" + + ">;", + options: [50], parserOptions: { ecmaFeatures: { jsx: true } } }, { - code: "var jsx = \n" + - " attr = \n" + - " {/* this line has 45 characters */}\n" + - ";", - options: [20, { ignoreComments: true }], + code: "var jsx = (<>\n" + + " <> {/* this line with two separate comments */} {/* have 80 characters */}\n" + + ")", + options: [80], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have 80 characters */}\n" + + ")", + options: [37, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have 80 characters */}\n" + + ")", + options: [37, { ignoreComments: true }], parserOptions: { ecmaFeatures: { jsx: true } } } ], @@ -804,9 +819,9 @@ ruleTester.run("max-len", rule, { ] }, { - code: "var jsx = \n" + + code: "var jsx = ;", + ">;", options: [56], parserOptions: { ecmaFeatures: { jsx: true } }, errors: [ @@ -820,9 +835,9 @@ ruleTester.run("max-len", rule, { ] }, { - code: "var jsx = \n" + + code: "var jsx = ;", + ">;", options: [56], parserOptions: { ecmaFeatures: { jsx: true } }, errors: [ @@ -836,9 +851,9 @@ ruleTester.run("max-len", rule, { ] }, { - code: "var jsx = \n" + + code: "var jsx = ;", + ">;", options: [55, { ignoreTrailingComments: true }], parserOptions: { ecmaFeatures: { jsx: true } }, errors: [ @@ -852,16 +867,66 @@ ruleTester.run("max-len", rule, { ] }, { - code: "var jsx = \n" + + code: "var jsx = ;", - options: [80, { comments: 44 }], + " {a & b /* this line has 51 characters */}\n" + + ">;", + options: [30, { comments: 44 }], parserOptions: { ecmaFeatures: { jsx: true } }, errors: [ { - messageId: "maxComment", - data: { lineLength: 45, maxCommentLength: 44 }, + messageId: "max", + data: { lineLength: 51, maxLength: 30 }, + type: "Program", + line: 3, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have 80 characters */}\n" + + ")", + options: [79], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 80, maxLength: 79 }, + type: "Program", + line: 3, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " <> {/* this line with two separate comments */} {/* have 87 characters */} <> \n" + + ")", + options: [85, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 87, maxLength: 85 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have 87 characters */} <> \n" + + ")", + options: [37, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 87, maxLength: 37 }, type: "Program", line: 3, column: 1 From ffcf5d2bbe6580587ac559226adc9fa09643630a Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 1 Jan 2020 13:46:10 +0900 Subject: [PATCH 4/6] push a unique node only --- lib/rules/max-len.js | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/lib/rules/max-len.js b/lib/rules/max-len.js index 9d204f6ef0d..e06b75805ce 100644 --- a/lib/rules/max-len.js +++ b/lib/rules/max-len.js @@ -274,17 +274,24 @@ module.exports = { * @returns {ASTNode[]} An array of comment nodes */ function getAllComments() { - return sourceCode - .getAllComments() - .map(comment => { - const containingNode = sourceCode.getNodeByRangeIndex(comment.range[0]); + const comments = []; + + sourceCode.getAllComments() + .forEach(commentNode => { + const containingNode = sourceCode.getNodeByRangeIndex(commentNode.range[0]); if (isJSXEmptyExpressionInSingleLineContainer(containingNode)) { - return containingNode.parent; - } - return comment; + // push a unique node only + if (comments[comments.length - 1] !== commentNode) { + comments.push(containingNode.parent); + } + } else { + comments.push(commentNode); + } }); + + return comments; } /** From 7505f50e5f87aa508047a54048be022e0f7ca660 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Wed, 1 Jan 2020 14:39:57 +0900 Subject: [PATCH 5/6] fix checking node, add more test cases --- lib/rules/max-len.js | 2 +- tests/lib/rules/max-len.js | 135 ++++++++++++++++++++++++++++++++++++- 2 files changed, 135 insertions(+), 2 deletions(-) diff --git a/lib/rules/max-len.js b/lib/rules/max-len.js index e06b75805ce..82229c3be48 100644 --- a/lib/rules/max-len.js +++ b/lib/rules/max-len.js @@ -283,7 +283,7 @@ module.exports = { if (isJSXEmptyExpressionInSingleLineContainer(containingNode)) { // push a unique node only - if (comments[comments.length - 1] !== commentNode) { + if (comments[comments.length - 1] !== containingNode.parent) { comments.push(containingNode.parent); } } else { diff --git a/tests/lib/rules/max-len.js b/tests/lib/rules/max-len.js index b335a7742ec..ad6421e9f09 100644 --- a/tests/lib/rules/max-len.js +++ b/tests/lib/rules/max-len.js @@ -279,6 +279,56 @@ ruleTester.run("max-len", rule, { ")", options: [37, { ignoreComments: true }], parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have > 80 characters */ /* another comment in same braces */}\n" + + ")", + options: [37, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have > 80 characters */ /* another comment in same braces */}\n" + + ")", + options: [37, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/*\n" + + " this line has 34 characters\n" + + " */}\n" + + ")", + options: [33, { comments: 34 }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {/*\n" + + " this line has 34 characters\n" + + " */}\n" + + ")", + options: [33, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {a & b /* this line has 34 characters\n" + + " */}\n" + + ")", + options: [33, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } + }, + { + code: "var jsx = (<>\n" + + " {a & b /* this line has 34 characters\n" + + " */}\n" + + ")", + options: [33, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } } } ], @@ -773,7 +823,7 @@ ruleTester.run("max-len", rule, { code: "var jsx = (<>\n" + "{ 38/* this line has 38 characters */}\n" + ")", - options: [15, { comments: 37 }], + options: [15, { comments: 38 }], parserOptions: { ecmaFeatures: { jsx: true } }, errors: [ { @@ -785,6 +835,38 @@ ruleTester.run("max-len", rule, { } ] }, + { + code: "var jsx = (<>\n" + + "{ 38/* this line has 38 characters */}\n" + + ")", + options: [37, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 38, maxLength: 37 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + "{ 38/* this line has 38 characters */}\n" + + ")", + options: [37, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 38, maxLength: 37 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, { code: "var jsx = (<>\n" + " <> 50 { 50/* this line has 50 characters */}\n" + @@ -932,6 +1014,57 @@ ruleTester.run("max-len", rule, { column: 1 } ] + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this line with two separate comments */} {/* have > 80 characters */ /* another comment in same braces */}\n" + + ")", + options: [37], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 119, maxLength: 37 }, + type: "Program", + line: 3, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this is not treated as a comment */ a & b} {/* trailing */ /* comments */}\n" + + ")", + options: [37, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 55, maxLength: 37 }, + type: "Program", + line: 3, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + " {/* this line has 37 characters */}\n" + + " <> {/* this is not treated as a comment */ a & b} {/* trailing */ /* comments */}\n" + + ")", + options: [37, { ignoreComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 55, maxLength: 37 }, + type: "Program", + line: 3, + column: 1 + } + ] } ] }); From ca6e4423d1e7b407137f19399dbda8d6ad3f8859 Mon Sep 17 00:00:00 2001 From: yeonjuan Date: Thu, 2 Jan 2020 11:54:32 +0900 Subject: [PATCH 6/6] add more test cases --- tests/lib/rules/max-len.js | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/lib/rules/max-len.js b/tests/lib/rules/max-len.js index ad6421e9f09..f6296b636b9 100644 --- a/tests/lib/rules/max-len.js +++ b/tests/lib/rules/max-len.js @@ -1065,6 +1065,40 @@ ruleTester.run("max-len", rule, { column: 1 } ] + }, + { + code: "var jsx = (<>\n" + + "12345678901234{/*\n" + + "*/}\n" + + ")", + options: [14, { ignoreTrailingComments: true }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 15, maxLength: 14 }, + type: "Program", + line: 2, + column: 1 + } + ] + }, + { + code: "var jsx = (<>\n" + + "{/*\n" + + "this line has 31 characters */}\n" + + ")", + options: [30, { comments: 100 }], + parserOptions: { ecmaFeatures: { jsx: true } }, + errors: [ + { + messageId: "max", + data: { lineLength: 31, maxLength: 30 }, + type: "Program", + line: 3, + column: 1 + } + ] } ] });