Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(eslint-plugin): [ban-ts-comment] add descriptionFormat option #5026

Merged
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
127 changes: 70 additions & 57 deletions 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 };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, it would be nice to have minimumDescriptionLength as an option in each object, so individual directives can override it... but that feels like a pretty niche use case. Maybe a followup to wait for someone to file an issue on.


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',
Expand All @@ -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,
Expand All @@ -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
bradzacher marked this conversation as resolved.
Show resolved Hide resolved
*/
const commentDirectiveRegExSingleLine =
/^\/*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
/^\/*\s*@ts-(?<directive>expect-error|ignore|check|nocheck)(?<description>.*)/;
const commentDirectiveRegExMultiLine =
/^\s*(?:\/|\*)*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
/^\s*(?:\/|\*)*\s*@ts-(?<directive>expect-error|ignore|check|nocheck)(?<description>.*)/;
const sourceCode = context.getSourceCode();

const descriptionFormats = new Map<string, RegExp>();
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;

Expand All @@ -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',
});
}
}
});
Expand Down