Add no-duplicate-modifiers rule (fixes #137) (#138)

Author: jfmengels

docs/rules/max-asserts.md

@@ -11,7 +11,7 @@ Skipped assertions are counted.
 
 ```js
 /*eslint ava/max-asserts: ["error", 5]*/
-import test = from 'ava';
+import test from 'ava';
 
 test('getSomeObject should define the players\' names', t => {
 	const object = lib.getSomeObject();
@@ -30,7 +30,7 @@ test('getSomeObject should define the players\' names', t => {
 ## Pass
 
 ```js
-import test = from 'ava';
+import test from 'ava';
 
 test('getSomeObject should define the player\'s name', t => {
 	const object = lib.getSomeObject();

docs/rules/no-duplicate-modifiers.md

@@ -0,0 +1,30 @@
+# Ensure tests do not have duplicate modifiers
+
+Translations: [Français](https://github.com/avajs/ava-docs/blob/master/fr_FR/related/eslint-plugin-ava/docs/rules/no-duplicate-modifiers.md)
+
+Prevent the use of duplicate [test modifiers](https://github.com/avajs/ava#api).
+
+
+## Fail
+
+```js
+import test from 'ava';
+
+test.only.only(t => {});
+test.serial.serial(t => {});
+test.cb.cb(t => {});
+test.beforeEach.beforeEach(t => {});
+test.only.only.cb(t => {});
+```
+
+
+## Pass
+
+```js
+import test from 'ava';
+
+test.only(t => {});
+test.serial(t => {});
+test.cb.only(t => {});
+test.beforeEach(t => {});
+```

docs/rules/no-unknown-modifiers.md

@@ -2,13 +2,13 @@
 
 Translations: [Français](https://github.com/avajs/ava-docs/blob/master/fr_FR/related/eslint-plugin-ava/docs/rules/no-unknown-modifiers.md)
 
-Prevent the use of unknown test modifiers.
+Prevent the use of unknown [test modifiers](https://github.com/avajs/ava#api).
 
 
 ## Fail
 
 ```js
-import test = from 'ava';
+import test from 'ava';
 
 test.onlu(t => {});
 test.seril(t => {});
@@ -21,7 +21,7 @@ test.unknown(t => {});
 ## Pass
 
 ```js
-import test = from 'ava';
+import test from 'ava';
 
 test.only(t => {});
 test.serial(t => {});

index.js

@@ -17,6 +17,7 @@ module.exports = {
 				'ava/assertion-message': ['off', 'always'],
 				'ava/max-asserts': ['off', 5],
 				'ava/no-cb-test': 'off',
+				'ava/no-duplicate-modifiers': 'error',
 				'ava/no-identical-title': 'error',
 				'ava/no-ignored-test-files': 'error',
 				'ava/no-invalid-end': 'error',

readme.md

@@ -35,6 +35,7 @@ Configure it in `package.json`.
 			"ava/assertion-message": ["off", "always"],
 			"ava/max-asserts": ["off", 5],
 			"ava/no-cb-test": "off",
+			"ava/no-duplicate-modifiers": "error",
 			"ava/no-identical-title": "error",
 			"ava/no-ignored-test-files": "error",
 			"ava/no-invalid-end": "error",
@@ -68,11 +69,12 @@ The rules will only activate in test files.
 - [assertion-message](docs/rules/assertion-message.md) - Enforce or disallow assertion messages.
 - [max-asserts](docs/rules/max-asserts.md) - Limit the number of assertions in a test.
 - [no-cb-test](docs/rules/no-cb-test.md) - Ensure no `test.cb()` is used.
+- [no-duplicate-modifiers](docs/rules/no-duplicate-modifiers.md) - Ensure tests do not have duplicate modifiers.
 - [no-identical-title](docs/rules/no-identical-title.md) - Ensure no tests have the same title.
 - [no-ignored-test-files](docs/rules/no-ignored-test-files.md) - Ensure no tests are written in ignored files.
 - [no-invalid-end](docs/rules/no-invalid-end.md) - Ensure `t.end()` is only called inside `test.cb()`.
-- [no-only-test](docs/rules/no-only-test.md) - Ensure no `test.only()` are present.
 - [no-nested-tests](docs/rules/no-nested-tests.md) - Ensure no tests are nested.
+- [no-only-test](docs/rules/no-only-test.md) - Ensure no `test.only()` are present.
 - [no-skip-assert](docs/rules/no-skip-assert.md) - Ensure no assertions are skipped.
 - [no-skip-test](docs/rules/no-skip-test.md) - Ensure no tests are skipped.
 - [no-statement-after-end](docs/rules/no-statement-after-end.md) - Ensure `t.end()` is the last statement executed.

rules/no-duplicate-modifiers.js

@@ -0,0 +1,35 @@
+'use strict';
+const visitIf = require('enhance-visitors').visitIf;
+const util = require('../util');
+const createAvaRule = require('../create-ava-rule');
+
+const create = context => {
+	const ava = createAvaRule();
+
+	return ava.merge({
+		CallExpression: visitIf([
+			ava.isInTestFile,
+			ava.isTestNode
+		])(node => {
+			const testModifiers = util.getTestModifiers(node).sort();
+			if (testModifiers.length === 0) {
+				return;
+			}
+
+			testModifiers.reduce((prev, current) => {
+				if (prev === current) {
+					context.report({
+						node,
+						message: `Duplicate test modifier \`${current}\`.`
+					});
+				}
+				return current;
+			});
+		})
+	});
+};
+
+module.exports = {
+	create,
+	meta: {}
+};

rules/no-unknown-modifiers.js

@@ -1,5 +1,6 @@
 'use strict';
 const visitIf = require('enhance-visitors').visitIf;
+const util = require('../util');
 const createAvaRule = require('../create-ava-rule');
 
 const modifiers = [
@@ -16,19 +17,7 @@ const modifiers = [
 	'failing'
 ];
 
-function getTestModifiers(node) {
-	if (node.type === 'CallExpression') {
-		return getTestModifiers(node.callee);
-	}
-
-	if (node.type === 'MemberExpression') {
-		return getTestModifiers(node.object).concat(node.property.name);
-	}
-
-	return [];
-}
-
-const unknownModifiers = node => getTestModifiers(node)
+const unknownModifiers = node => util.getTestModifiers(node)
 	.filter(modifier => modifiers.indexOf(modifier) === -1);
 
 const create = context => {

test/no-duplicate-modifiers.js

@@ -0,0 +1,58 @@
+import test from 'ava';
+import avaRuleTester from 'eslint-ava-rule-tester';
+import rule from '../rules/no-duplicate-modifiers';
+
+const ruleTester = avaRuleTester(test, {
+	env: {
+		es6: true
+	}
+});
+
+const ruleError = {ruleId: 'no-duplicate-modifiers'};
+const header = `const test = require('ava');\n`;
+
+const modifiers = [
+	'after',
+	'afterEach',
+	'always',
+	'before',
+	'beforeEach',
+	'cb',
+	'failing',
+	'only',
+	'serial',
+	'skip',
+	'todo'
+];
+
+const valid = modifiers.map(modifier => `${header} test.${modifier}(t => {});`);
+const invalid = modifiers.map(modifier => ({
+	code: `${header} test.${modifier}.${modifier}(t => {});`,
+	errors: [{
+		...ruleError,
+		message: `Duplicate test modifier \`${modifier}\`.`
+	}]
+}));
+
+ruleTester.run('no-duplicate-modifiers', rule, {
+	valid: valid.concat([
+		`${header} test(t => {});`,
+		`${header} test.cb.only(t => {});`,
+		`${header} test.after.always(t => {});`,
+		`${header} test.afterEach.always(t => {});`,
+		`${header} test.failing.cb(t => {});`,
+		// shouldn't be triggered since it's not a test file
+		`test.serial.serial(t => {});`
+	]),
+	invalid: invalid.concat([
+		{
+			code: `${header} test.serial.cb.only.serial(t => {});`,
+			errors: [
+				{
+					...ruleError,
+					message: 'Duplicate test modifier `serial`.'
+				}
+			]
+		}
+	])
+});

util.js

@@ -40,3 +40,15 @@ exports.getAvaConfig = filepath => {
 exports.isFunctionExpression = node => {
 	return node && functionExpressions.indexOf(node.type) !== -1;
 };
+
+exports.getTestModifiers = function getTestModifiers(node) {
+	if (node.type === 'CallExpression') {
+		return getTestModifiers(node.callee);
+	}
+
+	if (node.type === 'MemberExpression') {
+		return getTestModifiers(node.object).concat(node.property.name);
+	}
+
+	return [];
+};