From 273e75052d53dc21da7ab30982bdcafc3ad108b0 Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Fri, 20 May 2022 16:43:46 +0800 Subject: [PATCH 1/4] feat(eslint-plugin): [ban-ts-comment] add descriptionFormat option for ts-expect-error --- .../eslint-plugin/src/rules/ban-ts-comment.ts | 92 ++++++++++++++++--- .../tests/rules/ban-ts-comment.test.ts | 66 +++++++++++++ 2 files changed, 145 insertions(+), 13 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 96b01d8efc2f..f35557bc929f 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -2,10 +2,22 @@ import { AST_TOKEN_TYPES } from '@typescript-eslint/utils'; import * as util from '../util'; interface Options { - 'ts-expect-error'?: boolean | 'allow-with-description'; - 'ts-ignore'?: boolean | 'allow-with-description'; - 'ts-nocheck'?: boolean | 'allow-with-description'; - 'ts-check'?: boolean | 'allow-with-description'; + 'ts-expect-error'?: + | boolean + | 'allow-with-description' + | { descriptionFormat: string }; + 'ts-ignore'?: + | boolean + | 'allow-with-description' + | { descriptionFormat: string }; + 'ts-nocheck'?: + | boolean + | 'allow-with-description' + | { descriptionFormat: string }; + 'ts-check'?: + | boolean + | 'allow-with-description' + | { descriptionFormat: string }; minimumDescriptionLength?: number; } @@ -13,7 +25,8 @@ export const defaultMinimumDescriptionLength = 3; type MessageIds = | 'tsDirectiveComment' - | 'tsDirectiveCommentRequiresDescription'; + | 'tsDirectiveCommentRequiresDescription' + | 'tsDirectiveCommentDescriptionNotMatchPattern'; export default util.createRule<[Options], MessageIds>({ name: 'ban-ts-comment', @@ -29,6 +42,8 @@ export default util.createRule<[Options], MessageIds>({ 'Do not use "@ts-{{directive}}" because it alters compilation errors.', tsDirectiveCommentRequiresDescription: 'Include a description after the "@ts-{{directive}}" directive to explain why the @ts-{{directive}} is necessary. The description must be {{minimumDescriptionLength}} characters or longer.', + tsDirectiveCommentDescriptionNotMatchPattern: + 'The description for "@ts-{{directive}}" directive must match the {{format}} format.', }, schema: [ { @@ -43,6 +58,12 @@ export default util.createRule<[Options], MessageIds>({ { enum: ['allow-with-description'], }, + { + type: 'object', + properties: { + descriptionFormat: { type: 'string' }, + }, + }, ], }, 'ts-ignore': { @@ -54,6 +75,12 @@ export default util.createRule<[Options], MessageIds>({ { enum: ['allow-with-description'], }, + { + type: 'object', + properties: { + descriptionFormat: { type: 'string' }, + }, + }, ], }, 'ts-nocheck': { @@ -65,6 +92,12 @@ export default util.createRule<[Options], MessageIds>({ { enum: ['allow-with-description'], }, + { + type: 'object', + properties: { + descriptionFormat: { type: 'string' }, + }, + }, ], }, 'ts-check': { @@ -76,6 +109,12 @@ export default util.createRule<[Options], MessageIds>({ { enum: ['allow-with-description'], }, + { + type: 'object', + properties: { + descriptionFormat: { type: 'string' }, + }, + }, ], }, minimumDescriptionLength: { @@ -99,25 +138,42 @@ export default util.createRule<[Options], MessageIds>({ create(context, [options]) { /* The regex used are taken from the ones used in the official TypeScript repo - - https://github.com/microsoft/TypeScript/blob/main/src/compiler/scanner.ts#L281-L289 + https://github.com/microsoft/TypeScript/blob/408c760fae66080104bc85c449282c2d207dfe8e/src/compiler/scanner.ts#L288-L296 */ const commentDirectiveRegExSingleLine = - /^\/*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/; + /^\/*\s*@ts-(?expect-error|ignore|check|nocheck)(?.*)/; const commentDirectiveRegExMultiLine = - /^\s*(?:\/|\*)*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/; + /^\s*(?:\/|\*)*\s*@ts-(?expect-error|ignore|check|nocheck)(?.*)/; const sourceCode = context.getSourceCode(); + const descriptionFormats = new Map(); + for (const directive of [ + 'ts-expect-error', + 'ts-ignore', + 'ts-nocheck', + 'ts-check', + ] as const) { + const option = options[directive]; + if (typeof option === 'object' && option.descriptionFormat) { + descriptionFormats.set(directive, new RegExp(option.descriptionFormat)); + } + } + return { Program(): void { const comments = sourceCode.getAllComments(); comments.forEach(comment => { - let regExp = commentDirectiveRegExSingleLine; + const regExp = + comment.type === AST_TOKEN_TYPES.Line + ? commentDirectiveRegExSingleLine + : commentDirectiveRegExMultiLine; - if (comment.type !== AST_TOKEN_TYPES.Line) { - regExp = commentDirectiveRegExMultiLine; + const match = regExp.exec(comment.value); + if (!match) { + return; } - const [, directive, description] = regExp.exec(comment.value) ?? []; + const { directive, description } = match.groups!; const fullDirective = `ts-${directive}` as keyof Options; @@ -130,16 +186,26 @@ export default util.createRule<[Options], MessageIds>({ }); } - if (option === 'allow-with-description') { + if ( + option === 'allow-with-description' || + (typeof option === 'object' && option.descriptionFormat) + ) { const { minimumDescriptionLength = defaultMinimumDescriptionLength, } = options; + const format = descriptionFormats.get(fullDirective); if (description.trim().length < minimumDescriptionLength) { context.report({ data: { directive, minimumDescriptionLength }, node: comment, messageId: 'tsDirectiveCommentRequiresDescription', }); + } else if (format && !format.test(description)) { + context.report({ + data: { directive, format: format.source }, + node: comment, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + }); } } }); 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 b9d10b005b6f..ddb8c7dce9e8 100644 --- a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -34,6 +34,17 @@ ruleTester.run('ts-expect-error', rule, { }, ], }, + { + code: '// @ts-expect-error: TS1234 because xyz', + options: [ + { + 'ts-expect-error': { + descriptionFormat: ': TS\\d+ because .+', + }, + minimumDescriptionLength: 10, + }, + ], + }, ], invalid: [ { @@ -162,6 +173,61 @@ if (false) { }, ], }, + { + code: '// @ts-expect-error: TS1234 because xyz', + options: [ + { + 'ts-expect-error': { + descriptionFormat: ': TS\\d+ because .+', + }, + minimumDescriptionLength: 25, + }, + ], + errors: [ + { + data: { directive: 'expect-error', minimumDescriptionLength: 25 }, + messageId: 'tsDirectiveCommentRequiresDescription', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-expect-error: TS1234', + options: [ + { + 'ts-expect-error': { + descriptionFormat: ': TS\\d+ because .+', + }, + }, + ], + errors: [ + { + data: { directive: 'expect-error', format: ': TS\\d+ because .+' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, + { + code: noFormat`// @ts-expect-error : TS1234`, + options: [ + { + 'ts-expect-error': { + descriptionFormat: ': TS\\d+ because .+', + }, + }, + ], + errors: [ + { + data: { directive: 'expect-error', format: ': TS\\d+ because .+' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, ], }); From a1686bbfadbf791fb045fe3a6464e1e9f7014153 Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Fri, 20 May 2022 17:18:59 +0800 Subject: [PATCH 2/4] test: test --- .../tests/rules/ban-ts-comment.test.ts | 218 +++++++++++++++++- 1 file changed, 208 insertions(+), 10 deletions(-) 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 ddb8c7dce9e8..3843df63ef41 100644 --- a/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts +++ b/packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts @@ -39,7 +39,7 @@ ruleTester.run('ts-expect-error', rule, { options: [ { 'ts-expect-error': { - descriptionFormat: ': TS\\d+ because .+', + descriptionFormat: '^: TS\\d+ because .+$', }, minimumDescriptionLength: 10, }, @@ -178,7 +178,7 @@ if (false) { options: [ { 'ts-expect-error': { - descriptionFormat: ': TS\\d+ because .+', + descriptionFormat: '^: TS\\d+ because .+$', }, minimumDescriptionLength: 25, }, @@ -197,13 +197,13 @@ if (false) { options: [ { 'ts-expect-error': { - descriptionFormat: ': TS\\d+ because .+', + descriptionFormat: '^: TS\\d+ because .+$', }, }, ], errors: [ { - data: { directive: 'expect-error', format: ': TS\\d+ because .+' }, + data: { directive: 'expect-error', format: '^: TS\\d+ because .+$' }, messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', line: 1, column: 1, @@ -211,17 +211,17 @@ if (false) { ], }, { - code: noFormat`// @ts-expect-error : TS1234`, + code: noFormat`// @ts-expect-error : TS1234 because xyz`, options: [ { 'ts-expect-error': { - descriptionFormat: ': TS\\d+ because .+', + descriptionFormat: '^: TS\\d+ because .+$', }, }, ], errors: [ { - data: { directive: 'expect-error', format: ': TS\\d+ because .+' }, + data: { directive: 'expect-error', format: '^: TS\\d+ because .+$' }, messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', line: 1, column: 1, @@ -255,6 +255,17 @@ ruleTester.run('ts-ignore', rule, { }, ], }, + { + code: '// @ts-ignore: TS1234 because xyz', + options: [ + { + 'ts-ignore': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 10, + }, + ], + }, ], invalid: [ { @@ -394,6 +405,61 @@ if (false) { }, ], }, + { + code: '// @ts-ignore: TS1234 because xyz', + options: [ + { + 'ts-ignore': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 25, + }, + ], + errors: [ + { + data: { directive: 'ignore', minimumDescriptionLength: 25 }, + messageId: 'tsDirectiveCommentRequiresDescription', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-ignore: TS1234', + options: [ + { + 'ts-ignore': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'ignore', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, + { + code: noFormat`// @ts-ignore : TS1234 because xyz`, + options: [ + { + 'ts-ignore': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'ignore', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, ], }); @@ -421,6 +487,17 @@ ruleTester.run('ts-nocheck', rule, { }, ], }, + { + code: '// @ts-nocheck: TS1234 because xyz', + options: [ + { + 'ts-nocheck': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 10, + }, + ], + }, ], invalid: [ { @@ -536,6 +613,61 @@ if (false) { }, ], }, + { + code: '// @ts-nocheck: TS1234 because xyz', + options: [ + { + 'ts-nocheck': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 25, + }, + ], + errors: [ + { + data: { directive: 'nocheck', minimumDescriptionLength: 25 }, + messageId: 'tsDirectiveCommentRequiresDescription', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-nocheck: TS1234', + options: [ + { + 'ts-nocheck': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'nocheck', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, + { + code: noFormat`// @ts-nocheck : TS1234 because xyz`, + options: [ + { + 'ts-nocheck': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'nocheck', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, ], }); @@ -557,6 +689,17 @@ ruleTester.run('ts-check', rule, { { 'ts-check': 'allow-with-description', minimumDescriptionLength: 3 }, ], }, + { + code: '// @ts-check: TS1234 because xyz', + options: [ + { + 'ts-check': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 10, + }, + ], + }, ], invalid: [ { @@ -654,16 +797,71 @@ if (false) { ], }, { - code: '// @ts-ignore', - options: [{ 'ts-ignore': 'allow-with-description' }], + code: '// @ts-check', + options: [{ 'ts-check': 'allow-with-description' }], errors: [ { - data: { directive: 'ignore', minimumDescriptionLength: 3 }, + data: { directive: 'check', minimumDescriptionLength: 3 }, + messageId: 'tsDirectiveCommentRequiresDescription', + line: 1, + column: 1, + }, + ], + }, + { + code: '// @ts-check: TS1234 because xyz', + options: [ + { + 'ts-check': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 25, + }, + ], + errors: [ + { + data: { directive: 'check', minimumDescriptionLength: 25 }, messageId: 'tsDirectiveCommentRequiresDescription', line: 1, column: 1, }, ], }, + { + code: '// @ts-check: TS1234', + options: [ + { + 'ts-check': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'check', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, + { + code: noFormat`// @ts-check : TS1234 because xyz`, + options: [ + { + 'ts-check': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'check', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, ], }); From 88430b3227bd42ddff55c5e731320f12762db0c3 Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Sat, 21 May 2022 10:37:33 +0800 Subject: [PATCH 3/4] Update packages/eslint-plugin/src/rules/ban-ts-comment.ts Co-authored-by: Josh Goldberg --- packages/eslint-plugin/src/rules/ban-ts-comment.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index f35557bc929f..0323f6d26545 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -43,7 +43,7 @@ export default util.createRule<[Options], MessageIds>({ tsDirectiveCommentRequiresDescription: 'Include a description after the "@ts-{{directive}}" directive to explain why the @ts-{{directive}} is necessary. The description must be {{minimumDescriptionLength}} characters or longer.', tsDirectiveCommentDescriptionNotMatchPattern: - 'The description for "@ts-{{directive}}" directive must match the {{format}} format.', + 'The description for the "@ts-{{directive}}" directive must match the {{format}} format.', }, schema: [ { From 620bf20407433b5de5a3fe8cbd7848e470d1c5e7 Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Sat, 21 May 2022 10:43:24 +0800 Subject: [PATCH 4/4] refactor: deduplicate --- .../eslint-plugin/src/rules/ban-ts-comment.ts | 115 +++++------------- 1 file changed, 31 insertions(+), 84 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 0323f6d26545..50fa39c626f8 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -1,26 +1,37 @@ import { AST_TOKEN_TYPES } from '@typescript-eslint/utils'; import * as util from '../util'; +type DirectiveConfig = + | boolean + | 'allow-with-description' + | { descriptionFormat: string }; + interface Options { - 'ts-expect-error'?: - | boolean - | 'allow-with-description' - | { descriptionFormat: string }; - 'ts-ignore'?: - | boolean - | 'allow-with-description' - | { descriptionFormat: string }; - 'ts-nocheck'?: - | boolean - | 'allow-with-description' - | { descriptionFormat: string }; - 'ts-check'?: - | boolean - | 'allow-with-description' - | { descriptionFormat: string }; + 'ts-expect-error'?: DirectiveConfig; + 'ts-ignore'?: DirectiveConfig; + 'ts-nocheck'?: DirectiveConfig; + 'ts-check'?: DirectiveConfig; minimumDescriptionLength?: number; } +const directiveConfigSchema = { + oneOf: [ + { + type: 'boolean', + default: true, + }, + { + enum: ['allow-with-description'], + }, + { + type: 'object', + properties: { + descriptionFormat: { type: 'string' }, + }, + }, + ], +}; + export const defaultMinimumDescriptionLength = 3; type MessageIds = @@ -49,74 +60,10 @@ export default util.createRule<[Options], MessageIds>({ { type: 'object', properties: { - 'ts-expect-error': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - { - type: 'object', - properties: { - descriptionFormat: { type: 'string' }, - }, - }, - ], - }, - 'ts-ignore': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - { - type: 'object', - properties: { - descriptionFormat: { type: 'string' }, - }, - }, - ], - }, - 'ts-nocheck': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - { - type: 'object', - properties: { - descriptionFormat: { type: 'string' }, - }, - }, - ], - }, - 'ts-check': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - { - type: 'object', - properties: { - descriptionFormat: { type: 'string' }, - }, - }, - ], - }, + 'ts-expect-error': directiveConfigSchema, + 'ts-ignore': directiveConfigSchema, + 'ts-nocheck': directiveConfigSchema, + 'ts-check': directiveConfigSchema, minimumDescriptionLength: { type: 'number', default: defaultMinimumDescriptionLength,