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

fix: no-warning-comments rule escapes special RegEx characters in terms #16090

Merged
merged 7 commits into from Jul 9, 2022
Merged

fix: no-warning-comments rule escapes special RegEx characters in terms #16090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b3de940
fix: no-warning-comments rule escapes special RegEx characters in terms
lachlanhunt Jul 3, 2022
b5e779a
Fix unit test and explanatory comment
lachlanhunt Jul 5, 2022
2a2569a
Fix comment about $ matching
lachlanhunt Jul 8, 2022
e66b7aa
Undo accidental formatting.
lachlanhunt Jul 8, 2022
7b2f71e
Only include \W when the term starts or ends with a non-word character
lachlanhunt Jul 9, 2022
8d05998
Performance improvements
lachlanhunt Jul 9, 2022
6315060
Simplify regexes
lachlanhunt Jul 9, 2022
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
66 changes: 26 additions & 40 deletions lib/rules/no-warning-comments.js
View file
Open in desktop
Expand Up @@ -64,59 +64,45 @@ module.exports = {
*/
function convertToRegExp(term) {
const escaped = escapeRegExp(term);
const wordBoundary = "\\b";
const eitherOrWordBoundary = `|${wordBoundary}`;
let prefix;

/*
* If the term ends in a word character (a-z0-9_), ensure a word
* boundary at the end, so that substrings do not get falsely
* matched. eg "todo" in a string such as "mastodon".
* If the term ends in a non-word character, then \b won't match on
* the boundary to the next non-word character, which would likely
* be a space. For example `/\bFIX!\b/.test('FIX! blah') === false`.
* In these cases, use no bounding match. Same applies for the
* prefix, handled below.
* When matching at the start, ignore leading whitespace, and
* there's no need to worry about word boundaries.
*
* These expressions for the prefix and suffix are designed as follows:
* ^ handles any terms at the beginning of a comment.
* e.g. terms ["TODO"] matches `//TODO something`
* $ handles any terms at the end of a comment
* e.g. terms ["TODO"] matches `// something TODO`
* \s* handles optional leading spaces (for "start" location only)
* e.g. terms ["TODO"] matches `// TODO something`
* \b handles terms preceded/followed by word boundary
* e.g. terms: ["!FIX", "FIX!"] matches `// FIX!something` or `// something!FIX`
* terms: ["FIX"] matches `// FIX!` or `// !FIX`, but not `// fixed or affix`
*/
const suffix = /\w$/u.test(term) ? "\\b" : "";
const wordBoundary = "\\b";

if (location === "start") {
let prefix = "";

/*
* When matching at the start, ignore leading whitespace, and
* there's no need to worry about word boundaries.
*/
if (location === "start") {
prefix = "^\\s*";
} else if (/^\w/u.test(term)) {
prefix = wordBoundary;
} else {
prefix = "";
}

if (location === "start") {

/*
* For location "start" the regex should be
* ^\s*TERM\b. This checks the word boundary
* at the beginning of the comment.
*/
return new RegExp(prefix + escaped + suffix, "iu");
}
const suffix = /\w$/u.test(term) ? wordBoundary : "";
const flags = "iu"; // Case-insensitive with Unicode case folding.

/*
* For location "anywhere" the regex should be
* \bTERM\b|\bTERM\b, this checks the entire comment
* for the term.
* For location "start", the typical regex is:
* /^\s*ESCAPED_TERM\b/iu.
*
* For location "anywhere" the typical regex is
* /\bESCAPED_TERM\b/iu
*
* If it starts or ends with non-word character, the prefix and suffix empty, respectively.
*/
return new RegExp(
prefix +
escaped +
suffix +
eitherOrWordBoundary +
term +
wordBoundary,
"iu"
);
return new RegExp(`${prefix}${escaped}${suffix}`, flags);
}

const warningRegExps = warningTerms.map(convertToRegExp);
Expand Down
134 changes: 132 additions & 2 deletions tests/lib/rules/no-warning-comments.js
View file
Open in desktop
Expand Up @@ -34,10 +34,10 @@ ruleTester.run("no-warning-comments", rule, {
"/* any block comment with TODO, FIXME or XXX */",
"/* any block comment with (TODO, FIXME's or XXX!) */",
{ code: "// comments containing terms as substrings like TodoMVC", options: [{ terms: ["todo"], location: "anywhere" }] },
{ code: "// special regex characters don't cause problems", options: [{ terms: ["[aeiou]"], location: "anywhere" }] },
{ code: "// special regex characters don't cause a problem", options: [{ terms: ["[aeiou]"], location: "anywhere" }] },
"/*eslint no-warning-comments: [2, { \"terms\": [\"todo\", \"fixme\", \"any other term\"], \"location\": \"anywhere\" }]*/\n\nvar x = 10;\n",
{ code: "/*eslint no-warning-comments: [2, { \"terms\": [\"todo\", \"fixme\", \"any other term\"], \"location\": \"anywhere\" }]*/\n\nvar x = 10;\n", options: [{ location: "anywhere" }] },
{ code: "foo", options: [{ terms: ["foo-bar"] }] }
{ code: "// foo", options: [{ terms: ["foo-bar"] }] }
],
invalid: [
{
Expand Down Expand Up @@ -257,6 +257,136 @@ ruleTester.run("no-warning-comments", rule, {
}
}
]
},
{
code: "// Comment ending with term followed by punctuation TODO!",
options: [{ terms: ["todo"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo",
comment: "Comment ending with term followed by..."
}
}
]
},
{
code: "// Comment ending with term including punctuation TODO!",
options: [{ terms: ["todo!"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo!",
comment: "Comment ending with term including..."
}
}
]
},
{
code: "// Comment ending with term including punctuation followed by more TODO!!!",
options: [{ terms: ["todo!"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo!",
comment: "Comment ending with term including..."
}
}
]
},
{
code: "// !TODO comment starting with term preceded by punctuation",
options: [{ terms: ["todo"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo",
comment: "!TODO comment starting with term..."
}
}
]
},
{
code: "// !TODO comment starting with term including punctuation",
options: [{ terms: ["!todo"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "!todo",
comment: "!TODO comment starting with term..."
}
}
]
},
{
code: "// !!!TODO comment starting with term including punctuation preceded by more",
options: [{ terms: ["!todo"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "!todo",
comment: "!!!TODO comment starting with term..."
}
}
]
},
{
code: "// FIX!term ending with punctuation followed word character",
options: [{ terms: ["FIX!"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "FIX!",
comment: "FIX!term ending with punctuation..."
}
}
]
},
{
code: "// Term starting with punctuation preceded word character!FIX",
options: [{ terms: ["!FIX"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "!FIX",
comment: "Term starting with punctuation preceded..."
}
}
]
},
{
code: "//!XXX comment starting with no spaces (anywhere)",
options: [{ terms: ["!xxx"], location: "anywhere" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "!xxx",
comment: "!XXX comment starting with no spaces..."
}
}
]
},
{
code: "//!XXX comment starting with no spaces (start)",
options: [{ terms: ["!xxx"], location: "start" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "!xxx",
comment: "!XXX comment starting with no spaces..."
}
}
]
}
]
});