diff --git a/packages/eslint-plugin/docs/rules/unbound-method.md b/packages/eslint-plugin/docs/rules/unbound-method.md index a9a4fe396a2..15063dbb4f3 100644 --- a/packages/eslint-plugin/docs/rules/unbound-method.md +++ b/packages/eslint-plugin/docs/rules/unbound-method.md @@ -4,6 +4,8 @@ Warns when a method is used outside of a method call. Class functions don't preserve the class scope when passed as standalone variables. +If your function does not access `this`, [you can annotate it with `this: void`](https://www.typescriptlang.org/docs/handbook/2/functions.html#declaring-this-in-a-function), or consider using an arrow function instead. + ## Rule Details Examples of **incorrect** code for this rule diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts index 2ec66bc2aa3..ecf9e652819 100644 --- a/packages/eslint-plugin/src/rules/unbound-method.ts +++ b/packages/eslint-plugin/src/rules/unbound-method.ts @@ -16,7 +16,7 @@ interface Config { export type Options = [Config]; -export type MessageIds = 'unbound'; +export type MessageIds = 'unbound' | 'unboundWithoutThisAnnotation'; /** * The following is a list of exceptions to the rule @@ -121,6 +121,9 @@ const getNodeName = (node: TSESTree.Node): string | null => const getMemberFullName = (node: TSESTree.MemberExpression): string => `${getNodeName(node.object)}.${getNodeName(node.property)}`; +const BASE_MESSAGE = + 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.'; + export default util.createRule({ name: 'unbound-method', meta: { @@ -132,8 +135,11 @@ export default util.createRule({ requiresTypeChecking: true, }, messages: { - unbound: - 'Avoid referencing unbound methods which may cause unintentional scoping of `this`.', + unbound: BASE_MESSAGE, + unboundWithoutThisAnnotation: + BASE_MESSAGE + + '\n' + + 'If your function does not access `this`, you can annotate it with `this: void`, or consider using an arrow function instead.', }, schema: [ { @@ -160,6 +166,26 @@ export default util.createRule({ context.getFilename(), ); + function checkMethodAndReport( + node: TSESTree.Node, + symbol: ts.Symbol | undefined, + ): void { + if (!symbol) { + return; + } + + const { dangerous, firstParamIsThis } = checkMethod(symbol, ignoreStatic); + if (dangerous) { + context.report({ + messageId: + firstParamIsThis === false + ? 'unboundWithoutThisAnnotation' + : 'unbound', + node, + }); + } + } + return { MemberExpression(node: TSESTree.MemberExpression): void { if (isSafeUse(node)) { @@ -179,14 +205,8 @@ export default util.createRule({ } const originalNode = parserServices.esTreeNodeToTSNodeMap.get(node); - const symbol = checker.getSymbolAtLocation(originalNode); - if (symbol && isDangerousMethod(symbol, ignoreStatic)) { - context.report({ - messageId: 'unbound', - node, - }); - } + checkMethodAndReport(node, checker.getSymbolAtLocation(originalNode)); }, 'VariableDeclarator, AssignmentExpression'( node: TSESTree.VariableDeclarator | TSESTree.AssignmentExpression, @@ -219,13 +239,10 @@ export default util.createRule({ return; } - const symbol = initTypes.getProperty(property.key.name); - if (symbol && isDangerousMethod(symbol, ignoreStatic)) { - context.report({ - messageId: 'unbound', - node, - }); - } + checkMethodAndReport( + node, + initTypes.getProperty(property.key.name), + ); } }); } @@ -234,44 +251,52 @@ export default util.createRule({ }, }); -function isDangerousMethod(symbol: ts.Symbol, ignoreStatic: boolean): boolean { +function checkMethod( + symbol: ts.Symbol, + ignoreStatic: boolean, +): { dangerous: boolean; firstParamIsThis?: boolean } { const { valueDeclaration } = symbol; if (!valueDeclaration) { // working around https://github.com/microsoft/TypeScript/issues/31294 - return false; + return { dangerous: false }; } switch (valueDeclaration.kind) { case ts.SyntaxKind.PropertyDeclaration: - return ( - (valueDeclaration as ts.PropertyDeclaration).initializer?.kind === - ts.SyntaxKind.FunctionExpression - ); + return { + dangerous: + (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[0]; - const thisArgIsVoid = + const firstParamIsThis = firstParam?.name.kind === ts.SyntaxKind.Identifier && - firstParam?.name.escapedText === 'this' && + firstParam?.name.escapedText === 'this'; + const thisArgIsVoid = + firstParamIsThis && firstParam?.type?.kind === ts.SyntaxKind.VoidKeyword; - return ( - !thisArgIsVoid && - !( - ignoreStatic && - tsutils.hasModifier( - valueDeclaration.modifiers, - ts.SyntaxKind.StaticKeyword, - ) - ) - ); + return { + dangerous: + !thisArgIsVoid && + !( + ignoreStatic && + tsutils.hasModifier( + valueDeclaration.modifiers, + ts.SyntaxKind.StaticKeyword, + ) + ), + firstParamIsThis, + }; } } - return false; + return { dangerous: false }; } function isSafeUse(node: TSESTree.Node): boolean { diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts index 911206b1f21..3075e93fe67 100644 --- a/packages/eslint-plugin/tests/rules/unbound-method.test.ts +++ b/packages/eslint-plugin/tests/rules/unbound-method.test.ts @@ -42,7 +42,7 @@ function addContainsMethodsClassInvalid( errors: [ { line: 18, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], })); @@ -298,7 +298,7 @@ Promise.resolve().then(console.log); errors: [ { line: 10, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -310,7 +310,7 @@ const x = console.log; errors: [ { line: 3, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -325,15 +325,15 @@ function foo(arg: ContainsMethods | null) { errors: [ { line: 20, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, { line: 21, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, { line: 22, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -375,7 +375,7 @@ ContainsMethods.unboundStatic; errors: [ { line: 8, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -390,7 +390,7 @@ const x = CommunicationError.prototype.foo; errors: [ { line: 5, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -400,7 +400,7 @@ const x = CommunicationError.prototype.foo; errors: [ { line: 1, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -419,7 +419,7 @@ instance.unbound = x; // THIS SHOULD NOT errors: [ { line: 9, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -447,7 +447,7 @@ const { unbound } = new Foo(); errors: [ { line: 5, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -476,7 +476,7 @@ let unbound; errors: [ { line: 6, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -505,7 +505,7 @@ const { foo } = CommunicationError.prototype; errors: [ { line: 5, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -520,7 +520,7 @@ let foo; errors: [ { line: 6, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -532,7 +532,7 @@ const { log } = console; errors: [ { line: 3, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -541,7 +541,7 @@ const { log } = console; errors: [ { line: 1, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -562,7 +562,7 @@ class OtherClass extends BaseClass { { line: 8, column: 15, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], }, @@ -584,7 +584,7 @@ class OtherClass extends BaseClass { { line: 9, column: 9, - messageId: 'unbound', + messageId: 'unboundWithoutThisAnnotation', }, ], },