fix(eslint-plugin): [ban-ts-comment] counts graphemes instead of `Str…

…ing.prototype.length` (#5704) * fix: counts graphemes instead of String length * Update packages/eslint-plugin/src/rules/ban-ts-comment.ts Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com> * chore: add valid tests * Add grapheme-splitter to peerDependencies * Move to standard dependency * No more peerDependency Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com> Co-authored-by: Josh Goldberg <me@joshuakgoldberg.com>
typescript-eslint · Jan 24, 2023 · 09d57ce · 09d57ce
1 parent 47074b0
commit 09d57ce
Show file tree

Hide file tree

Showing 4 changed files with 121 additions and 1 deletion.
diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json
@@ -48,6 +48,7 @@
     "@typescript-eslint/type-utils": "5.49.0",
     "@typescript-eslint/utils": "5.49.0",
     "debug": "^4.3.4",
+    "grapheme-splitter": "^1.0.4",
     "ignore": "^5.2.0",
     "natural-compare-lite": "^1.4.0",
     "regexpp": "^3.2.0",
@@ -61,6 +62,7 @@
     "@types/natural-compare-lite": "^1.4.0",
     "@types/prettier": "*",
     "chalk": "^5.0.1",
+    "grapheme-splitter": "^1.0.4",
     "cross-fetch": "^3.1.5",
     "json-schema": "*",
     "markdown-table": "^3.0.2",

diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts
@@ -1,7 +1,22 @@
 import { AST_TOKEN_TYPES } from '@typescript-eslint/utils';
+import GraphemeSplitter from 'grapheme-splitter';
 
 import * as util from '../util';
 
+let splitter: GraphemeSplitter;
+function isASCII(value: string): boolean {
+  return /^[\u0020-\u007f]*$/u.test(value);
+}
+function getStringLength(value: string): number {
+  if (isASCII(value)) {
+    return value.length;
+  }
+
+  splitter ??= new GraphemeSplitter();
+
+  return splitter.countGraphemes(value);
+}
+
 type DirectiveConfig =
   | boolean
   | 'allow-with-description'
@@ -147,7 +162,9 @@ export default util.createRule<[Options], MessageIds>({
               minimumDescriptionLength = defaultMinimumDescriptionLength,
             } = options;
             const format = descriptionFormats.get(fullDirective);
-            if (description.trim().length < minimumDescriptionLength) {
+            if (
+              getStringLength(description.trim()) < minimumDescriptionLength
+            ) {
               context.report({
                 data: { directive, minimumDescriptionLength },
                 node: comment,

diff --git a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts
@@ -45,6 +45,14 @@ ruleTester.run('ts-expect-error', rule, {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-expect-error 👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-expect-error': 'allow-with-description',
+        },
+      ],
+    },
   ],
   invalid: [
     {
@@ -228,6 +236,22 @@ if (false) {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-expect-error 👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-expect-error': 'allow-with-description',
+        },
+      ],
+      errors: [
+        {
+          data: { directive: 'expect-error', minimumDescriptionLength: 3 },
+          messageId: 'tsDirectiveCommentRequiresDescription',
+          line: 1,
+          column: 1,
+        },
+      ],
+    },
   ],
 });
 
@@ -266,6 +290,14 @@ ruleTester.run('ts-ignore', rule, {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-ignore 👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-ignore': 'allow-with-description',
+        },
+      ],
+    },
   ],
   invalid: [
     {
@@ -460,6 +492,22 @@ if (false) {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-ignore 👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-ignore': 'allow-with-description',
+        },
+      ],
+      errors: [
+        {
+          data: { directive: 'ignore', minimumDescriptionLength: 3 },
+          messageId: 'tsDirectiveCommentRequiresDescription',
+          line: 1,
+          column: 1,
+        },
+      ],
+    },
   ],
 });
 
@@ -498,6 +546,14 @@ ruleTester.run('ts-nocheck', rule, {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-nocheck 👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-nocheck': 'allow-with-description',
+        },
+      ],
+    },
   ],
   invalid: [
     {
@@ -668,6 +724,22 @@ if (false) {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-nocheck 👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-nocheck': 'allow-with-description',
+        },
+      ],
+      errors: [
+        {
+          data: { directive: 'nocheck', minimumDescriptionLength: 3 },
+          messageId: 'tsDirectiveCommentRequiresDescription',
+          line: 1,
+          column: 1,
+        },
+      ],
+    },
   ],
 });
 
@@ -700,6 +772,14 @@ ruleTester.run('ts-check', rule, {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-check 👨‍👩‍👧‍👦👨‍👩‍👧‍👦👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-check': 'allow-with-description',
+        },
+      ],
+    },
   ],
   invalid: [
     {
@@ -863,5 +943,21 @@ if (false) {
         },
       ],
     },
+    {
+      code: noFormat`// @ts-check 👨‍👩‍👧‍👦`,
+      options: [
+        {
+          'ts-check': 'allow-with-description',
+        },
+      ],
+      errors: [
+        {
+          data: { directive: 'check', minimumDescriptionLength: 3 },
+          messageId: 'tsDirectiveCommentRequiresDescription',
+          line: 1,
+          column: 1,
+        },
+      ],
+    },
   ],
 });
diff --git a/yarn.lock b/yarn.lock
@@ -8086,6 +8086,11 @@ graceful-fs@^4.1.11, graceful-fs@^4.1.15, graceful-fs@^4.1.2, graceful-fs@^4.1.6
   resolved "https://registry.yarnpkg.com/graceful-fs/-/graceful-fs-4.2.9.tgz#041b05df45755e587a24942279b9d113146e1c96"
   integrity sha512-NtNxqUcXgpW2iMrfqSfR73Glt39K+BLwWsPs94yR63v45T0Wbej7eRmL5cWfwEgqXnmjQp3zaJTshdRW/qC2ZQ==
 
+grapheme-splitter@^1.0.4:
+  version "1.0.4"
+  resolved "https://registry.yarnpkg.com/grapheme-splitter/-/grapheme-splitter-1.0.4.tgz#9cf3a665c6247479896834af35cf1dbb4400767e"
+  integrity sha512-bzh50DW9kTPM00T8y4o8vQg89Di9oLJVLW/KaOGIXJWP/iqCN6WKYkbNOF04vFLJhwcpYUh9ydh/+5vpOqV4YQ==
+
 gray-matter@^4.0.3:
   version "4.0.3"
   resolved "https://registry.yarnpkg.com/gray-matter/-/gray-matter-4.0.3.tgz#e893c064825de73ea1f5f7d88c7a9f7274288798"