Skip to content

Commit

Permalink
Add consistent-indexof-check
Browse files Browse the repository at this point in the history
  • Loading branch information
axetroy committed Aug 19, 2024
1 parent e511ffd commit 5ff5b8a
Show file tree
Hide file tree
Showing 8 changed files with 428 additions and 2 deletions.
36 changes: 36 additions & 0 deletions docs/rules/consistent-indexof-check.md
Original file line number Diff line number Diff line change
@@ -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) {}
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. || | |
Expand Down
175 changes: 175 additions & 0 deletions rules/consistent-indexof-check.js
Original file line number Diff line number Diff line change
@@ -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,
},
};
4 changes: 2 additions & 2 deletions rules/utils/resolve-variable-name.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
64 changes: 64 additions & 0 deletions test/consistent-non-exists-index-check.mjs
Original file line number Diff line number Diff line change
@@ -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) {}
}
`,
],
});
1 change: 1 addition & 0 deletions test/package.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -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 => {
Expand Down
Loading

0 comments on commit 5ff5b8a

Please sign in to comment.