Skip to content

Commit

Permalink
adds "allow-with-description" option to ban-ts-comments rule
Browse files Browse the repository at this point in the history
  • Loading branch information
@dimitropoulos
dimitropoulos committed May 25, 2020
1 parent a71b9c9 commit f80ba23
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 19 deletions.
2 changes: 1 addition & 1 deletion packages/eslint-plugin/README.md
View file
Expand Up @@ -92,7 +92,7 @@ Pro Tip: For larger codebases you may want to consider splitting our linting int
| [`@typescript-eslint/adjacent-overload-signatures`](./docs/rules/adjacent-overload-signatures.md) | Require that member overloads be consecutive | :heavy_check_mark: | | |
| [`@typescript-eslint/array-type`](./docs/rules/array-type.md) | Requires using either `T[]` or `Array<T>` for arrays | | :wrench: | |
| [`@typescript-eslint/await-thenable`](./docs/rules/await-thenable.md) | Disallows awaiting a value that is not a Thenable | :heavy_check_mark: | | :thought_balloon: |
| [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-<directive>` comments from being used | :heavy_check_mark: | | |
| [`@typescript-eslint/ban-ts-comment`](./docs/rules/ban-ts-comment.md) | Bans `// @ts-<directive>` comments from being used or requires descriptions after directive | :heavy_check_mark: | | |
| [`@typescript-eslint/ban-types`](./docs/rules/ban-types.md) | Bans specific types from being used | :heavy_check_mark: | :wrench: | |
| [`@typescript-eslint/class-literal-property-style`](./docs/rules/class-literal-property-style.md) | Ensures that literals on classes are exposed in a consistent style | | :wrench: | |
| [`@typescript-eslint/consistent-type-assertions`](./docs/rules/consistent-type-assertions.md) | Enforces consistent usage of type assertions | | | |
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-plugin/docs/rules/ban-ts-comment.md
View file
@@ -1,4 +1,4 @@
# Bans `// @ts-<directive>` comments from being used (`ban-ts-comment`)
# Bans `// @ts-<directive>` comments from being used or requires descriptions after directive (`ban-ts-comment`)

TypeScript provides several directive comments that can be used to alter how it processes files.
Using these to suppress TypeScript Compiler Errors reduces the effectiveness of TypeScript overall.
Expand Down
79 changes: 62 additions & 17 deletions packages/eslint-plugin/src/rules/ban-ts-comment.ts
View file
Expand Up @@ -2,10 +2,10 @@ import { AST_TOKEN_TYPES } from '@typescript-eslint/experimental-utils';
import * as util from '../util';

interface Options {
'ts-expect-error'?: boolean;
'ts-ignore'?: boolean;
'ts-nocheck'?: boolean;
'ts-check'?: boolean;
'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';
}

const defaultOptions: [Options] = [
Expand All @@ -17,40 +17,73 @@ const defaultOptions: [Options] = [
},
];

type MessageIds = 'tsDirectiveComment';
type MessageIds =
| 'tsDirectiveComment'
| 'tsDirectiveCommentRequiresDescription';

export default util.createRule<[Options], MessageIds>({
name: 'ban-ts-comment',
meta: {
type: 'problem',
docs: {
description: 'Bans `// @ts-<directive>` comments from being used',
description:
'Bans `// @ts-<directive>` comments from being used or requires descriptions after directive',
category: 'Best Practices',
recommended: 'error',
},
messages: {
tsDirectiveComment:
'Do not use "// @ts-{{directive}}" because it alters compilation errors.',
tsDirectiveCommentRequiresDescription:
'When using "// @ts-{{directive}}" you must also provide a description after the the directive.',
},
schema: [
{
type: 'object',
properties: {
'ts-expect-error': {
type: 'boolean',
default: true,
oneOf: [
{
type: 'boolean',
default: true,
},
{
enum: ['allow-with-description'],
},
],
},
'ts-ignore': {
type: 'boolean',
default: true,
oneOf: [
{
type: 'boolean',
default: true,
},
{
enum: ['allow-with-description'],
},
],
},
'ts-nocheck': {
type: 'boolean',
default: true,
oneOf: [
{
type: 'boolean',
default: true,
},
{
enum: ['allow-with-description'],
},
],
},
'ts-check': {
type: 'boolean',
default: false,
oneOf: [
{
type: 'boolean',
default: true,
},
{
enum: ['allow-with-description'],
},
],
},
},
additionalProperties: false,
Expand All @@ -59,7 +92,7 @@ export default util.createRule<[Options], MessageIds>({
},
defaultOptions,
create(context, [options]) {
const tsCommentRegExp = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)/;
const tsCommentRegExp = /^\/*\s*@ts-(expect-error|ignore|check|nocheck)(.*)/;
const sourceCode = context.getSourceCode();

return {
Expand All @@ -71,17 +104,29 @@ export default util.createRule<[Options], MessageIds>({
return;
}

const [, directive] = tsCommentRegExp.exec(comment.value) ?? [];
const [, directive, description] =
tsCommentRegExp.exec(comment.value) ?? [];

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

if (options[fullDirective]) {
const option = options[fullDirective];
if (option === true) {
context.report({
data: { directive },
node: comment,
messageId: 'tsDirectiveComment',
});
}

if (option === 'allow-with-description') {
if (description.length === 0) {
context.report({
data: { directive },
node: comment,
messageId: 'tsDirectiveCommentRequiresDescription',
});
}
}
});
},
};
Expand Down
66 changes: 66 additions & 0 deletions packages/eslint-plugin/tests/rules/ban-ts-comment.test.ts
View file
Expand Up @@ -19,6 +19,10 @@ ruleTester.run('ts-expect-error', rule, {
code: '// @ts-expect-error',
options: [{ 'ts-expect-error': false }],
},
{
code: '// @ts-expect-error here is why the error is expected',
options: [{ 'ts-expect-error': 'allow-with-description' }],
},
],
invalid: [
{
Expand Down Expand Up @@ -74,6 +78,18 @@ if (false) {
},
],
},
{
code: '// @ts-expect-error',
options: [{ 'ts-expect-error': 'allow-with-description' }],
errors: [
{
data: { directive: 'expect-error' },
messageId: 'tsDirectiveCommentRequiresDescription',
line: 1,
column: 1,
},
],
},
],
});

Expand All @@ -91,6 +107,10 @@ ruleTester.run('ts-ignore', rule, {
code: '// @ts-ignore',
options: [{ 'ts-ignore': false }],
},
{
code: "// @ts-ignore I don't care about this line for... reasons",
options: [{ 'ts-ignore': 'allow-with-description' }],
},
],
invalid: [
{
Expand Down Expand Up @@ -154,6 +174,18 @@ if (false) {
},
],
},
{
code: '// @ts-ignore',
options: [{ 'ts-ignore': 'allow-with-description' }],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveCommentRequiresDescription',
line: 1,
column: 1,
},
],
},
],
});

Expand All @@ -171,6 +203,11 @@ ruleTester.run('ts-nocheck', rule, {
code: '// @ts-nocheck',
options: [{ 'ts-nocheck': false }],
},
{
code:
'// @ts-nocheck no doubt, people will put nonsense here from time to time just to get the rule to stop reporting, perhaps even long messages with other nonsense in them like other // @ts-nocheck or // @ts-ignore things',
options: [{ 'ts-nocheck': 'allow-with-description' }],
},
],
invalid: [
{
Expand Down Expand Up @@ -234,6 +271,18 @@ if (false) {
},
],
},
{
code: '// @ts-nocheck',
options: [{ 'ts-nocheck': 'allow-with-description' }],
errors: [
{
data: { directive: 'nocheck' },
messageId: 'tsDirectiveCommentRequiresDescription',
line: 1,
column: 1,
},
],
},
],
});

Expand All @@ -251,6 +300,11 @@ ruleTester.run('ts-check', rule, {
code: '// @ts-check',
options: [{ 'ts-check': false }],
},
{
code:
'// @ts-check with a description and also with a no-op // @ts-ignore',
options: [{ 'ts-check': 'allow-with-description' }],
},
],
invalid: [
{
Expand Down Expand Up @@ -307,5 +361,17 @@ if (false) {
},
],
},
{
code: '// @ts-ignore',
options: [{ 'ts-ignore': 'allow-with-description' }],
errors: [
{
data: { directive: 'ignore' },
messageId: 'tsDirectiveCommentRequiresDescription',
line: 1,
column: 1,
},
],
},
],
});

0 comments on commit f80ba23

Please sign in to comment.