Improve prefer-includes rule (#411)

Author: fisker

rules/prefer-includes.js

@@ -2,21 +2,30 @@
 const getDocsUrl = require('./utils/get-docs-url');
 const isMethodNamed = require('./utils/is-method-named');
 
+// Ignore {_,lodash,underscore}.indexOf
+const ignoredVariables = new Set(['_', 'lodash', 'underscore']);
 const isNegativeOne = (operator, value) => operator === '-' && value === 1;
+const isIgnoredTarget = node => node.type === 'Identifier' && ignoredVariables.has(node.name);
 
-const report = (context, node, target, pattern) => {
+const report = (context, node, target, argumentsNodes) => {
 	const sourceCode = context.getSourceCode();
 	const memberExpressionNode = target.parent;
 	const dotToken = sourceCode.getTokenBefore(memberExpressionNode.property);
 	const targetSource = sourceCode.getText().slice(memberExpressionNode.range[0], dotToken.range[0]);
-	const patternSource = sourceCode.getText(pattern);
+
+	// Strip default `fromIndex`
+	if (argumentsNodes.length === 2 && argumentsNodes[1].type === 'Literal' && argumentsNodes[1].value === 0) {
+		argumentsNodes = argumentsNodes.slice(0, 1);
+	}
+
+	const argumentsSource = argumentsNodes.map(argument => sourceCode.getText(argument));
 
 	context.report({
 		node,
 		message: 'Use `.includes()`, rather than `.indexOf()`, when checking for existence.',
 		fix: fixer => {
 			const isNot = node => ['===', '==', '<'].includes(node.operator) ? '!' : '';
-			const replacement = `${isNot(node)}${targetSource}.includes(${patternSource})`;
+			const replacement = `${isNot(node)}${targetSource}.includes(${argumentsSource.join(', ')})`;
 			return fixer.replaceText(node, replacement);
 		}
 	});
@@ -26,29 +35,39 @@ const create = context => ({
 	BinaryExpression: node => {
 		const {left, right} = node;
 
-		if (isMethodNamed(left, 'indexOf')) {
-			const target = left.callee.object;
-			const pattern = left.arguments[0];
+		if (!isMethodNamed(left, 'indexOf')) {
+			return;
+		}
+
+		const target = left.callee.object;
 
-			if (right.type === 'UnaryExpression') {
-				const {argument} = right;
+		if (isIgnoredTarget(target)) {
+			return;
+		}
+
+		const {arguments: argumentsNodes} = left;
 
-				if (argument.type !== 'Literal') {
-					return false;
-				}
+		// Ignore something.indexOf(foo, 0, another)
+		if (argumentsNodes.length > 2) {
+			return;
+		}
 
-				const {value} = argument;
+		if (right.type === 'UnaryExpression') {
+			const {argument} = right;
 
-				if (['!==', '!=', '>', '===', '=='].includes(node.operator) && isNegativeOne(right.operator, value)) {
-					report(context, node, target, pattern);
-				}
+			if (argument.type !== 'Literal') {
+				return;
 			}
 
-			if (right.type === 'Literal' && ['>=', '<'].includes(node.operator) && right.value === 0) {
-				report(context, node, target, pattern);
+			const {value} = argument;
+
+			if (['!==', '!=', '>', '===', '=='].includes(node.operator) && isNegativeOne(right.operator, value)) {
+				report(context, node, target, argumentsNodes);
 			}
+		}
 
-			return false;
+		if (right.type === 'Literal' && ['>=', '<'].includes(node.operator) && right.value === 0) {
+			report(context, node, target, argumentsNodes);
 		}
 	}
 });

test/prefer-includes.js

@@ -17,13 +17,18 @@ ruleTester.run('prefer-includes', rule, {
 	valid: [
 		'str.indexOf(\'foo\') !== -n',
 		'str.indexOf(\'foo\') !== 1',
+		'str.indexOf(\'foo\') === -2',
 		'!str.indexOf(\'foo\') === 1',
 		'!str.indexOf(\'foo\') === -n',
 		'str.includes(\'foo\')',
 		'\'foobar\'.includes(\'foo\')',
 		'[1,2,3].includes(4)',
 		'null.indexOf(\'foo\') !== 1',
-		'f(0) < 0'
+		'f(0) < 0',
+		'something.indexOf(foo, 0, another) !== -1',
+		'_.indexOf(foo, bar) !== -1',
+		'lodash.indexOf(foo, bar) !== -1',
+		'underscore.indexOf(foo, bar) !== -1'
 	],
 	invalid: [
 		{
@@ -41,6 +46,11 @@ ruleTester.run('prefer-includes', rule, {
 			output: 'str.includes(\'foo\')',
 			errors
 		},
+		{
+			code: 'str.indexOf(\'foo\') == -1',
+			output: '!str.includes(\'foo\')',
+			errors
+		},
 		{
 			code: '\'foobar\'.indexOf(\'foo\') >= 0',
 			output: '\'foobar\'.includes(\'foo\')',
@@ -65,6 +75,16 @@ ruleTester.run('prefer-includes', rule, {
 			code: '(a || b).indexOf(\'foo\') === -1',
 			output: '!(a || b).includes(\'foo\')',
 			errors
+		},
+		{
+			code: 'foo.indexOf(bar, 0) !== -1',
+			output: 'foo.includes(bar)',
+			errors
+		},
+		{
+			code: 'foo.indexOf(bar, 1) !== -1',
+			output: 'foo.includes(bar, 1)',
+			errors
 		}
 	]
 });