feat(eslint-plugin)!: recommended-requiring-type-checking config (#846)

BREAKING CHANGE: removed some rules from recommended config
typescript-eslint · Aug 13, 2019 · d3470c9 · d3470c9
1 parent 90b36dd
commit d3470c9
Show file tree

Hide file tree

Showing 36 changed files with 317 additions and 19 deletions.
diff --git a/.eslintrc.js b/.eslintrc.js
@@ -15,6 +15,7 @@ module.exports = {
     'eslint:recommended',
     'plugin:@typescript-eslint/eslint-recommended',
     'plugin:@typescript-eslint/recommended',
+    'plugin:@typescript-eslint/recommended-requiring-type-checking',
   ],
   rules: {
     //

diff --git a/packages/eslint-plugin/README.md b/packages/eslint-plugin/README.md
@@ -51,7 +51,7 @@ You can also enable all the recommended rules for our plugin. Add `plugin:@types
 }
 ```
 
-You can also use [eslint:recommended](https://eslint.org/docs/rules/) with this plugin. Add both `eslint:recommended` and `plugin:@typescript-eslint/eslint-recommended`:
+You can also use [eslint:recommended](https://eslint.org/docs/rules/) (the set of rules which are recommended for all projects by the ESLint Team) with this plugin. As noted in the root README, not all eslint core rules are compatible with TypeScript, so you need to add both `eslint:recommended` and `plugin:@typescript-eslint/eslint-recommended` (which will adjust the one from eslint appropriately for TypeScript) to your config:
 
 ```json
 {
@@ -63,7 +63,24 @@ You can also use [eslint:recommended](https://eslint.org/docs/rules/) with this
 }
 ```
 
-If you want to use rules which require type information, you will need to specify a path to your tsconfig.json file in the "project" property of "parserOptions".
+As of version 2 of this plugin, _by design_, none of the rules in the main `recommended` config require type-checking in order to run. This means that they are more lightweight and faster to run.
+
+Some highly valuable rules simply require type-checking in order to be implemented correctly, however, so we provide an additional config you can extend from called `recommended-requiring-type-checking`. You wou apply this _in addition_ to the recommended configs previously mentioned, e.g.:
+
+```json
+{
+  "extends": [
+    "eslint:recommended",
+    "plugin:@typescript-eslint/eslint-recommended",
+    "plugin:@typescript-eslint/recommended",
+    "plugin:@typescript-eslint/recommended-requiring-type-checking"
+  ]
+}
+```
+
+Pro Tip: For larger codebases you may want to consider splitting our linting into two separate stages: 1. fast feedback rules which operate purely based on syntax (no type-checking), 2. rules which are based on semantics (type-checking).
+
+NOTE: If you want to use rules which require type information, you will need to specify a path to your tsconfig.json file in the "project" property of "parserOptions". If you do not do this, you will get a runtime error which explains this.
 
 ```json
 {

diff --git a/packages/eslint-plugin/package.json b/packages/eslint-plugin/package.json
@@ -30,11 +30,11 @@
   "main": "dist/index.js",
   "scripts": {
     "build": "tsc -p tsconfig.build.json",
-    "check:docs": "ts-node --files ./tools/validate-docs/index.ts",
-    "check:configs": "ts-node --files ./tools/validate-configs/index.ts",
+    "check:docs": "../../node_modules/.bin/ts-node --files ./tools/validate-docs/index.ts",
+    "check:configs": "../../node_modules/.bin/ts-node --files ./tools/validate-configs/index.ts",
     "clean": "rimraf dist/",
     "format": "prettier --write \"./**/*.{ts,js,json,md}\" --ignore-path ../../.prettierignore",
-    "generate:configs": "ts-node --files tools/generate-configs.ts",
+    "generate:configs": "../../node_modules/.bin/ts-node --files tools/generate-configs.ts",
     "prebuild": "npm run clean",
     "test": "jest --coverage",
     "typecheck": "tsc --noEmit"

diff --git a/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json b/packages/eslint-plugin/src/configs/recommended-requiring-type-checking.json
@@ -0,0 +1,19 @@
+{
+  "extends": "./configs/base.json",
+  "rules": {
+    "@typescript-eslint/await-thenable": "error",
+    "@typescript-eslint/no-for-in-array": "error",
+    "@typescript-eslint/no-misused-promises": "error",
+    "@typescript-eslint/no-unnecessary-type-assertion": "error",
+    "@typescript-eslint/prefer-includes": "error",
+    "@typescript-eslint/prefer-regexp-exec": "error",
+    "@typescript-eslint/prefer-string-starts-ends-with": "error",
+    "require-await": "off",
+    "@typescript-eslint/require-await": "error",
+    "@typescript-eslint/unbound-method": "error",
+    "no-var": "error",
+    "prefer-const": "error",
+    "prefer-rest-params": "error",
+    "prefer-spread": "error"
+  }
+}
diff --git a/packages/eslint-plugin/src/configs/recommended.json b/packages/eslint-plugin/src/configs/recommended.json
@@ -2,7 +2,6 @@
   "extends": "./configs/base.json",
   "rules": {
     "@typescript-eslint/adjacent-overload-signatures": "error",
-    "@typescript-eslint/await-thenable": "error",
     "@typescript-eslint/ban-ts-ignore": "error",
     "@typescript-eslint/ban-types": "error",
     "camelcase": "off",
@@ -18,28 +17,19 @@
     "@typescript-eslint/no-empty-function": "error",
     "@typescript-eslint/no-empty-interface": "error",
     "@typescript-eslint/no-explicit-any": "warn",
-    "@typescript-eslint/no-for-in-array": "error",
     "@typescript-eslint/no-inferrable-types": "error",
     "@typescript-eslint/no-misused-new": "error",
-    "@typescript-eslint/no-misused-promises": "error",
     "@typescript-eslint/no-namespace": "error",
     "@typescript-eslint/no-non-null-assertion": "warn",
     "@typescript-eslint/no-this-alias": "error",
-    "@typescript-eslint/no-unnecessary-type-assertion": "error",
     "no-unused-vars": "off",
     "@typescript-eslint/no-unused-vars": "warn",
     "no-use-before-define": "off",
     "@typescript-eslint/no-use-before-define": "error",
     "@typescript-eslint/no-var-requires": "error",
-    "@typescript-eslint/prefer-includes": "error",
     "@typescript-eslint/prefer-namespace-keyword": "error",
-    "@typescript-eslint/prefer-regexp-exec": "error",
-    "@typescript-eslint/prefer-string-starts-ends-with": "error",
-    "require-await": "off",
-    "@typescript-eslint/require-await": "error",
     "@typescript-eslint/triple-slash-reference": "error",
     "@typescript-eslint/type-annotation-spacing": "error",
-    "@typescript-eslint/unbound-method": "error",
     "no-var": "error",
     "prefer-const": "error",
     "prefer-rest-params": "error",

diff --git a/packages/eslint-plugin/src/index.ts b/packages/eslint-plugin/src/index.ts
@@ -3,6 +3,7 @@ import rules from './rules';
 import all from './configs/all.json';
 import base from './configs/base.json';
 import recommended from './configs/recommended.json';
+import recommendedRequiringTypeChecking from './configs/recommended-requiring-type-checking.json';
 import eslintRecommended from './configs/eslint-recommended';
 
 export = {
@@ -12,5 +13,6 @@ export = {
     base,
     recommended,
     'eslint-recommended': eslintRecommended,
+    'recommended-requiring-type-checking': recommendedRequiringTypeChecking,
   },
 };
diff --git a/packages/eslint-plugin/src/rules/await-thenable.ts b/packages/eslint-plugin/src/rules/await-thenable.ts
@@ -10,6 +10,7 @@ export default util.createRule({
       description: 'Disallows awaiting a value that is not a Thenable',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     messages: {
       await: 'Unexpected `await` of a non-Promise (non-"Thenable") value.',

diff --git a/packages/eslint-plugin/src/rules/no-floating-promises.ts b/packages/eslint-plugin/src/rules/no-floating-promises.ts
@@ -10,6 +10,7 @@ export default util.createRule({
       description: 'Requires Promise-like values to be handled appropriately.',
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     messages: {
       floating: 'Promises must be handled appropriately',

diff --git a/packages/eslint-plugin/src/rules/no-for-in-array.ts b/packages/eslint-plugin/src/rules/no-for-in-array.ts
@@ -8,6 +8,7 @@ export default util.createRule({
       description: 'Disallow iterating over an array with a for-in loop',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     messages: {
       forInViolation:

diff --git a/packages/eslint-plugin/src/rules/no-misused-promises.ts b/packages/eslint-plugin/src/rules/no-misused-promises.ts
@@ -18,6 +18,7 @@ export default util.createRule<Options, 'conditional' | 'voidReturn'>({
       description: 'Avoid using promises in places not designed to handle them',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     messages: {
       voidReturn:

diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-qualifier.ts b/packages/eslint-plugin/src/rules/no-unnecessary-qualifier.ts
@@ -10,6 +10,7 @@ export default util.createRule({
       category: 'Best Practices',
       description: 'Warns when a namespace qualifier is unnecessary',
       recommended: false,
+      requiresTypeChecking: true,
     },
     fixable: 'code',
     messages: {

diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-arguments.ts
@@ -29,6 +29,7 @@ export default util.createRule<[], MessageIds>({
         'Warns if an explicitly specified type argument is the default for that type parameter',
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     fixable: 'code',
     messages: {

diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts
@@ -28,6 +28,7 @@ export default util.createRule<Options, MessageIds>({
         'Warns if a type assertion does not change the type of an expression',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     fixable: 'code',
     messages: {

diff --git a/packages/eslint-plugin/src/rules/prefer-includes.ts b/packages/eslint-plugin/src/rules/prefer-includes.ts
@@ -14,6 +14,7 @@ export default createRule({
       description: 'Enforce `includes` method over `indexOf` method',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     fixable: 'code',
     messages: {

diff --git a/packages/eslint-plugin/src/rules/prefer-readonly.ts b/packages/eslint-plugin/src/rules/prefer-readonly.ts
@@ -32,6 +32,7 @@ export default util.createRule<Options, MessageIds>({
         "Requires that private members are marked as `readonly` if they're never modified outside of the constructor",
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     fixable: 'code',
     messages: {

diff --git a/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts b/packages/eslint-plugin/src/rules/prefer-regexp-exec.ts
@@ -13,6 +13,7 @@ export default createRule({
         'Prefer RegExp#exec() over String#match() if no global flag is provided',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     messages: {
       regExpExecOverStringMatch: 'Use the `RegExp#exec()` method instead.',

diff --git a/packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts b/packages/eslint-plugin/src/rules/prefer-string-starts-ends-with.ts
@@ -21,6 +21,7 @@ export default createRule({
         'Enforce the use of `String#startsWith` and `String#endsWith` instead of other equivalent methods of checking substrings',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     messages: {
       preferStartsWith: "Use 'String#startsWith' method instead.",

diff --git a/packages/eslint-plugin/src/rules/promise-function-async.ts b/packages/eslint-plugin/src/rules/promise-function-async.ts
@@ -22,6 +22,7 @@ export default util.createRule<Options, MessageIds>({
         'Requires any function or method that returns a Promise to be marked async',
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     messages: {
       missingAsync: 'Functions that return promises must be async.',

diff --git a/packages/eslint-plugin/src/rules/require-array-sort-compare.ts b/packages/eslint-plugin/src/rules/require-array-sort-compare.ts
@@ -12,6 +12,7 @@ export default util.createRule({
       description: 'Enforce giving `compare` argument to `Array#sort`',
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     messages: {
       requireCompare: "Require 'compare' argument.",

diff --git a/packages/eslint-plugin/src/rules/require-await.ts b/packages/eslint-plugin/src/rules/require-await.ts
@@ -24,6 +24,7 @@ export default util.createRule<Options, MessageIds>({
       description: 'Disallow async functions which have no `await` expression',
       category: 'Best Practices',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     schema: baseRule.meta.schema,
     messages: baseRule.meta.messages,

diff --git a/packages/eslint-plugin/src/rules/restrict-plus-operands.ts b/packages/eslint-plugin/src/rules/restrict-plus-operands.ts
@@ -11,6 +11,7 @@ export default util.createRule({
         'When adding two variables, operands must both be of type number or of type string',
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     messages: {
       notNumbers:

diff --git a/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts b/packages/eslint-plugin/src/rules/strict-boolean-expressions.ts
@@ -27,6 +27,7 @@ export default util.createRule<Options, 'strictBooleanExpression'>({
       description: 'Restricts the types allowed in boolean expressions',
       category: 'Best Practices',
       recommended: false,
+      requiresTypeChecking: true,
     },
     schema: [
       {

diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts
@@ -26,6 +26,7 @@ export default util.createRule<Options, MessageIds>({
       description:
         'Enforces unbound methods are called with their expected scope',
       recommended: 'error',
+      requiresTypeChecking: true,
     },
     messages: {
       unbound:

diff --git a/packages/eslint-plugin/tools/generate-configs.ts b/packages/eslint-plugin/tools/generate-configs.ts
@@ -58,6 +58,7 @@ function reducer<TMessageIds extends string>(
   settings: {
     errorLevel?: 'error' | 'warn';
     filterDeprecated: boolean;
+    filterRequiresTypeChecking?: 'include' | 'exclude';
   },
 ): LinterConfigRules {
   const key = entry[0];
@@ -67,6 +68,22 @@ function reducer<TMessageIds extends string>(
     return config;
   }
 
+  // Explicitly exclude rules requiring type-checking
+  if (
+    settings.filterRequiresTypeChecking === 'exclude' &&
+    value.meta.docs.requiresTypeChecking === true
+  ) {
+    return config;
+  }
+
+  // Explicitly include rules requiring type-checking
+  if (
+    settings.filterRequiresTypeChecking === 'include' &&
+    value.meta.docs.requiresTypeChecking !== true
+  ) {
+    return config;
+  }
+
   const ruleName = `${RULE_NAME_PREFIX}${key}`;
   const recommendation = value.meta.docs.recommended;
   const usedSetting = settings.errorLevel
@@ -119,7 +136,7 @@ writeConfig(baseConfig, path.resolve(__dirname, '../src/configs/base.json'));
 
 console.log();
 console.log(
-  '---------------------------------- all.json ----------------------------------',
+  '------------------------------------------------ all.json ------------------------------------------------',
 );
 const allConfig: LinterConfig = {
   extends: './configs/base.json',
@@ -133,12 +150,16 @@ writeConfig(allConfig, path.resolve(__dirname, '../src/configs/all.json'));
 
 console.log();
 console.log(
-  '------------------------------ recommended.json ------------------------------',
+  '------------------------------ recommended.json (should not require program) ------------------------------',
 );
 const recommendedRules = ruleEntries
   .filter(entry => !!entry[1].meta.docs.recommended)
   .reduce<LinterConfigRules>(
-    (config, entry) => reducer(config, entry, { filterDeprecated: false }),
+    (config, entry) =>
+      reducer(config, entry, {
+        filterDeprecated: false,
+        filterRequiresTypeChecking: 'exclude',
+      }),
     {},
   );
 BASE_RULES_THAT_ARE_RECOMMENDED.forEach(ruleName => {
@@ -152,3 +173,32 @@ writeConfig(
   recommendedConfig,
   path.resolve(__dirname, '../src/configs/recommended.json'),
 );
+
+console.log();
+console.log(
+  '--------------------------------- recommended-requiring-type-checking.json ---------------------------------',
+);
+const recommendedRulesRequiringProgram = ruleEntries
+  .filter(entry => !!entry[1].meta.docs.recommended)
+  .reduce<LinterConfigRules>(
+    (config, entry) =>
+      reducer(config, entry, {
+        filterDeprecated: false,
+        filterRequiresTypeChecking: 'include',
+      }),
+    {},
+  );
+BASE_RULES_THAT_ARE_RECOMMENDED.forEach(ruleName => {
+  recommendedRulesRequiringProgram[ruleName] = 'error';
+});
+const recommendedRequiringTypeCheckingConfig: LinterConfig = {
+  extends: './configs/base.json',
+  rules: recommendedRulesRequiringProgram,
+};
+writeConfig(
+  recommendedRequiringTypeCheckingConfig,
+  path.resolve(
+    __dirname,
+    '../src/configs/recommended-requiring-type-checking.json',
+  ),
+);
diff --git a/packages/eslint-plugin/tools/validate-configs/checkConfigRecommended.ts b/packages/eslint-plugin/tools/validate-configs/checkConfigRecommended.ts
@@ -10,7 +10,11 @@ function checkConfigRecommended(): boolean {
   const recommendedNames = new Set(Object.keys(recommended));
 
   return Object.entries(rules).reduce<boolean>((acc, [ruleName, rule]) => {
-    if (!rule.meta.deprecated && rule.meta.docs.recommended !== false) {
+    if (
+      !rule.meta.deprecated &&
+      rule.meta.docs.recommended !== false &&
+      rule.meta.docs.requiresTypeChecking !== true
+    ) {
       const prefixed = `${prefix}${ruleName}` as keyof typeof recommended;
       if (recommendedNames.has(prefixed)) {
         if (recommended[prefixed] !== rule.meta.docs.recommended) {