From ed64767859d776145d68145419a61f5379b4dd63 Mon Sep 17 00:00:00 2001 From: Anix Date: Sat, 29 Aug 2020 06:01:26 +0530 Subject: [PATCH] Update: add comment to message in no-warning-comments (fixes #12327) (#13522) * Update: added the comment in no-warning-comments (fixes #12327) * Update: changed display logic * Update: changed the char limit and early break * Chore: refactor the logic * Chore: smallchanges * Chore: refactor small * Chore: added tests and refactor * Chore: correct tests --- lib/rules/no-warning-comments.js | 47 ++++++++-- tests/lib/rules/no-warning-comments.js | 122 ++++++++++++++++++------- 2 files changed, 128 insertions(+), 41 deletions(-) diff --git a/lib/rules/no-warning-comments.js b/lib/rules/no-warning-comments.js index d70bd5dd5c4..0691a31f77e 100644 --- a/lib/rules/no-warning-comments.js +++ b/lib/rules/no-warning-comments.js @@ -8,6 +8,8 @@ const { escapeRegExp } = require("lodash"); const astUtils = require("./utils/ast-utils"); +const CHAR_LIMIT = 40; + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -42,12 +44,11 @@ module.exports = { ], messages: { - unexpectedComment: "Unexpected '{{matchedTerm}}' comment." + unexpectedComment: "Unexpected '{{matchedTerm}}' comment: '{{comment}}'." } }, create(context) { - const sourceCode = context.getSourceCode(), configuration = context.options[0] || {}, warningTerms = configuration.terms || ["todo", "fixme", "xxx"], @@ -107,7 +108,15 @@ module.exports = { * \bTERM\b|\bTERM\b, this checks the entire comment * for the term. */ - return new RegExp(prefix + escaped + suffix + eitherOrWordBoundary + term + wordBoundary, "iu"); + return new RegExp( + prefix + + escaped + + suffix + + eitherOrWordBoundary + + term + + wordBoundary, + "iu" + ); } const warningRegExps = warningTerms.map(convertToRegExp); @@ -135,18 +144,40 @@ module.exports = { * @returns {void} undefined. */ function checkComment(node) { - if (astUtils.isDirectiveComment(node) && selfConfigRegEx.test(node.value)) { + const comment = node.value; + + if ( + astUtils.isDirectiveComment(node) && + selfConfigRegEx.test(comment) + ) { return; } - const matches = commentContainsWarningTerm(node.value); + const matches = commentContainsWarningTerm(comment); matches.forEach(matchedTerm => { + let commentToDisplay = ""; + let truncated = false; + + for (const c of comment.trim().split(/\s+/u)) { + const tmp = commentToDisplay ? `${commentToDisplay} ${c}` : c; + + if (tmp.length <= CHAR_LIMIT) { + commentToDisplay = tmp; + } else { + truncated = true; + break; + } + } + context.report({ node, messageId: "unexpectedComment", data: { - matchedTerm + matchedTerm, + comment: `${commentToDisplay}${ + truncated ? "..." : "" + }` } }); }); @@ -156,7 +187,9 @@ module.exports = { Program() { const comments = sourceCode.getAllComments(); - comments.filter(token => token.type !== "Shebang").forEach(checkComment); + comments + .filter(token => token.type !== "Shebang") + .forEach(checkComment); } }; } diff --git a/tests/lib/rules/no-warning-comments.js b/tests/lib/rules/no-warning-comments.js index 0359b870673..c2689defab1 100644 --- a/tests/lib/rules/no-warning-comments.js +++ b/tests/lib/rules/no-warning-comments.js @@ -43,165 +43,219 @@ ruleTester.run("no-warning-comments", rule, { { code: "// fixme", errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "fixme" } } ] }, { code: "// any fixme", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fixme" } } ] }, { code: "// any fixme", options: [{ terms: ["fixme"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fixme" } } ] }, { code: "// any FIXME", options: [{ terms: ["fixme"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any FIXME" } } ] }, { code: "// any fIxMe", options: [{ terms: ["fixme"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fIxMe" } } ] }, { code: "/* any fixme */", options: [{ terms: ["FIXME"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "FIXME" } } + { messageId: "unexpectedComment", data: { matchedTerm: "FIXME", comment: "any fixme" } } ] }, { code: "/* any FIXME */", options: [{ terms: ["FIXME"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "FIXME" } } + { messageId: "unexpectedComment", data: { matchedTerm: "FIXME", comment: "any FIXME" } } ] }, { code: "/* any fIxMe */", options: [{ terms: ["FIXME"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "FIXME" } } + { messageId: "unexpectedComment", data: { matchedTerm: "FIXME", comment: "any fIxMe" } } ] }, { code: "// any fixme or todo", options: [{ terms: ["fixme", "todo"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fixme or todo" } }, + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "any fixme or todo" } } ] }, { code: "/* any fixme or todo */", options: [{ terms: ["fixme", "todo"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fixme or todo" } }, + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "any fixme or todo" } } ] }, { code: "/* any fixme or todo */", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "any fixme or todo" } }, + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fixme or todo" } } ] }, { code: "/* fixme and todo */", errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "fixme and todo" } } ] }, { code: "/* fixme and todo */", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "fixme and todo" } }, + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "fixme and todo" } } ] }, { code: "/* any fixme */", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any fixme" } } ] }, { code: "/* fixme! */", options: [{ terms: ["fixme"] }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } } + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "fixme!" } } ] }, { code: "// regex [litera|$]", options: [{ terms: ["[litera|$]"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "[litera|$]" } } + { messageId: "unexpectedComment", data: { matchedTerm: "[litera|$]", comment: "regex [litera|$]" } } ] }, { code: "/* eslint one-var: 2 */", options: [{ terms: ["eslint"] }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "eslint" } } + { messageId: "unexpectedComment", data: { matchedTerm: "eslint", comment: "eslint one-var: 2" } } ] }, { code: "/* eslint one-var: 2 */", options: [{ terms: ["one"], location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "one" } } + { messageId: "unexpectedComment", data: { matchedTerm: "one", comment: "eslint one-var: 2" } } ] }, { code: "/* any block comment with TODO, FIXME or XXX */", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "xxx" } } + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "any block comment with TODO, FIXME or..." } }, + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any block comment with TODO, FIXME or..." } }, + { messageId: "unexpectedComment", data: { matchedTerm: "xxx", comment: "any block comment with TODO, FIXME or..." } } ] }, { code: "/* any block comment with (TODO, FIXME's or XXX!) */", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "xxx" } } + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "any block comment with (TODO, FIXME's or..." } }, + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any block comment with (TODO, FIXME's or..." } }, + { messageId: "unexpectedComment", data: { matchedTerm: "xxx", comment: "any block comment with (TODO, FIXME's or..." } } ] }, { code: "/** \n *any block comment \n*with (TODO, FIXME's or XXX!) **/", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "xxx" } } + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "* *any block comment *with (TODO,..." } }, + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "* *any block comment *with (TODO,..." } }, + { messageId: "unexpectedComment", data: { matchedTerm: "xxx", comment: "* *any block comment *with (TODO,..." } } ] }, { code: "// any comment with TODO, FIXME or XXX", options: [{ location: "anywhere" }], errors: [ - { messageId: "unexpectedComment", data: { matchedTerm: "todo" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "fixme" } }, - { messageId: "unexpectedComment", data: { matchedTerm: "xxx" } } + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "any comment with TODO, FIXME or XXX" } }, + { messageId: "unexpectedComment", data: { matchedTerm: "fixme", comment: "any comment with TODO, FIXME or XXX" } }, + { messageId: "unexpectedComment", data: { matchedTerm: "xxx", comment: "any comment with TODO, FIXME or XXX" } } + ] + }, + { + code: "// TODO: something small", + options: [{ location: "anywhere" }], + errors: [ + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "TODO: something small" } } + ] + }, + { + code: "// TODO: something really longer than 40 characters", + options: [{ location: "anywhere" }], + errors: [ + { messageId: "unexpectedComment", data: { matchedTerm: "todo", comment: "TODO: something really longer than 40..." } } + ] + }, + { + code: + "/* TODO: something \n really longer than 40 characters \n and also a new line */", + options: [{ location: "anywhere" }], + errors: [ + { + messageId: "unexpectedComment", + data: { + matchedTerm: "todo", + comment: "TODO: something really longer than 40..." + } + } + ] + }, + { + code: "// TODO: small", + options: [{ location: "anywhere" }], + errors: [ + { + messageId: "unexpectedComment", + data: { + matchedTerm: "todo", + comment: "TODO: small" + } + } + ] + }, + { + code: "// https://github.com/eslint/eslint/pull/13522#discussion_r470293411 TODO", + options: [{ location: "anywhere" }], + errors: [ + { + messageId: "unexpectedComment", + data: { + matchedTerm: "todo", + comment: "..." + } + } ] } ]