diff --git a/.eslintignore b/.eslintignore index 30e85ed0b..224070368 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,3 +1,4 @@ coverage/ lib/ !.eslintrc.js +src/rules/__tests__/fixtures/ diff --git a/README.md b/README.md index 667847434..2c38ceb35 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,7 @@ installations requiring long-term consistency. ## Rules - + | Rule | Description | Configurations | Fixable | | ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ | @@ -173,7 +173,32 @@ installations requiring long-term consistency. | [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | | | [valid-title](docs/rules/valid-title.md) | Enforce valid titles | ![recommended][] | ![fixable][] | - + + +## TypeScript Rules + +In addition to the above rules, this plugin also includes a few advanced rules +that are powered by type-checking information provided by TypeScript. + +In order to use these rules, you must be using `@typescript-eslint/parser` & +adjust your eslint config as outlined +[here](https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/TYPED_LINTING.md) + +Note that unlike the type-checking rules in `@typescript-eslint/eslint-plugin`, +the rules here will fallback to doing nothing if type information is not +available, meaning its safe to include them in shared configs that could be used +on JavaScript and TypeScript projects. + +Also note that `unbound-method` depends on `@typescript-eslint/eslint-plugin`, +as it extends the original `unbound-method` rule from that plugin. + + + +| Rule | Description | Configurations | Fixable | +| ---------------------------------------------- | ------------------------------------------------------------- | -------------- | ------- | +| [unbound-method](docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | + + ## Credit diff --git a/babel.config.js b/babel.config.js index 9da6d0228..35692b174 100644 --- a/babel.config.js +++ b/babel.config.js @@ -7,4 +7,5 @@ module.exports = { '@babel/preset-typescript', ['@babel/preset-env', { targets: { node: 10 } }], ], + ignore: ['src/**/__tests__/fixtures/**'], }; diff --git a/docs/rules/unbound-method.md b/docs/rules/unbound-method.md new file mode 100644 index 000000000..75e38923e --- /dev/null +++ b/docs/rules/unbound-method.md @@ -0,0 +1,54 @@ +# Enforces unbound methods are called with their expected scope (`unbound-method`) + +## Rule Details + +This rule extends the base [`@typescript-eslint/unbound-method`][original-rule] +rule, meaning you must depend on `@typescript-eslint/eslint-plugin` for it to +work. It adds support for understanding when it's ok to pass an unbound method +to `expect` calls. + +See the [`@typescript-eslint` documentation][original-rule] for more details on +the `unbound-method` rule. + +Note that while this rule requires type information to work, it will fail +silently when not available allowing you to safely enable it on projects that +are not using TypeScript. + +## How to use + +```json5 +{ + parser: '@typescript-eslint/parser', + parserOptions: { + project: 'tsconfig.json', + ecmaVersion: 2020, + sourceType: 'module', + }, + overrides: [ + { + files: ['test/**'], + extends: ['jest'], + rules: { + // you should turn the original rule off *only* for test files + '@typescript-eslint/unbound-method': 'off', + 'jest/unbound-method': 'error', + }, + }, + ], + rules: { + '@typescript-eslint/unbound-method': 'error', + }, +} +``` + +This rule should be applied to your test files in place of the original rule, +which should be applied to the rest of your codebase. + +## Options + +See [`@typescript-eslint/unbound-method`][original-rule] options. + +Taken with ❤️ [from `@typescript-eslint` core][original-rule] + +[original-rule]: + https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md diff --git a/package.json b/package.json index 9608cd93d..96956d3b8 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,8 @@ "displayName": "test", "testEnvironment": "node", "testPathIgnorePatterns": [ - "/lib/.*" + "/lib/.*", + "/src/rules/__tests__/fixtures/*" ] }, { @@ -121,8 +122,14 @@ "typescript": "^4.0.0" }, "peerDependencies": { + "@typescript-eslint/eslint-plugin": ">= 4", "eslint": ">=5" }, + "peerDependenciesMeta": { + "@typescript-eslint/eslint-plugin": { + "optional": true + } + }, "engines": { "node": ">=10" }, diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index 3d7972b03..414f67032 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/src/__tests__/__snapshots__/rules.test.ts.snap @@ -46,6 +46,7 @@ Object { "jest/prefer-todo": "error", "jest/require-to-throw-message": "error", "jest/require-top-level-describe": "error", + "jest/unbound-method": "error", "jest/valid-describe": "error", "jest/valid-expect": "error", "jest/valid-expect-in-promise": "error", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index d37e1bd4a..6a8ff2df1 100644 --- a/src/__tests__/rules.test.ts +++ b/src/__tests__/rules.test.ts @@ -2,7 +2,7 @@ import { existsSync } from 'fs'; import { resolve } from 'path'; import plugin from '../'; -const numberOfRules = 44; +const numberOfRules = 45; const ruleNames = Object.keys(plugin.rules); const deprecatedRules = Object.entries(plugin.rules) .filter(([, rule]) => rule.meta.deprecated) diff --git a/src/rules/__tests__/fixtures/class.ts b/src/rules/__tests__/fixtures/class.ts new file mode 100644 index 000000000..89f06af9e --- /dev/null +++ b/src/rules/__tests__/fixtures/class.ts @@ -0,0 +1,13 @@ +// used by no-throw-literal test case to validate custom error +export class Error {} + +// used by unbound-method test case to test imports +export const console = { log() {} }; + +// used by prefer-reduce-type-parameter to test native vs userland check +export class Reducable { + reduce() {} +} + +// used by no-implied-eval test function imports +export class Function {} diff --git a/src/rules/__tests__/fixtures/file.ts b/src/rules/__tests__/fixtures/file.ts new file mode 100644 index 000000000..e69de29bb diff --git a/src/rules/__tests__/fixtures/foo.ts b/src/rules/__tests__/fixtures/foo.ts new file mode 100644 index 000000000..fda4aa0a1 --- /dev/null +++ b/src/rules/__tests__/fixtures/foo.ts @@ -0,0 +1 @@ +export type T = number; diff --git a/src/rules/__tests__/fixtures/indent/indent-invalid-fixture-1.js b/src/rules/__tests__/fixtures/indent/indent-invalid-fixture-1.js new file mode 100644 index 000000000..f03507ff6 --- /dev/null +++ b/src/rules/__tests__/fixtures/indent/indent-invalid-fixture-1.js @@ -0,0 +1,530 @@ +if (a) { + var b = c; + var d = e + * f; + var e = f; // <- +// -> + function g() { + if (h) { + var i = j; + } // <- + } // <- + + while (k) l++; + while (m) { + n--; // -> + } // <- + + do { + o = p + + q; // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + o = p + + q; + } while(r); // <- + + for (var s in t) { + u++; + } + + for (;;) { + v++; // <- + } + + if ( w ) { + x++; + } else if (y) { + z++; // <- + aa++; + } else { // <- + bb++; // -> +} // -> +} + +/**/var b; // NO ERROR: single line multi-line comments followed by code is OK +/* + * + */ var b; // NO ERROR: multi-line comments followed by code is OK + +var arr = [ + a, + b, + c, + function (){ + d + }, // <- + {}, + { + a: b, + c: d, + d: e + }, + [ + f, + g, + h, + i + ], + [j] +]; + +var obj = { + a: { + b: { + c: d, + e: f, + g: h + + i // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + } + }, + g: [ + h, + i, + j, + k + ] +}; + +var arrObject = {a:[ + a, + b, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c +]}; + +var objArray = [{ + a: b, + b: c, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c: d +}]; + +var arrArray = [[ + a, + b, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c +]]; + +var objObject = {a:{ + a: b, + b: c, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c: d +}}; + + +switch (a) { + case 'a': + var a = 'b'; // -> + break; + case 'b': + var a = 'b'; + break; + case 'c': + var a = 'b'; // <- + break; + case 'd': + var a = 'b'; + break; // -> + case 'f': + var a = 'b'; + break; + case 'g': { + var a = 'b'; + break; + } + case 'z': + default: + break; // <- +} + +a.b('hi') + .c(a.b()) // <- + .d(); // <- + +if ( a ) { + if ( b ) { +d.e(f) // -> + .g() // -> + .h(); // -> + + i.j(m) + .k() // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + .l(); // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + + n.o(p) // <- + .q() // <- + .r(); // <- + } +} + +var a = b, + c = function () { + h = i; // -> + j = k; + l = m; // <- + }, + e = { + f: g, + n: o, + p: q + }, + r = [ + s, + t, + u + ]; + +var a = function () { +b = c; // -> + d = e; + f = g; // <- +}; + +function c(a, b) { + if (a || (a && + b)) { // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + return d; + } +} + +if ( a + || b ) { +var x; // -> + var c, + d = function(a, + b) { // <- + a; // -> + b; + c; // <- + } +} + + +a({ + d: 1 +}); + +a( +1 +); + +a( + b({ + d: 1 + }) +); + +a( + b( + c({ + d: 1, + e: 1, + f: 1 + }) + ) +); + +a({ d: 1 }); + +aa( + b({ // NO ERROR: CallExpression args not linted by default + c: d, // -> + e: f, + f: g + }) // -> +); + +aaaaaa( + b, + c, + { + d: a + } +); + +a(b, c, + d, e, + f, g // NO ERROR: alignment of arguments of callExpression not checked + ); // <- + +a( + ); // <- + +aaaaaa( + b, + c, { + d: a + }, { + e: f + } +); + +a.b() + .c(function(){ + var a; + }).d.e; + +if (a == 'b') { + if (c && d) e = f + else g('h').i('j') +} + +a = function (b, c) { + return a(function () { + var d = e + var f = g + var h = i + + if (!j) k('l', (m = n)) + if (o) p + else if (q) r + }) +} + +var a = function() { + "b" + .replace(/a/, "a") + .replace(/bc?/, function(e) { + return "b" + (e.f === 2 ? "c" : "f"); + }) + .replace(/d/, "d"); +}; + +$(b) + .on('a', 'b', function() { $(c).e('f'); }) + .on('g', 'h', function() { $(i).j('k'); }); + +a + .b('c', + 'd'); // NO ERROR: CallExpression args not linted by default + +a + .b('c', [ 'd', function(e) { + e++; + }]); + +var a = function() { + a++; + b++; // <- + c++; // <- + }, + b; + +var b = [ + a, + b, + c + ], + c; + +var c = { + a: 1, + b: 2, + c: 3 + }, + d; + +// holes in arrays indentation +x = [ + 1, + 1, + 1, + 1, + 1, + 1, + 1, + 1, + 1, + 1 +]; + +try { + a++; + b++; // <- +c++; // -> +} catch (d) { + e++; + f++; // <- +g++; // -> +} finally { + h++; + i++; // <- +j++; // -> +} + +if (array.some(function(){ + return true; +})) { +a++; // -> + b++; + c++; // <- +} + +var a = b.c(function() { + d++; + }), + e; + +switch (true) { + case (a + && b): +case (c // -> +&& d): + case (e // <- + && f): + case (g +&& h): + var i = j; // <- + var k = l; + var m = n; // -> +} + +if (a) { + b(); +} +else { +c(); // -> + d(); + e(); // <- +} + +if (a) b(); +else { +c(); // -> + d(); + e(); // <- +} + +if (a) { + b(); +} else c(); + +if (a) { + b(); +} +else c(); + +a(); + +if( "very very long multi line" + + "with weird indentation" ) { + b(); +a(); // -> + c(); // <- +} + +a( "very very long multi line" + + "with weird indentation", function() { + b(); +a(); // -> + c(); // <- + }); // <- + +a = function(content, dom) { + b(); + c(); // <- +d(); // -> +}; + +a = function(content, dom) { + b(); + c(); // <- + d(); // -> + }; + +a = function(content, dom) { + b(); // -> + }; + +a = function(content, dom) { +b(); // -> + }; + +a('This is a terribly long description youll ' + + 'have to read', function () { + b(); // <- + c(); // <- + }); // <- + +if ( + array.some(function(){ + return true; + }) +) { +a++; // -> + b++; + c++; // <- +} + +function c(d) { + return { + e: function(f, g) { + } + }; +} + +function a(b) { + switch(x) { + case 1: + if (foo) { + return 5; + } + } +} + +function a(b) { + switch(x) { + case 1: + c; + } +} + +function a(b) { + switch(x) { + case 1: c; + } +} + +function test() { + var a = 1; + { + a(); + } +} + +{ + a(); +} + +function a(b) { + switch(x) { + case 1: + { // <- + a(); // -> + } + break; + default: + { + b(); + } + } +} + +switch (a) { + default: + if (b) + c(); +} + +function test(x) { + switch (x) { + case 1: + return function() { + var a = 5; + return a; + }; + } +} + +switch (a) { + default: + if (b) + c(); +} diff --git a/src/rules/__tests__/fixtures/indent/indent-valid-fixture-1.js b/src/rules/__tests__/fixtures/indent/indent-valid-fixture-1.js new file mode 100644 index 000000000..5c298429f --- /dev/null +++ b/src/rules/__tests__/fixtures/indent/indent-valid-fixture-1.js @@ -0,0 +1,530 @@ +if (a) { + var b = c; + var d = e + * f; + var e = f; // <- + // -> + function g() { + if (h) { + var i = j; + } // <- + } // <- + + while (k) l++; + while (m) { + n--; // -> + } // <- + + do { + o = p + + q; // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + o = p + + q; + } while(r); // <- + + for (var s in t) { + u++; + } + + for (;;) { + v++; // <- + } + + if ( w ) { + x++; + } else if (y) { + z++; // <- + aa++; + } else { // <- + bb++; // -> + } // -> +} + +/**/var b; // NO ERROR: single line multi-line comments followed by code is OK +/* + * + */ var b; // NO ERROR: multi-line comments followed by code is OK + +var arr = [ + a, + b, + c, + function (){ + d + }, // <- + {}, + { + a: b, + c: d, + d: e + }, + [ + f, + g, + h, + i + ], + [j] +]; + +var obj = { + a: { + b: { + c: d, + e: f, + g: h + + i // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + } + }, + g: [ + h, + i, + j, + k + ] +}; + +var arrObject = {a:[ + a, + b, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c +]}; + +var objArray = [{ + a: b, + b: c, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c: d +}]; + +var arrArray = [[ + a, + b, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c +]]; + +var objObject = {a:{ + a: b, + b: c, // NO ERROR: INDENT ONCE WHEN MULTIPLE INDENTED EXPRESSIONS ARE ON SAME LINE + c: d +}}; + + +switch (a) { + case 'a': + var a = 'b'; // -> + break; + case 'b': + var a = 'b'; + break; + case 'c': + var a = 'b'; // <- + break; + case 'd': + var a = 'b'; + break; // -> + case 'f': + var a = 'b'; + break; + case 'g': { + var a = 'b'; + break; + } + case 'z': + default: + break; // <- +} + +a.b('hi') + .c(a.b()) // <- + .d(); // <- + +if ( a ) { + if ( b ) { + d.e(f) // -> + .g() // -> + .h(); // -> + + i.j(m) + .k() // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + .l(); // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + + n.o(p) // <- + .q() // <- + .r(); // <- + } +} + +var a = b, + c = function () { + h = i; // -> + j = k; + l = m; // <- + }, + e = { + f: g, + n: o, + p: q + }, + r = [ + s, + t, + u + ]; + +var a = function () { + b = c; // -> + d = e; + f = g; // <- +}; + +function c(a, b) { + if (a || (a && + b)) { // NO ERROR: DON'T VALIDATE MULTILINE STATEMENTS + return d; + } +} + +if ( a + || b ) { + var x; // -> + var c, + d = function(a, + b) { // <- + a; // -> + b; + c; // <- + } +} + + +a({ + d: 1 +}); + +a( +1 +); + +a( + b({ + d: 1 + }) +); + +a( + b( + c({ + d: 1, + e: 1, + f: 1 + }) + ) +); + +a({ d: 1 }); + +aa( + b({ // NO ERROR: CallExpression args not linted by default + c: d, // -> + e: f, + f: g + }) // -> +); + +aaaaaa( + b, + c, + { + d: a + } +); + +a(b, c, + d, e, + f, g // NO ERROR: alignment of arguments of callExpression not checked +); // <- + +a( +); // <- + +aaaaaa( + b, + c, { + d: a + }, { + e: f + } +); + +a.b() + .c(function(){ + var a; + }).d.e; + +if (a == 'b') { + if (c && d) e = f + else g('h').i('j') +} + +a = function (b, c) { + return a(function () { + var d = e + var f = g + var h = i + + if (!j) k('l', (m = n)) + if (o) p + else if (q) r + }) +} + +var a = function() { + "b" + .replace(/a/, "a") + .replace(/bc?/, function(e) { + return "b" + (e.f === 2 ? "c" : "f"); + }) + .replace(/d/, "d"); +}; + +$(b) + .on('a', 'b', function() { $(c).e('f'); }) + .on('g', 'h', function() { $(i).j('k'); }); + +a + .b('c', + 'd'); // NO ERROR: CallExpression args not linted by default + +a + .b('c', [ 'd', function(e) { + e++; + }]); + +var a = function() { + a++; + b++; // <- + c++; // <- + }, + b; + +var b = [ + a, + b, + c + ], + c; + +var c = { + a: 1, + b: 2, + c: 3 + }, + d; + +// holes in arrays indentation +x = [ + 1, + 1, + 1, + 1, + 1, + 1, + 1, + 1, + 1, + 1 +]; + +try { + a++; + b++; // <- + c++; // -> +} catch (d) { + e++; + f++; // <- + g++; // -> +} finally { + h++; + i++; // <- + j++; // -> +} + +if (array.some(function(){ + return true; +})) { + a++; // -> + b++; + c++; // <- +} + +var a = b.c(function() { + d++; + }), + e; + +switch (true) { + case (a + && b): + case (c // -> +&& d): + case (e // <- + && f): + case (g +&& h): + var i = j; // <- + var k = l; + var m = n; // -> +} + +if (a) { + b(); +} +else { + c(); // -> + d(); + e(); // <- +} + +if (a) b(); +else { + c(); // -> + d(); + e(); // <- +} + +if (a) { + b(); +} else c(); + +if (a) { + b(); +} +else c(); + +a(); + +if( "very very long multi line" + + "with weird indentation" ) { + b(); + a(); // -> + c(); // <- +} + +a( "very very long multi line" + + "with weird indentation", function() { + b(); + a(); // -> + c(); // <- +}); // <- + +a = function(content, dom) { + b(); + c(); // <- + d(); // -> +}; + +a = function(content, dom) { + b(); + c(); // <- + d(); // -> +}; + +a = function(content, dom) { + b(); // -> +}; + +a = function(content, dom) { + b(); // -> +}; + +a('This is a terribly long description youll ' + + 'have to read', function () { + b(); // <- + c(); // <- +}); // <- + +if ( + array.some(function(){ + return true; + }) +) { + a++; // -> + b++; + c++; // <- +} + +function c(d) { + return { + e: function(f, g) { + } + }; +} + +function a(b) { + switch(x) { + case 1: + if (foo) { + return 5; + } + } +} + +function a(b) { + switch(x) { + case 1: + c; + } +} + +function a(b) { + switch(x) { + case 1: c; + } +} + +function test() { + var a = 1; + { + a(); + } +} + +{ + a(); +} + +function a(b) { + switch(x) { + case 1: + { // <- + a(); // -> + } + break; + default: + { + b(); + } + } +} + +switch (a) { + default: + if (b) + c(); +} + +function test(x) { + switch (x) { + case 1: + return function() { + var a = 5; + return a; + }; + } +} + +switch (a) { + default: + if (b) + c(); +} diff --git a/src/rules/__tests__/fixtures/react.tsx b/src/rules/__tests__/fixtures/react.tsx new file mode 100644 index 000000000..e69de29bb diff --git a/src/rules/__tests__/fixtures/tsconfig-withmeta.json b/src/rules/__tests__/fixtures/tsconfig-withmeta.json new file mode 100644 index 000000000..14fede7cc --- /dev/null +++ b/src/rules/__tests__/fixtures/tsconfig-withmeta.json @@ -0,0 +1,6 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "emitDecoratorMetadata": true, + } +} diff --git a/src/rules/__tests__/fixtures/tsconfig.json b/src/rules/__tests__/fixtures/tsconfig.json new file mode 100644 index 000000000..1e56addad --- /dev/null +++ b/src/rules/__tests__/fixtures/tsconfig.json @@ -0,0 +1,16 @@ +{ + "compilerOptions": { + "jsx": "preserve", + "target": "es5", + "module": "commonjs", + "strict": true, + "esModuleInterop": true, + "lib": ["es2015", "es2017", "esnext"], + "experimentalDecorators": true + }, + "files": ["estree.ts"], + "include": [ + "file.ts", + "react.tsx" + ] +} diff --git a/src/rules/__tests__/fixtures/unstrict/file.ts b/src/rules/__tests__/fixtures/unstrict/file.ts new file mode 100644 index 000000000..e69de29bb diff --git a/src/rules/__tests__/fixtures/unstrict/react.tsx b/src/rules/__tests__/fixtures/unstrict/react.tsx new file mode 100644 index 000000000..e69de29bb diff --git a/src/rules/__tests__/fixtures/unstrict/tsconfig.json b/src/rules/__tests__/fixtures/unstrict/tsconfig.json new file mode 100644 index 000000000..751747ef2 --- /dev/null +++ b/src/rules/__tests__/fixtures/unstrict/tsconfig.json @@ -0,0 +1,15 @@ +{ + "compilerOptions": { + "jsx": "preserve", + "target": "es5", + "module": "commonjs", + "strict": false, + "esModuleInterop": true, + "lib": ["es2015", "es2017", "esnext"], + "experimentalDecorators": true + }, + "include": [ + "file.ts", + "react.tsx" + ] +} diff --git a/src/rules/__tests__/unbound-method.test.ts b/src/rules/__tests__/unbound-method.test.ts new file mode 100644 index 000000000..a425f721a --- /dev/null +++ b/src/rules/__tests__/unbound-method.test.ts @@ -0,0 +1,728 @@ +import path from 'path'; +import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils'; +import dedent from 'dedent'; +import type { MessageIds, Options } from '../unbound-method'; + +function getFixturesRootDir(): string { + return path.join(__dirname, 'fixtures'); +} + +const rootPath = getFixturesRootDir(); + +const ruleTester = new ESLintUtils.RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, +}); + +const ConsoleClassAndVariableCode = dedent` + class Console { + log(str) { + process.stdout.write(str); + } + } + + const console = new Console(); +`; + +const toThrowMatchers = [ + 'toThrow', + 'toThrowError', + 'toThrowErrorMatchingSnapshot', + 'toThrowErrorMatchingInlineSnapshot', +]; + +const validTestCases: string[] = [ + ...[ + 'expect(Console.prototype.log).toHaveBeenCalledTimes(1);', + 'expect(Console.prototype.log).not.toHaveBeenCalled();', + 'expect(Console.prototype.log).toStrictEqual(somethingElse);', + ].map(code => [ConsoleClassAndVariableCode, code].join('\n')), + dedent` + expect(() => { + ${ConsoleClassAndVariableCode} + + expect(Console.prototype.log).toHaveBeenCalledTimes(1); + }).not.toThrow(); + `, + 'expect(() => Promise.resolve().then(console.log)).not.toThrow();', + ...toThrowMatchers.map(matcher => `expect(console.log).not.${matcher}();`), + ...toThrowMatchers.map(matcher => `expect(console.log).${matcher}();`), +]; + +const invalidTestCases: Array> = [ + { + code: dedent` + ${ConsoleClassAndVariableCode} + + expect(Console.prototype.log) + `, + errors: [ + { + line: 9, + messageId: 'unbound', + }, + ], + }, + { + code: 'expect(Console.prototype.log).toHaveBeenCalledTimes', + errors: [ + { + line: 1, + messageId: 'unbound', + }, + ], + }, + { + code: dedent` + expect(() => { + ${ConsoleClassAndVariableCode} + + Promise.resolve().then(console.log); + }).not.toThrow(); + `, + errors: [ + { + line: 10, + messageId: 'unbound', + }, + ], + }, + // toThrow matchers call the expected value (which is expected to be a function) + ...toThrowMatchers.map(matcher => ({ + code: dedent` + ${ConsoleClassAndVariableCode} + + expect(console.log).${matcher}(); + `, + errors: [ + { + line: 9, + messageId: 'unbound' as const, + }, + ], + })), + // toThrow matchers call the expected value (which is expected to be a function) + ...toThrowMatchers.map(matcher => ({ + code: dedent` + ${ConsoleClassAndVariableCode} + + expect(console.log).not.${matcher}(); + `, + errors: [ + { + line: 9, + messageId: 'unbound' as const, + }, + ], + })), +]; + +const requireRule = (throwWhenRequiring: boolean) => { + jest.resetModuleRegistry(); + + TSESLintPluginRef.throwWhenRequiring = throwWhenRequiring; + + // eslint-disable-next-line @typescript-eslint/no-require-imports,node/no-missing-require + return require('../unbound-method').default; +}; + +const TSESLintPluginRef: { throwWhenRequiring: boolean } = { + throwWhenRequiring: false, +}; + +jest.mock('@typescript-eslint/eslint-plugin', () => { + if (TSESLintPluginRef.throwWhenRequiring) { + throw new (class extends Error { + public code; + + constructor(message?: string) { + super(message); + this.code = 'MODULE_NOT_FOUND'; + } + })(); + } + + return jest.requireActual('@typescript-eslint/eslint-plugin'); +}); + +describe('error handling', () => { + describe('when an error is thrown accessing the base rule', () => { + it('re-throws the error', () => { + jest.mock('@typescript-eslint/eslint-plugin', () => { + throw new Error('oh noes!'); + }); + + jest.resetModuleRegistry(); + + // eslint-disable-next-line @typescript-eslint/no-require-imports,node/no-missing-require + expect(() => require('../unbound-method').default).toThrow(/oh noes!/iu); + }); + }); + + describe('when @typescript-eslint/eslint-plugin is not available', () => { + const ruleTester = new ESLintUtils.RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + tsconfigRootDir: rootPath, + project: './tsconfig.json', + }, + }); + + ruleTester.run( + 'unbound-method jest edition without type service', + requireRule(true), + { + valid: validTestCases.concat(invalidTestCases.map(({ code }) => code)), + invalid: [], + }, + ); + }); + + describe('when "project" is not set', () => { + const ruleTester = new ESLintUtils.RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + tsconfigRootDir: rootPath, + }, + }); + + ruleTester.run( + 'unbound-method jest edition without "project" property', + requireRule(false), + { + valid: validTestCases.concat(invalidTestCases.map(({ code }) => code)), + invalid: [], + }, + ); + }); +}); + +ruleTester.run('unbound-method jest edition', requireRule(false), { + valid: validTestCases, + invalid: invalidTestCases, +}); + +function addContainsMethodsClass(code: string): string { + return ` +class ContainsMethods { + bound?: () => void; + unbound?(): void; + + static boundStatic?: () => void; + static unboundStatic?(): void; +} + +let instance = new ContainsMethods(); + +const arith = { + double(this: void, x: number): number { + return x * 2; + } +}; + +${code} + `; +} +function addContainsMethodsClassInvalid( + code: string[], +): Array> { + return code.map(c => ({ + code: addContainsMethodsClass(c), + errors: [ + { + line: 18, + messageId: 'unbound', + }, + ], + })); +} + +ruleTester.run('unbound-method', requireRule(false), { + valid: [ + 'Promise.resolve().then(console.log);', + "['1', '2', '3'].map(Number.parseInt);", + '[5.2, 7.1, 3.6].map(Math.floor);', + 'const x = console.log;', + ...[ + 'instance.bound();', + 'instance.unbound();', + + 'ContainsMethods.boundStatic();', + 'ContainsMethods.unboundStatic();', + + 'const bound = instance.bound;', + 'const boundStatic = ContainsMethods;', + + 'const { bound } = instance;', + 'const { boundStatic } = ContainsMethods;', + + '(instance.bound)();', + '(instance.unbound)();', + + '(ContainsMethods.boundStatic)();', + '(ContainsMethods.unboundStatic)();', + + 'instance.bound``;', + 'instance.unbound``;', + + 'if (instance.bound) { }', + 'if (instance.unbound) { }', + + 'if (instance.bound !== undefined) { }', + 'if (instance.unbound !== undefined) { }', + + 'if (ContainsMethods.boundStatic) { }', + 'if (ContainsMethods.unboundStatic) { }', + + 'if (ContainsMethods.boundStatic !== undefined) { }', + 'if (ContainsMethods.unboundStatic !== undefined) { }', + + 'if (ContainsMethods.boundStatic && instance) { }', + 'if (ContainsMethods.unboundStatic && instance) { }', + + 'if (instance.bound || instance) { }', + 'if (instance.unbound || instance) { }', + + 'ContainsMethods.unboundStatic && 0 || ContainsMethods;', + + '(instance.bound || instance) ? 1 : 0', + '(instance.unbound || instance) ? 1 : 0', + + 'while (instance.bound) { }', + 'while (instance.unbound) { }', + + 'while (instance.bound !== undefined) { }', + 'while (instance.unbound !== undefined) { }', + + 'while (ContainsMethods.boundStatic) { }', + 'while (ContainsMethods.unboundStatic) { }', + + 'while (ContainsMethods.boundStatic !== undefined) { }', + 'while (ContainsMethods.unboundStatic !== undefined) { }', + + 'instance.bound as any;', + 'ContainsMethods.boundStatic as any;', + + 'instance.bound++;', + '+instance.bound;', + '++instance.bound;', + 'instance.bound--;', + '-instance.bound;', + '--instance.bound;', + 'instance.bound += 1;', + 'instance.bound -= 1;', + 'instance.bound *= 1;', + 'instance.bound /= 1;', + + 'instance.bound || 0;', + 'instance.bound && 0;', + + 'instance.bound ? 1 : 0;', + 'instance.unbound ? 1 : 0;', + + 'ContainsMethods.boundStatic++;', + '+ContainsMethods.boundStatic;', + '++ContainsMethods.boundStatic;', + 'ContainsMethods.boundStatic--;', + '-ContainsMethods.boundStatic;', + '--ContainsMethods.boundStatic;', + 'ContainsMethods.boundStatic += 1;', + 'ContainsMethods.boundStatic -= 1;', + 'ContainsMethods.boundStatic *= 1;', + 'ContainsMethods.boundStatic /= 1;', + + 'ContainsMethods.boundStatic || 0;', + 'instane.boundStatic && 0;', + + 'ContainsMethods.boundStatic ? 1 : 0;', + 'ContainsMethods.unboundStatic ? 1 : 0;', + + "typeof instance.bound === 'function';", + "typeof instance.unbound === 'function';", + + "typeof ContainsMethods.boundStatic === 'function';", + "typeof ContainsMethods.unboundStatic === 'function';", + + 'instance.unbound = () => {};', + 'instance.unbound = instance.unbound.bind(instance);', + 'if (!!instance.unbound) {}', + 'void instance.unbound', + 'delete instance.unbound', + + 'const { double } = arith;', + ].map(addContainsMethodsClass), + ` +interface RecordA { + readonly type: 'A'; + readonly a: {}; +} +interface RecordB { + readonly type: 'B'; + readonly b: {}; +} +type AnyRecord = RecordA | RecordB; + +function test(obj: AnyRecord) { + switch (obj.type) { + } +} + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/496 + ` +class CommunicationError { + constructor() { + const x = CommunicationError.prototype; + } +} + `, + ` +class CommunicationError {} +const x = CommunicationError.prototype; + `, + // optional chain + ` +class ContainsMethods { + bound?: () => void; + unbound?(): void; + + static boundStatic?: () => void; + static unboundStatic?(): void; +} + +function foo(instance: ContainsMethods | null) { + instance?.bound(); + instance?.unbound(); + + instance?.bound++; + + if (instance?.bound) { + } + if (instance?.unbound) { + } + + typeof instance?.bound === 'function'; + typeof instance?.unbound === 'function'; +} + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/1425 + ` +interface OptionalMethod { + mightBeDefined?(): void; +} + +const x: OptionalMethod = {}; +declare const myCondition: boolean; +if (myCondition || x.mightBeDefined) { + console.log('hello world'); +} + `, + // https://github.com/typescript-eslint/typescript-eslint/issues/1256 + ` +class A { + unbound(): void { + this.unbound = undefined; + this.unbound = this.unbound.bind(this); + } +} + `, + 'const { parseInt } = Number;', + 'const { log } = console;', + ` +let parseInt; +({ parseInt } = Number); + `, + ` +let log; +({ log } = console); + `, + ` +const foo = { + bar: 'bar', +}; +const { bar } = foo; + `, + ` +class Foo { + unbnound() {} + bar = 4; +} +const { bar } = new Foo(); + `, + ` +class Foo { + bound = () => 'foo'; +} +const { bound } = new Foo(); + `, + ], + invalid: [ + { + code: ` +class Console { + log(str) { + process.stdout.write(str); + } +} + +const console = new Console(); + +Promise.resolve().then(console.log); + `, + errors: [ + { + line: 10, + messageId: 'unbound', + }, + ], + }, + { + code: ` +import { console } from './class'; +const x = console.log; + `, + errors: [ + { + line: 3, + messageId: 'unbound', + }, + ], + }, + { + code: addContainsMethodsClass(` +function foo(arg: ContainsMethods | null) { + const unbound = arg?.unbound; + arg.unbound += 1; + arg?.unbound as any; +} + `), + errors: [ + { + line: 20, + messageId: 'unbound', + }, + { + line: 21, + messageId: 'unbound', + }, + { + line: 22, + messageId: 'unbound', + }, + ], + }, + ...addContainsMethodsClassInvalid([ + 'const unbound = instance.unbound;', + 'const unboundStatic = ContainsMethods.unboundStatic;', + + 'const { unbound } = instance;', + 'const { unboundStatic } = ContainsMethods;', + + 'instance.unbound;', + 'instance.unbound as any;', + + 'ContainsMethods.unboundStatic;', + 'ContainsMethods.unboundStatic as any;', + + 'instance.unbound || 0;', + 'ContainsMethods.unboundStatic || 0;', + + 'instance.unbound ? instance.unbound : null', + ]), + { + code: ` +class ContainsMethods { + unbound?(): void; + + static unboundStatic?(): void; +} + +new ContainsMethods().unbound; + +ContainsMethods.unboundStatic; + `, + options: [ + { + ignoreStatic: true, + }, + ], + errors: [ + { + line: 8, + messageId: 'unbound', + }, + ], + }, + // https://github.com/typescript-eslint/typescript-eslint/issues/496 + { + code: ` +class CommunicationError { + foo() {} +} +const x = CommunicationError.prototype.foo; + `, + errors: [ + { + line: 5, + messageId: 'unbound', + }, + ], + }, + { + // Promise.all is not auto-bound to Promise + code: 'const x = Promise.all;', + errors: [ + { + line: 1, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class Foo { + unbound() {} +} +const instance = new Foo(); + +let x; + +x = instance.unbound; // THIS SHOULD ERROR +instance.unbound = x; // THIS SHOULD NOT + `, + errors: [ + { + line: 9, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class Foo { + unbound = function () {}; +} +const unbound = new Foo().unbound; + `, + errors: [ + { + line: 5, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class Foo { + unbound() {} +} +const { unbound } = new Foo(); + `, + errors: [ + { + line: 5, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class Foo { + unbound = function () {}; +} +const { unbound } = new Foo(); + `, + errors: [ + { + line: 5, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class Foo { + unbound() {} +} +let unbound; +({ unbound } = new Foo()); + `, + errors: [ + { + line: 6, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class Foo { + unbound = function () {}; +} +let unbound; +({ unbound } = new Foo()); + `, + errors: [ + { + line: 6, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class CommunicationError { + foo() {} +} +const { foo } = CommunicationError.prototype; + `, + errors: [ + { + line: 5, + messageId: 'unbound', + }, + ], + }, + { + code: ` +class CommunicationError { + foo() {} +} +let foo; +({ foo } = CommunicationError.prototype); + `, + errors: [ + { + line: 6, + messageId: 'unbound', + }, + ], + }, + { + code: ` +import { console } from './class'; +const { log } = console; + `, + errors: [ + { + line: 3, + messageId: 'unbound', + }, + ], + }, + { + code: 'const { all } = Promise;', + errors: [ + { + line: 1, + messageId: 'unbound', + }, + ], + }, + ], +}); diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts new file mode 100644 index 000000000..33bf27522 --- /dev/null +++ b/src/rules/unbound-method.ts @@ -0,0 +1,111 @@ +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; +import { createRule, isExpectCall, parseExpectCall } from './utils'; + +const toThrowMatchers = [ + 'toThrow', + 'toThrowError', + 'toThrowErrorMatchingSnapshot', + 'toThrowErrorMatchingInlineSnapshot', +]; + +const isJestExpectToThrowCall = (node: TSESTree.CallExpression) => { + if (!isExpectCall(node)) { + return false; + } + + const { matcher } = parseExpectCall(node); + + if (!matcher) { + return false; + } + + return !toThrowMatchers.includes(matcher.name); +}; + +const baseRule = (() => { + try { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const TSESLintPlugin = require('@typescript-eslint/eslint-plugin'); + + return TSESLintPlugin.rules['unbound-method'] as TSESLint.RuleModule< + MessageIds, + Options + >; + } catch (e: unknown) { + const error = e as { code: string }; + + if (error.code === 'MODULE_NOT_FOUND') { + return null; + } + + throw error; + } +})(); + +const tryCreateBaseRule = ( + context: Readonly>, +) => { + try { + return baseRule?.create(context); + } catch { + return null; + } +}; + +interface Config { + ignoreStatic: boolean; +} + +export type Options = [Config]; + +export type MessageIds = 'unbound'; + +export default createRule({ + defaultOptions: [{ ignoreStatic: false }], + ...baseRule, + name: __filename, + meta: { + messages: { + unbound: 'This rule requires `@typescript-eslint/eslint-plugin`', + }, + schema: [], + type: 'problem', + ...baseRule?.meta, + docs: { + category: 'Best Practices', + description: + 'Enforces unbound methods are called with their expected scope', + requiresTypeChecking: true, + ...baseRule?.meta.docs, + recommended: false, + }, + }, + create(context) { + const baseSelectors = tryCreateBaseRule(context); + + if (!baseSelectors) { + return {}; + } + + let inExpectToThrowCall = false; + + return { + ...baseSelectors, + CallExpression(node: TSESTree.CallExpression): void { + inExpectToThrowCall = isJestExpectToThrowCall(node); + }, + 'CallExpression:exit'(node: TSESTree.CallExpression): void { + if (inExpectToThrowCall && isJestExpectToThrowCall(node)) { + inExpectToThrowCall = false; + } + }, + MemberExpression(node: TSESTree.MemberExpression): void { + if (inExpectToThrowCall) { + return; + } + + baseSelectors.MemberExpression?.(node); + }, + }; + }, +}); diff --git a/tools/regenerate-docs.ts b/tools/regenerate-docs.ts index a6d07252c..b8707a132 100644 --- a/tools/regenerate-docs.ts +++ b/tools/regenerate-docs.ts @@ -22,6 +22,7 @@ interface RuleDetails { name: string; description: string; fixable: FixType | false; + requiresTypeChecking: boolean; } type RuleModule = TSESLint.RuleModule & { @@ -63,9 +64,13 @@ const generateRulesListMarkdown = (details: RuleDetails[]): string => .map(column => [...column, ' '].join('|')) .join('\n'); -const updateRulesList = (details: RuleDetails[], markdown: string): string => { - const listBeginMarker = ``; - const listEndMarker = ``; +const updateRulesList = ( + listName: 'base' | 'type', + details: RuleDetails[], + markdown: string, +): string => { + const listBeginMarker = ``; + const listEndMarker = ``; const listStartIndex = markdown.indexOf(listBeginMarker); const listEndIndex = markdown.indexOf(listEndMarker); @@ -112,6 +117,7 @@ const details: RuleDetails[] = Object.keys(config.configs.all.rules) : rule.meta.docs.suggestion ? 'suggest' : false, + requiresTypeChecking: rule.meta.docs.requiresTypeChecking ?? false, }), ); @@ -125,8 +131,18 @@ details.forEach(({ name, description }) => { fs.writeFileSync(pathToDoc, format(contents.join('\n'))); }); +const [baseRules, typeRules] = details.reduce<[RuleDetails[], RuleDetails[]]>( + (groups, details) => { + groups[details.requiresTypeChecking ? 1 : 0].push(details); + + return groups; + }, + [[], []], +); + let readme = fs.readFileSync(pathTo.readme, 'utf8'); -readme = updateRulesList(details, readme); +readme = updateRulesList('base', baseRules, readme); +readme = updateRulesList('type', typeRules, readme); fs.writeFileSync(pathTo.readme, format(readme), 'utf8'); diff --git a/yarn.lock b/yarn.lock index 1ff0461d6..1c5067d8f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -4602,7 +4602,11 @@ __metadata: ts-node: ^9.0.0 typescript: ^4.0.0 peerDependencies: + "@typescript-eslint/eslint-plugin": ">= 4" eslint: ">=5" + peerDependenciesMeta: + "@typescript-eslint/eslint-plugin": + optional: true languageName: unknown linkType: soft