From 5d7e622e6563cddb3538f303316324b2ebe17b64 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Fri, 1 Jul 2022 00:34:16 +0800 Subject: [PATCH] Add `prefer-logical-operator-over-ternary` rule (#1830) --- configs/recommended.js | 1 + .../prefer-logical-operator-over-ternary.md | 52 +++ readme.md | 1 + rules/prefer-logical-operator-over-ternary.js | 155 +++++++++ ...parentheses-to-logical-expression-child.js | 13 +- test/prefer-logical-operator-over-ternary.mjs | 45 +++ ...refer-logical-operator-over-ternary.mjs.md | 301 ++++++++++++++++++ ...fer-logical-operator-over-ternary.mjs.snap | Bin 0 -> 1029 bytes 8 files changed, 565 insertions(+), 3 deletions(-) create mode 100644 docs/rules/prefer-logical-operator-over-ternary.md create mode 100644 rules/prefer-logical-operator-over-ternary.js create mode 100644 test/prefer-logical-operator-over-ternary.mjs create mode 100644 test/snapshots/prefer-logical-operator-over-ternary.mjs.md create mode 100644 test/snapshots/prefer-logical-operator-over-ternary.mjs.snap diff --git a/configs/recommended.js b/configs/recommended.js index c8d4c73bcb..672c155b54 100644 --- a/configs/recommended.js +++ b/configs/recommended.js @@ -83,6 +83,7 @@ module.exports = { 'unicorn/prefer-includes': 'error', 'unicorn/prefer-json-parse-buffer': 'off', 'unicorn/prefer-keyboard-event-key': 'error', + 'unicorn/prefer-logical-operator-over-ternary': 'error', 'unicorn/prefer-math-trunc': 'error', 'unicorn/prefer-modern-dom-apis': 'error', 'unicorn/prefer-modern-math-apis': 'error', diff --git a/docs/rules/prefer-logical-operator-over-ternary.md b/docs/rules/prefer-logical-operator-over-ternary.md new file mode 100644 index 0000000000..e4efe87490 --- /dev/null +++ b/docs/rules/prefer-logical-operator-over-ternary.md @@ -0,0 +1,52 @@ +# Prefer using a logical operator over a ternary + + + +βœ… *This rule is part of the [recommended](https://github.com/sindresorhus/eslint-plugin-unicorn#recommended-config) config.* + +πŸ’‘ *This rule provides [suggestions](https://eslint.org/docs/developer-guide/working-with-rules#providing-suggestions).* + + +Disallow ternary operators when simpler logical operator alternatives exist. + +Ideally, most reported cases have an equivalent [`Logical OR(||)`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_OR) expression. The rule intentionally provides suggestions instead of auto-fixes, because in many cases, the [nullish coalescing operator (`??`)](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing_operator) should be preferred. + +## Fail + +```js +foo ? foo : bar; +``` + +```js +foo.bar ? foo.bar : foo.baz +``` + +```js +foo?.bar ? foo.bar : baz +``` + +```js +!bar ? foo : bar; +``` + +## Pass + +```js +foo ?? bar; +``` + +```js +foo || bar; +``` + +```js +foo ? bar : baz; +``` + +```js +foo.bar ?? foo.baz +``` + +```js +foo?.bar ?? baz +``` diff --git a/readme.md b/readme.md index 94a88f6444..611391227f 100644 --- a/readme.md +++ b/readme.md @@ -123,6 +123,7 @@ Each rule has emojis denoting: | [prefer-includes](docs/rules/prefer-includes.md) | Prefer `.includes()` over `.indexOf()` and `Array#some()` when checking for existence or non-existence. | βœ… | πŸ”§ | πŸ’‘ | | [prefer-json-parse-buffer](docs/rules/prefer-json-parse-buffer.md) | Prefer reading a JSON file as a buffer. | | πŸ”§ | | | [prefer-keyboard-event-key](docs/rules/prefer-keyboard-event-key.md) | Prefer `KeyboardEvent#key` over `KeyboardEvent#keyCode`. | βœ… | πŸ”§ | | +| [prefer-logical-operator-over-ternary](docs/rules/prefer-logical-operator-over-ternary.md) | Prefer using a logical operator over a ternary. | βœ… | | πŸ’‘ | | [prefer-math-trunc](docs/rules/prefer-math-trunc.md) | Enforce the use of `Math.trunc` instead of bitwise operators. | βœ… | πŸ”§ | πŸ’‘ | | [prefer-modern-dom-apis](docs/rules/prefer-modern-dom-apis.md) | Prefer `.before()` over `.insertBefore()`, `.replaceWith()` over `.replaceChild()`, prefer one of `.before()`, `.after()`, `.append()` or `.prepend()` over `insertAdjacentText()` and `insertAdjacentElement()`. | βœ… | πŸ”§ | | | [prefer-modern-math-apis](docs/rules/prefer-modern-math-apis.md) | Prefer modern `Math` APIs over legacy patterns. | βœ… | πŸ”§ | | diff --git a/rules/prefer-logical-operator-over-ternary.js b/rules/prefer-logical-operator-over-ternary.js new file mode 100644 index 0000000000..0939f1dd3e --- /dev/null +++ b/rules/prefer-logical-operator-over-ternary.js @@ -0,0 +1,155 @@ +'use strict'; +const {isParenthesized, getParenthesizedText} = require('./utils/parentheses.js'); +const isSameReference = require('./utils/is-same-reference.js'); +const shouldAddParenthesesToLogicalExpressionChild = require('./utils/should-add-parentheses-to-logical-expression-child.js'); +const needsSemicolon = require('./utils/needs-semicolon.js'); + +const MESSAGE_ID_ERROR = 'prefer-logical-operator-over-ternary/error'; +const MESSAGE_ID_SUGGESTION = 'prefer-logical-operator-over-ternary/suggestion'; +const messages = { + [MESSAGE_ID_ERROR]: 'Prefer using a logical operator over a ternary.', + [MESSAGE_ID_SUGGESTION]: 'Switch to `{{operator}}` operator.', +}; + +function isSameNode(left, right, sourceCode) { + if (isSameReference(left, right)) { + return true; + } + + if (left.type !== right.type) { + return false; + } + + switch (left.type) { + case 'AwaitExpression': + return isSameNode(left.argument, right.argument, sourceCode); + + case 'LogicalExpression': + return ( + left.operator === right.operator + && isSameNode(left.left, right.left, sourceCode) + && isSameNode(left.right, right.right, sourceCode) + ); + + case 'UnaryExpression': + return ( + left.operator === right.operator + && left.prefix === right.prefix + && isSameNode(left.argument, right.argument, sourceCode) + ); + + case 'UpdateExpression': + return false; + + // No default + } + + return sourceCode.getText(left) === sourceCode.getText(right); +} + +function fix({ + fixer, + sourceCode, + conditionalExpression, + left, + right, + operator, +}) { + let text = [left, right].map((node, index) => { + const isNodeParenthesized = isParenthesized(node, sourceCode); + let text = isNodeParenthesized ? getParenthesizedText(node, sourceCode) : sourceCode.getText(node); + + if ( + !isNodeParenthesized + && shouldAddParenthesesToLogicalExpressionChild(node, {operator, property: index === 0 ? 'left' : 'right'}) + ) { + text = `(${text})`; + } + + return text; + }).join(` ${operator} `); + + // According to https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Operator_Precedence#table + // There should be no cases need add parentheses when switching ternary to logical expression + + // ASI + if (needsSemicolon(sourceCode.getTokenBefore(conditionalExpression), sourceCode, text)) { + text = `;${text}`; + } + + return fixer.replaceText(conditionalExpression, text); +} + +function getProblem({ + sourceCode, + conditionalExpression, + left, + right, +}) { + return { + node: conditionalExpression, + messageId: MESSAGE_ID_ERROR, + suggest: ['??', '||'].map(operator => ({ + messageId: MESSAGE_ID_SUGGESTION, + data: {operator}, + fix: fixer => fix({ + fixer, + sourceCode, + conditionalExpression, + left, + right, + operator, + }), + })), + }; +} + +/** @param {import('eslint').Rule.RuleContext} context */ +const create = context => { + const sourceCode = context.getSourceCode(); + + return { + ConditionalExpression(conditionalExpression) { + const {test, consequent, alternate} = conditionalExpression; + + // `foo ? foo : bar` + if (isSameNode(test, consequent, sourceCode)) { + return getProblem({ + sourceCode, + conditionalExpression, + left: test, + right: alternate, + }); + } + + // `!bar ? foo : bar` + if ( + test.type === 'UnaryExpression' + && test.operator === '!' + && test.prefix + && isSameNode(test.argument, alternate, sourceCode) + ) { + return getProblem({ + sourceCode, + conditionalExpression, + left: test.argument, + right: consequent, + }); + } + }, + }; +}; + +/** @type {import('eslint').Rule.RuleModule} */ +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + description: 'Prefer using a logical operator over a ternary.', + }, + + hasSuggestions: true, + messages, + }, +}; diff --git a/rules/utils/should-add-parentheses-to-logical-expression-child.js b/rules/utils/should-add-parentheses-to-logical-expression-child.js index b57f85c0b0..91a844efa9 100644 --- a/rules/utils/should-add-parentheses-to-logical-expression-child.js +++ b/rules/utils/should-add-parentheses-to-logical-expression-child.js @@ -7,10 +7,17 @@ Check if parentheses should be added to a `node` when it's used as child of `Log @returns {boolean} */ function shouldAddParenthesesToLogicalExpressionChild(node, {operator, property}) { - // When operator or property is different, need check `LogicalExpression` operator precedence, not implemented + // We are not using this, but we can improve this function with it /* c8 ignore next 3 */ - if (operator !== '??' || property !== 'left') { - throw new Error('Not supported.'); + if (!property) { + throw new Error('`property` is required.'); + } + + if ( + node.type === 'LogicalExpression' + && node.operator === operator + ) { + return false; } // Not really needed, but more readable diff --git a/test/prefer-logical-operator-over-ternary.mjs b/test/prefer-logical-operator-over-ternary.mjs new file mode 100644 index 0000000000..5791fdb028 --- /dev/null +++ b/test/prefer-logical-operator-over-ternary.mjs @@ -0,0 +1,45 @@ +import outdent from 'outdent'; +import {getTester} from './utils/test.mjs'; + +const {test} = getTester(import.meta); + +test.snapshot({ + valid: [ + 'foo ? foo1 : bar;', + 'foo.bar ? foo.bar1 : foo.baz', + 'foo.bar ? foo1.bar : foo.baz', + '++foo ? ++foo : bar;', + + // Not checking + '!!bar ? foo : bar;', + ], + invalid: [ + 'foo ? foo : bar;', + 'foo.bar ? foo.bar : foo.baz', + 'foo?.bar ? foo.bar : baz', + '!bar ? foo : bar;', + '!!bar ? foo : !bar;', + + 'foo() ? foo() : bar', + + // Children parentheses + 'foo ? foo : a && b', + 'foo ? foo : a || b', + 'foo ? foo : a ?? b', + 'a && b ? a && b : bar', + 'a || b ? a || b : bar', + 'a ?? b ? a ?? b : bar', + 'foo ? foo : await a', + 'await a ? await a : foo', + + // ASI + outdent` + const foo = [] + !+a ? b : +a + `, + outdent` + const foo = [] + a && b ? a && b : 1 + `, + ], +}); diff --git a/test/snapshots/prefer-logical-operator-over-ternary.mjs.md b/test/snapshots/prefer-logical-operator-over-ternary.mjs.md new file mode 100644 index 0000000000..f19537bc36 --- /dev/null +++ b/test/snapshots/prefer-logical-operator-over-ternary.mjs.md @@ -0,0 +1,301 @@ +# Snapshot report for `test/prefer-logical-operator-over-ternary.mjs` + +The actual snapshot is saved in `prefer-logical-operator-over-ternary.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | foo ? foo : bar; + +> Error 1/1 + + `␊ + > 1 | foo ? foo : bar;␊ + | ^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo ?? bar;␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo || bar;␊ + ` + +## Invalid #2 + 1 | foo.bar ? foo.bar : foo.baz + +> Error 1/1 + + `␊ + > 1 | foo.bar ? foo.bar : foo.baz␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo.bar ?? foo.baz␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo.bar || foo.baz␊ + ` + +## Invalid #3 + 1 | foo?.bar ? foo.bar : baz + +> Error 1/1 + + `␊ + > 1 | foo?.bar ? foo.bar : baz␊ + | ^^^^^^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo?.bar ?? baz␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo?.bar || baz␊ + ` + +## Invalid #4 + 1 | !bar ? foo : bar; + +> Error 1/1 + + `␊ + > 1 | !bar ? foo : bar;␊ + | ^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | bar ?? foo;␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | bar || foo;␊ + ` + +## Invalid #5 + 1 | !!bar ? foo : !bar; + +> Error 1/1 + + `␊ + > 1 | !!bar ? foo : !bar;␊ + | ^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | !bar ?? foo;␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | !bar || foo;␊ + ` + +## Invalid #6 + 1 | foo() ? foo() : bar + +> Error 1/1 + + `␊ + > 1 | foo() ? foo() : bar␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo() ?? bar␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo() || bar␊ + ` + +## Invalid #7 + 1 | foo ? foo : a && b + +> Error 1/1 + + `␊ + > 1 | foo ? foo : a && b␊ + | ^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo ?? (a && b)␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo || (a && b)␊ + ` + +## Invalid #8 + 1 | foo ? foo : a || b + +> Error 1/1 + + `␊ + > 1 | foo ? foo : a || b␊ + | ^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo ?? (a || b)␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo || a || b␊ + ` + +## Invalid #9 + 1 | foo ? foo : a ?? b + +> Error 1/1 + + `␊ + > 1 | foo ? foo : a ?? b␊ + | ^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo ?? a ?? b␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo || (a ?? b)␊ + ` + +## Invalid #10 + 1 | a && b ? a && b : bar + +> Error 1/1 + + `␊ + > 1 | a && b ? a && b : bar␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | (a && b) ?? bar␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | (a && b) || bar␊ + ` + +## Invalid #11 + 1 | a || b ? a || b : bar + +> Error 1/1 + + `␊ + > 1 | a || b ? a || b : bar␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | (a || b) ?? bar␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | a || b || bar␊ + ` + +## Invalid #12 + 1 | a ?? b ? a ?? b : bar + +> Error 1/1 + + `␊ + > 1 | a ?? b ? a ?? b : bar␊ + | ^^^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | a ?? b ?? bar␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | (a ?? b) || bar␊ + ` + +## Invalid #13 + 1 | foo ? foo : await a + +> Error 1/1 + + `␊ + > 1 | foo ? foo : await a␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | foo ?? (await a)␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | foo || (await a)␊ + ` + +## Invalid #14 + 1 | await a ? await a : foo + +> Error 1/1 + + `␊ + > 1 | await a ? await a : foo␊ + | ^^^^^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | (await a) ?? foo␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | (await a) || foo␊ + ` + +## Invalid #15 + 1 | const foo = [] + 2 | !+a ? b : +a + +> Error 1/1 + + `␊ + 1 | const foo = []␊ + > 2 | !+a ? b : +a␊ + | ^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | const foo = []␊ + 2 | ;+a ?? b␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | const foo = []␊ + 2 | ;+a || b␊ + ` + +## Invalid #16 + 1 | const foo = [] + 2 | a && b ? a && b : 1 + +> Error 1/1 + + `␊ + 1 | const foo = []␊ + > 2 | a && b ? a && b : 1␊ + | ^^^^^^^^^^^^^^^^^^^ Prefer using a logical operator over a ternary.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/2: Switch to \`??\` operator.␊ + 1 | const foo = []␊ + 2 | ;(a && b) ?? 1␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 2/2: Switch to \`||\` operator.␊ + 1 | const foo = []␊ + 2 | ;(a && b) || 1␊ + ` diff --git a/test/snapshots/prefer-logical-operator-over-ternary.mjs.snap b/test/snapshots/prefer-logical-operator-over-ternary.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..e88fdf8d6e59417e472267f3de2880387a522b91 GIT binary patch literal 1029 zcmV+g1p50yRzVI@YaWE%|<61Iy4dzKFB13#gLtL-CAa6PCG>^7(Q$km+*ne z+~O88B9UNm-`tWVL^AhV<`RsIkP(USp68sMwRXDsK-$yup?&-GH2MGg%kzJq^ByJu z{lIXl;CA!U3#+fqx;5vQsiBq?*|n3sjof%PQ=ZFNw>jN&pmFFXg4L6~0eHif%qI_e#VxS7X6SQs$_ZAd!~rZz zEp^OkDD8PI^WjhP`}{=&t9Rl7$}^L6JFZUq($e2_z?zu#oM6?}mwj_(%K4A_&AJ-} z3D5R_IDVF3wIujGHmHkFcKQWVF^@;(YJM=^%Bo_4DDu`|} zwNO7TR z7BWy7tY}Q-7$`f#l&aP_+Jp0lONuVDMVHKG_)&$F$N1TU!L3(sCcH5kn@KblInAU- zWLp3cTe_VSBrLeHQaiUQ^xmUgI0-@@8|0fGr8vbK$g(Wt~2V