Skip to content

Commit 43f03aa

Browse files
lachlanhuntmdjermanovic
lachlanhunt
and
mdjermanovic
authored
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>
1 parent b1918da commit 43f03aa

3 files changed

Lines changed: 134 additions & 9 deletions

File tree

‎docs/src/rules/no-warning-comments.md‎

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@ This rule reports comments that include any of the predefined terms specified in
2020

2121
This rule has an options object literal:
2222

23-
* `"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`.
24-
* `"location"`: optional string that configures where in your comments to check for matches. Defaults to `"start"`. The other value is match `anywhere` in comments.
23+
* `"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`.
24+
* `"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.
25+
* `"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"`.
2526

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

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

34+
/*
35+
FIXME
36+
*/
3337
function callback(err, results) {
3438
if (err) {
3539
console.error(err);
@@ -72,7 +76,7 @@ Examples of **incorrect** code for the `{ "terms": ["todo", "fixme", "any other
7276
// TODO: this
7377
// todo: this too
7478
// Even this: TODO
75-
/* /*
79+
/*
7680
* The same goes for this TODO comment
7781
* Or a fixme
7882
* as well as any other term
@@ -100,6 +104,54 @@ Examples of **correct** code for the `{ "terms": ["todo", "fixme", "any other te
100104

101105
:::
102106

107+
### Decoration Characters
108+
109+
Examples of **incorrect** code for the `{ "decoration": ["*"] }` options:
110+
111+
::: incorrect
112+
113+
```js
114+
/*eslint no-warning-comments: ["error", { "decoration": ["*"] }]*/
115+
116+
//***** todo decorative asterisks are ignored *****//
117+
/**
118+
* TODO new lines and asterisks are also ignored in block comments.
119+
*/
120+
```
121+
122+
:::
123+
124+
Examples of **incorrect** code for the `{ "decoration": ["/", "*"] }` options:
125+
126+
::: incorrect
127+
128+
```js
129+
/*eslint no-warning-comments: ["error", { "decoration": ["/", "*"] }]*/
130+
131+
////// TODO decorative slashes and whitespace are ignored //////
132+
//***** todo decorative asterisks are also ignored *****//
133+
/**
134+
* TODO new lines are also ignored in block comments.
135+
*/
136+
```
137+
138+
:::
139+
140+
Examples of **correct** code for the `{ "decoration": ["/", "*"] }` options:
141+
142+
::: correct
143+
144+
```js
145+
/*eslint no-warning-comments: ["error", { "decoration": ["/", "*"] }]*/
146+
147+
//!TODO preceded by non-decoration character
148+
/**
149+
*!TODO preceded by non-decoration character in a block comment
150+
*/
151+
```
152+
153+
:::
154+
103155
## When Not To Use It
104156

105157
* 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.

‎lib/rules/no-warning-comments.js‎

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,15 @@ module.exports = {
3737
},
3838
location: {
3939
enum: ["start", "anywhere"]
40+
},
41+
decoration: {
42+
type: "array",
43+
items: {
44+
type: "string",
45+
pattern: "^\\S$"
46+
},
47+
minItems: 1,
48+
uniqueItems: true
4049
}
4150
},
4251
additionalProperties: false
@@ -53,6 +62,7 @@ module.exports = {
5362
configuration = context.options[0] || {},
5463
warningTerms = configuration.terms || ["todo", "fixme", "xxx"],
5564
location = configuration.location || "start",
65+
decoration = [...configuration.decoration || []].join(""),
5666
selfConfigRegEx = /\bno-warning-comments\b/u;
5767

5868
/**
@@ -64,6 +74,7 @@ module.exports = {
6474
*/
6575
function convertToRegExp(term) {
6676
const escaped = escapeRegExp(term);
77+
const escapedDecoration = escapeRegExp(decoration);
6778

6879
/*
6980
* When matching at the start, ignore leading whitespace, and
@@ -74,18 +85,23 @@ module.exports = {
7485
* e.g. terms ["TODO"] matches `//TODO something`
7586
* $ handles any terms at the end of a comment
7687
* e.g. terms ["TODO"] matches `// something TODO`
77-
* \s* handles optional leading spaces (for "start" location only)
78-
* e.g. terms ["TODO"] matches `// TODO something`
7988
* \b handles terms preceded/followed by word boundary
8089
* e.g. terms: ["!FIX", "FIX!"] matches `// FIX!something` or `// something!FIX`
8190
* terms: ["FIX"] matches `// FIX!` or `// !FIX`, but not `// fixed or affix`
91+
*
92+
* For location start:
93+
* [\s]* handles optional leading spaces
94+
* e.g. terms ["TODO"] matches `// TODO something`
95+
* [\s\*]* (where "\*" is the escaped string of decoration)
96+
* handles optional leading spaces or decoration characters (for "start" location only)
97+
* e.g. terms ["TODO"] matches `/**** TODO something ... `
8298
*/
8399
const wordBoundary = "\\b";
84100

85101
let prefix = "";
86102

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

96112
/*
97113
* For location "start", the typical regex is:
98-
* /^\s*ESCAPED_TERM\b/iu.
114+
* /^[\s]*ESCAPED_TERM\b/iu.
115+
* Or if decoration characters are specified (e.g. "*"), then any of
116+
* those characters may appear in any order at the start:
117+
* /^[\s\*]*ESCAPED_TERM\b/iu.
99118
*
100119
* For location "anywhere" the typical regex is
101120
* /\bESCAPED_TERM\b/iu
102121
*
103-
* If it starts or ends with non-word character, the prefix and suffix empty, respectively.
122+
* If it starts or ends with non-word character, the prefix and suffix are empty, respectively.
104123
*/
105124
return new RegExp(`${prefix}${escaped}${suffix}`, flags);
106125
}

‎tests/lib/rules/no-warning-comments.js‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,9 @@ ruleTester.run("no-warning-comments", rule, {
3737
{ code: "// special regex characters don't cause a problem", options: [{ terms: ["[aeiou]"], location: "anywhere" }] },
3838
"/*eslint no-warning-comments: [2, { \"terms\": [\"todo\", \"fixme\", \"any other term\"], \"location\": \"anywhere\" }]*/\n\nvar x = 10;\n",
3939
{ code: "/*eslint no-warning-comments: [2, { \"terms\": [\"todo\", \"fixme\", \"any other term\"], \"location\": \"anywhere\" }]*/\n\nvar x = 10;\n", options: [{ location: "anywhere" }] },
40-
{ code: "// foo", options: [{ terms: ["foo-bar"] }] }
40+
{ code: "// foo", options: [{ terms: ["foo-bar"] }] },
41+
"/** multi-line block comment with lines starting with\nTODO\nFIXME or\nXXX\n*/",
42+
{ code: "//!TODO ", options: [{ decoration: ["*"] }] }
4143
],
4244
invalid: [
4345
{
@@ -387,6 +389,58 @@ ruleTester.run("no-warning-comments", rule, {
387389
}
388390
}
389391
]
392+
},
393+
{
394+
code: "/*\nTODO undecorated multi-line block comment (start)\n*/",
395+
options: [{ terms: ["todo"], location: "start" }],
396+
errors: [
397+
{
398+
messageId: "unexpectedComment",
399+
data: {
400+
matchedTerm: "todo",
401+
comment: "TODO undecorated multi-line block..."
402+
}
403+
}
404+
]
405+
},
406+
{
407+
code: "///// TODO decorated single-line comment with decoration array \n /////",
408+
options: [{ terms: ["todo"], location: "start", decoration: ["*", "/"] }],
409+
errors: [
410+
{
411+
messageId: "unexpectedComment",
412+
data: {
413+
matchedTerm: "todo",
414+
comment: "/// TODO decorated single-line comment..."
415+
}
416+
}
417+
]
418+
},
419+
{
420+
code: "///*/*/ TODO decorated single-line comment with multiple decoration characters (start) \n /////",
421+
options: [{ terms: ["todo"], location: "start", decoration: ["*", "/"] }],
422+
errors: [
423+
{
424+
messageId: "unexpectedComment",
425+
data: {
426+
matchedTerm: "todo",
427+
comment: "/*/*/ TODO decorated single-line comment..."
428+
}
429+
}
430+
]
431+
},
432+
{
433+
code: "//**TODO term starts with a decoration character",
434+
options: [{ terms: ["*todo"], location: "start", decoration: ["*"] }],
435+
errors: [
436+
{
437+
messageId: "unexpectedComment",
438+
data: {
439+
matchedTerm: "*todo",
440+
comment: "**TODO term starts with a decoration..."
441+
}
442+
}
443+
]
390444
}
391445
]
392446
});

0 commit comments

Comments
 (0)