From 1fb31a4b3e05734f801ade0450fea33494e4d5e6 Mon Sep 17 00:00:00 2001 From: Joshua Chen Date: Thu, 26 May 2022 02:54:32 +0800 Subject: [PATCH] feat(eslint-plugin): [ban-ts-comment] add descriptionFormat option (#5026) * feat(eslint-plugin): [ban-ts-comment] add descriptionFormat option for ts-expect-error * test: test * Update packages/eslint-plugin/src/rules/ban-ts-comment.ts Co-authored-by: Josh Goldberg * refactor: deduplicate Co-authored-by: Josh Goldberg --- .../eslint-plugin/src/rules/ban-ts-comment.ts | 127 ++++---- .../tests/rules/ban-ts-comment.test.ts | 270 +++++++++++++++++- 2 files changed, 337 insertions(+), 60 deletions(-) diff --git a/packages/eslint-plugin/src/rules/ban-ts-comment.ts b/packages/eslint-plugin/src/rules/ban-ts-comment.ts index 96b01d8efc2..50fa39c626f 100644 --- a/packages/eslint-plugin/src/rules/ban-ts-comment.ts +++ b/packages/eslint-plugin/src/rules/ban-ts-comment.ts @@ -1,19 +1,43 @@ 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'; - 'ts-ignore'?: boolean | 'allow-with-description'; - 'ts-nocheck'?: boolean | 'allow-with-description'; - 'ts-check'?: boolean | 'allow-with-description'; + '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 = | 'tsDirectiveComment' - | 'tsDirectiveCommentRequiresDescription'; + | 'tsDirectiveCommentRequiresDescription' + | 'tsDirectiveCommentDescriptionNotMatchPattern'; export default util.createRule<[Options], MessageIds>({ name: 'ban-ts-comment', @@ -29,55 +53,17 @@ 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 the "@ts-{{directive}}" directive must match the {{format}} format.', }, schema: [ { type: 'object', properties: { - 'ts-expect-error': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - ], - }, - 'ts-ignore': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - ], - }, - 'ts-nocheck': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - ], - }, - 'ts-check': { - oneOf: [ - { - type: 'boolean', - default: true, - }, - { - enum: ['allow-with-description'], - }, - ], - }, + 'ts-expect-error': directiveConfigSchema, + 'ts-ignore': directiveConfigSchema, + 'ts-nocheck': directiveConfigSchema, + 'ts-check': directiveConfigSchema, minimumDescriptionLength: { type: 'number', default: defaultMinimumDescriptionLength, @@ -99,25 +85,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 +133,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 b9d10b005b6..3843df63ef4 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 because xyz`, + options: [ + { + 'ts-expect-error': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + }, + ], + errors: [ + { + data: { directive: 'expect-error', format: '^: TS\\d+ because .+$' }, + messageId: 'tsDirectiveCommentDescriptionNotMatchPattern', + line: 1, + column: 1, + }, + ], + }, ], }); @@ -189,6 +255,17 @@ ruleTester.run('ts-ignore', rule, { }, ], }, + { + code: '// @ts-ignore: TS1234 because xyz', + options: [ + { + 'ts-ignore': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 10, + }, + ], + }, ], invalid: [ { @@ -328,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, + }, + ], + }, ], }); @@ -355,6 +487,17 @@ ruleTester.run('ts-nocheck', rule, { }, ], }, + { + code: '// @ts-nocheck: TS1234 because xyz', + options: [ + { + 'ts-nocheck': { + descriptionFormat: '^: TS\\d+ because .+$', + }, + minimumDescriptionLength: 10, + }, + ], + }, ], invalid: [ { @@ -470,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, + }, + ], + }, ], }); @@ -491,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: [ { @@ -588,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, + }, + ], + }, ], });