Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update: add comment to message in no-warning-comments (fixes #12327) #13522

Merged
merged 8 commits into from Aug 29, 2020
Merged

Update: add comment to message in no-warning-comments (fixes #12327) #13522

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dbaeef1
Update: added the comment in no-warning-comments (fixes #12327)
anikethsaha Jul 22, 2020
52fe254
Update: changed display logic
anikethsaha Jul 28, 2020
264e415
Update: changed the char limit and early break
anikethsaha Jul 31, 2020
c51cd4e
Chore: refactor the logic
anikethsaha Aug 8, 2020
787c1ce
Chore: smallchanges
anikethsaha Aug 11, 2020
1a2cf7e
Chore: refactor small
anikethsaha Aug 13, 2020
c0c867d
Chore: added tests and refactor
anikethsaha Aug 19, 2020
07dd444
Chore: correct tests
anikethsaha Aug 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 40 additions & 7 deletions lib/rules/no-warning-comments.js
View file
Open in desktop
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
Open in desktop
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