Add hooks-order rule (#265)

Co-authored-by: Sindre Sorhus <sindresorhus@gmail.com>

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);
+});
+```

index.js

@@ -18,6 +18,7 @@ module.exports = {
 			],
 			rules: {
 				'ava/assertion-arguments': 'error',
+				'ava/hooks-order': 'error',
 				'ava/max-asserts': [
 					'off',
 					5

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"
 	},

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.

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'
+	}
+};