Catch & fix out-of-order assertion arguments (#307)

Co-authored-by: Sindre Sorhus <[email protected]>

Author: ninevra

docs/rules/assertion-arguments.md

@@ -6,6 +6,8 @@ Enforces passing the right number of arguments to assertion methods like `t.is()
 
 Assertion messages are optional arguments that can be given to any assertion call to improve the error message, should the assertion fail.
 
+This rule also attempts to enforce passing actual values before expected values. If exactly one of the first two arguments to a two-argument assertion is a static expression such as `{a: 1}`, then the static expression must come second. (`t.regex()` and `t.notRegex()` are excluded from this check, because either their `contents` argument or their `regex` argument could plausibly be the actual or expected value.) If the argument to a one-argument assertion is a binary relation such as `'static' === dynamic`, a similar check is performed on its left- and right-hand sides. Errors of these kinds are usually fixable.
+
 ## Fail
 
 ```js

package.json

@@ -32,6 +32,7 @@
 	"dependencies": {
 		"deep-strict-equal": "^0.2.0",
 		"enhance-visitors": "^1.0.0",
+		"eslint-utils": "^2.1.0",
 		"espree": "^7.1.0",
 		"espurify": "^2.0.1",
 		"import-modules": "^2.0.0",

rules/assertion-arguments.js

@@ -1,9 +1,14 @@
 'use strict';
 const {visitIf} = require('enhance-visitors');
+const {getStaticValue, isOpeningParenToken, isCommaToken} = require('eslint-utils');
 const util = require('../util');
 const createAvaRule = require('../create-ava-rule');
 
 const expectedNbArguments = {
+	assert: {
+		min: 1,
+		max: 2
+	},
 	deepEqual: {
 		min: 2,
 		max: 3
@@ -44,6 +49,10 @@ const expectedNbArguments = {
 		min: 1,
 		max: 2
 	},
+	notThrowsAsync: {
+		min: 1,
+		max: 2
+	},
 	pass: {
 		min: 0,
 		max: 1
@@ -72,6 +81,10 @@ const expectedNbArguments = {
 		min: 1,
 		max: 3
 	},
+	throwsAsync: {
+		min: 1,
+		max: 3
+	},
 	true: {
 		min: 1,
 		max: 2
@@ -86,6 +99,111 @@ const expectedNbArguments = {
 	}
 };
 
+const actualExpectedAssertions = new Set([
+	'deepEqual',
+	'is',
+	'like',
+	'not',
+	'notDeepEqual',
+	'throws',
+	'throwsAsync'
+]);
+
+const relationalActualExpectedAssertions = new Set([
+	'assert',
+	'truthy',
+	'falsy',
+	'true',
+	'false'
+]);
+
+const comparisonOperators = new Map([
+	['>', '<'],
+	['>=', '<='],
+	['==', '=='],
+	['===', '==='],
+	['!=', '!='],
+	['!==', '!=='],
+	['<=', '>='],
+	['<', '>']
+]);
+
+const flipOperator = operator => comparisonOperators.get(operator);
+
+function isStatic(node) {
+	const staticValue = getStaticValue(node);
+	return staticValue !== null && typeof staticValue.value !== 'function';
+}
+
+function * sourceRangesOfArguments(sourceCode, callExpression) {
+	const openingParen = sourceCode.getTokenAfter(
+		callExpression.callee,
+		{filter: token => isOpeningParenToken(token)}
+	);
+
+	const closingParen = sourceCode.getLastToken(callExpression);
+
+	for (const [index, argument] of callExpression.arguments.entries()) {
+		const previousToken = index === 0 ?
+			openingParen :
+			sourceCode.getTokenBefore(
+				argument,
+				{filter: token => isCommaToken(token)}
+			);
+
+		const nextToken = index === callExpression.arguments.length - 1 ?
+			closingParen :
+			sourceCode.getTokenAfter(
+				argument,
+				{filter: token => isCommaToken(token)}
+			);
+
+		const firstToken = sourceCode.getTokenAfter(
+			previousToken,
+			{includeComments: true}
+		);
+
+		const lastToken = sourceCode.getTokenBefore(
+			nextToken,
+			{includeComments: true}
+		);
+
+		yield [firstToken.range[0], lastToken.range[1]];
+	}
+}
+
+function sourceOfBinaryExpressionComponents(sourceCode, node) {
+	const {operator, left, right} = node;
+
+	const operatorToken = sourceCode.getFirstTokenBetween(
+		left,
+		right,
+		{filter: token => token.value === operator}
+	);
+
+	const previousToken = sourceCode.getTokenBefore(node);
+	const nextToken = sourceCode.getTokenAfter(node);
+
+	const leftRange = [
+		sourceCode.getTokenAfter(previousToken, {includeComments: true}).range[0],
+		sourceCode.getTokenBefore(operatorToken, {includeComments: true}).range[1]
+	];
+
+	const rightRange = [
+		sourceCode.getTokenAfter(operatorToken, {includeComments: true}).range[0],
+		sourceCode.getTokenBefore(nextToken, {includeComments: true}).range[1]
+	];
+
+	return [leftRange, operatorToken, rightRange];
+}
+
+function noComments(sourceCode, ...nodes) {
+	return nodes.every(node => {
+		const {leading, trailing} = sourceCode.getComments(node);
+		return leading.length === 0 && trailing.length === 0;
+	});
+}
+
 const create = context => {
 	const ava = createAvaRule();
 	const options = context.options[0] || {};
@@ -141,19 +259,102 @@ const create = context => {
 				report(node, `Not enough arguments. Expected at least ${nArgs.min}.`);
 			} else if (node.arguments.length > nArgs.max) {
 				report(node, `Too many arguments. Expected at most ${nArgs.max}.`);
-			} else if (enforcesMessage && nArgs.min !== nArgs.max) {
-				const hasMessage = gottenArgs === nArgs.max;
+			} else {
+				if (enforcesMessage && nArgs.min !== nArgs.max) {
+					const hasMessage = gottenArgs === nArgs.max;
 
-				if (!hasMessage && shouldHaveMessage) {
-					report(node, 'Expected an assertion message, but found none.');
-				} else if (hasMessage && !shouldHaveMessage) {
-					report(node, 'Expected no assertion message, but found one.');
+					if (!hasMessage && shouldHaveMessage) {
+						report(node, 'Expected an assertion message, but found none.');
+					} else if (hasMessage && !shouldHaveMessage) {
+						report(node, 'Expected no assertion message, but found one.');
+					}
 				}
+
+				checkArgumentOrder({node, assertion: members[0], context});
 			}
 		})
 	});
 };
 
+function checkArgumentOrder({node, assertion, context}) {
+	const [first, second] = node.arguments;
+	if (actualExpectedAssertions.has(assertion) && second) {
+		const [leftNode, rightNode] = [first, second];
+		if (isStatic(leftNode) && !isStatic(rightNode)) {
+			context.report(
+				makeOutOfOrder2ArgumentReport({node, leftNode, rightNode, context})
+			);
+		}
+	} else if (
+		relationalActualExpectedAssertions.has(assertion) &&
+		first &&
+		first.type === 'BinaryExpression' &&
+		comparisonOperators.has(first.operator)
+	) {
+		const [leftNode, rightNode] = [first.left, first.right];
+		if (isStatic(leftNode) && !isStatic(rightNode)) {
+			context.report(
+				makeOutOfOrder1ArgumentReport({node: first, leftNode, rightNode, context})
+			);
+		}
+	}
+}
+
+function makeOutOfOrder2ArgumentReport({node, leftNode, rightNode, context}) {
+	const sourceCode = context.getSourceCode();
+	const [leftRange, rightRange] = sourceRangesOfArguments(sourceCode, node);
+	const report = {
+		message: 'Expected values should come after actual values.',
+		loc: {
+			start: sourceCode.getLocFromIndex(leftRange[0]),
+			end: sourceCode.getLocFromIndex(rightRange[1])
+		}
+	};
+
+	if (noComments(sourceCode, leftNode, rightNode)) {
+		report.fix = fixer => {
+			const leftText = sourceCode.getText().slice(...leftRange);
+			const rightText = sourceCode.getText().slice(...rightRange);
+			return [
+				fixer.replaceTextRange(leftRange, rightText),
+				fixer.replaceTextRange(rightRange, leftText)
+			];
+		};
+	}
+
+	return report;
+}
+
+function makeOutOfOrder1ArgumentReport({node, leftNode, rightNode, context}) {
+	const sourceCode = context.getSourceCode();
+	const [
+		leftRange,
+		operatorToken,
+		rightRange
+	] = sourceOfBinaryExpressionComponents(sourceCode, node);
+	const report = {
+		message: 'Expected values should come after actual values.',
+		loc: {
+			start: sourceCode.getLocFromIndex(leftRange[0]),
+			end: sourceCode.getLocFromIndex(rightRange[1])
+		}
+	};
+
+	if (noComments(sourceCode, leftNode, rightNode, node)) {
+		report.fix = fixer => {
+			const leftText = sourceCode.getText().slice(...leftRange);
+			const rightText = sourceCode.getText().slice(...rightRange);
+			return [
+				fixer.replaceTextRange(leftRange, rightText),
+				fixer.replaceText(operatorToken, flipOperator(node.operator)),
+				fixer.replaceTextRange(rightRange, leftText)
+			];
+		};
+	}
+
+	return report;
+}
+
 const schema = [{
 	type: 'object',
 	properties: {
@@ -170,6 +371,7 @@ const schema = [{
 module.exports = {
 	create,
 	meta: {
+		fixable: 'code',
 		docs: {
 			url: util.getDocsUrl(__filename)
 		},