From 0bfb31a983e59c5e70a408d48c8a19b9dfc02632 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 22 Jan 2021 14:34:11 +1300 Subject: [PATCH 01/12] feat(unbound-method): add original `unbound-method` rule --- src/rules/__tests__/fixtures/class.ts | 13 + src/rules/__tests__/fixtures/file.ts | 0 src/rules/__tests__/fixtures/foo.ts | 1 + .../indent/indent-invalid-fixture-1.js | 530 +++++++++++++++++ .../fixtures/indent/indent-valid-fixture-1.js | 530 +++++++++++++++++ src/rules/__tests__/fixtures/react.tsx | 0 .../__tests__/fixtures/tsconfig-withmeta.json | 6 + src/rules/__tests__/fixtures/tsconfig.json | 16 + src/rules/__tests__/fixtures/unstrict/file.ts | 0 .../__tests__/fixtures/unstrict/react.tsx | 0 .../__tests__/fixtures/unstrict/tsconfig.json | 15 + src/rules/__tests__/unbound-method.test.ts | 537 ++++++++++++++++++ src/rules/unbound-method.ts | 358 ++++++++++++ 13 files changed, 2006 insertions(+) create mode 100644 src/rules/__tests__/fixtures/class.ts create mode 100644 src/rules/__tests__/fixtures/file.ts create mode 100644 src/rules/__tests__/fixtures/foo.ts create mode 100644 src/rules/__tests__/fixtures/indent/indent-invalid-fixture-1.js create mode 100644 src/rules/__tests__/fixtures/indent/indent-valid-fixture-1.js create mode 100644 src/rules/__tests__/fixtures/react.tsx create mode 100644 src/rules/__tests__/fixtures/tsconfig-withmeta.json create mode 100644 src/rules/__tests__/fixtures/tsconfig.json create mode 100644 src/rules/__tests__/fixtures/unstrict/file.ts create mode 100644 src/rules/__tests__/fixtures/unstrict/react.tsx create mode 100644 src/rules/__tests__/fixtures/unstrict/tsconfig.json create mode 100644 src/rules/__tests__/unbound-method.test.ts create mode 100644 src/rules/unbound-method.ts 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..4987fc7e1 --- /dev/null +++ b/src/rules/__tests__/fixtures/tsconfig-withmeta.json @@ -0,0 +1,6 @@ +{ + "extends": "./tsconfig.json", + "compilerOptions": { + "emitDecoratorMetadata": true, + } +} \ No newline at end of file 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..11307f81e --- /dev/null +++ b/src/rules/__tests__/unbound-method.test.ts @@ -0,0 +1,537 @@ +import path from 'path'; +import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils'; +import rule, { 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', + }, +}); + +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', rule, { + 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..f1b778f30 --- /dev/null +++ b/src/rules/unbound-method.ts @@ -0,0 +1,358 @@ +import { + ASTUtils, + AST_NODE_TYPES, + ESLintUtils, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import * as ts from 'typescript'; +import { createRule } from './utils'; + +const tsutils = { + hasModifier( + modifiers: ts.ModifiersArray | undefined, + ...kinds: Array + ) { + if (modifiers === undefined) { + return false; + } + + for (const modifier of modifiers) { + if (kinds.includes(modifier.kind)) { + return true; + } + } + + return false; + }, +}; + +const util = { + createRule, + getParserServices: ESLintUtils.getParserServices, + isIdentifier: ASTUtils.isIdentifier, +}; + +//------------------------------------------------------------------------------ +// Rule Definition +//------------------------------------------------------------------------------ + +interface Config { + ignoreStatic: boolean; +} + +export type Options = [Config]; + +export type MessageIds = 'unbound'; + +/** + * The following is a list of exceptions to the rule + * Generated via the following script. + * This is statically defined to save making purposely invalid calls every lint run + * ``` + SUPPORTED_GLOBALS.flatMap(namespace => { + const object = window[namespace]; + return Object.getOwnPropertyNames(object) + .filter( + name => + !name.startsWith('_') && + typeof object[name] === 'function', + ) + .map(name => { + try { + const x = object[name]; + x(); + } catch (e) { + if (e.message.includes("called on non-object")) { + return `${namespace}.${name}`; + } + } + }); +}).filter(Boolean); + * ``` + */ +const nativelyNotBoundMembers = new Set([ + 'Promise.all', + 'Promise.race', + 'Promise.resolve', + 'Promise.reject', + 'Promise.allSettled', + 'Object.defineProperties', + 'Object.defineProperty', + 'Reflect.defineProperty', + 'Reflect.deleteProperty', + 'Reflect.get', + 'Reflect.getOwnPropertyDescriptor', + 'Reflect.getPrototypeOf', + 'Reflect.has', + 'Reflect.isExtensible', + 'Reflect.ownKeys', + 'Reflect.preventExtensions', + 'Reflect.set', + 'Reflect.setPrototypeOf', +]); +const SUPPORTED_GLOBALS = [ + 'Number', + 'Object', + 'String', + 'RegExp', + 'Symbol', + 'Array', + 'Proxy', + 'Date', + 'Infinity', + 'Atomics', + 'Reflect', + 'console', + 'Math', + 'JSON', + 'Intl', +] as const; +const nativelyBoundMembers = SUPPORTED_GLOBALS.map(namespace => { + if (!(namespace in global)) { + // node.js might not have namespaces like Intl depending on compilation options + // https://nodejs.org/api/intl.html#intl_options_for_building_node_js + return []; + } + const object = global[namespace]; + + return Object.getOwnPropertyNames(object) + .filter( + name => + !name.startsWith('_') && + typeof (object as Record)[name] === 'function', + ) + .map(name => `${namespace}.${name}`); +}) + .reduce((arr, names) => arr.concat(names), []) + .filter(name => !nativelyNotBoundMembers.has(name)); + +const isNotImported = ( + symbol: ts.Symbol, + currentSourceFile: ts.SourceFile | undefined, +): boolean => { + const { valueDeclaration } = symbol; + + if (!valueDeclaration) { + // working around https://github.com/microsoft/TypeScript/issues/31294 + return false; + } + + return ( + !!currentSourceFile && + currentSourceFile !== valueDeclaration.getSourceFile() + ); +}; + +const getNodeName = (node: TSESTree.Node): string | null => + node.type === AST_NODE_TYPES.Identifier ? node.name : null; + +const getMemberFullName = (node: TSESTree.MemberExpression): string => + `${getNodeName(node.object)}.${getNodeName(node.property)}`; + +export default util.createRule({ + name: 'unbound-method', + meta: { + docs: { + category: 'Best Practices', + description: + 'Enforces unbound methods are called with their expected scope', + recommended: 'error', + requiresTypeChecking: true, + }, + messages: { + unbound: + 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.', + }, + schema: [ + { + type: 'object', + properties: { + ignoreStatic: { + type: 'boolean', + }, + }, + additionalProperties: false, + }, + ], + type: 'problem', + }, + defaultOptions: [ + { + ignoreStatic: false, + }, + ], + create(context, [{ ignoreStatic }]) { + const parserServices = util.getParserServices(context); + const checker = parserServices.program.getTypeChecker(); + const currentSourceFile = parserServices.program.getSourceFile( + context.getFilename(), + ); + + return { + MemberExpression(node: TSESTree.MemberExpression): void { + if (isSafeUse(node)) { + return; + } + + const objectSymbol = checker.getSymbolAtLocation( + parserServices.esTreeNodeToTSNodeMap.get(node.object), + ); + + if ( + objectSymbol && + nativelyBoundMembers.includes(getMemberFullName(node)) && + isNotImported(objectSymbol, currentSourceFile) + ) { + return; + } + + const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); + const symbol = checker.getSymbolAtLocation(originalNode); + + if (symbol && isDangerousMethod(symbol, ignoreStatic)) { + context.report({ + messageId: 'unbound', + node, + }); + } + }, + 'VariableDeclarator, AssignmentExpression'( + node: TSESTree.VariableDeclarator | TSESTree.AssignmentExpression, + ): void { + const [idNode, initNode] = + node.type === AST_NODE_TYPES.VariableDeclarator + ? [node.id, node.init] + : [node.left, node.right]; + + if (initNode && idNode.type === AST_NODE_TYPES.ObjectPattern) { + const tsNode = parserServices.esTreeNodeToTSNodeMap.get(initNode); + const rightSymbol = checker.getSymbolAtLocation(tsNode); + const initTypes = checker.getTypeAtLocation(tsNode); + + const notImported = + rightSymbol && isNotImported(rightSymbol, currentSourceFile); + + idNode.properties.forEach(property => { + if ( + property.type === AST_NODE_TYPES.Property && + property.key.type === AST_NODE_TYPES.Identifier + ) { + if ( + notImported && + util.isIdentifier(initNode) && + nativelyBoundMembers.includes( + `${initNode.name}.${property.key.name}`, + ) + ) { + return; + } + + const symbol = initTypes.getProperty(property.key.name); + + if (symbol && isDangerousMethod(symbol, ignoreStatic)) { + context.report({ + messageId: 'unbound', + node, + }); + } + } + }); + } + }, + }; + }, +}); + +function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { + const { valueDeclaration } = symbol; + + if (!valueDeclaration) { + // working around https://github.com/microsoft/TypeScript/issues/31294 + return false; + } + + switch (valueDeclaration.kind) { + case ts.SyntaxKind.PropertyDeclaration: + return ( + (valueDeclaration as ts.PropertyDeclaration).initializer?.kind === + ts.SyntaxKind.FunctionExpression + ); + case ts.SyntaxKind.MethodDeclaration: + case ts.SyntaxKind.MethodSignature: { + const decl = valueDeclaration as + | ts.MethodDeclaration + | ts.MethodSignature; + const [firstParam] = decl.parameters; + const thisArgIsVoid = + firstParam?.name.kind === ts.SyntaxKind.Identifier && + firstParam?.name.escapedText === 'this' && + firstParam?.type?.kind === ts.SyntaxKind.VoidKeyword; + + return ( + !thisArgIsVoid && + !( + ignoreStatic && + tsutils.hasModifier( + valueDeclaration.modifiers, + ts.SyntaxKind.StaticKeyword, + ) + ) + ); + } + } + + return false; +} + +function isSafeUse(node: TSESTree.Node): boolean { + const { parent } = node; + + switch (parent?.type) { + case AST_NODE_TYPES.IfStatement: + case AST_NODE_TYPES.ForStatement: + case AST_NODE_TYPES.MemberExpression: + case AST_NODE_TYPES.SwitchStatement: + case AST_NODE_TYPES.UpdateExpression: + case AST_NODE_TYPES.WhileStatement: + return true; + + case AST_NODE_TYPES.CallExpression: + return parent.callee === node; + + case AST_NODE_TYPES.ConditionalExpression: + return parent.test === node; + + case AST_NODE_TYPES.TaggedTemplateExpression: + return parent.tag === node; + + case AST_NODE_TYPES.UnaryExpression: + // the first case is safe for obvious + // reasons. The second one is also fine + // since we're returning something falsy + return ['typeof', '!', 'void', 'delete'].includes(parent.operator); + + case AST_NODE_TYPES.BinaryExpression: + return ['instanceof', '==', '!=', '===', '!=='].includes(parent.operator); + + case AST_NODE_TYPES.AssignmentExpression: + return parent.operator === '=' && node === parent.left; + + case AST_NODE_TYPES.ChainExpression: + case AST_NODE_TYPES.TSNonNullExpression: + case AST_NODE_TYPES.TSAsExpression: + case AST_NODE_TYPES.TSTypeAssertion: + return isSafeUse(parent); + + case AST_NODE_TYPES.LogicalExpression: + if (parent.operator === '&&' && parent.left === node) { + // this is safe, as && will return the left if and only if it's falsy + return true; + } + + // in all other cases, it's likely the logical expression will return the method ref + // so make sure the parent is a safe usage + return isSafeUse(parent); + } + + return false; +} From aa1a3d6af3520515ef081ecac340b24bcd70cf92 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 21 Feb 2021 11:10:37 +1300 Subject: [PATCH 02/12] feat(unbound-method): modify rule for `jest` --- .eslintignore | 1 + README.md | 1 + docs/rules/unbound-method.md | 53 +++++++++ package.json | 3 +- .../__snapshots__/rules.test.ts.snap | 1 + src/__tests__/rules.test.ts | 2 +- src/rules/__tests__/unbound-method.test.ts | 108 ++++++++++++++++++ src/rules/unbound-method.ts | 66 ++++++++++- 8 files changed, 229 insertions(+), 6 deletions(-) create mode 100644 docs/rules/unbound-method.md 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..9e1d07a6a 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,7 @@ installations requiring long-term consistency. | [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] | | [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | | [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | +| [unbound-method](docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | | [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | | | [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | | | [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md) | Enforce having return statement when testing with promises | ![recommended][] | | diff --git a/docs/rules/unbound-method.md b/docs/rules/unbound-method.md new file mode 100644 index 000000000..df660ac46 --- /dev/null +++ b/docs/rules/unbound-method.md @@ -0,0 +1,53 @@ +# 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. 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..aeadd11b1 100644 --- a/package.json +++ b/package.json @@ -62,7 +62,8 @@ "displayName": "test", "testEnvironment": "node", "testPathIgnorePatterns": [ - "/lib/.*" + "/lib/.*", + "/src/rules/__tests__/fixtures/*" ] }, { 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__/unbound-method.test.ts b/src/rules/__tests__/unbound-method.test.ts index 11307f81e..91013b101 100644 --- a/src/rules/__tests__/unbound-method.test.ts +++ b/src/rules/__tests__/unbound-method.test.ts @@ -1,5 +1,6 @@ import path from 'path'; import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils'; +import dedent from 'dedent'; import rule, { MessageIds, Options } from '../unbound-method'; function getFixturesRootDir(): string { @@ -17,6 +18,113 @@ const ruleTester = new ESLintUtils.RuleTester({ }, }); +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)', + '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: '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, + }, + ], + })), +]; + +ruleTester.run('unbound-method jest edition', rule, { + valid: validTestCases, + invalid: invalidTestCases, +}); + +new ESLintUtils.RuleTester({ + parser: '@typescript-eslint/parser', + parserOptions: { + sourceType: 'module', + tsconfigRootDir: rootPath, + }, +}).run('unbound-method jest edition without type service', rule, { + valid: validTestCases.concat(invalidTestCases.map(({ code }) => code)), + invalid: [], +}); + function addContainsMethodsClass(code: string): string { return ` class ContainsMethods { diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts index f1b778f30..a65fb84dd 100644 --- a/src/rules/unbound-method.ts +++ b/src/rules/unbound-method.ts @@ -2,11 +2,49 @@ import { ASTUtils, AST_NODE_TYPES, ESLintUtils, + ParserServices, + TSESLint, TSESTree, } from '@typescript-eslint/experimental-utils'; import * as ts from 'typescript'; -import { createRule } from './utils'; +import { createRule, isExpectCall, parseExpectCall } from './utils'; +//------------------------------------------------------------------------------ +// eslint-plugin-jest extension +//------------------------------------------------------------------------------ + +const getParserServices = ( + context: Readonly>, +): ParserServices | null => { + try { + return ESLintUtils.getParserServices(context); + } catch { + return null; + } +}; + +const toThrowMatchers = [ + 'toThrow', + 'toThrowError', + 'toThrowErrorMatchingSnapshot', + 'toThrowErrorMatchingInlineSnapshot', +]; + +const isJestExpectCall = (node: TSESTree.CallExpression) => { + if (!isExpectCall(node)) { + return false; + } + + const { matcher } = parseExpectCall(node); + + return !toThrowMatchers.includes(matcher?.name ?? ''); +}; + +//------------------------------------------------------------------------------ +// Inlining of external dependencies +//------------------------------------------------------------------------------ + +/* istanbul ignore next */ const tsutils = { hasModifier( modifiers: ts.ModifiersArray | undefined, @@ -28,7 +66,6 @@ const tsutils = { const util = { createRule, - getParserServices: ESLintUtils.getParserServices, isIdentifier: ASTUtils.isIdentifier, }; @@ -108,6 +145,7 @@ const SUPPORTED_GLOBALS = [ 'Intl', ] as const; const nativelyBoundMembers = SUPPORTED_GLOBALS.map(namespace => { + /* istanbul ignore if */ if (!(namespace in global)) { // node.js might not have namespaces like Intl depending on compilation options // https://nodejs.org/api/intl.html#intl_options_for_building_node_js @@ -156,7 +194,7 @@ export default util.createRule({ category: 'Best Practices', description: 'Enforces unbound methods are called with their expected scope', - recommended: 'error', + recommended: false, requiresTypeChecking: true, }, messages: { @@ -182,14 +220,33 @@ export default util.createRule({ }, ], create(context, [{ ignoreStatic }]) { - const parserServices = util.getParserServices(context); + const parserServices = getParserServices(context); + + if (!parserServices) { + return {}; + } + const checker = parserServices.program.getTypeChecker(); const currentSourceFile = parserServices.program.getSourceFile( context.getFilename(), ); + let inExpectCall = false; + return { + CallExpression(node: TSESTree.CallExpression): void { + inExpectCall = isJestExpectCall(node); + }, + 'CallExpression:exit'(node: TSESTree.CallExpression): void { + if (inExpectCall && isJestExpectCall(node)) { + inExpectCall = false; + } + }, MemberExpression(node: TSESTree.MemberExpression): void { + if (inExpectCall) { + return; + } + if (isSafeUse(node)) { return; } @@ -233,6 +290,7 @@ export default util.createRule({ rightSymbol && isNotImported(rightSymbol, currentSourceFile); idNode.properties.forEach(property => { + /* istanbul ignore else */ if ( property.type === AST_NODE_TYPES.Property && property.key.type === AST_NODE_TYPES.Identifier From 2f79c3e579d545e978c2c1151bc530f8d535a5f8 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 21 Feb 2021 17:08:44 +1300 Subject: [PATCH 03/12] refactor(unbound-method): extend base rule --- src/rules/__tests__/unbound-method.test.ts | 79 ++++- src/rules/unbound-method.ts | 393 ++------------------- 2 files changed, 104 insertions(+), 368 deletions(-) diff --git a/src/rules/__tests__/unbound-method.test.ts b/src/rules/__tests__/unbound-method.test.ts index 91013b101..481bdd3a1 100644 --- a/src/rules/__tests__/unbound-method.test.ts +++ b/src/rules/__tests__/unbound-method.test.ts @@ -1,7 +1,7 @@ import path from 'path'; import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils'; import dedent from 'dedent'; -import rule, { MessageIds, Options } from '../unbound-method'; +import type { MessageIds, Options } from '../unbound-method'; function getFixturesRootDir(): string { return path.join(__dirname, 'fixtures'); @@ -109,20 +109,71 @@ const invalidTestCases: Array> = [ })), ]; -ruleTester.run('unbound-method jest edition', rule, { - valid: validTestCases, - invalid: invalidTestCases, +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 Error('oh noes!'); + } + + return jest.requireActual('@typescript-eslint/eslint-plugin'); }); -new ESLintUtils.RuleTester({ - parser: '@typescript-eslint/parser', - parserOptions: { - sourceType: 'module', - tsconfigRootDir: rootPath, - }, -}).run('unbound-method jest edition without type service', rule, { - valid: validTestCases.concat(invalidTestCases.map(({ code }) => code)), - invalid: [], +describe('error handling', () => { + 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 { @@ -160,7 +211,7 @@ function addContainsMethodsClassInvalid( })); } -ruleTester.run('unbound-method', rule, { +ruleTester.run('unbound-method', requireRule(false), { valid: [ 'Promise.resolve().then(console.log);', "['1', '2', '3'].map(Number.parseInt);", diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts index a65fb84dd..0d9912e75 100644 --- a/src/rules/unbound-method.ts +++ b/src/rules/unbound-method.ts @@ -1,28 +1,6 @@ -import { - ASTUtils, - AST_NODE_TYPES, - ESLintUtils, - ParserServices, - TSESLint, - TSESTree, -} from '@typescript-eslint/experimental-utils'; -import * as ts from 'typescript'; +import { TSESLint, TSESTree } from '@typescript-eslint/experimental-utils'; import { createRule, isExpectCall, parseExpectCall } from './utils'; -//------------------------------------------------------------------------------ -// eslint-plugin-jest extension -//------------------------------------------------------------------------------ - -const getParserServices = ( - context: Readonly>, -): ParserServices | null => { - try { - return ESLintUtils.getParserServices(context); - } catch { - return null; - } -}; - const toThrowMatchers = [ 'toThrow', 'toThrowError', @@ -40,39 +18,30 @@ const isJestExpectCall = (node: TSESTree.CallExpression) => { return !toThrowMatchers.includes(matcher?.name ?? ''); }; -//------------------------------------------------------------------------------ -// Inlining of external dependencies -//------------------------------------------------------------------------------ - -/* istanbul ignore next */ -const tsutils = { - hasModifier( - modifiers: ts.ModifiersArray | undefined, - ...kinds: Array - ) { - if (modifiers === undefined) { - return false; - } - - for (const modifier of modifiers) { - if (kinds.includes(modifier.kind)) { - return true; - } - } - - return false; - }, -}; +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) { + return null; + } +})(); -const util = { - createRule, - isIdentifier: ASTUtils.isIdentifier, +const tryCreateBaseRule = ( + context: Readonly>, +) => { + try { + return baseRule?.create(context); + } catch { + return null; + } }; -//------------------------------------------------------------------------------ -// Rule Definition -//------------------------------------------------------------------------------ - interface Config { ignoreStatic: boolean; } @@ -81,159 +50,37 @@ export type Options = [Config]; export type MessageIds = 'unbound'; -/** - * The following is a list of exceptions to the rule - * Generated via the following script. - * This is statically defined to save making purposely invalid calls every lint run - * ``` - SUPPORTED_GLOBALS.flatMap(namespace => { - const object = window[namespace]; - return Object.getOwnPropertyNames(object) - .filter( - name => - !name.startsWith('_') && - typeof object[name] === 'function', - ) - .map(name => { - try { - const x = object[name]; - x(); - } catch (e) { - if (e.message.includes("called on non-object")) { - return `${namespace}.${name}`; - } - } - }); -}).filter(Boolean); - * ``` - */ -const nativelyNotBoundMembers = new Set([ - 'Promise.all', - 'Promise.race', - 'Promise.resolve', - 'Promise.reject', - 'Promise.allSettled', - 'Object.defineProperties', - 'Object.defineProperty', - 'Reflect.defineProperty', - 'Reflect.deleteProperty', - 'Reflect.get', - 'Reflect.getOwnPropertyDescriptor', - 'Reflect.getPrototypeOf', - 'Reflect.has', - 'Reflect.isExtensible', - 'Reflect.ownKeys', - 'Reflect.preventExtensions', - 'Reflect.set', - 'Reflect.setPrototypeOf', -]); -const SUPPORTED_GLOBALS = [ - 'Number', - 'Object', - 'String', - 'RegExp', - 'Symbol', - 'Array', - 'Proxy', - 'Date', - 'Infinity', - 'Atomics', - 'Reflect', - 'console', - 'Math', - 'JSON', - 'Intl', -] as const; -const nativelyBoundMembers = SUPPORTED_GLOBALS.map(namespace => { - /* istanbul ignore if */ - if (!(namespace in global)) { - // node.js might not have namespaces like Intl depending on compilation options - // https://nodejs.org/api/intl.html#intl_options_for_building_node_js - return []; - } - const object = global[namespace]; - - return Object.getOwnPropertyNames(object) - .filter( - name => - !name.startsWith('_') && - typeof (object as Record)[name] === 'function', - ) - .map(name => `${namespace}.${name}`); -}) - .reduce((arr, names) => arr.concat(names), []) - .filter(name => !nativelyNotBoundMembers.has(name)); - -const isNotImported = ( - symbol: ts.Symbol, - currentSourceFile: ts.SourceFile | undefined, -): boolean => { - const { valueDeclaration } = symbol; - - if (!valueDeclaration) { - // working around https://github.com/microsoft/TypeScript/issues/31294 - return false; - } - - return ( - !!currentSourceFile && - currentSourceFile !== valueDeclaration.getSourceFile() - ); -}; - -const getNodeName = (node: TSESTree.Node): string | null => - node.type === AST_NODE_TYPES.Identifier ? node.name : null; - -const getMemberFullName = (node: TSESTree.MemberExpression): string => - `${getNodeName(node.object)}.${getNodeName(node.property)}`; - -export default util.createRule({ - name: 'unbound-method', +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', - recommended: false, requiresTypeChecking: true, + ...baseRule?.meta.docs, + recommended: false, }, - messages: { - unbound: - 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.', - }, - schema: [ - { - type: 'object', - properties: { - ignoreStatic: { - type: 'boolean', - }, - }, - additionalProperties: false, - }, - ], - type: 'problem', }, - defaultOptions: [ - { - ignoreStatic: false, - }, - ], - create(context, [{ ignoreStatic }]) { - const parserServices = getParserServices(context); + create(context) { + const baseSelectors = tryCreateBaseRule(context); - if (!parserServices) { + if (!baseSelectors) { return {}; } - const checker = parserServices.program.getTypeChecker(); - const currentSourceFile = parserServices.program.getSourceFile( - context.getFilename(), - ); - let inExpectCall = false; return { + ...baseSelectors, CallExpression(node: TSESTree.CallExpression): void { inExpectCall = isJestExpectCall(node); }, @@ -247,170 +94,8 @@ export default util.createRule({ return; } - if (isSafeUse(node)) { - return; - } - - const objectSymbol = checker.getSymbolAtLocation( - parserServices.esTreeNodeToTSNodeMap.get(node.object), - ); - - if ( - objectSymbol && - nativelyBoundMembers.includes(getMemberFullName(node)) && - isNotImported(objectSymbol, currentSourceFile) - ) { - return; - } - - const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); - const symbol = checker.getSymbolAtLocation(originalNode); - - if (symbol && isDangerousMethod(symbol, ignoreStatic)) { - context.report({ - messageId: 'unbound', - node, - }); - } - }, - 'VariableDeclarator, AssignmentExpression'( - node: TSESTree.VariableDeclarator | TSESTree.AssignmentExpression, - ): void { - const [idNode, initNode] = - node.type === AST_NODE_TYPES.VariableDeclarator - ? [node.id, node.init] - : [node.left, node.right]; - - if (initNode && idNode.type === AST_NODE_TYPES.ObjectPattern) { - const tsNode = parserServices.esTreeNodeToTSNodeMap.get(initNode); - const rightSymbol = checker.getSymbolAtLocation(tsNode); - const initTypes = checker.getTypeAtLocation(tsNode); - - const notImported = - rightSymbol && isNotImported(rightSymbol, currentSourceFile); - - idNode.properties.forEach(property => { - /* istanbul ignore else */ - if ( - property.type === AST_NODE_TYPES.Property && - property.key.type === AST_NODE_TYPES.Identifier - ) { - if ( - notImported && - util.isIdentifier(initNode) && - nativelyBoundMembers.includes( - `${initNode.name}.${property.key.name}`, - ) - ) { - return; - } - - const symbol = initTypes.getProperty(property.key.name); - - if (symbol && isDangerousMethod(symbol, ignoreStatic)) { - context.report({ - messageId: 'unbound', - node, - }); - } - } - }); - } + baseSelectors.MemberExpression?.(node); }, }; }, }); - -function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { - const { valueDeclaration } = symbol; - - if (!valueDeclaration) { - // working around https://github.com/microsoft/TypeScript/issues/31294 - return false; - } - - switch (valueDeclaration.kind) { - case ts.SyntaxKind.PropertyDeclaration: - return ( - (valueDeclaration as ts.PropertyDeclaration).initializer?.kind === - ts.SyntaxKind.FunctionExpression - ); - case ts.SyntaxKind.MethodDeclaration: - case ts.SyntaxKind.MethodSignature: { - const decl = valueDeclaration as - | ts.MethodDeclaration - | ts.MethodSignature; - const [firstParam] = decl.parameters; - const thisArgIsVoid = - firstParam?.name.kind === ts.SyntaxKind.Identifier && - firstParam?.name.escapedText === 'this' && - firstParam?.type?.kind === ts.SyntaxKind.VoidKeyword; - - return ( - !thisArgIsVoid && - !( - ignoreStatic && - tsutils.hasModifier( - valueDeclaration.modifiers, - ts.SyntaxKind.StaticKeyword, - ) - ) - ); - } - } - - return false; -} - -function isSafeUse(node: TSESTree.Node): boolean { - const { parent } = node; - - switch (parent?.type) { - case AST_NODE_TYPES.IfStatement: - case AST_NODE_TYPES.ForStatement: - case AST_NODE_TYPES.MemberExpression: - case AST_NODE_TYPES.SwitchStatement: - case AST_NODE_TYPES.UpdateExpression: - case AST_NODE_TYPES.WhileStatement: - return true; - - case AST_NODE_TYPES.CallExpression: - return parent.callee === node; - - case AST_NODE_TYPES.ConditionalExpression: - return parent.test === node; - - case AST_NODE_TYPES.TaggedTemplateExpression: - return parent.tag === node; - - case AST_NODE_TYPES.UnaryExpression: - // the first case is safe for obvious - // reasons. The second one is also fine - // since we're returning something falsy - return ['typeof', '!', 'void', 'delete'].includes(parent.operator); - - case AST_NODE_TYPES.BinaryExpression: - return ['instanceof', '==', '!=', '===', '!=='].includes(parent.operator); - - case AST_NODE_TYPES.AssignmentExpression: - return parent.operator === '=' && node === parent.left; - - case AST_NODE_TYPES.ChainExpression: - case AST_NODE_TYPES.TSNonNullExpression: - case AST_NODE_TYPES.TSAsExpression: - case AST_NODE_TYPES.TSTypeAssertion: - return isSafeUse(parent); - - case AST_NODE_TYPES.LogicalExpression: - if (parent.operator === '&&' && parent.left === node) { - // this is safe, as && will return the left if and only if it's falsy - return true; - } - - // in all other cases, it's likely the logical expression will return the method ref - // so make sure the parent is a safe usage - return isSafeUse(parent); - } - - return false; -} From 5715f91fb9eb1ef6cc3244ac48aeda9d18d71192 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 7 Mar 2021 09:00:08 +1300 Subject: [PATCH 04/12] docs(readme): add section about type-based rules --- README.md | 27 ++++++++++++++++++++++++--- tools/regenerate-docs.ts | 24 ++++++++++++++++++++---- 2 files changed, 44 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 9e1d07a6a..fbda8e3cd 100644 --- a/README.md +++ b/README.md @@ -128,7 +128,7 @@ installations requiring long-term consistency. ## Rules - + | Rule | Description | Configurations | Fixable | | ---------------------------------------------------------------------------- | --------------------------------------------------------------- | ---------------- | ------------ | @@ -168,13 +168,34 @@ installations requiring long-term consistency. | [prefer-todo](docs/rules/prefer-todo.md) | Suggest using `test.todo` | | ![fixable][] | | [require-to-throw-message](docs/rules/require-to-throw-message.md) | Require a message for `toThrow()` | | | | [require-top-level-describe](docs/rules/require-top-level-describe.md) | Require test cases and hooks to be inside a `describe` block | | | -| [unbound-method](docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | | [valid-describe](docs/rules/valid-describe.md) | Enforce valid `describe()` callback | ![recommended][] | | | [valid-expect](docs/rules/valid-expect.md) | Enforce valid `expect()` usage | ![recommended][] | | | [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. + + + +| Rule | Description | Configurations | Fixable | +| ---------------------------------------------- | ------------------------------------------------------------- | -------------- | ------- | +| [unbound-method](docs/rules/unbound-method.md) | Enforces unbound methods are called with their expected scope | | | + + ## Credit 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'); From d956d0d274d3c3764e08d454d24176dd32aa01d7 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 7 Mar 2021 09:01:26 +1300 Subject: [PATCH 05/12] chore: add `@typescript-eslint/eslint-plugin` as an optional peer dep --- package.json | 6 ++++++ yarn.lock | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/package.json b/package.json index aeadd11b1..96956d3b8 100644 --- a/package.json +++ b/package.json @@ -122,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/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 From 9df61ab7787c0941aee0014b690c00a747bb1a18 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 7 Mar 2021 09:05:50 +1300 Subject: [PATCH 06/12] fix(unbound-method): re-throw errors that are not `MODULE_NOT_FOUND` --- src/rules/__tests__/unbound-method.test.ts | 22 +++++++++++++++++++++- src/rules/unbound-method.ts | 10 ++++++++-- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/rules/__tests__/unbound-method.test.ts b/src/rules/__tests__/unbound-method.test.ts index 481bdd3a1..2478774b3 100644 --- a/src/rules/__tests__/unbound-method.test.ts +++ b/src/rules/__tests__/unbound-method.test.ts @@ -124,13 +124,33 @@ const TSESLintPluginRef: { throwWhenRequiring: boolean } = { jest.mock('@typescript-eslint/eslint-plugin', () => { if (TSESLintPluginRef.throwWhenRequiring) { - throw new Error('oh noes!'); + 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', diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts index 0d9912e75..57065a62d 100644 --- a/src/rules/unbound-method.ts +++ b/src/rules/unbound-method.ts @@ -27,8 +27,14 @@ const baseRule = (() => { MessageIds, Options >; - } catch (e) { - return null; + } catch (e: unknown) { + const error = e as { code: string }; + + if (error.code === 'MODULE_NOT_FOUND') { + return null; + } + + throw error; } })(); From 3e6eab84102c362ce81678a6594945a301757011 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 7 Mar 2021 09:09:47 +1300 Subject: [PATCH 07/12] chore(unbound-method): use early return instead of empty string --- src/rules/unbound-method.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts index 57065a62d..7aad84d0f 100644 --- a/src/rules/unbound-method.ts +++ b/src/rules/unbound-method.ts @@ -15,7 +15,11 @@ const isJestExpectCall = (node: TSESTree.CallExpression) => { const { matcher } = parseExpectCall(node); - return !toThrowMatchers.includes(matcher?.name ?? ''); + if (!matcher) { + return false; + } + + return !toThrowMatchers.includes(matcher.name); }; const baseRule = (() => { From b5f312bf4e7ce43b69c9fd3c9ca631cc6d0b74d4 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 7 Mar 2021 09:32:58 +1300 Subject: [PATCH 08/12] test(unbound-method): adjust test --- src/rules/__tests__/unbound-method.test.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/rules/__tests__/unbound-method.test.ts b/src/rules/__tests__/unbound-method.test.ts index 2478774b3..a425f721a 100644 --- a/src/rules/__tests__/unbound-method.test.ts +++ b/src/rules/__tests__/unbound-method.test.ts @@ -37,7 +37,6 @@ const toThrowMatchers = [ const validTestCases: string[] = [ ...[ - 'expect(Console.prototype.log)', 'expect(Console.prototype.log).toHaveBeenCalledTimes(1);', 'expect(Console.prototype.log).not.toHaveBeenCalled();', 'expect(Console.prototype.log).toStrictEqual(somethingElse);', @@ -55,6 +54,19 @@ const validTestCases: string[] = [ ]; const invalidTestCases: Array> = [ + { + code: dedent` + ${ConsoleClassAndVariableCode} + + expect(Console.prototype.log) + `, + errors: [ + { + line: 9, + messageId: 'unbound', + }, + ], + }, { code: 'expect(Console.prototype.log).toHaveBeenCalledTimes', errors: [ From 9610a0821c133a59f29582c48def4e9c075f44a3 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 7 Mar 2021 10:00:52 +1300 Subject: [PATCH 09/12] build: ignore test fixtures --- babel.config.js | 1 + 1 file changed, 1 insertion(+) 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/**'], }; From b6e175e181de66700102aa05e2f1e48976fae99b Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 8 Mar 2021 07:21:12 +1300 Subject: [PATCH 10/12] test: add end of file newline to fixture --- src/rules/__tests__/fixtures/tsconfig-withmeta.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rules/__tests__/fixtures/tsconfig-withmeta.json b/src/rules/__tests__/fixtures/tsconfig-withmeta.json index 4987fc7e1..14fede7cc 100644 --- a/src/rules/__tests__/fixtures/tsconfig-withmeta.json +++ b/src/rules/__tests__/fixtures/tsconfig-withmeta.json @@ -3,4 +3,4 @@ "compilerOptions": { "emitDecoratorMetadata": true, } -} \ No newline at end of file +} From ce08e5eceedb71cbc10ee559f14c47c68fd488a8 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Mon, 8 Mar 2021 07:25:26 +1300 Subject: [PATCH 11/12] docs(unbound-method): mention `@typescript-eslint/eslint-plugin` dep --- README.md | 3 +++ docs/rules/unbound-method.md | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index fbda8e3cd..2c38ceb35 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,9 @@ 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 | diff --git a/docs/rules/unbound-method.md b/docs/rules/unbound-method.md index df660ac46..75e38923e 100644 --- a/docs/rules/unbound-method.md +++ b/docs/rules/unbound-method.md @@ -3,7 +3,8 @@ ## Rule Details This rule extends the base [`@typescript-eslint/unbound-method`][original-rule] -rule. It adds support for understanding when it's ok to pass an unbound method +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 From 5d1612a6d3c497697607090a34936ef8a8c7ab11 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Sun, 14 Mar 2021 10:00:17 +1300 Subject: [PATCH 12/12] refactor(unbound-method): improve method & variable name --- src/rules/unbound-method.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts index 7aad84d0f..33bf27522 100644 --- a/src/rules/unbound-method.ts +++ b/src/rules/unbound-method.ts @@ -8,7 +8,7 @@ const toThrowMatchers = [ 'toThrowErrorMatchingInlineSnapshot', ]; -const isJestExpectCall = (node: TSESTree.CallExpression) => { +const isJestExpectToThrowCall = (node: TSESTree.CallExpression) => { if (!isExpectCall(node)) { return false; } @@ -87,20 +87,20 @@ export default createRule({ return {}; } - let inExpectCall = false; + let inExpectToThrowCall = false; return { ...baseSelectors, CallExpression(node: TSESTree.CallExpression): void { - inExpectCall = isJestExpectCall(node); + inExpectToThrowCall = isJestExpectToThrowCall(node); }, 'CallExpression:exit'(node: TSESTree.CallExpression): void { - if (inExpectCall && isJestExpectCall(node)) { - inExpectCall = false; + if (inExpectToThrowCall && isJestExpectToThrowCall(node)) { + inExpectToThrowCall = false; } }, MemberExpression(node: TSESTree.MemberExpression): void { - if (inExpectCall) { + if (inExpectToThrowCall) { return; }