Detect more assertions in prefer-t-regex

rules/prefer-t-regex.js

@@ -13,75 +13,186 @@ const create = context => {
 		'falsy'
 	]);
 
+	const equalityTests = new Set([
+		'is',
+		'deepEqual'
+	]);
+
+	// Find the latest reference to the given identifier's name.
 	const findReference = name => {
 		const reference = context.getScope().references.find(reference => reference.identifier.name === name);
-		const definitions = reference.resolved.defs;
-		return definitions[definitions.length - 1].node;
-	};
 
-	return ava.merge({
-		CallExpression: visitIf([
-			ava.isInTestFile,
-			ava.isInTestNode
-		])(node => {
-			// Call a boolean assertion, for example, `t.true`, `t.false`, …
-			const isBooleanAssertion = node.callee.type === 'MemberExpression' &&
-				booleanTests.has(node.callee.property.name) &&
-				util.getNameOfRootNodeObject(node.callee) === 't';
+		if (reference && reference.resolved) {
+			const definitions = reference.resolved.defs;
 
-			if (!isBooleanAssertion) {
+			if (definitions.length === 0) {
 				return;
 			}
 
-			const firstArg = node.arguments[0];
+			return definitions[definitions.length - 1].node;
+		}
+	};
 
-			// First argument is a call expression
-			const isFunctionCall = firstArg.type === 'CallExpression';
-			if (!isFunctionCall || !firstArg.callee.property) {
-				return;
-			}
+	/*
+	Recursively find the "origin" node of the given node.
 
-			const {name} = firstArg.callee.property;
-			let lookup = {};
-			let variable = {};
-
-			if (name === 'test') {
-				// `lookup.test(variable)`
-				lookup = firstArg.callee.object;
-				variable = firstArg.arguments[0];
-			} else if (['search', 'match'].includes(name)) {
-				// `variable.match(lookup)`
-				lookup = firstArg.arguments[0];
-				variable = firstArg.callee.object;
-			}
+	Note: `context.getScope()` doesn't contain information about the outer scope so in most cases this function will only find the reference directly above the current scope. So the following code will only find the reference in this order: y -> x, and it will have no knowledge of the number `0`. (assuming we run this function on the identifier `y`)
 
-			let isRegExp = lookup.regex;
+	```
+	const test = require('ava');
 
-			// It's not a regexp but an identifier
-			if (!isRegExp && lookup.type === 'Identifier') {
-				const reference = findReference(lookup.name);
-				isRegExp = reference.init.regex;
-			}
+	let x = 0;
+	let y = x;
 
-			if (!isRegExp) {
-				return;
+	test(t => {
+		t.is(y, 0);
+	});
+	```
+	*/
+	const findRootReference = node => {
+		if (node.type === 'Identifier') {
+			const reference = findReference(node.name);
+
+			if (reference && reference.init) {
+				return findRootReference(reference.init);
 			}
 
-			const assertion = ['true', 'truthy'].includes(node.callee.property.name) ? 'regex' : 'notRegex';
+			return node;
+		}
+
+		if (node.type === 'CallExpression' || node.type === 'NewExpression') {
+			return findRootReference(node.callee);
+		}
+
+		if (node.type === 'MemberExpression') {
+			return findRootReference(node.object);
+		}
+
+		return node;
+	};
+
+	/*
+	Determine if the given node is a regex expression.
+
+	There are two ways to create regex expressions in JavaScript: Regex literal and `RegExp` class.
+	1. Regex literal can be easily looked up using the `.regex` property on the node.
+	2. `RegExp` class can't be looked up so the function just checks for the name `RegExp`.
+	*/
+	const isRegExp = lookup => {
+		if (lookup.regex) {
+			return true;
+		}
+
+		// Look up references in case it's a variable or RegExp declaration.
+		const reference = findRootReference(lookup);
+		return reference.regex || reference.name === 'RegExp';
+	};
+
+	const booleanHandler = node => {
+		const firstArg = node.arguments[0];
+
+		const isFunctionCall = firstArg.type === 'CallExpression';
+		if (!isFunctionCall || !firstArg.callee.property) {
+			return;
+		}
+
+		const {name} = firstArg.callee.property;
+		let lookup = {};
+		let variable = {};
+
+		if (name === 'test') {
+			// Represent: `lookup.test(variable)`
+			lookup = firstArg.callee.object;
+			variable = firstArg.arguments[0];
+		} else if (['search', 'match'].includes(name)) {
+			// Represent: `variable.match(lookup)`
+			lookup = firstArg.arguments[0];
+			variable = firstArg.callee.object;
+		}
+
+		if (!isRegExp(lookup)) {
+			return;
+		}
+
+		const assertion = ['true', 'truthy'].includes(node.callee.property.name) ? 'regex' : 'notRegex';
+
+		const fix = fixer => {
+			const source = context.getSourceCode();
+			return [
+				fixer.replaceText(node.callee.property, assertion),
+				fixer.replaceText(firstArg, `${source.getText(variable)}, ${source.getText(lookup)}`)
+			];
+		};
+
+		context.report({
+			node,
+			message: `Prefer using the \`t.${assertion}()\` assertion.`,
+			fix
+		});
+	};
 
-			const fix = fixer => {
-				const source = context.getSourceCode();
-				return [
-					fixer.replaceText(node.callee.property, assertion),
-					fixer.replaceText(firstArg, `${source.getText(variable)}, ${source.getText(lookup)}`)
-				];
-			};
+	const equalityHandler = node => {
+		const [firstArg, secondArg] = node.arguments;
+
+		const firstArgumentIsRegex = isRegExp(firstArg);
+		const secondArgumentIsRegex = isRegExp(secondArg);
+
+		// If both are regex, or neither are, the expression is ok.
+		if (firstArgumentIsRegex === secondArgumentIsRegex) {
+			return;
+		}
+
+		const matchee = secondArgumentIsRegex ? firstArg : secondArg;
+		const regex = secondArgumentIsRegex ? secondArg : firstArg;
+
+		const booleanFixer = assertion => fixer => {
+			const source = context.getSourceCode();
+			return [
+				fixer.replaceText(node.callee.property, assertion),
+				fixer.replaceText(firstArg, `${source.getText(regex.arguments[0])}`),
+				fixer.replaceText(secondArg, `${source.getText(regex.callee.object)}`)
+			];
+		};
+
+		// Only fix a statically verifiable equality.
+		if (regex && matchee.type === 'Literal') {
+			let assertion;
+
+			if (matchee.raw === 'true') {
+				assertion = 'regex';
+			} else if (matchee.raw === 'false') {
+				assertion = 'notRegex';
+			} else {
+				return;
+			}
 
 			context.report({
 				node,
 				message: `Prefer using the \`t.${assertion}()\` assertion.`,
-				fix
+				fix: booleanFixer(assertion)
 			});
+		}
+	};
+
+	return ava.merge({
+		CallExpression: visitIf([
+			ava.isInTestFile,
+			ava.isInTestNode
+		])(node => {
+			const isAssertion = node.callee.type === 'MemberExpression' &&
+				util.getNameOfRootNodeObject(node.callee) === 't';
+
+			const isBooleanAssertion = isAssertion &&
+				booleanTests.has(node.callee.property.name);
+
+			const isEqualityAssertion = isAssertion &&
+				equalityTests.has(node.callee.property.name);
+
+			if (isBooleanAssertion) {
+				booleanHandler(node);
+			} else if (isEqualityAssertion) {
+				equalityHandler(node);
+			}
 		})
 	});
 };

test/prefer-t-regex.js

@@ -17,12 +17,21 @@ ruleTester.run('prefer-t-regex', rule, {
 	valid: [
 		header + 'test(t => t.regex("foo", /\\d+/));',
 		header + 'test(t => t.regex(foo(), /\\d+/));',
-		header + 'test(t => t.is(/\\d+/.test("foo")), true);',
+		header + 'test(t => t.is(/\\d+/.test("foo"), foo));',
+		header + 'test(t => t.is(RegExp("\\d+").test("foo"), foo));',
+		header + 'test(t => t.is(RegExp(/\\d+/).test("foo"), foo));',
+		header + 'test(t => t.is(/\\d+/, /\\w+/));',
+		header + 'test(t => t.is(123, /\\d+/));',
 		header + 'test(t => t.true(1 === 1));',
 		header + 'test(t => t.true(foo.bar()));',
+		header + 'test(t => t.is(foo, true));',
 		header + 'const a = /\\d+/;\ntest(t => t.truthy(a));',
 		header + 'const a = "not a regexp";\ntest(t => t.true(a.test("foo")));',
 		header + 'test("main", t => t.true(foo()));',
+		header + 'test(t => t.regex(foo, new RegExp("\\d+")));',
+		header + 'test(t => t.regex(foo, RegExp("\\d+")));',
+		header + 'test(t => t.regex(foo, new RegExp(/\\d+/)));',
+		header + 'test(t => t.regex(foo, RegExp(/\\d+/)));',
 		// Shouldn't be triggered since it's not a test file
 		'test(t => t.true(/\\d+/.test("foo")));'
 	],
@@ -57,10 +66,112 @@ ruleTester.run('prefer-t-regex', rule, {
 			output: header + 'test(t => t.regex(foo(), /\\d+/));',
 			errors: errors('regex')
 		},
+		{
+			code: header + 'test(t => t.is(/\\d+/.test(foo), true));',
+			output: header + 'test(t => t.regex(foo, /\\d+/));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.is(/\\d+/.test(foo), false));',
+			output: header + 'test(t => t.notRegex(foo, /\\d+/));',
+			errors: errors('notRegex')
+		},
 		{
 			code: header + 'const reg = /\\d+/;\ntest(t => t.true(reg.test(foo.bar())));',
 			output: header + 'const reg = /\\d+/;\ntest(t => t.regex(foo.bar(), reg));',
 			errors: errors('regex')
+		},
+		// The same as the above tests but with `RegExp()` object instead of a regex literal
+		{
+			code: header + 'test(t => t.true(new RegExp("\\d+").test("foo")));',
+			output: header + 'test(t => t.regex("foo", new RegExp("\\d+")));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.false(foo.search(new RegExp("\\d+"))));',
+			output: header + 'test(t => t.notRegex(foo, new RegExp("\\d+")));',
+			errors: errors('notRegex')
+		},
+		{
+			code: header + 'const regexp = RegExp("\\d+");\ntest(t => t.true(foo.search(regexp)));',
+			output: header + 'const regexp = RegExp("\\d+");\ntest(t => t.regex(foo, regexp));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.truthy(foo.match(new RegExp("\\d+"))));',
+			output: header + 'test(t => t.regex(foo, new RegExp("\\d+")));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.false(RegExp("\\d+").test("foo")));',
+			output: header + 'test(t => t.notRegex("foo", RegExp("\\d+")));',
+			errors: errors('notRegex')
+		},
+		{
+			code: header + 'test(t => t.true(new RegExp("\\d+").test(foo())));',
+			output: header + 'test(t => t.regex(foo(), new RegExp("\\d+")));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.is(new RegExp("\\d+").test(foo), true));',
+			output: header + 'test(t => t.regex(foo, new RegExp("\\d+")));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.is(new RegExp("\\d+").test(foo), false));',
+			output: header + 'test(t => t.notRegex(foo, new RegExp("\\d+")));',
+			errors: errors('notRegex')
+		},
+		{
+			code: header + 'const reg = RegExp("\\d+");\ntest(t => t.true(reg.test(foo.bar())));',
+			output: header + 'const reg = RegExp("\\d+");\ntest(t => t.regex(foo.bar(), reg));',
+			errors: errors('regex')
+		},
+		// The same as the above tests but with regex literal instead of string regex
+		{
+			code: header + 'test(t => t.true(new RegExp(/\\d+/).test("foo")));',
+			output: header + 'test(t => t.regex("foo", new RegExp(/\\d+/)));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.false(foo.search(new RegExp(/\\d+/))));',
+			output: header + 'test(t => t.notRegex(foo, new RegExp(/\\d+/)));',
+			errors: errors('notRegex')
+		},
+		{
+			code: header + 'const regexp = RegExp(/\\d+/);\ntest(t => t.true(foo.search(regexp)));',
+			output: header + 'const regexp = RegExp(/\\d+/);\ntest(t => t.regex(foo, regexp));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.truthy(foo.match(new RegExp(/\\d+/))));',
+			output: header + 'test(t => t.regex(foo, new RegExp(/\\d+/)));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.false(RegExp(/\\d+/).test("foo")));',
+			output: header + 'test(t => t.notRegex("foo", RegExp(/\\d+/)));',
+			errors: errors('notRegex')
+		},
+		{
+			code: header + 'test(t => t.true(new RegExp(/\\d+/).test(foo())));',
+			output: header + 'test(t => t.regex(foo(), new RegExp(/\\d+/)));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.is(new RegExp(/\\d+/).test(foo), true));',
+			output: header + 'test(t => t.regex(foo, new RegExp(/\\d+/)));',
+			errors: errors('regex')
+		},
+		{
+			code: header + 'test(t => t.is(new RegExp(/\\d+/).test(foo), false));',
+			output: header + 'test(t => t.notRegex(foo, new RegExp(/\\d+/)));',
+			errors: errors('notRegex')
+		},
+		{
+			code: header + 'const reg = RegExp(/\\d+/);\ntest(t => t.true(reg.test(foo.bar())));',
+			output: header + 'const reg = RegExp(/\\d+/);\ntest(t => t.regex(foo.bar(), reg));',
+			errors: errors('regex')
 		}
 	]
 });