TypeScript Specific Linter

[Skillz4Killz]

Hello 👋

I was hoping to open a discussion about possibly having a specific set of linter rules for TypeScript. Some rules in ESLint are because of the foot guns in JS which TS already prevents. An extra unnecessary clause for ESLint is just not nice to have. I just tried the deno lint on my project today and already noticed one of these situations.

This rule is good and 100% makes sense to have for JS. But when you move to TS, this becomes irrelevant.

In my opinion, a lot of rules of a linter is just clutter when you use TS. Your files/code just end up with silly comments ignoring stuff that makes it appear like the code isn't right.

A config file to disable rules is not the solution either. If you think about it why do people love the fact that Deno works so nicely with TS? User experience! When you have support for it built-in, user experience skyrockets. First-class TS support should also apply to the linter so that the linting rules take into consideration TS users. It should just work out the box without having to disable or deal with unnecessary rules.

Please 🙏

Thank You!

[magurotuna]

I agree with you. Indeed there are not a few rules that are not necessary if we use TypeScript. To give TS users good user experience out of the box, we should probably split the rules into two; (a) the rules for JavaScript, (b) for TypeScript. (of course, some rules will join both of them.) Then the linter will be able to automatically decide what rules should be used from a file extension. So we have to list such rules that should not join (b).

[David-Else]

I have not looked into if this is being done already by deno_lint, but when TypeScript is being used then the "plugin:@typescript-eslint/eslint-recommended" rules should override the eslint-recommended rules. This is how the typescript-eslint plugin operates:

https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/eslint-plugin/src/configs#eslint-recommended

https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/src/configs/eslint-recommended.ts

/**
 * This is a compatibility ruleset that:
 * - disables rules from eslint:recommended which are already handled by TypeScript.
 * - enables rules that make sense due to TS's typechecking / transpilation.
 */
export = {
  overrides: [
    {
      files: ['*.ts', '*.tsx'],
      rules: {
        'constructor-super': 'off', // ts(2335) & ts(2377)
        'getter-return': 'off', // ts(2378)
        'no-const-assign': 'off', // ts(2588)
        'no-dupe-args': 'off', // ts(2300)
        'no-dupe-class-members': 'off', // ts(2393) & ts(2300)
        'no-dupe-keys': 'off', // ts(1117)
        'no-func-assign': 'off', // ts(2539)
        'no-import-assign': 'off', // ts(2539) & ts(2540)
        'no-new-symbol': 'off', // ts(2588)
        'no-obj-calls': 'off', // ts(2349)
        'no-redeclare': 'off', // ts(2451)
        'no-setter-return': 'off', // ts(2408)
        'no-this-before-super': 'off', // ts(2376)
        'no-undef': 'off', // ts(2304)
        'no-unreachable': 'off', // ts(7027)
        'no-unsafe-negation': 'off', // ts(2365) & ts(2360) & ts(2358)
        'no-var': 'error', // ts transpiles let/const to var, so no need for vars any more
        'prefer-const': 'error', // ts provides better types with const
        'prefer-rest-params': 'error', // ts provides better types with rest args over arguments
        'prefer-spread': 'error', // ts transpiles spread to apply, so no need for manual apply
        'valid-typeof': 'off', // ts(2367)
      },
    },
  ],
};

[magurotuna]

Sounds great! Currently deno_lint doesn't do such override. It provides always the same rule set even when TypeScript is used, via https://github.com/denoland/deno_lint/blob/v0.1.29/src/rules/mod.rs#L94

So we'd prefer to have something like get_recommended_rules_js and get_recommended_rules_ts, or to allow get_recommended_rules to receive extension parameter. And then in TS case we'd make the rules that @typescript-eslint/eslint-recommended turns off disable.

@bartlomieju This feature is not listed in the roadmap, but it would be better to be introduced ASAP since vscode_deno users run into it and it can be kind of annoying. So I will work on it if you don't mind... WDYT?

[bartlomieju]

While I'm not specifically against this change, I'm also not super keen on it...

typescript-eslint disables some rules because they are handled by TSC (which is run by typescript-eslint) - however deno_lint does not use TSC. So it would be left to developer's IDE to catch those violations. However linter can be run from command line as well and it makes no sense to disable those checks for TS files because one would need to run other subcommand to get code type-checked to get TS diagnostics for same violations. Given than deno_lint runs a lot faster than TSC I think it makes sense to keep those rules for TS files as well - as long as diagnostics produced are consistent with what TSC would return.

Secondly this change would greatly complicate linter logic - eg. VSCode extension lints files by piping them to stdin - if we decide that some rules shouldn't apply to TS files, we will need a way to set what kind of file is being linted - which means adding a new flag to deno lint subcommand, eg. deno lint --extension=ts -, and we've been frugal in adding new flags if it can be avoided.

That said, I'm open to having further discussion about this issue.

[Skillz4Killz]

So it would be left to developer's IDE to catch those violations

Isn't this already true for Deno? Developers already depend on IDE to handle certain things. For example, you need the deno extension for vscode. You can't really code without it. Depending on a good IDE isn't really bad thing. We're okay with it.

However linter can be run from command line as well and it makes no sense to disable those checks for TS files because one would need to run other subcommand to get code type-checked to get TS diagnostics for same violations.

I think most devs when making a change, will run their code to test it locally. If you use TS, your gonna want to compile ur code before deploying. So i don't think this would force devs to do deno run cause were already doing that IMO. Linter won't catch all the errors TS will. We still need to type check by running the compiler, even with these rules and that is where we will find them.

Given than deno_lint runs a lot faster than TSC I think it makes sense to keep those rules for TS files as well - as long as diagnostics produced are consistent with what TSC would return.

I think you are right 100%, but I think we are seeing this from different perspectives. My concerns were never about performance but about user experience.

Question: Why does code need to be made "subjectively" worse because of lint rules that TS already handles for me. Simply put, I am basically losing the benefits of using TS by using the deno linter. Let me show you what I mean by using an example from my project.

// Without Linter
        case 6:
          const newValue = settings.count > 100 ? settings.count - 100 : 0;
          // ....
          break;
        case 7:
          // ....
          break;

// With Deno Lint
        case 6: {
          const newValue = settings.count > 100 ? settings.count - 100 : 0;
          // ....
          break;
         }
        case 7:
          // ....
          break;

So now this shows a very simplified example.

1. The linter creates extra lines that are not needed for the brackets.
2. The linter creates unnecessary complexity and confusion for developers. The fact that case 6 needs a {} and 7 doesn't increases the complexity of user code as now u need to learn and understand what that is for and why its needed in some but not others.

I know that isn't a huge thing. But rules like these add up. Changes like these here and there across more rules all add up to make code significantly less maintainable. The problem here is that if this code is written in a bad way, TS will tell us. Linter shouldn't be forcing modifications that increase complexity/confusion for something TS already handles.

---

IDK much about how this could be implemented but I think a simple solution is just removing rules like this from the base. Eventually, you are going to need some sort of plugin system to allow users to have custom rules be added in. You could reuse that plugin system to allow users to opt-in to rules like this. For example, if i wanted a rule to be added that didn't allow the word null to be used. This could be done through a custom plugin system. EDIT: Just found this #175

A simple rule that could be enforced is that any linter rule that requires configuration should be added in through a custom plugin. The only rules that should be built into deno lint by default are those that are enforced no matter what, for example the no-var rule.

[bartlomieju]

I think we can go ahead with this approach - each lint rule would define what "media types" it operates on - js/ts/jsx/tsx - that would be in line with deno's MediaType.

It would imply that Linter::lint() method would need to be changed to accept some config.

A huge benefit of this approach would be that only rules matching media type would be actually run.

I will look into it.

[dandv]

I'm curious about an update on this, and what people use for linting, Deno's linter which seems to have a much smaller set of rules and far less documentation than typescript-eslint, or the latter. Of particular interest is linting with type information, which could greatly benefit from the much higher performance of Deno's linter.