Improve target reporting (#182)

* no-only-test: Target `only` modifier, not the whole test

* no-unknown-modifiers: Target unknown modifier, not the whole test

* no-skip-test: Target `skip` modifier, not the whole test

* no-duplicate-modifiers: Target duplicate modifier, not the whole test

* no-async-fn-without-await: Target async modifier, not the whole test

* no-cb-test: Target `cb` modifier, not the whole test

* no-identical-title: Target title, not the whole test

create-ava-rule.js

@@ -2,6 +2,7 @@
 const espurify = require('espurify');
 const enhance = require('enhance-visitors');
 const deepStrictEqual = require('deep-strict-equal');
+const util = require('./util');
 
 const avaImportDeclarationAst = {
 	type: 'ImportDeclaration',
@@ -51,18 +52,8 @@ function isTestFunctionCall(node) {
 	return false;
 }
 
-function hasTestModifier(node, mod) {
-	if (node.type === 'CallExpression') {
-		return hasTestModifier(node.callee, mod);
-	} else if (node.type === 'MemberExpression') {
-		if (node.property.type === 'Identifier' && node.property.name === mod) {
-			return true;
-		}
-
-		return hasTestModifier(node.object, mod);
-	}
-
-	return false;
+function getTestModifierNames(node) {
+	return util.getTestModifiers(node).map(property => property.name);
 }
 
 module.exports = () => {
@@ -99,11 +90,14 @@ module.exports = () => {
 	};
 
 	return {
-		hasTestModifier: mod => hasTestModifier(currentTestNode, mod),
-		hasNoHookModifier: () => !hasTestModifier(currentTestNode, 'before') &&
-				!hasTestModifier(currentTestNode, 'beforeEach') &&
-				!hasTestModifier(currentTestNode, 'after') &&
-				!hasTestModifier(currentTestNode, 'afterEach'),
+		hasTestModifier: mod => getTestModifierNames(currentTestNode).indexOf(mod) >= 0,
+		hasNoHookModifier: () => {
+			const modifiers = getTestModifierNames(currentTestNode);
+			return modifiers.indexOf('before') === -1 &&
+				modifiers.indexOf('beforeEach') === -1 &&
+				modifiers.indexOf('after') === -1 &&
+				modifiers.indexOf('afterEach') === -1;
+		},
 		isInTestFile: () => isTestFile,
 		isInTestNode: () => currentTestNode,
 		isTestNode: node => currentTestNode === node,

rules/no-async-fn-without-await.js

@@ -4,11 +4,11 @@ const createAvaRule = require('../create-ava-rule');
 
 const create = context => {
 	const ava = createAvaRule();
-	let testIsAsync = false;
+	let asyncTest = null;
 	let testUsed = false;
 
 	const registerUseOfAwait = () => {
-		if (testIsAsync) {
+		if (asyncTest) {
 			testUsed = true;
 		}
 	};
@@ -20,21 +20,27 @@ const create = context => {
 			ava.isInTestFile,
 			ava.isTestNode
 		])(node => {
-			testIsAsync = isAsync(node.arguments[0]) || isAsync(node.arguments[1]);
+			asyncTest = (isAsync(node.arguments[0]) && node.arguments[0]) ||
+				(isAsync(node.arguments[1]) && node.arguments[1]) ||
+				null;
 		}),
 		AwaitExpression: registerUseOfAwait,
 		YieldExpression: registerUseOfAwait,
 		'CallExpression:exit': visitIf([
 			ava.isInTestFile,
 			ava.isTestNode
-		])(node => {
-			if (testIsAsync && !testUsed) {
+		])(() => {
+			if (asyncTest && !testUsed) {
 				context.report({
-					node,
+					node: asyncTest,
+					loc: {
+						start: asyncTest.loc.start,
+						end: asyncTest.loc.start + 5
+					},
 					message: 'Function was declared as `async` but doesn\'t use `await`'
 				});
 			}
-			testIsAsync = false;
+			asyncTest = null;
 			testUsed = false;
 		})
 	});

rules/no-cb-test.js

@@ -1,6 +1,7 @@
 'use strict';
 const visitIf = require('enhance-visitors').visitIf;
 const createAvaRule = require('../create-ava-rule');
+const util = require('../util');
 
 const create = context => {
 	const ava = createAvaRule();
@@ -12,8 +13,8 @@ const create = context => {
 		])(node => {
 			if (ava.hasTestModifier('cb')) {
 				context.report({
-					node,
-					message: '`test.cb()` should be not be used.'
+					node: util.getTestModifier(node, 'cb'),
+					message: '`test.cb()` should not be used.'
 				});
 			}
 		})

rules/no-duplicate-modifiers.js

@@ -3,6 +3,18 @@ const visitIf = require('enhance-visitors').visitIf;
 const util = require('../util');
 const createAvaRule = require('../create-ava-rule');
 
+function sortByName(a, b) {
+	if (a.name < b.name) {
+		return -1;
+	}
+
+	if (a.name > b.name) {
+		return 1;
+	}
+
+	return 0;
+}
+
 const create = context => {
 	const ava = createAvaRule();
 
@@ -11,16 +23,17 @@ const create = context => {
 			ava.isInTestFile,
 			ava.isTestNode
 		])(node => {
-			const testModifiers = util.getTestModifiers(node).sort();
+			const testModifiers = util.getTestModifiers(node).sort(sortByName);
+
 			if (testModifiers.length === 0) {
 				return;
 			}
 
 			testModifiers.reduce((prev, current) => {
-				if (prev === current) {
+				if (prev.name === current.name) {
 					context.report({
-						node,
-						message: `Duplicate test modifier \`${current}\`.`
+						node: current,
+						message: `Duplicate test modifier \`${current.name}\`.`
 					});
 				}
 				return current;

rules/no-identical-title.js

@@ -45,7 +45,7 @@ const create = context => {
 
 			if (isTitleUsed(usedTitleNodes, titleNode)) {
 				context.report({
-					node,
+					node: titleNode,
 					message: 'Test title is used multiple times in the same file.'
 				});
 				return;

rules/no-only-test.js

@@ -1,7 +1,7 @@
 'use strict';
 const visitIf = require('enhance-visitors').visitIf;
 const createAvaRule = require('../create-ava-rule');
-const getTestModifier = require('../util').getTestModifier;
+const util = require('../util');
 
 const create = context => {
 	const ava = createAvaRule();
@@ -11,12 +11,13 @@ const create = context => {
 			ava.isInTestFile,
 			ava.isTestNode
 		])(node => {
-			if (ava.hasTestModifier('only')) {
+			const propertyNode = util.getTestModifier(node, 'only');
+			if (propertyNode) {
 				context.report({
-					node,
+					node: propertyNode,
 					message: '`test.only()` should not be used.',
 					fix: fixer => {
-						const range = getTestModifier(node, 'only').range.slice();
+						const range = propertyNode.range.slice();
 						const source = context.getSourceCode().getText();
 						let dotPosition = range[0] - 1;
 						while (source.charAt(dotPosition) !== '.') {

rules/no-skip-test.js

@@ -1,6 +1,7 @@
 'use strict';
 const visitIf = require('enhance-visitors').visitIf;
 const createAvaRule = require('../create-ava-rule');
+const util = require('../util');
 
 const create = context => {
 	const ava = createAvaRule();
@@ -10,9 +11,10 @@ const create = context => {
 			ava.isInTestFile,
 			ava.isTestNode
 		])(node => {
-			if (ava.hasTestModifier('skip')) {
+			const propertyNode = util.getTestModifier(node, 'skip');
+			if (propertyNode) {
 				context.report({
-					node,
+					node: propertyNode,
 					message: 'No tests should be skipped.'
 				});
 			}

rules/no-unknown-modifiers.js

@@ -18,7 +18,7 @@ const modifiers = [
 ];
 
 const unknownModifiers = node => util.getTestModifiers(node)
-	.filter(modifier => modifiers.indexOf(modifier) === -1);
+	.filter(modifier => modifiers.indexOf(modifier.name) === -1);
 
 const create = context => {
 	const ava = createAvaRule();
@@ -32,8 +32,8 @@ const create = context => {
 
 			if (unknown.length !== 0) {
 				context.report({
-					node,
-					message: `Unknown test modifier \`${unknown[0]}\`.`
+					node: unknown[0],
+					message: `Unknown test modifier \`${unknown[0].name}\`.`
 				});
 			}
 		})

test/no-async-fn-without-await.js

@@ -2,10 +2,8 @@ import test from 'ava';
 import avaRuleTester from 'eslint-ava-rule-tester';
 import rule from '../rules/no-async-fn-without-await';
 
-const error = {
-	ruleId: 'no-async-fn-without-await',
-	message: 'Function was declared as `async` but doesn\'t use `await`'
-};
+const ruleId = 'no-async-fn-without-await';
+const message = 'Function was declared as `async` but doesn\'t use `await`';
 const header = `const test = require('ava');\n`;
 
 const ruleTesterOptions = [
@@ -27,45 +25,87 @@ ruleTesterOptions.forEach(options => {
 
 	ruleTester.run(`no-async-fn-without-await`, rule, {
 		valid: [
-			`${header} test(fn);`,
-			`${header} test(t => {});`,
-			`${header} test(function(t) {});`,
-			`${header} test(async t => { await foo(); });`,
-			`${header} test(async t => { t.is(await foo(), 1); });`,
-			`${header} test(async function(t) { await foo(); });`,
-			`${header} test(async t => { if (bar) { await foo(); } });`,
-			`${header} test(async t => { if (bar) {} else { await foo(); } });`,
-			`${header} test.after(async () => { await foo(); });`,
-			`${header} test('title', fn);`,
-			`${header} test('title', function(t) {});`,
-			`${header} test('title', async t => { await foo(); });`,
+			`${header}test(fn);`,
+			`${header}test(t => {});`,
+			`${header}test(function(t) {});`,
+			`${header}test(async t => { await foo(); });`,
+			`${header}test(async t => { t.is(await foo(), 1); });`,
+			`${header}test(async function(t) { await foo(); });`,
+			`${header}test(async t => { if (bar) { await foo(); } });`,
+			`${header}test(async t => { if (bar) {} else { await foo(); } });`,
+			`${header}test.after(async () => { await foo(); });`,
+			`${header}test('title', fn);`,
+			`${header}test('title', function(t) {});`,
+			`${header}test('title', async t => { await foo(); });`,
 			// Shouldn't be triggered since it's not a test file
 			'test(async t => {});'
 		],
 		invalid: [
 			{
-				code: `${header} test(async t => {});`,
-				errors: [error]
+				code: `${header}test(async t => {});`,
+				errors: [{
+					ruleId,
+					message,
+					type: 'ArrowFunctionExpression',
+					line: 2,
+					column: 6
+				}]
 			},
 			{
-				code: `${header} test(async function(t) {});`,
-				errors: [error]
+				code: `${header}test(async function(t) {});`,
+				errors: [{
+					ruleId,
+					message,
+					type: 'FunctionExpression',
+					line: 2,
+					column: 6
+				}]
 			},
 			{
-				code: `${header} test(async t => {}); test(async t => {});`,
-				errors: [error, error]
+				code: `${header}test(async t => {}); test(async t => {});`,
+				errors: [{
+					ruleId,
+					message,
+					type: 'ArrowFunctionExpression',
+					line: 2,
+					column: 6
+				}, {
+					ruleId,
+					message,
+					type: 'ArrowFunctionExpression',
+					line: 2,
+					column: 27
+				}]
 			},
 			{
-				code: `${header} test(async t => {}); test(async t => { await foo(); });`,
-				errors: [error]
+				code: `${header}test(async t => {}); test(async t => { await foo(); });`,
+				errors: [{
+					ruleId,
+					message,
+					type: 'ArrowFunctionExpression',
+					line: 2,
+					column: 6
+				}]
 			},
 			{
-				code: `${header} test(async t => { await foo(); }); test(async t => {});`,
-				errors: [error]
+				code: `${header}test(async t => { await foo(); }); test(async t => {});`,
+				errors: [{
+					ruleId,
+					message,
+					type: 'ArrowFunctionExpression',
+					line: 2,
+					column: 41
+				}]
 			},
 			{
-				code: `${header} test('title', async t => {});`,
-				errors: [error]
+				code: `${header}test('title', async t => {});`,
+				errors: [{
+					ruleId,
+					message,
+					type: 'ArrowFunctionExpression',
+					line: 2,
+					column: 15
+				}]
 			}
 		]
 	});