From 9fca36baa40a1e4b658c75066dd6570a3014a6d1 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Thu, 9 Aug 2018 17:55:00 +0100 Subject: [PATCH] feat(rules): add `no-test-return-statement` (#136) Adds a rule for not returning inside of a test. Related issue in Jest: https://github.com/facebook/jest/issues/6516 --- README.md | 2 + docs/rules/no-test-return-statement.md | 47 ++++++++++++++++ index.js | 2 + .../no-test-return-statement.test.js | 55 +++++++++++++++++++ rules/no-test-return-statement.js | 43 +++++++++++++++ 5 files changed, 149 insertions(+) create mode 100644 docs/rules/no-test-return-statement.md create mode 100644 rules/__tests__/no-test-return-statement.test.js create mode 100644 rules/no-test-return-statement.js diff --git a/README.md b/README.md index ff3e001a1..516e0d4e2 100644 --- a/README.md +++ b/README.md @@ -90,6 +90,7 @@ for more information about extending configuration files. | [no-jest-import][] | Disallow importing `jest` | ![recommended][] | | | [no-large-snapshots][] | Disallow large snapshots | | | | [no-test-prefixes][] | Disallow using `f` & `x` prefixes to define focused/skipped tests | | ![fixable-green][] | +| [no-test-return-statement][] | Disallow explicitly returning from tests | | | | [prefer-expect-assertions][] | Suggest using `expect.assertions()` OR `expect.hasAssertions()` | | | | [prefer-to-be-null][] | Suggest using `toBeNull()` | | ![fixable-green][] | | [prefer-to-be-undefined][] | Suggest using `toBeUndefined()` | | ![fixable-green][] | @@ -114,6 +115,7 @@ for more information about extending configuration files. [no-jest-import]: docs/rules/no-jest-import.md [no-large-snapshots]: docs/rules/no-large-snapshots.md [no-test-prefixes]: docs/rules/no-test-prefixes.md +[no-test-return-statement]: docs/rules/no-test-return-statement.md [prefer-expect-assertions]: docs/rules/prefer-expect-assertions.md [prefer-to-be-null]: docs/rules/prefer-to-be-null.md [prefer-to-be-undefined]: docs/rules/prefer-to-be-undefined.md diff --git a/docs/rules/no-test-return-statement.md b/docs/rules/no-test-return-statement.md new file mode 100644 index 000000000..a8ac7161e --- /dev/null +++ b/docs/rules/no-test-return-statement.md @@ -0,0 +1,47 @@ +# Disallow explicitly returning from tests (no-test-return-statement) + +Tests in Jest should be void and not return values. + +If you are returning Promises then you should update the test to use +`async/await`. + +## Rule details + +This rule triggers a warning if you use a return statement inside of a test +body. + +```js +/*eslint jest/no-test-return-statement: "error"*/ + +// valid: + +it('noop', function() {}); + +test('noop', () => {}); + +test('one arrow', () => expect(1).toBe(1)); + +test('empty'); + +test('one', () => { + expect(1).toBe(1); +}); + +it('one', function() { + expect(1).toBe(1); +}); + +it('returning a promise', async () => { + await new Promise(res => setTimeout(res, 100)); + expect(1).toBe(1); +}); + +// invalid: +test('return an expect', () => { + return expect(1).toBe(1); +}); + +it('returning a promise', function() { + return new Promise(res => setTimeout(res, 100)).then(() => expect(1).toBe(1)); +}); +``` diff --git a/index.js b/index.js index 0d7e3e483..a1ffd926b 100644 --- a/index.js +++ b/index.js @@ -10,6 +10,7 @@ const noJasmineGlobals = require('./rules/no-jasmine-globals'); const noJestImport = require('./rules/no-jest-import'); const noLargeSnapshots = require('./rules/no-large-snapshots'); const noTestPrefixes = require('./rules/no-test-prefixes'); +const noTestReturnStatement = require('./rules/no-test-return-statement'); const preferToBeNull = require('./rules/prefer-to-be-null'); const preferToBeUndefined = require('./rules/prefer-to-be-undefined'); const preferToHaveLength = require('./rules/prefer-to-have-length'); @@ -75,6 +76,7 @@ module.exports = { 'no-jest-import': noJestImport, 'no-large-snapshots': noLargeSnapshots, 'no-test-prefixes': noTestPrefixes, + 'no-test-return-statement': noTestReturnStatement, 'prefer-to-be-null': preferToBeNull, 'prefer-to-be-undefined': preferToBeUndefined, 'prefer-to-have-length': preferToHaveLength, diff --git a/rules/__tests__/no-test-return-statement.test.js b/rules/__tests__/no-test-return-statement.test.js new file mode 100644 index 000000000..506cd20a4 --- /dev/null +++ b/rules/__tests__/no-test-return-statement.test.js @@ -0,0 +1,55 @@ +'use strict'; + +const RuleTester = require('eslint').RuleTester; +const rule = require('../no-test-return-statement'); + +const ruleTester = new RuleTester({ parserOptions: { ecmaVersion: 2015 } }); + +ruleTester.run('no-test-prefixes', rule, { + valid: [ + 'it("noop", function () {});', + 'test("noop", () => {});', + 'test("one", () => expect(1).toBe(1));', + 'test("empty")', + ` + test("one", () => { + expect(1).toBe(1); + }); + `, + ` + it("one", function () { + expect(1).toBe(1); + }); + `, + ], + invalid: [ + { + code: ` + test("one", () => { + return expect(1).toBe(1); + }); + `, + errors: [ + { + message: 'Jest tests should not return a value.', + column: 9, + line: 3, + }, + ], + }, + { + code: ` + it("one", function () { + return expect(1).toBe(1); + }); + `, + errors: [ + { + message: 'Jest tests should not return a value.', + column: 9, + line: 3, + }, + ], + }, + ], +}); diff --git a/rules/no-test-return-statement.js b/rules/no-test-return-statement.js new file mode 100644 index 000000000..370070975 --- /dev/null +++ b/rules/no-test-return-statement.js @@ -0,0 +1,43 @@ +'use strict'; + +const getDocsUrl = require('./util').getDocsUrl; +const isFunction = require('./util').isFunction; +const isTestCase = require('./util').isTestCase; + +const MESSAGE = 'Jest tests should not return a value.'; +const RETURN_STATEMENT = 'ReturnStatement'; +const BLOCK_STATEMENT = 'BlockStatement'; + +const getBody = args => { + if ( + args.length > 1 && + isFunction(args[1]) && + args[1].body.type === BLOCK_STATEMENT + ) { + return args[1].body.body; + } + return []; +}; + +module.exports = { + meta: { + docs: { + url: getDocsUrl(__filename), + }, + }, + create(context) { + return { + CallExpression(node) { + if (!isTestCase(node)) return; + const body = getBody(node.arguments); + const returnStmt = body.find(t => t.type === RETURN_STATEMENT); + if (!returnStmt) return; + + context.report({ + message: MESSAGE, + node: returnStmt, + }); + }, + }; + }, +};