From d0b2ecd289ae0be1a99663167a420bc1802fc869 Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Sat, 1 Oct 2022 14:46:03 +0900 Subject: [PATCH 1/6] fix: counts graphemes instead of String length --- packages/eslint-plugin/package.json | 1 + .../eslint-plugin/src/rules/ban-ts-comment.ts | 19 +++++- .../tests/rules/ban-ts-comment.test.ts | 64 +++++++++++++++++++ yarn.lock | 5 ++ 4 files changed, 88 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 702e3db6dfa..bb4b59d68c2 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -59,6 +59,7 @@ "@types/marked": "*", "@types/prettier": "*", "chalk": "^5.0.1", + "grapheme-splitter": "^1.0.4", "json-schema": "*", "marked": "^4.0.15", "prettier": "*", diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index d9e3659ce0a..83ec06724fb 100644 --- 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; + } + if (!splitter) { + 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 index 7e8ef295ba9..333d3f362e2 100644 --- a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -228,6 +228,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, + }, + ], + }, ], }); @@ -460,6 +476,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, + }, + ], + }, ], }); @@ -668,6 +700,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, + }, + ], + }, ], }); @@ -863,5 +911,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 index 10a60b825f3..2f3247318e8 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8066,6 +8066,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" From 46548509344b0db3914a388280f42aa463fe52cb Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Sun, 2 Oct 2022 16:59:30 +0900 Subject: [PATCH 2/6] Update packages/eslint-plugin/src/rules/ban-ts-comment.ts Co-authored-by: Josh Goldberg --- packages/eslint-plugin/src/rules/ban-ts-comment.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 83ec06724fb..7fd20d9c31f 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -11,9 +11,9 @@ function getStringLength(value: string): number { if (isASCII(value)) { return value.length; } - if (!splitter) { - splitter = new GraphemeSplitter(); - } + + splitter ??= new GraphemeSplitter(); + return splitter.countGraphemes(value); } From 3d5307361405178205931e5066adab7491f9a2cc Mon Sep 17 00:00:00 2001 From: Sosuke Suzuki Date: Sun, 2 Oct 2022 17:17:13 +0900 Subject: [PATCH 3/6] chore: add valid tests --- .../tests/rules/ban-ts-comment.test.ts | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts index 333d3f362e2..54855f19cf3 100644 --- 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: [ { @@ -282,6 +290,14 @@ ruleTester.run('ts-ignore', rule, { }, ], }, + { + code: noFormat`// @ts-ignore ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ`, + options: [ + { + 'ts-ignore': 'allow-with-description', + }, + ], + }, ], invalid: [ { @@ -530,6 +546,14 @@ ruleTester.run('ts-nocheck', rule, { }, ], }, + { + code: noFormat`// @ts-nocheck ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ`, + options: [ + { + 'ts-nocheck': 'allow-with-description', + }, + ], + }, ], invalid: [ { @@ -748,6 +772,14 @@ ruleTester.run('ts-check', rule, { }, ], }, + { + code: noFormat`// @ts-check ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ๐Ÿ‘จโ€๐Ÿ‘ฉโ€๐Ÿ‘งโ€๐Ÿ‘ฆ`, + options: [ + { + 'ts-check': 'allow-with-description', + }, + ], + }, ], invalid: [ { From 64122ed2da9039715d87bfaec62235db2f49e9b6 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 18 Dec 2022 20:50:15 -0500 Subject: [PATCH 4/6] Add grapheme-splitter to peerDependencies --- packages/eslint-plugin/package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 3eecb874ffb..6053e62af91 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -72,7 +72,8 @@ }, "peerDependencies": { "@typescript-eslint/parser": "^5.0.0", - "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0" + "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0", + "grapheme-splitter": "^1.0.4" }, "peerDependenciesMeta": { "typescript": { From 47a3e78096341d7a943757be90a0853c82926ad5 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Sun, 18 Dec 2022 21:21:53 -0500 Subject: [PATCH 5/6] Move to standard dependency --- packages/eslint-plugin/package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 6053e62af91..88894d2cd18 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -48,6 +48,7 @@ "@typescript-eslint/type-utils": "5.46.1", "@typescript-eslint/utils": "5.46.1", "debug": "^4.3.4", + "grapheme-splitter": "^1.0.4", "ignore": "^5.2.0", "natural-compare-lite": "^1.4.0", "regexpp": "^3.2.0", From f17c78fc49906593c1cd2210ce5ce0dcc00c8e75 Mon Sep 17 00:00:00 2001 From: Josh Goldberg Date: Tue, 24 Jan 2023 17:42:23 -0500 Subject: [PATCH 6/6] No more peerDependency --- packages/eslint-plugin/package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json index 29c7f71e0f5..5e2304c49e0 100644 --- a/packages/eslint-plugin/package.json +++ b/packages/eslint-plugin/package.json @@ -73,8 +73,7 @@ }, "peerDependencies": { "@typescript-eslint/parser": "^5.0.0", - "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0", - "grapheme-splitter": "^1.0.4" + "eslint": "^6.0.0 || ^7.0.0 || ^8.0.0" }, "peerDependenciesMeta": { "typescript": {