Skip to content

Commit

Permalink
Rewrite the implement
Browse files Browse the repository at this point in the history
  • Loading branch information
axetroy committed Sep 10, 2024
1 parent 8246f20 commit d98c86c
Show file tree
Hide file tree
Showing 7 changed files with 502 additions and 10 deletions.
24 changes: 16 additions & 8 deletions rules/no-identical-alternatives-in-ternary.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
'use strict';

const {isSameReference} = require('./utils/index.js');
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.',
[MESSAGE_ID]:
'Replace repeated alternatives of ternary expressions with a logical expression.',
};

/** @param {import('eslint').Rule.RuleContext} context */
Expand All @@ -13,18 +14,25 @@ const create = context => ({
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 {test: _outerTest, consequent: _outerConsequent, alternate: outerAlternate} = node;
const {test: _innerTest, consequent: _innerConsequent, alternate: innerAlternate} = node.consequent;

const {sourceCode} = context;

if (isSameReference(outerAlternate, innerAlternate)) {
if (isNodesEqual(outerAlternate, innerAlternate)) {
context.report({
node,
messageId: MESSAGE_ID,
fix(fixer) {
const newExpression = `${sourceCode.getText(outerTest)} && ${sourceCode.getText(innerTest)} ? ${sourceCode.getText(innerConsequent)} : ${sourceCode.getText(outerAlternate)}`;
return fixer.replaceText(node, newExpression);
* 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));
},
});
}
Expand Down
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;
52 changes: 51 additions & 1 deletion test/no-identical-alternatives-in-ternary.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,66 @@ const {test} = getTester(import.meta);
test.snapshot({
valid: [
'a ? b ? c : 1 : 2',
'a ? b ? c : { foo: 1 } : { foo: 1 }',
'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;
}
`,
],
});
Loading

0 comments on commit d98c86c

Please sign in to comment.