From d2bdf81bfc0bde5e75c962551d4e198899d12fa0 Mon Sep 17 00:00:00 2001 From: Anix Date: Fri, 17 Apr 2020 13:31:05 +0000 Subject: [PATCH 1/7] chore: ignores super methods --- .../eslint-plugin/src/rules/unbound-method.ts | 5 ++++- .../tests/rules/unbound-method.test.ts | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 407d04c11ec..3842762b7d5 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -305,7 +305,10 @@ function isSafeUse(node: TSESTree.Node): boolean { return ['instanceof', '==', '!=', '===', '!=='].includes(parent.operator); case AST_NODE_TYPES.AssignmentExpression: - return parent.operator === '=' && node === parent.left; + return ( + parent.operator === '=' && + (node === parent.left || node.object.type === AST_NODE_TYPES.Super) + ); case AST_NODE_TYPES.ChainExpression: case AST_NODE_TYPES.TSNonNullExpression: diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index 83f3b93524c..d57549995e5 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -54,6 +54,25 @@ ruleTester.run('unbound-method', rule, { "['1', '2', '3'].map(Number.parseInt);", '[5.2, 7.1, 3.6].map(Math.floor);', 'const x = console.log;', + ` +class BaseClass { + x: number = 42; + logThis() { + console.log('x is '); + } +} + +class OtherClass extends BaseClass { + superLogThis: any; + constructor() { + super(); + this.superLogThis = super.logThis; // X - Incorrect eslint error + } +} + +const oc = new OtherClass(); +oc.superLogThis(); + `, ...[ 'instance.bound();', 'instance.unbound();', From eca791d185588fe459e74659aac1c16b55463f09 Mon Sep 17 00:00:00 2001 From: Anix Date: Tue, 21 Apr 2020 16:13:21 +0000 Subject: [PATCH 2/7] chore: removed comments --- packages/eslint-plugin/tests/rules/unbound-method.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index d57549995e5..02c3181bb39 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -66,7 +66,7 @@ class OtherClass extends BaseClass { superLogThis: any; constructor() { super(); - this.superLogThis = super.logThis; // X - Incorrect eslint error + this.superLogThis = super.logThis; } } From b6e1d4059aebd78664b0237d5d2707f3301c364c Mon Sep 17 00:00:00 2001 From: Anix Date: Tue, 21 Apr 2020 16:49:37 +0000 Subject: [PATCH 3/7] chore: options for allowSuper --- .../eslint-plugin/src/rules/unbound-method.ts | 14 +++++-- .../tests/rules/unbound-method.test.ts | 42 ++++++++++++++++++- 2 files changed, 52 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 3842762b7d5..84fec93a540 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -12,6 +12,7 @@ import * as util from '../util'; interface Config { ignoreStatic: boolean; + allowSuper?: boolean; } export type Options = [Config]; @@ -142,6 +143,9 @@ export default util.createRule({ ignoreStatic: { type: 'boolean', }, + allowSuper: { + type: 'boolean', + }, }, additionalProperties: false, }, @@ -151,9 +155,10 @@ export default util.createRule({ defaultOptions: [ { ignoreStatic: false, + allowSuper: false, }, ], - create(context, [{ ignoreStatic }]) { + create(context, [{ ignoreStatic, allowSuper }]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); const currentSourceFile = parserServices.program.getSourceFile( @@ -274,7 +279,7 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { return false; } -function isSafeUse(node: TSESTree.Node): boolean { +function isSafeUse(node: TSESTree.Node, allowSuper: boolean = false): boolean { const parent = node.parent; switch (parent?.type) { @@ -307,7 +312,10 @@ function isSafeUse(node: TSESTree.Node): boolean { case AST_NODE_TYPES.AssignmentExpression: return ( parent.operator === '=' && - (node === parent.left || node.object.type === AST_NODE_TYPES.Super) + (node === parent.left || + (allowSuper && + (node as TSESTree.MemberExpression) && + node.object.type === AST_NODE_TYPES.Super)) ); case AST_NODE_TYPES.ChainExpression: diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index 02c3181bb39..fda2e6f0e23 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -54,7 +54,8 @@ ruleTester.run('unbound-method', rule, { "['1', '2', '3'].map(Number.parseInt);", '[5.2, 7.1, 3.6].map(Math.floor);', 'const x = console.log;', - ` + { + code: ` class BaseClass { x: number = 42; logThis() { @@ -73,6 +74,12 @@ class OtherClass extends BaseClass { const oc = new OtherClass(); oc.superLogThis(); `, + options: [ + { + allowSuper: true, + }, + ], + }, ...[ 'instance.bound();', 'instance.unbound();', @@ -548,5 +555,38 @@ const { log } = console; }, ], }, + { + code: ` +class BaseClass { + x: number = 42; + logThis() { + console.log('x is '); + } +} + +class OtherClass extends BaseClass { + superLogThis: any; + constructor() { + super(); + this.superLogThis = super.logThis; + } +} + +const oc = new OtherClass(); +oc.superLogThis(); + `, + options: [ + { + allowSuper: false, + }, + ], + errors: [ + { + line: 13, + column: 25, + messageId: 'unbound', + }, + ], + }, ], }); From d7b8a6ce6864dbc062fbf81faccca1d042327f77 Mon Sep 17 00:00:00 2001 From: Anix Date: Tue, 21 Apr 2020 17:03:33 +0000 Subject: [PATCH 4/7] chore: refactored the docs and formatting --- .../docs/rules/unbound-method.md | 22 ++++++++++++++++++- .../eslint-plugin/src/rules/unbound-method.ts | 2 +- .../tests/rules/unbound-method.test.ts | 4 ++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unbound-method.md b/packages/eslint-plugin/docs/rules/unbound-method.md index a9a4fe396a2..3bc6de0cc45 100644 --- a/packages/eslint-plugin/docs/rules/unbound-method.md +++ b/packages/eslint-plugin/docs/rules/unbound-method.md @@ -68,6 +68,7 @@ const { double } = arith; The rule accepts an options object with the following property: - `ignoreStatic` to not check whether `static` methods are correctly bound +- `allowSuper` allows `Super` class methods to bound with base class. ### `ignoreStatic` @@ -86,6 +87,24 @@ const { log } = OtherClass; log(); ``` +### `allowSuper` + +Examples of **correct** code for this rule with `{ allowSuper: true }`: + +```ts +class SuperClass { + method1(){ ... } +} + +class baseClass extend SuperClass { + constructor(){ + super(); + this.baseVar = super.method1; + } +} + +``` + ### Example ```json @@ -93,7 +112,8 @@ log(); "@typescript-eslint/unbound-method": [ "error", { - "ignoreStatic": true + "ignoreStatic": true, + "allowSuper": false } ] } diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 84fec93a540..f753439d320 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -279,7 +279,7 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { return false; } -function isSafeUse(node: TSESTree.Node, allowSuper: boolean = false): boolean { +function isSafeUse(node: TSESTree.Node, allowSuper = false): boolean { const parent = node.parent; switch (parent?.type) { diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index fda2e6f0e23..515a44196de 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -73,7 +73,7 @@ class OtherClass extends BaseClass { const oc = new OtherClass(); oc.superLogThis(); - `, + `, options: [ { allowSuper: true, @@ -574,7 +574,7 @@ class OtherClass extends BaseClass { const oc = new OtherClass(); oc.superLogThis(); - `, + `, options: [ { allowSuper: false, From 9ea3e5d00296d508f66befca8220b8993b27ec2c Mon Sep 17 00:00:00 2001 From: Armano Date: Sun, 7 Feb 2021 03:48:40 +0100 Subject: [PATCH 5/7] fix(eslint-plugin): remove allowSuper, update tests and rebase code --- .../docs/rules/unbound-method.md | 18 ------ .../eslint-plugin/src/rules/unbound-method.ts | 12 +--- .../tests/rules/unbound-method.test.ts | 60 +++++++------------ 3 files changed, 25 insertions(+), 65 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unbound-method.md b/packages/eslint-plugin/docs/rules/unbound-method.md index 3bc6de0cc45..ff646272ee5 100644 --- a/packages/eslint-plugin/docs/rules/unbound-method.md +++ b/packages/eslint-plugin/docs/rules/unbound-method.md @@ -87,24 +87,6 @@ const { log } = OtherClass; log(); ``` -### `allowSuper` - -Examples of **correct** code for this rule with `{ allowSuper: true }`: - -```ts -class SuperClass { - method1(){ ... } -} - -class baseClass extend SuperClass { - constructor(){ - super(); - this.baseVar = super.method1; - } -} - -``` - ### Example ```json diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index f753439d320..dc74876682b 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -12,7 +12,6 @@ import * as util from '../util'; interface Config { ignoreStatic: boolean; - allowSuper?: boolean; } export type Options = [Config]; @@ -143,9 +142,6 @@ export default util.createRule({ ignoreStatic: { type: 'boolean', }, - allowSuper: { - type: 'boolean', - }, }, additionalProperties: false, }, @@ -155,10 +151,9 @@ export default util.createRule({ defaultOptions: [ { ignoreStatic: false, - allowSuper: false, }, ], - create(context, [{ ignoreStatic, allowSuper }]) { + create(context, [{ ignoreStatic }]) { const parserServices = util.getParserServices(context); const checker = parserServices.program.getTypeChecker(); const currentSourceFile = parserServices.program.getSourceFile( @@ -279,7 +274,7 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { return false; } -function isSafeUse(node: TSESTree.Node, allowSuper = false): boolean { +function isSafeUse(node: TSESTree.Node): boolean { const parent = node.parent; switch (parent?.type) { @@ -313,8 +308,7 @@ function isSafeUse(node: TSESTree.Node, allowSuper = false): boolean { return ( parent.operator === '=' && (node === parent.left || - (allowSuper && - (node as TSESTree.MemberExpression) && + (node.type === AST_NODE_TYPES.MemberExpression && node.object.type === AST_NODE_TYPES.Super)) ); diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index 515a44196de..30c2c5c99d0 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -54,32 +54,6 @@ ruleTester.run('unbound-method', rule, { "['1', '2', '3'].map(Number.parseInt);", '[5.2, 7.1, 3.6].map(Math.floor);', 'const x = console.log;', - { - code: ` -class BaseClass { - x: number = 42; - logThis() { - console.log('x is '); - } -} - -class OtherClass extends BaseClass { - superLogThis: any; - constructor() { - super(); - this.superLogThis = super.logThis; - } -} - -const oc = new OtherClass(); -oc.superLogThis(); - `, - options: [ - { - allowSuper: true, - }, - ], - }, ...[ 'instance.bound();', 'instance.unbound();', @@ -291,6 +265,24 @@ class Foo { } const { bound } = new Foo(); `, + // https://github.com/typescript-eslint/typescript-eslint/issues/1866 + ` +class BaseClass { + x: number = 42; + logThis() { + console.log('x is '); + } +} +class OtherClass extends BaseClass { + superLogThis: any; + constructor() { + super(); + this.superLogThis = super.logThis; + } +} +const oc = new OtherClass(); +oc.superLogThis(); + `, ], invalid: [ { @@ -555,6 +547,7 @@ const { log } = console; }, ], }, + // https://github.com/typescript-eslint/typescript-eslint/issues/1866 { code: ` class BaseClass { @@ -563,27 +556,18 @@ class BaseClass { console.log('x is '); } } - class OtherClass extends BaseClass { superLogThis: any; constructor() { super(); - this.superLogThis = super.logThis; + const x = super.logThis; // ERROR - unbound method } } - -const oc = new OtherClass(); -oc.superLogThis(); `, - options: [ - { - allowSuper: false, - }, - ], errors: [ { - line: 13, - column: 25, + line: 12, + column: 15, messageId: 'unbound', }, ], From c047e513c5bd79e2abe31381a0323074e1679d73 Mon Sep 17 00:00:00 2001 From: Armano Date: Sun, 7 Feb 2021 03:48:40 +0100 Subject: [PATCH 6/7] fix(eslint-plugin): remove invalid doc changes --- packages/eslint-plugin/docs/rules/unbound-method.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unbound-method.md b/packages/eslint-plugin/docs/rules/unbound-method.md index ff646272ee5..a9a4fe396a2 100644 --- a/packages/eslint-plugin/docs/rules/unbound-method.md +++ b/packages/eslint-plugin/docs/rules/unbound-method.md @@ -68,7 +68,6 @@ const { double } = arith; The rule accepts an options object with the following property: - `ignoreStatic` to not check whether `static` methods are correctly bound -- `allowSuper` allows `Super` class methods to bound with base class. ### `ignoreStatic` @@ -94,8 +93,7 @@ log(); "@typescript-eslint/unbound-method": [ "error", { - "ignoreStatic": true, - "allowSuper": false + "ignoreStatic": true } ] } From 1a24d0c11542884d86d7b5b01870779303b44313 Mon Sep 17 00:00:00 2001 From: Armano Date: Mon, 8 Feb 2021 18:01:46 +0100 Subject: [PATCH 7/7] fix(eslint-plugin): improve safeuse check --- .../eslint-plugin/src/rules/unbound-method.ts | 4 ++- .../tests/rules/unbound-method.test.ts | 36 +++++++++++++------ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index dc74876682b..2ec66bc2aa3 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -309,7 +309,9 @@ function isSafeUse(node: TSESTree.Node): boolean { parent.operator === '=' && (node === parent.left || (node.type === AST_NODE_TYPES.MemberExpression && - node.object.type === AST_NODE_TYPES.Super)) + node.object.type === AST_NODE_TYPES.Super && + parent.left.type === AST_NODE_TYPES.MemberExpression && + parent.left.object.type === AST_NODE_TYPES.ThisExpression)) ); case AST_NODE_TYPES.ChainExpression: diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index 30c2c5c99d0..911206b1f21 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -269,9 +269,7 @@ const { bound } = new Foo(); ` class BaseClass { x: number = 42; - logThis() { - console.log('x is '); - } + logThis() {} } class OtherClass extends BaseClass { superLogThis: any; @@ -551,26 +549,44 @@ const { log } = console; { code: ` class BaseClass { - x: number = 42; - logThis() { - console.log('x is '); - } + logThis() {} } class OtherClass extends BaseClass { - superLogThis: any; constructor() { super(); - const x = super.logThis; // ERROR - unbound method + const x = super.logThis; } } `, errors: [ { - line: 12, + line: 8, column: 15, messageId: 'unbound', }, ], }, + // https://github.com/typescript-eslint/typescript-eslint/issues/1866 + { + code: ` +class BaseClass { + logThis() {} +} +class OtherClass extends BaseClass { + constructor() { + super(); + let x; + x = super.logThis; + } +} + `, + errors: [ + { + line: 9, + column: 9, + messageId: 'unbound', + }, + ], + }, ], });