Skip to content

Commit

Permalink
fix: no-warning-comments rule escapes special RegEx characters in ter…
Browse files Browse the repository at this point in the history 
…ms (#16090)

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

* Fix unit test and explanatory comment

* Fix comment about $ matching

* Undo accidental formatting.

* Only include \W when the term starts or ends with a non-word character

* Performance improvements

* Simplify regexes
  • Loading branch information
@lachlanhunt
lachlanhunt committed Jul 9, 2022
1 parent ca83178 commit 30be0ed
Show file tree
Hide file tree
Showing 2 changed files with 158 additions and 42 deletions.
66 changes: 26 additions & 40 deletions lib/rules/no-warning-comments.js
View file
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
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..."
}
}
]
}
]
});

0 comments on commit 30be0ed

Please sign in to comment.