From 24f32f17a9de2b436d78ff1e22ea83387ae5955d Mon Sep 17 00:00:00 2001 From: eran shabi Date: Mon, 5 Aug 2019 11:22:40 +0300 Subject: [PATCH 1/6] add new rule no-expect-resolves --- docs/rules/no-expect-resolves.md | 29 ++++++++++++ src/__tests__/rules.test.js | 2 +- .../__tests__/no-expect-resolves.test.ts | 26 +++++++++++ src/rules/no-expect-resolves.ts | 46 +++++++++++++++++++ 4 files changed, 102 insertions(+), 1 deletion(-) create mode 100644 docs/rules/no-expect-resolves.md create mode 100644 src/rules/__tests__/no-expect-resolves.test.ts create mode 100644 src/rules/no-expect-resolves.ts diff --git a/docs/rules/no-expect-resolves.md b/docs/rules/no-expect-resolves.md new file mode 100644 index 000000000..9e6653252 --- /dev/null +++ b/docs/rules/no-expect-resolves.md @@ -0,0 +1,29 @@ +# Avoid using `expect().resolves (no-expect-resolves) + +Jest allows you to test a promise resolve value using `await expect().resolves`. +For consistency and readability this rule bans `expect().resolves` in favor of +`expect(await promise)`. + +## Rule details + +This rule triggers a warning if `expect().resolves` is used. + +This rule is disabled by default. + +### Default configuration + +The following patterns is considered warning: + +```js +test('some test', async () => { + await expect(Promise.resolve(1)).resolves.toBe(1); +}); +``` + +The following pattern is not considered warning: + +```js +test('some test', async () => { + expect(await Promise.resolve(1)).toBe(1); +}); +``` diff --git a/src/__tests__/rules.test.js b/src/__tests__/rules.test.js index 1167b20fb..a6da2132d 100644 --- a/src/__tests__/rules.test.js +++ b/src/__tests__/rules.test.js @@ -3,7 +3,7 @@ import { resolve } from 'path'; import { rules } from '../'; const ruleNames = Object.keys(rules); -const numberOfRules = 37; +const numberOfRules = 38; describe('rules', () => { it('should have a corresponding doc for each rule', () => { diff --git a/src/rules/__tests__/no-expect-resolves.test.ts b/src/rules/__tests__/no-expect-resolves.test.ts new file mode 100644 index 000000000..7223b085d --- /dev/null +++ b/src/rules/__tests__/no-expect-resolves.test.ts @@ -0,0 +1,26 @@ +import { TSESLint } from '@typescript-eslint/experimental-utils'; +import rule from '../no-expect-resolves'; + +const ruleTester = new TSESLint.RuleTester({ + parserOptions: { + ecmaVersion: 2017, + sourceType: 'module', + }, +}); + +ruleTester.run('no-expect-resolves', rule, { + valid: [ + `test('some test', async () => { + expect(await Promise.resolve(1)).toBe(1); + });`, + ], + invalid: [ + { + code: `test('some test', async () => { + await expect(Promise.resolve(1)).resolves.toBe(1); + });`, + parserOptions: { sourceType: 'module' }, + errors: [{ endColumn: 55, column: 20, messageId: 'expectResolves' }], + }, + ], +}); diff --git a/src/rules/no-expect-resolves.ts b/src/rules/no-expect-resolves.ts new file mode 100644 index 000000000..48e1c0181 --- /dev/null +++ b/src/rules/no-expect-resolves.ts @@ -0,0 +1,46 @@ +import { + AST_NODE_TYPES, + TSESTree, +} from '@typescript-eslint/experimental-utils'; +import { createRule } from './tsUtils'; + +const isCallExpressionExpect = (node: TSESTree.MemberExpression) => + node.object.type === AST_NODE_TYPES.CallExpression && + node.object.callee.type === AST_NODE_TYPES.Identifier && + node.object.callee.name === 'expect'; + +const isIdentifierResolves = (node: TSESTree.MemberExpression) => + node.property.type === AST_NODE_TYPES.Identifier && + node.property.name === 'resolves'; + +function isExpectResolves(node: TSESTree.MemberExpression) { + if (isCallExpressionExpect(node) && isIdentifierResolves(node)) { + return true; + } + + return false; +} + +export default createRule({ + name: __filename, + meta: { + docs: { + category: 'Best Practices', + description: 'Disallow expect.resolves', + recommended: false, + }, + messages: { + expectResolves: 'Use `expect(await promise)` instead.', + }, + schema: [], + type: 'suggestion', + }, + defaultOptions: [], + create: context => ({ + MemberExpression(node) { + if (isExpectResolves(node)) { + context.report({ node, messageId: 'expectResolves' }); + } + }, + }), +}); From 2b6838cd91ef35b90389201f33ed7b3a08381252 Mon Sep 17 00:00:00 2001 From: eran shabi Date: Mon, 5 Aug 2019 11:27:16 +0300 Subject: [PATCH 2/6] refactor --- src/rules/no-expect-resolves.ts | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/rules/no-expect-resolves.ts b/src/rules/no-expect-resolves.ts index 48e1c0181..3121c9f0d 100644 --- a/src/rules/no-expect-resolves.ts +++ b/src/rules/no-expect-resolves.ts @@ -4,21 +4,23 @@ import { } from '@typescript-eslint/experimental-utils'; import { createRule } from './tsUtils'; -const isCallExpressionExpect = (node: TSESTree.MemberExpression) => - node.object.type === AST_NODE_TYPES.CallExpression && - node.object.callee.type === AST_NODE_TYPES.Identifier && - node.object.callee.name === 'expect'; - -const isIdentifierResolves = (node: TSESTree.MemberExpression) => - node.property.type === AST_NODE_TYPES.Identifier && - node.property.name === 'resolves'; +function isCallExpressionExpect(node: TSESTree.MemberExpression) { + return ( + node.object.type === AST_NODE_TYPES.CallExpression && + node.object.callee.type === AST_NODE_TYPES.Identifier && + node.object.callee.name === 'expect' + ); +} -function isExpectResolves(node: TSESTree.MemberExpression) { - if (isCallExpressionExpect(node) && isIdentifierResolves(node)) { - return true; - } +function isIdentifierResolves(node: TSESTree.MemberExpression) { + return ( + node.property.type === AST_NODE_TYPES.Identifier && + node.property.name === 'resolves' + ); +} - return false; +function isExpectResolves(node: TSESTree.MemberExpression): boolean { + return !!(isCallExpressionExpect(node) && isIdentifierResolves(node)); } export default createRule({ From a9eb9cdf9ae831dfa201eeeb85ed752d31baea47 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 7 Aug 2019 18:01:29 +0200 Subject: [PATCH 3/6] reuse `isExpectCall` --- src/rules/no-expect-resolves.ts | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/rules/no-expect-resolves.ts b/src/rules/no-expect-resolves.ts index 3121c9f0d..9871d23e0 100644 --- a/src/rules/no-expect-resolves.ts +++ b/src/rules/no-expect-resolves.ts @@ -2,15 +2,7 @@ import { AST_NODE_TYPES, TSESTree, } from '@typescript-eslint/experimental-utils'; -import { createRule } from './tsUtils'; - -function isCallExpressionExpect(node: TSESTree.MemberExpression) { - return ( - node.object.type === AST_NODE_TYPES.CallExpression && - node.object.callee.type === AST_NODE_TYPES.Identifier && - node.object.callee.name === 'expect' - ); -} +import { createRule, isExpectCall } from './tsUtils'; function isIdentifierResolves(node: TSESTree.MemberExpression) { return ( @@ -19,8 +11,8 @@ function isIdentifierResolves(node: TSESTree.MemberExpression) { ); } -function isExpectResolves(node: TSESTree.MemberExpression): boolean { - return !!(isCallExpressionExpect(node) && isIdentifierResolves(node)); +function isExpectResolves(node: TSESTree.MemberExpression) { + return isExpectCall(node.object) && isIdentifierResolves(node); } export default createRule({ From b6865581a6af5eded92dc3f8daec35e93f2066fc Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 7 Aug 2019 18:04:00 +0200 Subject: [PATCH 4/6] update readme --- README.md | 2 ++ docs/rules/no-expect-resolves.md | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3c6e9616b..cb8ee5a72 100644 --- a/README.md +++ b/README.md @@ -114,6 +114,7 @@ installations requiring long-term consistency. | [no-disabled-tests][] | Disallow disabled tests | ![recommended][] | | | [no-duplicate-hooks][] | Disallow duplicate hooks within a `describe` block | | | | [no-empty-title][] | Disallow empty titles | | | +| [no-expect-resolves][] | Disallow using `expect().resolves` | | | | [no-export][] | Disallow export from test files | | | | [no-focused-tests][] | Disallow focused tests | ![recommended][] | | | [no-hooks][] | Disallow setup and teardown hooks | | | @@ -166,6 +167,7 @@ https://github.com/dangreenisrael/eslint-plugin-jest-formatting [no-disabled-tests]: docs/rules/no-disabled-tests.md [no-duplicate-hooks]: docs/rules/no-duplicate-hooks.md [no-empty-title]: docs/rules/no-empty-title.md +[no-expect-resolves]: docs/rules/no-expect-resolves.md [no-export]: docs/rules/no-export.md [no-focused-tests]: docs/rules/no-focused-tests.md [no-hooks]: docs/rules/no-hooks.md diff --git a/docs/rules/no-expect-resolves.md b/docs/rules/no-expect-resolves.md index 9e6653252..c15d08300 100644 --- a/docs/rules/no-expect-resolves.md +++ b/docs/rules/no-expect-resolves.md @@ -1,4 +1,4 @@ -# Avoid using `expect().resolves (no-expect-resolves) +# Avoid using `expect().resolves` (no-expect-resolves) Jest allows you to test a promise resolve value using `await expect().resolves`. For consistency and readability this rule bans `expect().resolves` in favor of From 8bb84072f7287aa4f644a65b14adf46b85e9ffbd Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 7 Aug 2019 18:06:32 +0200 Subject: [PATCH 5/6] remove unused parser options --- src/rules/__tests__/no-expect-resolves.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/rules/__tests__/no-expect-resolves.test.ts b/src/rules/__tests__/no-expect-resolves.test.ts index 7223b085d..813df5ddc 100644 --- a/src/rules/__tests__/no-expect-resolves.test.ts +++ b/src/rules/__tests__/no-expect-resolves.test.ts @@ -4,7 +4,6 @@ import rule from '../no-expect-resolves'; const ruleTester = new TSESLint.RuleTester({ parserOptions: { ecmaVersion: 2017, - sourceType: 'module', }, }); @@ -19,7 +18,6 @@ ruleTester.run('no-expect-resolves', rule, { code: `test('some test', async () => { await expect(Promise.resolve(1)).resolves.toBe(1); });`, - parserOptions: { sourceType: 'module' }, errors: [{ endColumn: 55, column: 20, messageId: 'expectResolves' }], }, ], From ed3b2d148bf9d0d9db7d35cf0b78bf12940199c4 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 7 Aug 2019 18:13:09 +0200 Subject: [PATCH 6/6] report `resolves` rather than `expect` --- src/rules/__tests__/no-expect-resolves.test.ts | 2 +- src/rules/no-expect-resolves.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/rules/__tests__/no-expect-resolves.test.ts b/src/rules/__tests__/no-expect-resolves.test.ts index 813df5ddc..c983499b5 100644 --- a/src/rules/__tests__/no-expect-resolves.test.ts +++ b/src/rules/__tests__/no-expect-resolves.test.ts @@ -18,7 +18,7 @@ ruleTester.run('no-expect-resolves', rule, { code: `test('some test', async () => { await expect(Promise.resolve(1)).resolves.toBe(1); });`, - errors: [{ endColumn: 55, column: 20, messageId: 'expectResolves' }], + errors: [{ endColumn: 55, column: 47, messageId: 'expectResolves' }], }, ], }); diff --git a/src/rules/no-expect-resolves.ts b/src/rules/no-expect-resolves.ts index 9871d23e0..586ca26c2 100644 --- a/src/rules/no-expect-resolves.ts +++ b/src/rules/no-expect-resolves.ts @@ -33,7 +33,7 @@ export default createRule({ create: context => ({ MemberExpression(node) { if (isExpectResolves(node)) { - context.report({ node, messageId: 'expectResolves' }); + context.report({ node: node.property, messageId: 'expectResolves' }); } }, }),