fix(eslint-plugin): [unbound-method] check method definition in objec…

…t literal using longhand form (#8637) * [unbound-method] Check for unbound methods in object literals with long-form syntax (#8636) * codecov
typescript-eslint · Mar 13, 2024 · 609a000 · 609a000
1 parent e95d90a
commit 609a000
Show file tree

Hide file tree

Showing 2 changed files with 88 additions and 29 deletions.
diff --git a/packages/eslint-plugin/src/rules/unbound-method.ts b/packages/eslint-plugin/src/rules/unbound-method.ts
@@ -132,15 +132,18 @@ export default createRule<Options, MessageIds>({
     const services = getParserServices(context);
     const currentSourceFile = services.program.getSourceFile(context.filename);
 
-    function checkMethodAndReport(
+    function checkIfMethodAndReport(
       node: TSESTree.Node,
       symbol: ts.Symbol | undefined,
     ): void {
       if (!symbol) {
         return;
       }
 
-      const { dangerous, firstParamIsThis } = checkMethod(symbol, ignoreStatic);
+      const { dangerous, firstParamIsThis } = checkIfMethod(
+        symbol,
+        ignoreStatic,
+      );
       if (dangerous) {
         context.report({
           messageId:
@@ -168,7 +171,7 @@ export default createRule<Options, MessageIds>({
           return;
         }
 
-        checkMethodAndReport(node, services.getSymbolAtLocation(node));
+        checkIfMethodAndReport(node, services.getSymbolAtLocation(node));
       },
       'VariableDeclarator, AssignmentExpression'(
         node: TSESTree.AssignmentExpression | TSESTree.VariableDeclarator,
@@ -200,7 +203,7 @@ export default createRule<Options, MessageIds>({
                 return;
               }
 
-              checkMethodAndReport(
+              checkIfMethodAndReport(
                 property.key,
                 initTypes.getProperty(property.key.name),
               );
@@ -212,10 +215,15 @@ export default createRule<Options, MessageIds>({
   },
 });
 
-function checkMethod(
+interface CheckMethodResult {
+  dangerous: boolean;
+  firstParamIsThis?: boolean;
+}
+
+function checkIfMethod(
   symbol: ts.Symbol,
   ignoreStatic: boolean,
-): { dangerous: boolean; firstParamIsThis?: boolean } {
+): CheckMethodResult {
   const { valueDeclaration } = symbol;
   if (!valueDeclaration) {
     // working around https://github.com/microsoft/TypeScript/issues/31294
@@ -229,37 +237,56 @@ function checkMethod(
           (valueDeclaration as ts.PropertyDeclaration).initializer?.kind ===
           ts.SyntaxKind.FunctionExpression,
       };
+    case ts.SyntaxKind.PropertyAssignment: {
+      const assignee = (valueDeclaration as ts.PropertyAssignment).initializer;
+      if (assignee.kind !== ts.SyntaxKind.FunctionExpression) {
+        return {
+          dangerous: false,
+        };
+      }
+      return checkMethod(assignee as ts.FunctionExpression, ignoreStatic);
+    }
     case ts.SyntaxKind.MethodDeclaration:
     case ts.SyntaxKind.MethodSignature: {
-      const decl = valueDeclaration as
-        | ts.MethodDeclaration
-        | ts.MethodSignature;
-      const firstParam = decl.parameters.at(0);
-      const firstParamIsThis =
-        firstParam?.name.kind === ts.SyntaxKind.Identifier &&
-        // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
-        firstParam.name.escapedText === 'this';
-      const thisArgIsVoid =
-        firstParamIsThis && firstParam.type?.kind === ts.SyntaxKind.VoidKeyword;
-
-      return {
-        dangerous:
-          !thisArgIsVoid &&
-          !(
-            ignoreStatic &&
-            tsutils.includesModifier(
-              getModifiers(valueDeclaration),
-              ts.SyntaxKind.StaticKeyword,
-            )
-          ),
-        firstParamIsThis,
-      };
+      return checkMethod(
+        valueDeclaration as ts.MethodDeclaration | ts.MethodSignature,
+        ignoreStatic,
+      );
     }
   }
 
   return { dangerous: false };
 }
 
+function checkMethod(
+  valueDeclaration:
+    | ts.MethodDeclaration
+    | ts.MethodSignature
+    | ts.FunctionExpression,
+  ignoreStatic: boolean,
+): CheckMethodResult {
+  const firstParam = valueDeclaration.parameters.at(0);
+  const firstParamIsThis =
+    firstParam?.name.kind === ts.SyntaxKind.Identifier &&
+    // eslint-disable-next-line @typescript-eslint/no-unsafe-enum-comparison
+    firstParam.name.escapedText === 'this';
+  const thisArgIsVoid =
+    firstParamIsThis && firstParam.type?.kind === ts.SyntaxKind.VoidKeyword;
+
+  return {
+    dangerous:
+      !thisArgIsVoid &&
+      !(
+        ignoreStatic &&
+        tsutils.includesModifier(
+          getModifiers(valueDeclaration),
+          ts.SyntaxKind.StaticKeyword,
+        )
+      ),
+    firstParamIsThis,
+  };
+}
+
 function isSafeUse(node: TSESTree.Node): boolean {
   const parent = node.parent;
 

diff --git a/packages/eslint-plugin/tests/rules/unbound-method.test.ts b/packages/eslint-plugin/tests/rules/unbound-method.test.ts
@@ -58,6 +58,23 @@ ruleTester.run('unbound-method', rule, {
     '[5.2, 7.1, 3.6].map(Math.floor);',
     'const x = console.log;',
     'const x = Object.defineProperty;',
+    `
+      const o = {
+        f: function (this: void) {},
+      };
+      const f = o.f;
+    `,
+    `
+      const { alert } = window;
+    `,
+    `
+      let b = window.blur;
+    `,
+    `
+      function foo() {}
+      const fooObject = { foo };
+      const { foo: bar } = fooObject;
+    `,
     ...[
       'instance.bound();',
       'instance.unbound();',
@@ -644,5 +661,20 @@ const { b, a } = values;
         },
       ],
     },
+    // https://github.com/typescript-eslint/typescript-eslint/issues/8636
+    {
+      code: `
+const objectLiteral = {
+  f: function () {},
+};
+const f = objectLiteral.f;
+      `,
+      errors: [
+        {
+          line: 5,
+          messageId: 'unboundWithoutThisAnnotation',
+        },
+      ],
+    },
   ],
 });