Skip to content

Commit

Permalink
Add no-identical-alternate
Browse files Browse the repository at this point in the history
  • Loading branch information
axetroy committed Sep 10, 2024
1 parent d3e4b80 commit 95b18ee
Show file tree
Hide file tree
Showing 10 changed files with 686 additions and 0 deletions.
32 changes: 32 additions & 0 deletions docs/rules/no-identical-alternatives-in-ternary.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# Disallow nested ternary expressions with repeated alternatives

💼 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` -->

Disallow nested ternary expressions with repeated alternatives, and simplify to a more readable format with logical operators.

## Examples

```js
a ? b ? c : 1 : 1; //
a && b ? c : 1; //
```

```js
a ? b ? c : { foo } : { foo }; //
a && b ? c : { foo }; //
```

```js
a ? b ? c : sameReference : sameReference; //
a && b ? c : sameReference; //
```

```js
a ? b ? c : foo.bar() : foo.bar(); //
a && b ? c : foo.bar(); //
```
1 change: 1 addition & 0 deletions readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ If you don't use the preset, ensure you use the same `env` and `parserOptions` c
| [no-empty-file](docs/rules/no-empty-file.md) | Disallow empty files. || | |
| [no-for-loop](docs/rules/no-for-loop.md) | Do not use a `for` loop that can be replaced with a `for-of` loop. || 🔧 | 💡 |
| [no-hex-escape](docs/rules/no-hex-escape.md) | Enforce the use of Unicode escapes instead of hexadecimal escapes. || 🔧 | |
| [no-identical-alternatives-in-ternary](docs/rules/no-identical-alternatives-in-ternary.md) | Disallow nested ternary expressions with repeated alternatives. || 🔧 | |
| [no-instanceof-array](docs/rules/no-instanceof-array.md) | Require `Array.isArray()` instead of `instanceof Array`. || 🔧 | |
| [no-invalid-fetch-options](docs/rules/no-invalid-fetch-options.md) | Disallow invalid options in `fetch()` and `new Request()`. || | |
| [no-invalid-remove-event-listener](docs/rules/no-invalid-remove-event-listener.md) | Prevent calling `EventTarget#removeEventListener()` with the result of an expression. || | |
Expand Down
55 changes: 55 additions & 0 deletions rules/no-identical-alternatives-in-ternary.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
'use strict';

const {isNodesEqual, findIndexOfQuestionMarkInConditionalExpression, getParenthesizedRange} = require('./utils/index.js');

const MESSAGE_ID = 'no-identical-alternatives-in-ternary';
const messages = {
[MESSAGE_ID]:
'Replace repeated alternatives of ternary expressions with a logical expression.',
};

/** @param {import('eslint').Rule.RuleContext} context */
const create = context => ({
/** @param {import('estree').ConditionalExpression} node */
ConditionalExpression(node) {
// Check if the alternate is a ConditionalExpression
if (node.alternate && node.consequent.type === 'ConditionalExpression') {
const {test: _outerTest, consequent: _outerConsequent, alternate: outerAlternate} = node;
const {test: _innerTest, consequent: _innerConsequent, alternate: innerAlternate} = node.consequent;

const {sourceCode} = context;

if (isNodesEqual(outerAlternate, innerAlternate)) {
context.report({
node,
messageId: MESSAGE_ID,
* fix(fixer) {
// Find the index of the question mark in the outer ConditionalExpression
const questionMarkIndex = findIndexOfQuestionMarkInConditionalExpression(node, sourceCode);

// Replace the ? with &&
yield fixer.replaceTextRange([questionMarkIndex, questionMarkIndex + 1], '&&');

// Remove the repeated alternative
const [alternativeStart, alternativeEnd] = getParenthesizedRange(innerAlternate, sourceCode);
yield fixer.replaceTextRange([alternativeStart, node.range[1]], sourceCode.getText().slice(alternativeStart, alternativeEnd));
},
});
}
}
},
});

/** @type {import('eslint').Rule.RuleModule} */
module.exports = {
create,
meta: {
type: 'problem',
docs: {
description: 'Disallow nested ternary expressions with repeated alternatives.',
recommended: true,
},
fixable: 'code',
messages,
},
};
34 changes: 34 additions & 0 deletions rules/utils/conditional-expression.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
'use strict';

/**
Find the index of the question mark in a ConditionalExpression node
@param {import('estree').ConditionalExpression} node
@param {import('eslint').SourceCode} sourceCode
@returns {number}
*/
function findIndexOfQuestionMarkInConditionalExpression(node, sourceCode) {
const testAfterComments = sourceCode.getCommentsAfter(node.test);
const consequentBeforeComments = sourceCode.getCommentsBefore(node.consequent);

let start = node.test.range[1];
let end = node.consequent.range[0];

if (testAfterComments.length > 0) {
const lastComment = testAfterComments.at(-1);

start = lastComment.range[1];
}

if (consequentBeforeComments.length > 0) {
const firstComment = consequentBeforeComments.at(0);

end = firstComment.range[0];
}

return start + sourceCode.getText().slice(start, end).indexOf('?');
}

module.exports = {
findIndexOfQuestionMarkInConditionalExpression,
};
3 changes: 3 additions & 0 deletions rules/utils/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const {
} = require('./array-or-object-prototype-property.js');
const {isNodeMatches, isNodeMatchesNameOrPath} = require('./is-node-matches.js');
const {isBooleanNode, getBooleanAncestor} = require('./boolean.js');
const {findIndexOfQuestionMarkInConditionalExpression} = require('./conditional-expression.js');

module.exports = {
avoidCapture: require('./avoid-capture.js'),
Expand Down Expand Up @@ -43,6 +44,7 @@ module.exports = {
isParenthesized,
isSameIdentifier: require('./is-same-identifier.js'),
isSameReference: require('./is-same-reference.js'),
isNodesEqual: require('./is-node-equals.js'),
isShadowed: require('./is-shadowed.js'),
isValueNotUsable: require('./is-value-not-usable.js'),
needsSemicolon: require('./needs-semicolon.js'),
Expand All @@ -52,5 +54,6 @@ module.exports = {
singular: require('./singular.js'),
toLocation: require('./to-location.js'),
getAncestor: require('./get-ancestor.js'),
findIndexOfQuestionMarkInConditionalExpression,
};

56 changes: 56 additions & 0 deletions rules/utils/is-node-equals.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';

const ignoreKeys = new Set(['loc', 'start', 'end', 'parent', 'range']);

/**
Compare two AST nodes for equality by structure and value.
Different from `deepStrictEqual` in that it ignores `loc`, `start`, `end`, `parent`, and `range` properties.
Different from `isSameReference` in that `isSameReference` just checks if the nodes are the same reference, but cannot compare two identical data.
eg.:
```js
const node1 = { foo: bar }
const node2 = { foo: bar }
```
isNodesEqual(node1, node2) => true
isSameReference(node1, node2) => false
@param {import('estree').Node} node1 - The first AST node.
@param {import('estree').Node} node2 - The second AST node.
@returns {boolean} - True if the nodes are structurally and value-wise equal, false otherwise.
*/
function isNodesEqual(node1, node2) {
if (node1 === node2) {
return true;
}

if (typeof node1 === 'string' || typeof node1 === 'boolean' || typeof node1 === 'number') {
return node1 === node2;
}

// If one of them is null or undefined, they are not equal
if (!node1 || !node2) {
return false;
}

// If they are of different types, they are not equal
if (node1.type !== node2.type) {
return false;
}

if (node1.type === 'Literal') {
return node1.value === node2.value;
}

// Compare properties recursively
for (const key in node1) {
if (Object.hasOwn(node1, key) && !ignoreKeys.has(key) && !isNodesEqual(node1[key], node2[key])) {
return false;
}
}

return true;
}

module.exports = isNodesEqual;
71 changes: 71 additions & 0 deletions test/no-identical-alternatives-in-ternary.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
import {outdent} from 'outdent';
import {getTester} from './utils/test.mjs';

const {test} = getTester(import.meta);

test.snapshot({
valid: [
'a ? b ? c : 1 : 2',
'a && b ? c : 1',
],
invalid: [
'a ? b ? c : 1 : 1',
'a ? b ? c : "str" : "str"',
'a ? b ? c : sameReference : sameReference',
'a ? b ? c : { foo: 1 } : { foo: 1 }',
outdent`
const foo = { bar: 1 };
a ? b ? c : foo.bar : foo.bar
`,
// With comments
'/** comment before a */ a /** comment after a */ ? b ? c : 1 : 1',
outdent`
/** comment before a */
a ?
/** comment after a */ b ? c : 1 : 1
`,
outdent`
/** comment before a */
a ?
/** comment after a */ b /** comment after b */ ? c /** comment after c */ : 1 : 1 /** comment after value */
`,
'/** comment includes ? */ a /** comment includes ? */ ? b ? c : 1 : 1',

// Don't use outdent for the following cases, outdent will causes eslint parsing errors
`
a ?
b ?
c :
1 :
1
`,
`
a ? // comment a
b ? // comment b
c : // comment c
1 : // comment 1
1 // comment repeat 1
`,
`
/** comment before a */ a ? // comment a
/** comment before b */ b ? // comment b
/** comment before c */ c : // comment c
/** comment before 1 */ 1 : // comment 1
/** comment before repeat 1 */ 1 // comment repeat 1
`,

// Edge cases
'a ? b ? c : (1, 2) : (1, 2)',
'a ? b ? c : a === 100 : a === 100',
outdent`
async function foo() {
return a ? b ? c : await 1 : await 1;
}
`,
outdent`
function* foo() {
return a ? b ? c : yield 1 : yield 1;
}
`,
],
});
1 change: 1 addition & 0 deletions test/package.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const RULES_WITHOUT_PASS_FAIL_SECTIONS = new Set([
'prefer-modern-math-apis',
'prefer-math-min-max',
'consistent-existence-index-check',
'no-identical-alternatives-in-ternary',
]);

test('Every rule is defined in index file in alphabetical order', t => {
Expand Down
Loading

0 comments on commit 95b18ee

Please sign in to comment.