From 8ac58afabbae58200dc04f5975d8828990ed80fb Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Wed, 30 Jun 2021 10:38:31 +0800 Subject: [PATCH 1/3] Add `prefer-nullish-coalescing` rule --- docs/rules/prefer-nullish-coalescing.md | 27 ++++++ index.js | 2 + readme.md | 2 + rules/prefer-nullish-coalescing.js | 78 ++++++++++++++++++ test/prefer-nullish-coalescing.mjs | 32 +++++++ .../prefer-nullish-coalescing.mjs.md | 47 +++++++++++ .../prefer-nullish-coalescing.mjs.snap | Bin 0 -> 283 bytes 7 files changed, 188 insertions(+) create mode 100644 docs/rules/prefer-nullish-coalescing.md create mode 100644 rules/prefer-nullish-coalescing.js create mode 100644 test/prefer-nullish-coalescing.mjs create mode 100644 test/snapshots/prefer-nullish-coalescing.mjs.md create mode 100644 test/snapshots/prefer-nullish-coalescing.mjs.snap diff --git a/docs/rules/prefer-nullish-coalescing.md b/docs/rules/prefer-nullish-coalescing.md new file mode 100644 index 0000000000..4a157eacab --- /dev/null +++ b/docs/rules/prefer-nullish-coalescing.md @@ -0,0 +1,27 @@ +# Prefer the nullish coalescing operator(`??`) over the logical OR operator(`||`) + +The [nullish coalescing operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing_operator) only coalesces when the value is `null` or `undefined`, it is safer than [logical OR operator](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_OR) which coalesces on any [falsy value](https://developer.mozilla.org/en-US/docs/Glossary/Falsy). + +## Fail + +```js +const foo = bar || value; +``` + +```js +foo ||= value; +``` + +```js +if (foo || value) {} +``` + +## Pass + +```js +const foo = bar ?? value; +``` + +```js +foo ??= value; +``` diff --git a/index.js b/index.js index ec977e5188..9bacd38ad4 100644 --- a/index.js +++ b/index.js @@ -102,6 +102,8 @@ module.exports = { 'unicorn/prefer-module': 'error', 'unicorn/prefer-negative-index': 'error', 'unicorn/prefer-node-protocol': 'error', + // TODO: Enable this by default when targeting Node.js 14. + 'unicorn/prefer-nullish-coalescing': 'off', 'unicorn/prefer-number-properties': 'error', // TODO: Enable this by default when targeting a Node.js version that supports `Object.hasOwn`. 'unicorn/prefer-object-has-own': 'off', diff --git a/readme.md b/readme.md index 6dcfdee20e..a986972b1a 100644 --- a/readme.md +++ b/readme.md @@ -98,6 +98,7 @@ Configure it in `package.json`. "unicorn/prefer-module": "error", "unicorn/prefer-negative-index": "error", "unicorn/prefer-node-protocol": "error", + "unicorn/prefer-nullish-coalescing": "off", "unicorn/prefer-number-properties": "error", "unicorn/prefer-object-has-own": "off", "unicorn/prefer-optional-catch-binding": "error", @@ -202,6 +203,7 @@ Each rule has emojis denoting: | [prefer-module](docs/rules/prefer-module.md) | Prefer JavaScript modules (ESM) over CommonJS. | ✅ | 🔧 | 💡 | | [prefer-negative-index](docs/rules/prefer-negative-index.md) | Prefer negative index over `.length - index` for `{String,Array,TypedArray}#slice()`, `Array#splice()` and `Array#at()`. | ✅ | 🔧 | | | [prefer-node-protocol](docs/rules/prefer-node-protocol.md) | Prefer using the `node:` protocol when importing Node.js builtin modules. | ✅ | 🔧 | | +| [prefer-nullish-coalescing](docs/rules/prefer-nullish-coalescing.md) | Prefer the nullish coalescing operator(`??`) over the logical OR operator(`||`). | | | 💡 | | [prefer-number-properties](docs/rules/prefer-number-properties.md) | Prefer `Number` static properties over global ones. | ✅ | 🔧 | 💡 | | [prefer-object-has-own](docs/rules/prefer-object-has-own.md) | Prefer `Object.hasOwn(…)` over `Object.prototype.hasOwnProperty.call(…)`. | | 🔧 | | | [prefer-optional-catch-binding](docs/rules/prefer-optional-catch-binding.md) | Prefer omitting the `catch` binding parameter. | ✅ | 🔧 | | diff --git a/rules/prefer-nullish-coalescing.js b/rules/prefer-nullish-coalescing.js new file mode 100644 index 0000000000..d25231702f --- /dev/null +++ b/rules/prefer-nullish-coalescing.js @@ -0,0 +1,78 @@ +'use strict'; +const {getStaticValue} = require('eslint-utils'); +const {matches} = require('./selectors/index.js'); +const {isBooleanNode} = require('./utils/boolean.js'); + +const ERROR = 'error'; +const SUGGESTION = 'suggestion'; +const messages = { + [ERROR]: 'Prefer `{{replacement}}` over `{{original}}`.', + [SUGGESTION]: 'Use `{{replacement}}`' +}; + +const selector = matches([ + 'LogicalExpression[operator="||"]', + 'AssignmentExpression[operator="||="]' +]); + +/** @param {import('eslint').Rule.RuleContext} context */ +const create = context => { + return { + [selector](node) { + if ( + [node.parent, node.left, node.right].some(node => node.type === 'LogicalExpression') || + isBooleanNode(node) + ) { + return; + } + + const {left} = node; + + const staticValue = getStaticValue(left, context.getScope()); + if (staticValue) { + const {value} = staticValue; + if (!(typeof value === 'undefined' || value === null)) { + return; + } + } + + const isAssignment = node.type === 'AssignmentExpression'; + const originalOperator = isAssignment ? '||=' : '||'; + const replacementOperator = isAssignment ? '??=' : '??'; + const operatorToken = context.getSourceCode() + .getTokenAfter( + node.left, + token => token.type === 'Punctuator' && token.value === originalOperator + ); + + const messageData = { + original: originalOperator, + replacement: replacementOperator + }; + return { + node: operatorToken, + messageId: ERROR, + data: messageData, + suggest: [ + { + messageId: SUGGESTION, + data: messageData, + fix: fixer => fixer.replaceText(operatorToken, replacementOperator) + } + ] + }; + } + }; +}; + +module.exports = { + create, + meta: { + type: 'suggestion', + docs: { + description: 'Prefer the nullish coalescing operator(`??`) over the logical OR operator(`||`).' + }, + messages, + hasSuggestions: true + } +}; diff --git a/test/prefer-nullish-coalescing.mjs b/test/prefer-nullish-coalescing.mjs new file mode 100644 index 0000000000..0ea908121d --- /dev/null +++ b/test/prefer-nullish-coalescing.mjs @@ -0,0 +1,32 @@ +import {getTester} from './utils/test.mjs'; + +const {test} = getTester(import.meta); + +test.snapshot({ + valid: [ + 'const a = 0; const foo = a || 1;', + 'const a = {b: false}; const foo = a.b || 1;', + 'const foo = 0n || 1;', + 'const foo = "" || 1;', + 'const foo = `` || 1;', + 'const foo = NaN || 1;', + // Boolean + 'const foo = !(a || b)', + 'const foo = Boolean(a || b)', + 'if (a || b);', + 'const foo = (a || b) ? c : d;', + 'while (a || b);', + 'do {} while (a || b);', + 'for (;a || b;);', + // Mixed + 'const foo = a || (b && c)', + 'const foo = (a || b) && c', + 'const foo = a ?? (b || c)', + 'const foo = (a ?? b) || c' + ], + invalid: [ + 'const foo = a || b', + 'foo ||= b', + 'const foo = (( a )) || b' + ] +}); diff --git a/test/snapshots/prefer-nullish-coalescing.mjs.md b/test/snapshots/prefer-nullish-coalescing.mjs.md new file mode 100644 index 0000000000..1698ec7fee --- /dev/null +++ b/test/snapshots/prefer-nullish-coalescing.mjs.md @@ -0,0 +1,47 @@ +# Snapshot report for `test/prefer-nullish-coalescing.mjs` + +The actual snapshot is saved in `prefer-nullish-coalescing.mjs.snap`. + +Generated by [AVA](https://avajs.dev). + +## Invalid #1 + 1 | const foo = a || b + +> Error 1/1 + + `␊ + > 1 | const foo = a || b␊ + | ^^ Prefer \`??\` over \`||\`.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Use \`??\`␊ + 1 | const foo = a ?? b␊ + ` + +## Invalid #2 + 1 | foo ||= b + +> Error 1/1 + + `␊ + > 1 | foo ||= b␊ + | ^^^ Prefer \`??=\` over \`||=\`.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Use \`??=\`␊ + 1 | foo ??= b␊ + ` + +## Invalid #3 + 1 | const foo = (( a )) || b + +> Error 1/1 + + `␊ + > 1 | const foo = (( a )) || b␊ + | ^^ Prefer \`??\` over \`||\`.␊ + ␊ + --------------------------------------------------------------------------------␊ + Suggestion 1/1: Use \`??\`␊ + 1 | const foo = (( a )) ?? b␊ + ` diff --git a/test/snapshots/prefer-nullish-coalescing.mjs.snap b/test/snapshots/prefer-nullish-coalescing.mjs.snap new file mode 100644 index 0000000000000000000000000000000000000000..396adaafc1b08bdcc477f7280381827a11fdff60 GIT binary patch literal 283 zcmV+$0p$KcRzVk>@&lWKZ^B%U>DetX&w>=z)Yzi< hE)K;zwg~^&($hn>Fdu=O2;>5t2mm_e>|x&m000xSb!Pwo literal 0 HcmV?d00001 From 4abe6243baf1cca98d2402282eb173d828517672 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Wed, 30 Jun 2021 11:18:53 +0800 Subject: [PATCH 2/3] Ignore boolean left --- rules/prefer-nullish-coalescing.js | 3 ++- rules/utils/boolean.js | 43 ++++++++++++++++++++++++++---- test/prefer-nullish-coalescing.mjs | 14 ++++++++++ 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/rules/prefer-nullish-coalescing.js b/rules/prefer-nullish-coalescing.js index d25231702f..26af8e3c8f 100644 --- a/rules/prefer-nullish-coalescing.js +++ b/rules/prefer-nullish-coalescing.js @@ -21,7 +21,8 @@ const create = context => { [selector](node) { if ( [node.parent, node.left, node.right].some(node => node.type === 'LogicalExpression') || - isBooleanNode(node) + isBooleanNode(node) || + isBooleanNode(node.left) ) { return; } diff --git a/rules/utils/boolean.js b/rules/utils/boolean.js index 82bf5e1d0c..cd70ceba58 100644 --- a/rules/utils/boolean.js +++ b/rules/utils/boolean.js @@ -3,7 +3,6 @@ const isLogicalExpression = require('./is-logical-expression.js'); const isLogicNot = node => - node && node.type === 'UnaryExpression' && node.operator === '!'; const isLogicNotArgument = node => @@ -13,14 +12,22 @@ const isBooleanCallArgument = node => isBooleanCall(node.parent) && node.parent.arguments[0] === node; const isBooleanCall = node => - node && node.type === 'CallExpression' && - node.callee && + !node.optional && node.callee.type === 'Identifier' && node.callee.name === 'Boolean' && node.arguments.length === 1; +const isObjectIsCall = node => + node.type === 'CallExpression' && + !node.optional && + node.callee.type === 'MemberExpression' && + !node.callee.computed && + !node.callee.optional && + node.callee.object.type === 'Identifier' && + node.callee.object.name === 'Object' && + node.callee.property.type === 'Identifier' && + node.callee.property.name === 'is'; const isVueBooleanAttributeValue = node => - node && node.type === 'VExpressionContainer' && node.parent.type === 'VAttribute' && node.parent.directive && @@ -32,6 +39,25 @@ const isVueBooleanAttributeValue = node => node.parent.key.name.rawName === 'else-if' || node.parent.key.name.rawName === 'show' ); +const isBooleanLiteral = node => + node.type === 'Literal' && + typeof node.value === 'boolean'; +// https://github.com/estree/estree/blob/master/es5.md#binaryoperator +const comparisonOperators = new Set([ + '==', + '!=', + '===', + '!==', + '<', + '<=', + '>', + '>=', + 'in', + 'instanceof' +]); +const isComparison = node => + node.type === 'BinaryExpression' && + comparisonOperators.has(node.operator); /** Check if the value of node is a `boolean`. @@ -40,11 +66,18 @@ Check if the value of node is a `boolean`. @returns {boolean} */ function isBooleanNode(node) { + if (!node) { + return false; + } + if ( isLogicNot(node) || isLogicNotArgument(node) || isBooleanCall(node) || - isBooleanCallArgument(node) + isBooleanCallArgument(node) || + isBooleanLiteral(node) || + isComparison(node) || + isObjectIsCall(node) ) { return true; } diff --git a/test/prefer-nullish-coalescing.mjs b/test/prefer-nullish-coalescing.mjs index 0ea908121d..bcad0c943b 100644 --- a/test/prefer-nullish-coalescing.mjs +++ b/test/prefer-nullish-coalescing.mjs @@ -18,6 +18,20 @@ test.snapshot({ 'while (a || b);', 'do {} while (a || b);', 'for (;a || b;);', + // Left is boolean + 'const foo = false || a', + 'const foo = !bar || a', + 'const foo = a == 1 || bar', + 'const foo = a != 1 || bar', + 'const foo = a === b || a === c', + 'const foo = a !== b || bar', + 'const foo = a < 1 || bar', + 'const foo = a <= 1 || bar', + 'const foo = a > 1 || bar', + 'const foo = a >= 1 || bar', + 'const foo = Object.is(a, -0) || a < 0', + 'const foo = ("key" in object) || bar', + 'const foo = (object instanceof Foo) || Array.isArray(object)', // Mixed 'const foo = a || (b && c)', 'const foo = (a || b) && c', From 6a9ea177dc99291bc95318bdd6b63f0baa954368 Mon Sep 17 00:00:00 2001 From: fisker Cheung Date: Wed, 30 Jun 2021 11:33:22 +0800 Subject: [PATCH 3/3] Remove dead code --- rules/utils/boolean.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/rules/utils/boolean.js b/rules/utils/boolean.js index cd70ceba58..15a73f0855 100644 --- a/rules/utils/boolean.js +++ b/rules/utils/boolean.js @@ -66,10 +66,6 @@ Check if the value of node is a `boolean`. @returns {boolean} */ function isBooleanNode(node) { - if (!node) { - return false; - } - if ( isLogicNot(node) || isLogicNotArgument(node) ||