Skip to content

Commit

Permalink
feat(unbound-method): modify rule for jest
Browse files Browse the repository at this point in the history
  • Loading branch information
G-Rath committed Feb 21, 2021
1 parent c0037ae commit dc2f4dc
Show file tree
Hide file tree
Showing 8 changed files with 220 additions and 6 deletions.
1 change: 1 addition & 0 deletions .eslintignore
@@ -1,3 +1,4 @@
coverage/
lib/
!.eslintrc.js
src/rules/__tests__/fixtures/
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -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][] | |
Expand Down
53 changes: 53 additions & 0 deletions 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
3 changes: 2 additions & 1 deletion package.json
Expand Up @@ -65,7 +65,8 @@
"displayName": "test",
"testEnvironment": "node",
"testPathIgnorePatterns": [
"<rootDir>/lib/.*"
"<rootDir>/lib/.*",
"<rootDir>/src/rules/__tests__/fixtures/*"
]
},
{
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/__snapshots__/rules.test.ts.snap
Expand Up @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/rules.test.ts
Expand Up @@ -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)
Expand Down
99 changes: 99 additions & 0 deletions 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 {
Expand All @@ -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 {
Expand Down
66 changes: 62 additions & 4 deletions src/rules/unbound-method.ts
Expand Up @@ -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,
Expand All @@ -28,7 +66,6 @@ const tsutils = {

const util = {
createRule,
getParserServices: ESLintUtils.getParserServices,
isIdentifier: ASTUtils.isIdentifier,
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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: {
Expand All @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit dc2f4dc

Please sign in to comment.