Add prefer-includes rule - fixes #8

docs/rules/prefer-includes.md

@@ -0,0 +1,48 @@
+# Prefer .includes() over .indexOf() when checking for existence.
+
+Since ESLint has no type analysis we'll have to assume all properties named .indexOf() have a .includes() counterpart.
+
+This is luckily true for all builtins: String#includes, Array#includes, TypedArray#includes, Buffer#includes.
+
+
+## Fail
+
+```js
+const str = 'foobar';
+
+str.indexOf('foo') !== -1
+str.indexOf('foo') != -1
+str.indexOf('foo') > -1
+str.indexOf('foo') >= 0
+```
+
+```js
+const str = 'foobar';
+
+!str.includes === -1
+!str.includes == -1
+!str.includes < 0
+```
+
+
+## Pass
+
+```js
+const str = 'foobar';
+
+str.includes('foo')
+```
+
+TODO:
+
+Would also be useful to catch cases where .includes() would be better than a regex:
+
+```js
+/\r\n/.test(foo);
+```
+
+Could be:
+
+```js
+foo.includes('\r\n');
+```

index.js

@@ -21,6 +21,7 @@ module.exports = {
 				'unicorn/throw-new-error': 'error',
 				'unicorn/number-literal-case': 'error',
 				'unicorn/no-array-instanceof': 'error',
+				'unicorn/prefer-includes': 'error',
 				'unicorn/no-new-buffer': 'error',
 				'unicorn/no-hex-escape': 'error'
 			}

rules/prefer-includes.js

@@ -0,0 +1,116 @@
+'use strict';
+
+const isIndexOfCallExpression = node => {
+	if (node.type !== 'CallExpression') {
+		return false;
+	}
+
+	const property = node.callee.property;
+
+	return property.name === 'indexOf';
+};
+
+const isUnaryNotExpression = node => (
+	node.type === 'UnaryExpression' && node.operator === '!'
+);
+
+const isNegativeOne = (operator, value) => operator === '-' && value === 1;
+
+const getSourceCode = (context, node) => (
+	context.getSourceCode().text.slice(node.range[0], node.range[1])
+);
+
+const report = (context, node, target, pattern) => {
+	const targetSource = getSourceCode(context, target);
+	const patternSource = getSourceCode(context, pattern);
+	context.report({
+		node,
+		message: 'Use `.includes()`, not .indexOf(), when checking for existence.',
+		fix: fixer => fixer.replaceText(node, `${targetSource}.includes(${patternSource})`)
+	});
+};
+
+const create = context => ({
+	BinaryExpression: node => {
+		const left = node.left;
+		const right = node.right;
+
+		if (isIndexOfCallExpression(left)) {
+			const target = left.callee.object;
+			const pattern = left.arguments[0];
+
+			if (right.type === 'UnaryExpression') {
+				const argument = right.argument;
+
+				if (argument.type !== 'Literal') {
+					return false;
+				}
+
+				const value = argument.value;
+
+				if (node.operator === '!==' && isNegativeOne(right.operator, value)) {
+					report(context, node, target, pattern);
+				}
+				if (node.operator === '!=' && isNegativeOne(right.operator, value)) {
+					report(context, node, target, pattern);
+				}
+				if (node.operator === '>' && isNegativeOne(right.operator, value)) {
+					report(context, node, target, pattern);
+				}
+			}
+
+			if (right.type !== 'Literal') {
+				return false;
+			}
+
+			if (node.operator === '>=' && right.value === 0) {
+				report(context, node, target, pattern);
+			}
+
+			return false;
+		}
+
+		if (isUnaryNotExpression(left)) {
+			const argument = left.argument;
+
+			if (isIndexOfCallExpression(argument)) {
+				const target = argument.callee.object;
+				const pattern = argument.arguments[0];
+
+				if (right.type === 'UnaryExpression') {
+					const argument = right.argument;
+
+					if (argument.type !== 'Literal') {
+						return false;
+					}
+
+					const value = argument.value;
+
+					if (node.operator === '===' && isNegativeOne(right.operator, value)) {
+						report(context, node, target, pattern);
+					}
+					if (node.operator === '==' && isNegativeOne(right.operator, value)) {
+						report(context, node, target, pattern);
+					}
+				}
+
+				if (right.type !== 'Literal') {
+					return false;
+				}
+
+				if (node.operator === '<' && right.value === 0) {
+					report(context, node, target, pattern);
+				}
+
+				return false;
+			}
+		}
+	}
+});
+
+module.exports = {
+	create,
+	meta: {
+		fixable: 'code'
+	}
+};

test/prefer-includes.js

@@ -0,0 +1,69 @@
+import test from 'ava';
+import avaRuleTester from 'eslint-ava-rule-tester';
+import rule from '../rules/prefer-includes';
+
+const ruleTester = avaRuleTester(test, {
+	env: {
+		es6: true
+	}
+});
+
+const errors = [{
+	ruleId: 'prefer-includes',
+	message: 'Use `.includes()`, not .indexOf(), when checking for existence.'
+}];
+
+ruleTester.run('prefer-includes', rule, {
+	valid: [
+		'str.indexOf(\'foo\') !== -n',
+		'str.indexOf(\'foo\') !== 1',
+		'!str.indexOf(\'foo\') === 1',
+		'!str.indexOf(\'foo\') === -n',
+		'str.includes(\'foo\')',
+		'\'foobar\'.includes(\'foo\')',
+		'[1,2,3].includes(4)',
+		'"str.indexOf(\'foo\') !== -1"'
+	],
+	invalid: [
+		{
+			code: '\'foobar\'.indexOf(\'foo\') !== -1',
+			output: '\'foobar\'.includes(\'foo\')',
+			errors
+		},
+		{
+			code: 'str.indexOf(\'foo\') != -1',
+			output: 'str.includes(\'foo\')',
+			errors
+		},
+		{
+			code: 'str.indexOf(\'foo\') > -1',
+			output: 'str.includes(\'foo\')',
+			errors
+		},
+		{
+			code: '\'foobar\'.indexOf(\'foo\') >= 0',
+			output: '\'foobar\'.includes(\'foo\')',
+			errors
+		},
+		{
+			code: '!str.indexOf(\'foo\') === -1',
+			output: 'str.includes(\'foo\')',
+			errors
+		},
+		{
+			code: '!str.indexOf(\'foo\') == -1',
+			output: 'str.includes(\'foo\')',
+			errors
+		},
+		{
+			code: '!\'foobar\'.indexOf(\'foo\') < 0',
+			output: '\'foobar\'.includes(\'foo\')',
+			errors
+		},
+		{
+			code: '[1,2,3].indexOf(4) !== -1',
+			output: '[1,2,3].includes(4)',
+			errors
+		}
+	]
+});