diff --git a/docs/rules/hooks-order.md b/docs/rules/hooks-order.md new file mode 100644 index 00000000..670cb664 --- /dev/null +++ b/docs/rules/hooks-order.md @@ -0,0 +1,79 @@ +# Enforce test hook ordering + +Hooks should be placed before any tests and in the proper semantic order: + +- `test.before(…);` +- `test.after(…);` +- `test.after.always(…);` +- `test.beforeEach(…);` +- `test.afterEach(…);` +- `test.afterEach.always(…);` +- `test(…);` + +This rule is fixable as long as no other code is between the hooks that need to be reordered. + + +## Fail + +```js +import test from 'ava'; + +test.after(t => { + doFoo(); +}); + +test.before(t => { + doFoo(); +}); + +test('foo', t => { + t.true(true); +}); +``` + +```js +import test from 'ava'; + +test('foo', t => { + t.true(true); +}); + +test.before(t => { + doFoo(); +}); +``` + + +## Pass + +```js +import test from 'ava'; + +test.before(t => { + doFoo(); +}); + +test.after(t => { + doFoo(); +}); + +test.after.always(t => { + doFoo(); +}); + +test.beforeEach(t => { + doFoo(); +}); + +test.afterEach(t => { + doFoo(); +}); + +test.afterEach.always(t => { + doFoo(); +}); + +test('foo', t => { + t.true(true); +}); +``` diff --git a/index.js b/index.js index 12fcddf6..ef5418f2 100644 --- a/index.js +++ b/index.js @@ -18,6 +18,7 @@ module.exports = { ], rules: { 'ava/assertion-arguments': 'error', + 'ava/hooks-order': 'error', 'ava/max-asserts': [ 'off', 5 diff --git a/package.json b/package.json index 6413bc1d..9bd70109 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ "eslint-plugin-eslint-plugin": "2.1.0", "js-combinatorics": "^0.5.4", "nyc": "^14.1.1", + "outdent": "^0.7.0", "pify": "^4.0.1", "xo": "^0.24.0" }, diff --git a/readme.md b/readme.md index 8e40eac8..74397b03 100644 --- a/readme.md +++ b/readme.md @@ -36,6 +36,7 @@ Configure it in `package.json`. ], "rules": { "ava/assertion-arguments": "error", + "ava/hooks-order": "error", "ava/max-asserts": [ "off", 5 @@ -77,6 +78,7 @@ Configure it in `package.json`. The rules will only activate in test files. - [assertion-arguments](docs/rules/assertion-arguments.md) - Enforce passing correct arguments to assertions. +- [hooks-order](docs/rules/hooks-order.md) - Enforce test hook ordering. *(fixable)* - [max-asserts](docs/rules/max-asserts.md) - Limit the number of assertions in a test. - [no-async-fn-without-await](docs/rules/no-async-fn-without-await.md) - Ensure that async tests use `await`. - [no-cb-test](docs/rules/no-cb-test.md) - Ensure no `test.cb()` is used. diff --git a/rules/hooks-order.js b/rules/hooks-order.js new file mode 100644 index 00000000..b421a48d --- /dev/null +++ b/rules/hooks-order.js @@ -0,0 +1,158 @@ +'use strict'; +const {visitIf} = require('enhance-visitors'); +const createAvaRule = require('../create-ava-rule'); +const util = require('../util'); + +const MESSAGE_ID = 'hooks-order'; + +const buildOrders = names => { + const orders = {}; + for (const nameLater of names) { + for (const nameEarlier in orders) { + if (orders[nameEarlier]) { + orders[nameEarlier].push(nameLater); + } + } + + orders[nameLater] = []; + } + + return orders; +}; + +const buildMessage = (name, orders, visited) => { + const checks = orders[name] || []; + + for (const check of checks) { + const nodeEarlier = visited[check]; + if (nodeEarlier) { + return { + messageId: MESSAGE_ID, + data: { + current: name, + invalid: check + }, + node: nodeEarlier + }; + } + } + + return null; +}; + +const create = context => { + const ava = createAvaRule(); + + const orders = buildOrders([ + 'before', + 'after', + 'after.always', + 'beforeEach', + 'afterEach', + 'afterEach.always', + 'test' + ]); + + const visited = {}; + + const checks = [ + { + selector: 'CallExpression[callee.object.name="test"][callee.property.name="before"]', + name: 'before' + }, + { + selector: 'CallExpression[callee.object.name="test"][callee.property.name="after"]', + name: 'after' + }, + { + selector: 'CallExpression[callee.object.object.name="test"][callee.object.property.name="after"][callee.property.name="always"]', + name: 'after.always' + }, + { + selector: 'CallExpression[callee.object.name="test"][callee.property.name="beforeEach"]', + name: 'beforeEach' + }, + { + selector: 'CallExpression[callee.object.name="test"][callee.property.name="afterEach"]', + name: 'afterEach' + }, + { + selector: 'CallExpression[callee.object.object.name="test"][callee.object.property.name="afterEach"][callee.property.name="always"]', + name: 'afterEach.always' + }, + { + selector: 'CallExpression[callee.name="test"]', + name: 'test' + } + ]; + + const sourceCode = context.getSourceCode(); + + const selectors = checks.reduce((result, check) => { + result[check.selector] = visitIf([ + ava.isInTestFile, + ava.isTestNode + ])(node => { + visited[check.name] = node; + + const message = buildMessage(check.name, orders, visited); + if (message) { + const nodeEarlier = message.node; + + context.report({ + node, + messageId: message.messageId, + data: message.data, + fix: fixer => { + const tokensBetween = sourceCode.getTokensBetween(nodeEarlier.parent, node.parent); + + if (tokensBetween && tokensBetween.length > 0) { + return; + } + + const source = sourceCode.getText(); + let [insertStart, insertEnd] = nodeEarlier.parent.range; + + // Grab the node and all comments and whitespace before the node + const start = nodeEarlier.parent.range[1]; + const end = node.parent.range[1]; + + let text = sourceCode.getText().substring(start, end); + + // Preserve newline previously between hooks + if (source.length >= (start + 1) && source[start + 1] === '\n') { + text = text.substring(1) + '\n'; + } + + // Preserve newline that was previously before hooks + if ((insertStart - 1) > 0 && source[insertStart - 1] === '\n') { + insertStart -= 1; + } + + return [ + fixer.insertTextBeforeRange([insertStart, insertEnd], text), + fixer.removeRange([start, end]) + ]; + } + }); + } + }); + return result; + }, {}); + + return ava.merge(selectors); +}; + +module.exports = { + create, + meta: { + docs: { + url: util.getDocsUrl(__filename) + }, + messages: { + [MESSAGE_ID]: '`{{current}}` hook must come before `{{invalid}}`' + }, + type: 'suggestion', + fixable: 'code' + } +}; diff --git a/test/hooks-order.js b/test/hooks-order.js new file mode 100644 index 00000000..f91d8586 --- /dev/null +++ b/test/hooks-order.js @@ -0,0 +1,723 @@ +import test from 'ava'; +import avaRuleTester from 'eslint-ava-rule-tester'; +import {outdent} from 'outdent'; +import rule from '../rules/hooks-order'; + +const ruleTester = avaRuleTester(test, { + env: { + es6: true + } +}); + +const errors = [{ruleId: 'no-todo-test'}]; +const header = 'const test = require(\'ava\');'; + +ruleTester.run('no-todo-test', rule, { + valid: [ + outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test.after.always(t => { + doFoo(); + }); + + test.beforeEach(t => { + doFoo(); + }); + + test.afterEach(t => { + doFoo(); + }); + + test.afterEach.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + console.log('foo'); + + test.after.always(t => { + doFoo(); + }); + + const foo = 'foo'; + + test.afterEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + outdent` + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test.after.always(t => { + doFoo(); + }); + + test.beforeEach(t => { + doFoo(); + }); + + test.afterEach(t => { + doFoo(); + }); + + test.afterEach.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + `, + outdent` + test.after(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + `, + outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + ` + ], + invalid: [ + { + code: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.after.always(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.after.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + + test.after.always(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.after.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.beforeEach(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.beforeEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.afterEach(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.afterEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.afterEach.always(t => { + doFoo(); + }); + + test.before(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test.afterEach.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + + test.before(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.before(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + + { + code: outdent` + ${header} + + test.after.always(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test.after.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.beforeEach(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test.beforeEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.afterEach(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test.afterEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.afterEach.always(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test.afterEach.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + + test.after(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + + { + code: outdent` + ${header} + + test.afterEach(t => { + doFoo(); + }); + + test.beforeEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + output: outdent` + ${header} + + test.beforeEach(t => { + doFoo(); + }); + + test.afterEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + + test.beforeEach(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.beforeEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + + { + code: outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + + test.afterEach(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.afterEach(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + + { + code: outdent` + ${header} + + test('foo', t => { + t.true(true); + }); + + test.afterEach.always(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.afterEach.always(t => { + doFoo(); + }); + + test('foo', t => { + t.true(true); + }); + `, + errors + }, + + { + code: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + console.log('foo'); + + test.before(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + console.log('foo'); + + test.before(t => { + doFoo(); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + // comments + test.before(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + // comments + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + `, + errors + }, + { + code: outdent` + ${header} + + test.after(t => { + doFoo(); + }); + + /* comments */ + test.before(t => { + doFoo(); + }); + `, + output: outdent` + ${header} + + /* comments */ + test.before(t => { + doFoo(); + }); + + test.after(t => { + doFoo(); + }); + `, + errors + } + ] +});