From c0037ae79cef22b45ffddf8a74d4cbc7744cc870 Mon Sep 17 00:00:00 2001 From: Gareth Jones Date: Fri, 22 Jan 2021 14:34:11 +1300 Subject: [PATCH] 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; +}