Add consistent-indexof-check

docs/rules/consistent-indexof-check.md

@@ -0,0 +1,36 @@
+# Enforce consistent styling when checking for element existence using `indexOf()`
+
+💼 This rule is enabled in the ✅ `recommended` [config](https://github.com/sindresorhus/eslint-plugin-unicorn#preset-configs-eslintconfigjs).
+
+🔧 This rule is automatically fixable by the [`--fix` CLI option](https://eslint.org/docs/latest/user-guide/command-line-interface#--fix).
+
+<!-- end auto-generated rule header -->
+<!-- Do not manually modify this header. Run: `npm run fix:eslint-docs` -->
+
+Enforce consistent style when checking for element existence with `indexOf()`.
+
+Prefer using `index === -1` to check if an element does not exist and `index !== -1` to check if an element does exist.
+
+Similar to the [`explicit-length-check`](explicit-length-check.md) rule.
+
+## Examples
+
+```js
+// ❌
+const index = foo.indexOf('bar');
+if (index < 0) {}
+
+// ✅
+const index = foo.indexOf('bar');
+if (index === -1) {}
+```
+
+```js
+// ❌
+const index = foo.indexOf('bar');
+if (index >= 0) {}
+
+// ✅
+const index = foo.indexOf('bar');
+if (index !== -1) {}
+```

readme.md

@@ -115,6 +115,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
 | [consistent-destructuring](docs/rules/consistent-destructuring.md)                               | Use destructured variables over properties.                                                                                                                                                                       |    | 🔧 | 💡 |
 | [consistent-empty-array-spread](docs/rules/consistent-empty-array-spread.md)                     | Prefer consistent types when spreading a ternary in an array literal.                                                                                                                                             | ✅  | 🔧 |    |
 | [consistent-function-scoping](docs/rules/consistent-function-scoping.md)                         | Move function definitions to the highest possible scope.                                                                                                                                                          | ✅  |    |    |
+| [consistent-indexof-check](docs/rules/consistent-indexof-check.md)                               | Enforce consistent styling when checking for element existence using `indexOf()`                                                                                                                                  | ✅  | 🔧 |    |
 | [custom-error-definition](docs/rules/custom-error-definition.md)                                 | Enforce correct `Error` subclassing.                                                                                                                                                                              |    | 🔧 |    |
 | [empty-brace-spaces](docs/rules/empty-brace-spaces.md)                                           | Enforce no spaces between braces.                                                                                                                                                                                 | ✅  | 🔧 |    |
 | [error-message](docs/rules/error-message.md)                                                     | Enforce passing a `message` value when creating a built-in error.                                                                                                                                                 | ✅  |    |    |

rules/consistent-indexof-check.js

@@ -0,0 +1,175 @@
+'use strict';
+const resolveVariableName = require('./utils/resolve-variable-name.js');
+
+const MESSAGE_ID = 'consistent-indexof-check';
+const messages = {
+	[MESSAGE_ID]: 'Prefer `{{replacement}}` over `{{value}}`.',
+};
+
+/**
+Check if the node is a call expression of `indexOf` method.
+
+@param {import('estree').Node} node
+@returns {node is import('estree').CallExpression}
+*/
+function isIndexOfCallExpression(node) {
+	return (
+		node
+		&& node.type === 'CallExpression'
+		&& node.callee.type === 'MemberExpression'
+		&& node.callee.property.name === 'indexOf'
+		&& node.arguments.length === 1
+	);
+}
+
+/**
+Check if the operator is `<` or `<=` and the value is `0` or `-1`.
+
+@param {string} operator
+@param {string} value
+@returns {boolean}
+*/
+function isCheckNotExists(operator, value) {
+	return (
+		(operator === '<' && value <= 0)
+		|| (operator === '<=' && value <= -1)
+	);
+}
+
+/**
+Check if the operator is `>` or `>=` and the value is `-1` or `0`.
+
+@param {string} operator
+@param {number} value
+@returns {boolean}
+*/
+function isCheckExists(operator, value) {
+	return (
+		(operator === '>' && value === -1)
+		|| (operator === '>=' && value === 0)
+	);
+}
+
+/**
+Reverse the operator.
+
+@param {string} operator
+@returns {string}
+*/
+function reverseOperator(operator) {
+	switch (operator) {
+		case '<': {
+			return '>';
+		}
+
+		case '<=': {
+			return '>=';
+		}
+
+		case '>': {
+			return '<';
+		}
+
+		case '>=': {
+			return '<=';
+		}
+
+		default: {
+			return operator;
+		}
+	}
+}
+
+/** @param {import('eslint').Rule.RuleContext} context */
+const create = context => ({
+	/** @param {import('estree').IfStatement} node */
+	IfStatement(node) {
+		if (node.test.type !== 'BinaryExpression') {
+			return;
+		}
+
+		/** @type {import('estree').BinaryExpression | undefined} */
+		let testNode;
+		let variableName = '';
+		let literalValue = 0;
+		let operator = '';
+
+		if (
+			// Match case: index === -1
+			node.test.left.type === 'Identifier'
+			&& node.test.right.type === 'Literal'
+		) {
+			testNode = node.test;
+			variableName = testNode.left.name;
+			literalValue = testNode.right.value;
+			operator = testNode.operator;
+		} else if (
+			// Match case: -1 === index
+			node.test.right.type === 'Identifier'
+			&& node.test.left.type === 'Literal'
+		) {
+			testNode = node.test;
+			variableName = testNode.right.name;
+			literalValue = testNode.left.value;
+			operator = reverseOperator(testNode.operator);
+		}
+
+		if (!testNode) {
+			return;
+		}
+
+		let replacement = '';
+
+		// For better performance, early checking of operators can avoid looking up variables in scope.
+		if (isCheckNotExists(operator, literalValue)) {
+			replacement = `${variableName} === -1`;
+		} else if (isCheckExists(operator, literalValue)) {
+			replacement = `${variableName} !== -1`;
+		}
+
+		if (!replacement) {
+			return;
+		}
+
+		const variable = resolveVariableName(
+			variableName,
+			context.sourceCode.getScope(node),
+		);
+
+		if (!variable) {
+			return;
+		}
+
+		for (const definition of variable.defs) {
+			if (definition.type === 'Variable') {
+				if (!isIndexOfCallExpression(definition.node.init)) {
+					break;
+				}
+
+				context.report({
+					node: testNode,
+					messageId: MESSAGE_ID,
+					data: {
+						value: context.sourceCode.getText(testNode),
+						replacement,
+					},
+					fix: fixer => fixer.replaceText(testNode, replacement),
+				});
+			}
+		}
+	},
+});
+
+/** @type {import('eslint').Rule.RuleModule} */
+module.exports = {
+	create,
+	meta: {
+		type: 'problem',
+		docs: {
+			description: 'Enforce consistent styling when checking for element existence using `indexOf()`',
+			recommended: true,
+		},
+		fixable: 'code',
+		messages,
+	},
+};

rules/utils/resolve-variable-name.js

@@ -4,8 +4,8 @@
 Finds a variable named `name` in the scope `scope` (or it's parents).
 
 @param {string} name - The variable name to be resolve.
-@param {Scope} scope - The scope to look for the variable in.
-@returns {Variable?} - The found variable, if any.
+@param {import('eslint').Scope.Scope} scope - The scope to look for the variable in.
+@returns {import('eslint').Scope.Variable | void} - The found variable, if any.
 */
 module.exports = function resolveVariableName(name, scope) {
 	while (scope) {

test/consistent-indexof-check.mjs

@@ -0,0 +1,64 @@
+import outdent from 'outdent';
+import {getTester} from './utils/test.mjs';
+
+const {test} = getTester(import.meta);
+
+test.snapshot({
+	valid: [
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (index === -1) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (-1 === index) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (index !== -1) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (-1 !== index) {}
+		`,
+		outdent`
+			const index = 0; // index not from indexOf
+
+			if (index < 0) {}
+		`,
+	],
+	invalid: [
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (index < 0) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (0 > index) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (index >= 0) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			if (0 <= index) {}
+		`,
+		outdent`
+			const index = foo.indexOf('bar');
+
+			function foo () {
+				// It will search the scope chain for 'index' and find the 'index' variable declared above.
+				if (index < 0) {}
+			}
+		`,
+	],
+});

test/package.mjs

@@ -30,6 +30,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
 	'filename-case',
 	// Intended to not use `pass`/`fail` section in this rule.
 	'prefer-modern-math-apis',
+	'consistent-indexof-check',
 ]);
 
 test('Every rule is defined in index file in alphabetical order', t => {