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

lachlanhunt · mdjermanovic · web-flow · commit 43f03aa96b63 · 2022-08-24T15:22:02.000-07:00
* 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 &lt;milos.djermanovic@gmail.com&gt;

* 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 &lt;milos.djermanovic@gmail.com&gt;
diff --git a/docs/src/rules/no-warning-comments.md b/docs/src/rules/no-warning-comments.md
@@ -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:
 
@@ -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);
@@ -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
@@ -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.
diff --git a/lib/rules/no-warning-comments.js b/lib/rules/no-warning-comments.js
@@ -37,6 +37,15 @@ module.exports = {
                     },
                     location: {
                         enum: ["start", "anywhere"]
+                    },
+                    decoration: {
+                        type: "array",
+                        items: {
+                            type: "string",
+                            pattern: "^\\S$"
+                        },
+                        minItems: 1,
+                        uniqueItems: true
                     }
                 },
                 additionalProperties: false
@@ -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;
 
         /**
@@ -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
@@ -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;
             }
@@ -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);
         }
diff --git a/tests/lib/rules/no-warning-comments.js b/tests/lib/rules/no-warning-comments.js
@@ -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: [
         {
@@ -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..."
+                    }
+                }
+            ]
         }
     ]
 });
