Skip to content

Commit

Permalink
fix(eslint-plugin): [ban-ts-comment] more accurate handling of multil…
Browse files Browse the repository at this point in the history
…ine comments (#8416)

* fix(eslint-plugin): [ban-ts-comment] more accurate handling of multiline comments

* chore: revert suggesstion message

---------

Co-authored-by: Josh Goldberg <git@joshuakgoldberg.com>
  • Loading branch information
auvred and JoshuaKGoldberg committed Mar 18, 2024
1 parent 73ce4ad commit da006b1
Show file tree
Hide file tree
Showing 2 changed files with 464 additions and 114 deletions.
91 changes: 73 additions & 18 deletions packages/eslint-plugin/src/rules/ban-ts-comment.ts
@@ -1,7 +1,7 @@
import type { TSESLint } from '@typescript-eslint/utils';
import type { TSESLint, TSESTree } from '@typescript-eslint/utils';
import { AST_TOKEN_TYPES } from '@typescript-eslint/utils';

import { createRule, getStringLength } from '../util';
import { createRule, getStringLength, nullThrows } from '../util';

type DirectiveConfig =
| boolean
Expand All @@ -16,7 +16,7 @@ interface Options {
minimumDescriptionLength?: number;
}

export const defaultMinimumDescriptionLength = 3;
const defaultMinimumDescriptionLength = 3;

type MessageIds =
| 'tsDirectiveComment'
Expand All @@ -25,6 +25,11 @@ type MessageIds =
| 'tsDirectiveCommentRequiresDescription'
| 'replaceTsIgnoreWithTsExpectError';

interface MatchedTSDirective {
directive: string;
description: string;
}

export default createRule<[Options], MessageIds>({
name: 'ban-ts-comment',
meta: {
Expand Down Expand Up @@ -98,14 +103,18 @@ export default createRule<[Options], MessageIds>({
},
],
create(context, [options]) {
// https://github.com/microsoft/TypeScript/blob/6f1ad5ad8bec5671f7e951a3524b62d82ec4be68/src/compiler/parser.ts#L10591
const singleLinePragmaRegEx =
/^\/\/\/?\s*@ts-(?<directive>check|nocheck)(?<description>.*)$/;

/*
The regex used are taken from the ones used in the official TypeScript repo -
https://github.com/microsoft/TypeScript/blob/408c760fae66080104bc85c449282c2d207dfe8e/src/compiler/scanner.ts#L288-L296
https://github.com/microsoft/TypeScript/blob/6f1ad5ad8bec5671f7e951a3524b62d82ec4be68/src/compiler/scanner.ts#L340-L348
*/
const commentDirectiveRegExSingleLine =
/^\/*\s*@ts-(?<directive>expect-error|ignore|check|nocheck)(?<description>.*)/;
/^\/*\s*@ts-(?<directive>expect-error|ignore)(?<description>.*)/;
const commentDirectiveRegExMultiLine =
/^\s*(?:\/|\*)*\s*@ts-(?<directive>expect-error|ignore|check|nocheck)(?<description>.*)/;
/^\s*(?:\/|\*)*\s*@ts-(?<directive>expect-error|ignore)(?<description>.*)/;

const descriptionFormats = new Map<string, RegExp>();
for (const directive of [
Expand All @@ -120,22 +129,66 @@ export default createRule<[Options], MessageIds>({
}
}

function execDirectiveRegEx(
regex: RegExp,
str: string,
): MatchedTSDirective | null {
const match = regex.exec(str);
if (!match) {
return null;
}

const { directive, description } = nullThrows(
match.groups,
'RegExp should contain groups',
);
return {
directive: nullThrows(
directive,
'RegExp should contain "directive" group',
),
description: nullThrows(
description,
'RegExp should contain "description" group',
),
};
}

function findDirectiveInComment(
comment: TSESTree.Comment,
): MatchedTSDirective | null {
if (comment.type === AST_TOKEN_TYPES.Line) {
const matchedPragma = execDirectiveRegEx(
singleLinePragmaRegEx,
`//${comment.value}`,
);
if (matchedPragma) {
return matchedPragma;
}

return execDirectiveRegEx(
commentDirectiveRegExSingleLine,
comment.value,
);
}

const commentLines = comment.value.split('\n');
return execDirectiveRegEx(
commentDirectiveRegExMultiLine,
commentLines[commentLines.length - 1],
);
}

return {
Program(): void {
const comments = context.sourceCode.getAllComments();

comments.forEach(comment => {
const regExp =
comment.type === AST_TOKEN_TYPES.Line
? commentDirectiveRegExSingleLine
: commentDirectiveRegExMultiLine;

const match = regExp.exec(comment.value);
const match = findDirectiveInComment(comment);
if (!match) {
return;
}
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
const { directive, description } = match.groups!;
const { directive, description } = match;

const fullDirective = `ts-${directive}` as keyof Options;

Expand Down Expand Up @@ -177,12 +230,14 @@ export default createRule<[Options], MessageIds>({
option === 'allow-with-description' ||
(typeof option === 'object' && option.descriptionFormat)
) {
const {
minimumDescriptionLength = defaultMinimumDescriptionLength,
} = options;
const { minimumDescriptionLength } = options;
const format = descriptionFormats.get(fullDirective);
if (
getStringLength(description.trim()) < minimumDescriptionLength
getStringLength(description.trim()) <
nullThrows(
minimumDescriptionLength,
'Expected minimumDescriptionLength to be set',
)
) {
context.report({
data: { directive, minimumDescriptionLength },
Expand Down

0 comments on commit da006b1

Please sign in to comment.