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.

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

feat: no-warning-comments support comments with decoration #16120

Merged
merged 10 commits into from Aug 24, 2022
58 changes: 55 additions & 3 deletions docs/src/rules/no-warning-comments.md
Expand Up @@ -21,8 +21,9 @@ This rule reports comments that include any of the predefined terms specified in

This rule has an options object literal:

* `"terms"`: optional array of terms to match. Defaults to `["todo", "fixme", "xxx"]`. Terms are matched case-insensitive and as whole words: `fix` would match `FIX` but not `fixing`. Terms can consist of multiple words: `really bad idea`.
* `"location"`: optional string that configures where in your comments to check for matches. Defaults to `"start"`. The other value is match `anywhere` in comments.
* `"terms"`: optional array of terms to match. Defaults to `["todo", "fixme", "xxx"]`. Terms are matched case-insensitively and as whole words: `fix` would match `FIX` but not `fixing`. Terms can consist of multiple words: `really bad idea`.
* `"location"`: optional string that configures where in your comments to check for matches. Defaults to `"start"`. The start is from the first non-decorative character, ignoring whitespace, new lines and characters specified in `decoration`. The other value is match `anywhere` in comments.
* `"decoration"`: optional array of characters that are ignored at the start of a comment, when location is `"start"`. Defaults to `[]`. Any sequence of whitespace or the characters from this property are ignored. This option is ignored when location is `"anywhere"`.

Example of **incorrect** code for the default `{ "terms": ["todo", "fixme", "xxx"], "location": "start" }` options:

Expand All @@ -31,6 +32,9 @@ Example of **incorrect** code for the default `{ "terms": ["todo", "fixme", "xxx
```js
/*eslint no-warning-comments: "error"*/

/*
FIXME
*/
function callback(err, results) {
if (err) {
console.error(err);
Expand Down Expand Up @@ -73,7 +77,7 @@ Examples of **incorrect** code for the `{ "terms": ["todo", "fixme", "any other
// TODO: this
// todo: this too
// Even this: TODO
/* /*
/*
* The same goes for this TODO comment
* Or a fixme
* as well as any other term
Expand Down Expand Up @@ -101,6 +105,54 @@ Examples of **correct** code for the `{ "terms": ["todo", "fixme", "any other te

:::

### Decoration Characters

Examples of **incorrect** code for the `{ "decoration": "*" }` options:
lachlanhunt marked this conversation as resolved.
Show resolved Hide resolved

::: incorrect

```js
/*eslint no-warning-comments: ["error", { "decoration": "*" }]*/
lachlanhunt marked this conversation as resolved.
Show resolved Hide resolved

//***** todo decorative asterisks are ignored *****//
/**
* TODO new lines and asterisks are also ignored in block comments.
*/
```

:::

Examples of **incorrect** code for the `{ "decoration": ["/", "*"] }` options:

::: incorrect

```js
/*eslint no-warning-comments: ["error", { "decoration": ["/", "*"] }]*/

////// TODO decorative slashes and whitespace are ignored //////
//***** todo decorative asterisks are also ignored *****//
/**
* TODO new lines are also ignored in block comments.
*/
```

:::

Examples of **correct** code for the `{ "decoration": ["/", "*"] }` options:

::: correct

```js
/*eslint no-warning-comments: ["error", { "decoration": ["/", "*"] }]*/

//!TODO preceded by non-decoration character
/**
*!TODO preceded by non-decoration character in a block comment
*/
```

:::

## When Not To Use It

* If you have a large code base that was not developed with a policy to not use such warning terms, you might get hundreds of warnings / errors which might be counter-productive if you can't fix all of them (e.g. if you don't get the time to do it) as you might overlook other warnings / errors or get used to many of them and don't pay attention on it anymore.
Expand Down
29 changes: 24 additions & 5 deletions lib/rules/no-warning-comments.js
Expand Up @@ -37,6 +37,15 @@ module.exports = {
},
location: {
enum: ["start", "anywhere"]
},
decoration: {
type: "array",
items: {
type: "string",
pattern: "^\\S$"
},
minItems: 1,
uniqueItems: true
}
},
additionalProperties: false
Expand All @@ -53,6 +62,7 @@ module.exports = {
configuration = context.options[0] || {},
warningTerms = configuration.terms || ["todo", "fixme", "xxx"],
location = configuration.location || "start",
decoration = [...configuration.decoration || []].join(""),
selfConfigRegEx = /\bno-warning-comments\b/u;

/**
Expand All @@ -64,6 +74,7 @@ module.exports = {
*/
function convertToRegExp(term) {
const escaped = escapeRegExp(term);
const escapedDecoration = escapeRegExp(decoration);

/*
* When matching at the start, ignore leading whitespace, and
Expand All @@ -74,18 +85,23 @@ module.exports = {
* 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`
*
* For location start:
* [\s]* handles optional leading spaces
* e.g. terms ["TODO"] matches `// TODO something`
* [\s\*] (where "\*" is the escaped string of decoration) handles
* handles optional leading spaces or decoration characters (for "start" location only)
lachlanhunt marked this conversation as resolved.
Show resolved Hide resolved
* e.g. terms ["TODO"] matches `/**** TODO something ... `
*/
const wordBoundary = "\\b";

let prefix = "";

if (location === "start") {
prefix = "^\\s*";
prefix = `^[\\s${escapedDecoration}]*`;
} else if (/^\w/u.test(term)) {
prefix = wordBoundary;
}
Expand All @@ -95,12 +111,15 @@ module.exports = {

/*
* For location "start", the typical regex is:
* /^\s*ESCAPED_TERM\b/iu.
* /^[\s]*ESCAPED_TERM\b/iu.
* Or if decoration characters are specified (e.g. "*"), then any of
* those characters may appear in any order at the start:
* /^[\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.
* If it starts or ends with non-word character, the prefix and suffix are empty, respectively.
*/
return new RegExp(`${prefix}${escaped}${suffix}`, flags);
}
Expand Down
43 changes: 42 additions & 1 deletion tests/lib/rules/no-warning-comments.js
Expand Up @@ -37,7 +37,9 @@ ruleTester.run("no-warning-comments", rule, {
{ 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"] }] },
"/** multi-line block comment with lines starting with\nTODO\nFIXME or\nXXX\n*/",
{ code: "//!TODO ", options: [{ decoration: ["*"] }] }
],
invalid: [
{
Expand Down Expand Up @@ -387,6 +389,45 @@ ruleTester.run("no-warning-comments", rule, {
}
}
]
},
{
code: "/*\nTODO undecorated multi-line block comment (start)\n*/",
options: [{ terms: ["todo"], location: "start" }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo",
comment: "TODO undecorated multi-line block..."
}
}
]
},
{
code: "///// TODO decorated single-line comment with decoration array \n /////",
options: [{ terms: ["todo"], location: "start", decoration: ["*", "/"] }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo",
comment: "/// TODO decorated single-line comment..."
}
}
]
},
{
code: "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) \n /////",
options: [{ terms: ["todo"], location: "start", decoration: ["*", "/"] }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "todo",
comment: "/*/*/ TODO decorated single-line comment..."
}
}
]
lachlanhunt marked this conversation as resolved.
Show resolved Hide resolved
}
]
});