Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
feat: no-warning-comments support comments with decoration (#16120)
* Implement support for skipping decoration characters in no-warning-comments

* Update unit tests for no-warning-comments

* Update docs for no-warning-comments

* Support array of decoration strings in no-warning-comments

* Enforce single character decoration strings in no-warning-comments

* Enforce minimum number of unique decoration characters

* Update docs/src/rules/no-warning-comments.md

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>

* Change decoration property to array only, update docs for no-warning-comments

* Fix mistakes in README

* Add test for term starting with decoration character

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
lachlanhunt and mdjermanovic committed Aug 24, 2022
1 parent b1918da commit 43f03aa
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 9 deletions.
58 changes: 55 additions & 3 deletions docs/src/rules/no-warning-comments.md
Expand Up @@ -20,8 +20,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 @@ -30,6 +31,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 @@ -72,7 +76,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 @@ -100,6 +104,54 @@ Examples of **correct** code for the `{ "terms": ["todo", "fixme", "any other te

:::

### Decoration Characters

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

::: incorrect

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

//***** 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 optional leading spaces or decoration characters (for "start" location only)
* 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
56 changes: 55 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,58 @@ 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..."
}
}
]
},
{
code: "//**TODO term starts with a decoration character",
options: [{ terms: ["*todo"], location: "start", decoration: ["*"] }],
errors: [
{
messageId: "unexpectedComment",
data: {
matchedTerm: "*todo",
comment: "**TODO term starts with a decoration..."
}
}
]
}
]
});

0 comments on commit 43f03aa

Please sign in to comment.