From 878dd4ae8c408f1eb42790a8fac37f85040b7f3c Mon Sep 17 00:00:00 2001 From: Ryota Kameoka Date: Thu, 26 Nov 2020 05:28:44 +0900 Subject: [PATCH] feat(eslint-plugin): [unbound-method] add support for methods with a `this: void` parameter (#2796) --- .../docs/rules/unbound-method.md | 16 ++++++++++++ .../eslint-plugin/src/rules/unbound-method.ts | 25 ++++++++++++++----- .../tests/rules/unbound-method.test.ts | 16 +++++++++--- 3 files changed, 47 insertions(+), 10 deletions(-) diff --git a/packages/eslint-plugin/docs/rules/unbound-method.md b/packages/eslint-plugin/docs/rules/unbound-method.md index 0de7b17abcf..a9a4fe396a2 100644 --- a/packages/eslint-plugin/docs/rules/unbound-method.md +++ b/packages/eslint-plugin/docs/rules/unbound-method.md @@ -23,6 +23,14 @@ myLog(); // This log might later be called with an incorrect scope const { log } = instance; + +// arith.double may refer to `this` internally +const arith = { + double(x: number): number { + return x * 2; + }, +}; +const { double } = arith; ``` Examples of **correct** code for this rule @@ -45,6 +53,14 @@ logBound(); // .bind and lambdas will also add a correct scope const dotBindLog = instance.logBound.bind(instance); const innerLog = () => instance.logBound(); + +// arith.double explicitly declares that it does not refer to `this` internally +const arith = { + double(this: void, x: number): number { + return x * 2; + }, +}; +const { double } = arith; ``` ## Options diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 246023fce10..407d04c11ec 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -248,14 +248,27 @@ function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { ts.SyntaxKind.FunctionExpression ); case ts.SyntaxKind.MethodDeclaration: - case ts.SyntaxKind.MethodSignature: - return !( - ignoreStatic && - tsutils.hasModifier( - valueDeclaration.modifiers, - ts.SyntaxKind.StaticKeyword, + case ts.SyntaxKind.MethodSignature: { + const decl = valueDeclaration as + | ts.MethodDeclaration + | ts.MethodSignature; + const firstParam = decl.parameters[0]; + 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; diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index 4c0cb8f58d3..83f3b93524c 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -25,6 +25,12 @@ class ContainsMethods { let instance = new ContainsMethods(); +const arith = { + double(this: void, x: number): number { + return x * 2; + } +}; + ${code} `; } @@ -35,7 +41,7 @@ function addContainsMethodsClassInvalid( code: addContainsMethodsClass(c), errors: [ { - line: 12, + line: 18, messageId: 'unbound', }, ], @@ -153,6 +159,8 @@ ruleTester.run('unbound-method', rule, { 'if (!!instance.unbound) {}', 'void instance.unbound', 'delete instance.unbound', + + 'const { double } = arith;', ].map(addContainsMethodsClass), ` interface RecordA { @@ -300,15 +308,15 @@ function foo(arg: ContainsMethods | null) { `), errors: [ { - line: 14, + line: 20, messageId: 'unbound', }, { - line: 15, + line: 21, messageId: 'unbound', }, { - line: 16, + line: 22, messageId: 'unbound', }, ],