diff --git a/.eslintignore b/.eslintignore index 30e85ed0b..224070368 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1,3 +1,4 @@ coverage/ lib/ !.eslintrc.js +src/rules/__tests__/fixtures/ diff --git a/README.md b/README.md index 4a2d0e96d..062fe47dc 100644 --- a/README.md +++ b/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][] | | diff --git a/docs/rules/unbound-method.md b/docs/rules/unbound-method.md new file mode 100644 index 000000000..df660ac46 --- /dev/null +++ b/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. + +Taken with ❤️ [from `@typescript-eslint` core][original-rule] + +[original-rule]: + https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/unbound-method.md diff --git a/package.json b/package.json index f70a8dbad..5a56777a5 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,8 @@ "displayName": "test", "testEnvironment": "node", "testPathIgnorePatterns": [ - "/lib/.*" + "/lib/.*", + "/src/rules/__tests__/fixtures/*" ] }, { diff --git a/src/__tests__/__snapshots__/rules.test.ts.snap b/src/__tests__/__snapshots__/rules.test.ts.snap index 3d7972b03..414f67032 100644 --- a/src/__tests__/__snapshots__/rules.test.ts.snap +++ b/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", diff --git a/src/__tests__/rules.test.ts b/src/__tests__/rules.test.ts index d37e1bd4a..6a8ff2df1 100644 --- a/src/__tests__/rules.test.ts +++ b/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) diff --git a/src/rules/__tests__/unbound-method.test.ts b/src/rules/__tests__/unbound-method.test.ts index 11307f81e..4e55e2cfb 100644 --- a/src/rules/__tests__/unbound-method.test.ts +++ b/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,67 @@ const ruleTester = new ESLintUtils.RuleTester({ }, }); +const ConsoleClassAndVariableCode = dedent` + class Console { + log(str) { + process.stdout.write(str); + } + } + + const console = new Console(); +`; + +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();', +]; + +const invalidTestCases: Array> = [ + { + code: dedent` + expect(() => { + ${ConsoleClassAndVariableCode} + + Promise.resolve().then(console.log); + }).not.toThrow(); + `, + errors: [ + { + line: 10, + messageId: 'unbound', + }, + ], + }, +]; + +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 { diff --git a/src/rules/unbound-method.ts b/src/rules/unbound-method.ts index f1b778f30..040388b0b 100644 --- a/src/rules/unbound-method.ts +++ b/src/rules/unbound-method.ts @@ -2,11 +2,32 @@ 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 } from './utils'; +//------------------------------------------------------------------------------ +// eslint-plugin-jest extension +//------------------------------------------------------------------------------ + +const getParserServices = ( + context: Readonly>, +): ParserServices | null => { + try { + return ESLintUtils.getParserServices(context); + } catch { + return null; + } +}; + +//------------------------------------------------------------------------------ +// Inlining of external dependencies +//------------------------------------------------------------------------------ + +/* istanbul ignore next */ const tsutils = { hasModifier( modifiers: ts.ModifiersArray | undefined, @@ -28,7 +49,6 @@ const tsutils = { const util = { createRule, - getParserServices: ESLintUtils.getParserServices, isIdentifier: ASTUtils.isIdentifier, }; @@ -108,6 +128,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 +177,7 @@ export default util.createRule({ category: 'Best Practices', description: 'Enforces unbound methods are called with their expected scope', - recommended: 'error', + recommended: false, requiresTypeChecking: true, }, messages: { @@ -182,14 +203,33 @@ export default util.createRule({ }, ], 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 = isExpectCall(node); + }, + 'CallExpression:exit'(node: TSESTree.CallExpression): void { + if (inExpectCall && isExpectCall(node)) { + inExpectCall = false; + } + }, MemberExpression(node: TSESTree.MemberExpression): void { + if (inExpectCall) { + return; + } + if (isSafeUse(node)) { return; } @@ -233,6 +273,7 @@ export default util.createRule({ 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