feat(unbound-method): modify rule for jest

Author: G-Rath

.eslintignore

@@ -1,3 +1,4 @@
 coverage/
 lib/
 !.eslintrc.js
+src/rules/__tests__/fixtures/

README.md

@@ -168,6 +168,7 @@ installations requiring long-term consistency.
 | [prefer-todo](docs/rules/prefer-todo.md)                                     | Suggest using `test.todo`                                       |                  | ![fixable][] |
 | [require-to-throw-message](docs/rules/require-to-throw-message.md)           | Require a message for `toThrow()`                               |                  |              |
 | [require-top-level-describe](docs/rules/require-top-level-describe.md)       | Require test cases and hooks to be inside a `describe` block    |                  |              |
+| [unbound-method](docs/rules/unbound-method.md)                               | Enforces unbound methods are called with their expected scope   |                  |              |
 | [valid-describe](docs/rules/valid-describe.md)                               | Enforce valid `describe()` callback                             | ![recommended][] |              |
 | [valid-expect](docs/rules/valid-expect.md)                                   | Enforce valid `expect()` usage                                  | ![recommended][] |              |
 | [valid-expect-in-promise](docs/rules/valid-expect-in-promise.md)             | Enforce having return statement when testing with promises      | ![recommended][] |              |

docs/rules/unbound-method.md

@@ -0,0 +1,53 @@
+# Enforces unbound methods are called with their expected scope (`unbound-method`)
+
+## Rule Details
+
+This rule extends the base [`@typescript-eslint/unbound-method`][original-rule]
+rule. It adds support for understanding when it's ok to pass an unbound method
+to `expect` calls.
+
+See the [`@typescript-eslint` documentation][original-rule] for more details on
+the `unbound-method` rule.
+
+Note that while this rule requires type information to work, it will fail
+silently when not available allowing you to safely enable it on projects that
+are not using TypeScript.
+
+## How to use
+
+```json5
+{
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    project: 'tsconfig.json',
+    ecmaVersion: 2020,
+    sourceType: 'module',
+  },
+  overrides: [
+    {
+      files: ['test/**'],
+      extends: ['jest'],
+      rules: {
+        // you should turn the original rule off *only* for test files
+        '@typescript-eslint/unbound-method': 'off',
+        'jest/unbound-method': 'error',
+      },
+    },
+  ],
+  rules: {
+    '@typescript-eslint/unbound-method': 'error',
+  },
+}
+```
+
+This rule should be applied to your test files in place of the original rule,
+which should be applied to the rest of your codebase.
+
+## Options
+
+See [`@typescript-eslint/unbound-method`][original-rule] options.
+
+<sup>Taken with ❤️ [from `@typescript-eslint` core][original-rule]</sup>
+
+[original-rule]:
+  https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md

package.json

@@ -65,7 +65,8 @@
         "displayName": "test",
         "testEnvironment": "node",
         "testPathIgnorePatterns": [
-          "<rootDir>/lib/.*"
+          "<rootDir>/lib/.*",
+          "<rootDir>/src/rules/__tests__/fixtures/*"
         ]
       },
       {

src/__tests__/__snapshots__/rules.test.ts.snap

@@ -46,6 +46,7 @@ Object {
       "jest/prefer-todo": "error",
       "jest/require-to-throw-message": "error",
       "jest/require-top-level-describe": "error",
+      "jest/unbound-method": "error",
       "jest/valid-describe": "error",
       "jest/valid-expect": "error",
       "jest/valid-expect-in-promise": "error",

src/__tests__/rules.test.ts

@@ -2,7 +2,7 @@ import { existsSync } from 'fs';
 import { resolve } from 'path';
 import plugin from '../';
 
-const numberOfRules = 44;
+const numberOfRules = 45;
 const ruleNames = Object.keys(plugin.rules);
 const deprecatedRules = Object.entries(plugin.rules)
   .filter(([, rule]) => rule.meta.deprecated)

src/rules/__tests__/unbound-method.test.ts

@@ -1,5 +1,6 @@
 import path from 'path';
 import { ESLintUtils, TSESLint } from '@typescript-eslint/experimental-utils';
+import dedent from 'dedent';
 import rule, { MessageIds, Options } from '../unbound-method';
 
 function getFixturesRootDir(): string {
@@ -17,6 +18,104 @@ const ruleTester = new ESLintUtils.RuleTester({
   },
 });
 
+const ConsoleClassAndVariableCode = dedent`
+  class Console {
+    log(str) {
+      process.stdout.write(str);
+    }
+  }
+
+  const console = new Console();
+`;
+
+const toThrowMatchers = [
+  'toThrow',
+  'toThrowError',
+  'toThrowErrorMatchingSnapshot',
+  'toThrowErrorMatchingInlineSnapshot',
+];
+
+const validTestCases: string[] = [
+  ...[
+    'expect(Console.prototype.log)',
+    'expect(Console.prototype.log).toHaveBeenCalledTimes(1);',
+    'expect(Console.prototype.log).not.toHaveBeenCalled();',
+    'expect(Console.prototype.log).toStrictEqual(somethingElse);',
+  ].map(code => [ConsoleClassAndVariableCode, code].join('\n')),
+  dedent`
+    expect(() => {
+      ${ConsoleClassAndVariableCode}
+
+      expect(Console.prototype.log).toHaveBeenCalledTimes(1);
+    }).not.toThrow();
+  `,
+  'expect(() => Promise.resolve().then(console.log)).not.toThrow();',
+  ...toThrowMatchers.map(matcher => `expect(console.log).not.${matcher}();`),
+  ...toThrowMatchers.map(matcher => `expect(console.log).${matcher}();`),
+];
+
+const invalidTestCases: Array<TSESLint.InvalidTestCase<MessageIds, Options>> = [
+  {
+    code: dedent`
+      expect(() => {
+        ${ConsoleClassAndVariableCode}
+
+        Promise.resolve().then(console.log);
+      }).not.toThrow();
+    `,
+    errors: [
+      {
+        line: 10,
+        messageId: 'unbound',
+      },
+    ],
+  },
+  // toThrow matchers call the expected value (which is expected to be a function)
+  ...toThrowMatchers.map(matcher => ({
+    code: dedent`
+      ${ConsoleClassAndVariableCode}
+
+      expect(console.log).${matcher}();
+    `,
+    errors: [
+      {
+        line: 9,
+        messageId: 'unbound' as const,
+      },
+    ],
+  })),
+  // toThrow matchers call the expected value (which is expected to be a function)
+  ...toThrowMatchers.map(matcher => ({
+    code: dedent`
+      ${ConsoleClassAndVariableCode}
+
+      expect(console.log).not.${matcher}();
+    `,
+    errors: [
+      {
+        line: 9,
+        messageId: 'unbound' as const,
+      },
+    ],
+  })),
+];
+
+ruleTester.run('unbound-method jest edition', rule, {
+  valid: validTestCases,
+  invalid: invalidTestCases,
+});
+
+new ESLintUtils.RuleTester({
+  parser: '@typescript-eslint/parser',
+  parserOptions: {
+    sourceType: 'module',
+    tsconfigRootDir: rootPath,
+  },
+}).run('unbound-method jest edition without type service', rule, {
+  valid: validTestCases.concat(invalidTestCases.map(({ code }) => code)),
+  invalid: [],
+});
+
 function addContainsMethodsClass(code: string): string {
   return `
 class ContainsMethods {

src/rules/unbound-method.ts

@@ -2,11 +2,49 @@ import {
   ASTUtils,
   AST_NODE_TYPES,
   ESLintUtils,
+  ParserServices,
+  TSESLint,
   TSESTree,
 } from '@typescript-eslint/experimental-utils';
 import * as ts from 'typescript';
-import { createRule } from './utils';
+import { createRule, isExpectCall, parseExpectCall } from './utils';
 
+//------------------------------------------------------------------------------
+// eslint-plugin-jest extension
+//------------------------------------------------------------------------------
+
+const getParserServices = (
+  context: Readonly<TSESLint.RuleContext<MessageIds, Options>>,
+): ParserServices | null => {
+  try {
+    return ESLintUtils.getParserServices(context);
+  } catch {
+    return null;
+  }
+};
+
+const toThrowMatchers = [
+  'toThrow',
+  'toThrowError',
+  'toThrowErrorMatchingSnapshot',
+  'toThrowErrorMatchingInlineSnapshot',
+];
+
+const isJestExpectCall = (node: TSESTree.CallExpression) => {
+  if (!isExpectCall(node)) {
+    return false;
+  }
+
+  const { matcher } = parseExpectCall(node);
+
+  return !toThrowMatchers.includes(matcher?.name ?? '');
+};
+
+//------------------------------------------------------------------------------
+// Inlining of external dependencies
+//------------------------------------------------------------------------------
+
+/* istanbul ignore next */
 const tsutils = {
   hasModifier(
     modifiers: ts.ModifiersArray | undefined,
@@ -28,7 +66,6 @@ const tsutils = {
 
 const util = {
   createRule,
-  getParserServices: ESLintUtils.getParserServices,
   isIdentifier: ASTUtils.isIdentifier,
 };
 
@@ -108,6 +145,7 @@ const SUPPORTED_GLOBALS = [
   'Intl',
 ] as const;
 const nativelyBoundMembers = SUPPORTED_GLOBALS.map(namespace => {
+  /* istanbul ignore if */
   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
@@ -156,7 +194,7 @@ export default util.createRule<Options, MessageIds>({
       category: 'Best Practices',
       description:
         'Enforces unbound methods are called with their expected scope',
-      recommended: 'error',
+      recommended: false,
       requiresTypeChecking: true,
     },
     messages: {
@@ -182,14 +220,33 @@ export default util.createRule<Options, MessageIds>({
     },
   ],
   create(context, [{ ignoreStatic }]) {
-    const parserServices = util.getParserServices(context);
+    const parserServices = getParserServices(context);
+
+    if (!parserServices) {
+      return {};
+    }
+
     const checker = parserServices.program.getTypeChecker();
     const currentSourceFile = parserServices.program.getSourceFile(
       context.getFilename(),
     );
 
+    let inExpectCall = false;
+
     return {
+      CallExpression(node: TSESTree.CallExpression): void {
+        inExpectCall = isJestExpectCall(node);
+      },
+      'CallExpression:exit'(node: TSESTree.CallExpression): void {
+        if (inExpectCall && isJestExpectCall(node)) {
+          inExpectCall = false;
+        }
+      },
       MemberExpression(node: TSESTree.MemberExpression): void {
+        if (inExpectCall) {
+          return;
+        }
+
         if (isSafeUse(node)) {
           return;
         }
@@ -233,6 +290,7 @@ export default util.createRule<Options, MessageIds>({
             rightSymbol && isNotImported(rightSymbol, currentSourceFile);
 
           idNode.properties.forEach(property => {
+            /* istanbul ignore else */
             if (
               property.type === AST_NODE_TYPES.Property &&
               property.key.type === AST_NODE_TYPES.Identifier