Add rules that require type information #250
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| // This file is only used to test the config. | ||
| console.log('Hello, world!'); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,12 +15,19 @@ module.exports = { | |
| // (not pre-release) here: https://github.com/tc39/ecma262/releases | ||
| ecmaVersion: 2020, | ||
| sourceType: 'module', | ||
|
|
||
| // This enables support for linting rules that require type information. We | ||
| // assume that the project has a `tsconfig.json` file in the directory where | ||
| // ESLint is being run. | ||
| tsconfigRootDir: process.cwd(), | ||
| project: ['./tsconfig.json'], | ||
| }, | ||
|
|
||
| plugins: ['@typescript-eslint', 'jsdoc'], | ||
|
|
||
| extends: [ | ||
| 'plugin:@typescript-eslint/recommended', | ||
| 'plugin:@typescript-eslint/recommended-requiring-type-checking', | ||
| 'plugin:import/typescript', | ||
| ], | ||
|
|
||
|
|
@@ -59,14 +66,80 @@ module.exports = { | |
| }, | ||
| ], | ||
|
|
||
| // Recommended rules that require type information | ||
| '@typescript-eslint/no-unsafe-argument': 'off', | ||
| '@typescript-eslint/no-unsafe-assignment': 'off', | ||
| '@typescript-eslint/no-unsafe-call': 'off', | ||
| '@typescript-eslint/no-unsafe-member-access': 'off', | ||
| '@typescript-eslint/no-unsafe-return': 'off', | ||
|
Comment on lines
+70
to
+74
There was a problem hiding this comment. All of these have to do with the use of There was a problem hiding this comment. I would really like to enable these rules, but saving them for a future release might be a good idea anyway, to reduce the disruption these other rules will cause. I've created an issue to track this: #251 |
||
|
|
||
| // Our rules that require type information | ||
| '@typescript-eslint/consistent-type-exports': 'error', | ||
|
There was a problem hiding this comment. Up for debate if we want to enable this one. It enforces // foo.ts
export type Foo = string;
export const bar = 'baz';
// bar.ts
import type { Foo } from './foo';
import { bar } from './foo';I think personally it makes it more clear if something is just a type or not. There was a problem hiding this comment. I agree, this seems great to me. |
||
| '@typescript-eslint/naming-convention': [ | ||
| 'error', | ||
| { | ||
| selector: 'default', | ||
| format: ['camelCase'], | ||
| leadingUnderscore: 'allow', | ||
| trailingUnderscore: 'forbid', | ||
| }, | ||
| { | ||
| selector: 'enumMember', | ||
| format: ['PascalCase'], | ||
| }, | ||
| { | ||
| selector: 'interface', | ||
| format: ['PascalCase'], | ||
| custom: { | ||
| regex: '^I[A-Z]', | ||
| match: false, | ||
| }, | ||
| }, | ||
| { | ||
| selector: 'objectLiteralMethod', | ||
| format: ['camelCase', 'PascalCase', 'UPPER_CASE'], | ||
| }, | ||
| { | ||
| selector: 'objectLiteralProperty', | ||
| format: ['camelCase', 'PascalCase', 'UPPER_CASE'], | ||
| }, | ||
| { | ||
| selector: 'typeLike', | ||
| format: ['PascalCase'], | ||
| }, | ||
| { | ||
| selector: 'variable', | ||
| format: ['camelCase', 'UPPER_CASE', 'PascalCase'], | ||
| leadingUnderscore: 'allow', | ||
| }, | ||
| ], | ||
| '@typescript-eslint/no-meaningless-void-operator': 'error', | ||
|
There was a problem hiding this comment. This disables the use of the |
||
| '@typescript-eslint/no-unnecessary-boolean-literal-compare': 'error', | ||
| '@typescript-eslint/no-unnecessary-qualifier': 'error', | ||
|
There was a problem hiding this comment. This makes sure we don't use a qualifier in enums that reference other enum values. Not sure if we ever do this, but regardless I think it's fine to enable it. enum Foo {
Bar,
Baz = Foo.Bar, // Invalid
Qux = Baz, // Valid
} |
||
| '@typescript-eslint/no-unnecessary-type-arguments': 'error', | ||
|
There was a problem hiding this comment. In functions with generics, this makes sure don't specify the default generic value (if any). |
||
| '@typescript-eslint/prefer-includes': 'error', | ||
|
There was a problem hiding this comment. This is to prefer |
||
| '@typescript-eslint/prefer-nullish-coalescing': 'error', | ||
|
There was a problem hiding this comment. This makes sure we use |
||
| '@typescript-eslint/prefer-readonly': 'error', | ||
|
There was a problem hiding this comment. In classes with variables that are never mutated, this makes sure we define it as There was a problem hiding this comment. Hmm. I'm OK with trying this. Not entirely sure how I feel about it yet. There was a problem hiding this comment. We could 🤔 The problem with warnings is that since they don't get highlighted by CI, people don't notice them and they build up over time. That's why we haven't used warnings for anything here so far. There was a problem hiding this comment. I believe there is ways for warnings to show inline in GitHub, but in general I don't think we should use warnings. This rule is auto-fixable, so it's easy to implement regardless. |
||
| '@typescript-eslint/prefer-reduce-type-parameter': 'error', | ||
|
There was a problem hiding this comment. This makes sure we use the generic parameter of const foo = [];
foo.reduce<Bar>(...); // Valid
foo.reduce(reducer, defaultValue as Bar); // Invalid |
||
| '@typescript-eslint/prefer-string-starts-ends-with': 'error', | ||
|
There was a problem hiding this comment. This prefers using |
||
| '@typescript-eslint/promise-function-async': 'error', | ||
|
There was a problem hiding this comment. This makes sure that functions that return a |
||
| '@typescript-eslint/restrict-template-expressions': [ | ||
| 'error', | ||
| { | ||
| allowBoolean: true, | ||
| allowNumber: true, | ||
| }, | ||
| ], | ||
| '@typescript-eslint/switch-exhaustiveness-check': 'error', | ||
|
There was a problem hiding this comment. This makes sure that all the possible values of a union are covered in a switch statement that does not have a |
||
|
|
||
| 'default-param-last': 'off', | ||
| '@typescript-eslint/default-param-last': 'error', | ||
|
|
||
| 'no-shadow': 'off', | ||
| '@typescript-eslint/no-shadow': ['error', { builtinGlobals: true }], | ||
|
|
||
| 'no-throw-literal': 'off', | ||
| '@typescript-eslint/no-throw-literal': 'error', | ||
|
|
||
| 'no-unused-expressions': 'off', | ||
| '@typescript-eslint/no-unused-expressions': [ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| { | ||
| "include": ["src"] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that we always run
eslintfrom the root project here, but that may not always be the case, especially in monorepos. We may have to configure this on a per-project basis.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested this in
snaps-monorepo, and it does not work out of the box with this option. All monorepo packages need to have their own.eslintrc, and specify thetsconfigRootDir. Otherwise ESLint will complain that the files are not included intsconfig.json(since they aren't in the root config).