[ban-ts-comment] add option to allow a ts-comment if there is a description with it

[dimitropoulos]

[Feature Request] @typescript-eslint/require-ts-comment-description

Rule Details

Examples of 👎 incorrect code for this rule:

// @ts-expect-error
const x: string = 1;

// @ts-ignore
const x: string = 1;

Examples of 👍 correct code for this rule:

// @ts-expect-error because this code is testing a compiler I'm writing
const x: string = 1;

// @ts-ignore: because this code is testing a compiler I'm writing
const x: string = 1;

Options

{
  "rules": {
    "@typescript-eslint/require-ts-comment-description": "error"
  }
}

The configuration looks like this:

interface Options {
  'ts-expect-error'?: boolean;
  'ts-ignore'?: boolean;
  'ts-nocheck'?: boolean;
  'ts-check'?: boolean;
}

const defaultOptions: Options = {
  'ts-expect-error': true,
  'ts-ignore': true,
  'ts-nocheck': true,
  'ts-check': true,
};

Additional Info

There is a similar rule in https://mysticatea.github.io/eslint-plugin-eslint-comments/rules/require-description.html.

I thought typescript-eslint had such a rule, but apparently not. My apologies if I just missed it.

[bbarry]

I don't think a full rule is necessary but instead perhaps options to @typescript-eslint/prefer-ts-expect-error and/or @typescript-eslint/ban-ts-comment.

I'd kinda like it on the former (just for ts-expect-error) and to see that rule added to recommended in 4.0.

[bradzacher]

I agree! I think this makes sense as an option in ban-ts-comment.

I think something like the following configuration makes sense:

interface Options {
  'ts-expect-error'?: boolean | 'allow-with-comment';
  'ts-ignore'?: boolean | 'allow-with-comment';
  'ts-nocheck'?: boolean | 'allow-with-comment';
  'ts-check'?: boolean | 'allow-with-comment';
}

Also agree that with the next major, we can consider setting the default to
ts-expect-error: 'allow-with-comment'

[dimitropoulos]

I don't agree and think it makes sense to have them separate but I can see what you're both talking about and I'm not passionate about it either way - it's the functionality that I'm really interested in, haha. I do like, I'll say (against my own point), the result that you can't get into a situation where you banned a ts-comment but also require a comment (which could happen if they were different rules).

I have some experience with estree and a decent amount with the internals of eslint circa version 5, but it's been a minute. I'd be happy to contribute this update to ban-ts-comment. Do you have any pointers that will be useful before I take my first look?

[bradzacher]

Only the source code!

This rule is one of the simplest we have in our repo, so it's definitely not a huge task to add.

[dimitropoulos]

great, I'll take a look, thanks!

[dimitropoulos]

PR posted #2099

[dimitropoulos]

Also, I was meaning to inquire, does anyone here have any interest in functionality that would allow the user to dictate what kind of thing happens in the description. For example, say I only want to allow @ts-expect-errors that are like this:

• @ts-expect-error UPSTREAM: for errors in upstream types that we're trying to work around
• @ts-expect-error FIXME: for something I know is broken but don't have time to fix in the current PR
• @ts-expect-error TODO: for something that is happening in the course of working (or otherwise needs to be explored) that will hopefully not stay in the codebase for long.

I donno, don't wanna bikeshed but if there's any interest in such a thing now'd be the time to add it. It'd be simple enough to add but I don't feel strongly that it's necessary (although perhaps there are people out there would really like to categorize their directives! I can totally see that use case!).

[bradzacher]

It might be a bit niche? Unsure.
I wouldn't be opposed to adding an option to allow a regex for the description, but it would require some non-trivial reworking to fit a new option in in a backwards-compatible way.

I think that might be better suited to a local rule in your project to help enforce the coding guidelines for your project.

[dimitropoulos]

cool. yeah. I wasn't sure "how far" we wanna take it. I think the added feature of setting the minimum description length is enough of a power-user feature to suffice for now. If it comes up in the future that people want to impose some kind of regulations on the classification of the errors just lemme know.

And in case anyone is turning their nose at the idea of a codebase having so many ts-ignores (and ts-expect-errors) that they need to be classified consider the use-cases of migrating a large codebase from Flow (where you might have lots of loose ends to cinch up over the span of weeks or months due to differences in 3rd party typings or just differences in TypeScript vs Flow) or a large JavaScript project that you are trying to provide stopgap types for. In that case, I have seen conventions develop for the ignores such that people can search a set of ignores later to try attacking them in groups.

Anyway. I'm happy to leave it open ended for now so cool.