Skip to content

Commit

Permalink
Update: add comment to message in no-warning-comments (fixes #12327) (#…
Browse files Browse the repository at this point in the history 
…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
  • Loading branch information
@anikethsaha
anikethsaha committed Aug 29, 2020
1 parent e60ec07 commit ed64767
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 41 deletions.
47 changes: 40 additions & 7 deletions lib/rules/no-warning-comments.js
View file
Expand Up @@ -8,6 +8,8 @@
const { escapeRegExp } = require("lodash");
const astUtils = require("./utils/ast-utils");

const CHAR_LIMIT = 40;

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -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"],
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 ? "..." : ""
}`
}
});
});
Expand All @@ -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);
}
};
}
Expand Down
122 changes: 88 additions & 34 deletions tests/lib/rules/no-warning-comments.js
View file
Expand Up @@ -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: "..."
}
}
]
}
]
Expand Down

0 comments on commit ed64767

Please sign in to comment.