Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run prefer-nullish-coalescing on codebase #296

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 27 additions & 0 deletions 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;
```
2 changes: 2 additions & 0 deletions index.js
Expand Up @@ -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',
Expand Down
2 changes: 2 additions & 0 deletions readme.md
Expand Up @@ -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",
Expand Down Expand Up @@ -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. | ✅ | 🔧 | |
Expand Down
2 changes: 1 addition & 1 deletion rules/better-regex.js
Expand Up @@ -15,7 +15,7 @@ const newRegExp = [
].join('');

const create = context => {
const {sortCharacterClasses} = context.options[0] || {};
const {sortCharacterClasses} = context.options[0] ?? {};

const ignoreList = [];

Expand Down
2 changes: 1 addition & 1 deletion rules/custom-error-definition.js
Expand Up @@ -133,7 +133,7 @@ function * customErrorDefinition(context, node) {
yield fixer.insertTextAfterRange([
superExpression.range[0],
superExpression.range[0] + 6
], rhs.raw || rhs.name);
], rhs.raw ?? rhs.name);
}

yield fixer.removeRange([
Expand Down
6 changes: 3 additions & 3 deletions rules/expiring-todo-comments.js
Expand Up @@ -87,7 +87,7 @@ function parseTodoWithArguments(string, {terms}) {
function createArgumentGroup(arguments_) {
const groups = {};
for (const {value, type} of arguments_) {
groups[type] = groups[type] || [];
groups[type] = groups[type] ?? [];
groups[type].push(value);
}

Expand Down Expand Up @@ -226,7 +226,7 @@ function tryToCoerceVersion(rawVersion) {
try {
// Try to semver.parse a perfect match while semver.coerce tries to fix errors
// But coerce can't parse pre-releases.
return semver.parse(version) || semver.coerce(version);
return semver.parse(version) ?? semver.coerce(version);
} catch {
/* istanbul ignore next: We don't have this `package.json` to test */
return false;
Expand Down Expand Up @@ -420,7 +420,7 @@ const create = context => {
}
}

const packageEngines = packageJson.engines || {};
const packageEngines = packageJson.engines ?? {};

for (const engine of engines) {
uses++;
Expand Down
6 changes: 3 additions & 3 deletions rules/filename-case.js
Expand Up @@ -98,7 +98,7 @@ function fixFilename(words, caseFunctions, {leading, extension}) {

const leadingUnderscoresRegex = /^(?<leading>_+)(?<tailing>.*)$/;
function splitFilename(filename) {
const result = leadingUnderscoresRegex.exec(filename) || {groups: {}};
const result = leadingUnderscoresRegex.exec(filename) ?? {groups: {}};
const {leading = '', tailing = filename} = result.groups;

const words = [];
Expand Down Expand Up @@ -143,9 +143,9 @@ function englishishJoinWords(words) {
}

const create = context => {
const options = context.options[0] || {};
const options = context.options[0] ?? {};
const chosenCases = getChosenCases(options);
const ignore = (options.ignore || []).map(item => {
const ignore = (options.ignore ?? []).map(item => {
if (item instanceof RegExp) {
return item;
}
Expand Down
4 changes: 2 additions & 2 deletions rules/fix/remove-argument.js
Expand Up @@ -6,8 +6,8 @@ function removeArgument(fixer, node, sourceCode) {
const callExpression = node.parent;
const index = callExpression.arguments.indexOf(node);
const parentheses = getParentheses(node, sourceCode);
const firstToken = parentheses[0] || node;
const lastToken = parentheses[parentheses.length - 1] || node;
const firstToken = parentheses[0] ?? node;
const lastToken = parentheses[parentheses.length - 1] ?? node;

let [start] = firstToken.range;
let [, end] = lastToken.range;
Expand Down
2 changes: 1 addition & 1 deletion rules/import-index.js
Expand Up @@ -21,7 +21,7 @@ const importIndex = (context, node, argument) => {
};

const create = context => {
const options = context.options[0] || {};
const options = context.options[0] ?? {};

const rules = {
[STATIC_REQUIRE_SELECTOR]: node => importIndex(context, node, node.arguments[0])
Expand Down
2 changes: 1 addition & 1 deletion rules/no-array-push-push.js
Expand Up @@ -21,7 +21,7 @@ const selector = `${arrayPushExpressionStatement} + ${arrayPushExpressionStateme

function getFirstExpression(node, sourceCode) {
const {parent} = node;
const visitorKeys = sourceCode.visitorKeys[parent.type] || Object.keys(parent);
const visitorKeys = sourceCode.visitorKeys[parent.type] ?? Object.keys(parent);

for (const property of visitorKeys) {
const value = parent[property];
Expand Down
6 changes: 3 additions & 3 deletions rules/no-for-loop.js
Expand Up @@ -255,7 +255,7 @@ const isIndexVariableAssignedToInTheLoopBody = (indexVariable, bodyScope) => {
const someVariablesLeakOutOfTheLoop = (forStatement, variables, forScope) => {
return variables.some(variable => {
return !variable.references.every(reference => {
return scopeContains(forScope, reference.from) ||
return scopeContains(forScope, reference.from) ??
nodeContains(forStatement, reference.identifier);
});
});
Expand Down Expand Up @@ -354,8 +354,8 @@ const create = context => {
const shouldGenerateIndex = isIndexVariableUsedElsewhereInTheLoopBody(indexVariable, bodyScope, arrayIdentifierName);

const index = indexIdentifierName;
const element = elementIdentifierName ||
avoidCapture(singular(arrayIdentifierName) || defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion);
const element = elementIdentifierName ??
avoidCapture(singular(arrayIdentifierName) ?? defaultElementName, getChildScopesRecursive(bodyScope), context.parserOptions.ecmaVersion);
const array = arrayIdentifierName;

let declarationElement = element;
Expand Down
2 changes: 1 addition & 1 deletion rules/no-keyword-prefix.js
Expand Up @@ -11,7 +11,7 @@ const prepareOptions = ({
onlyCamelCase = true
} = {}) => {
return {
disallowedPrefixes: (disallowedPrefixes || [
disallowedPrefixes: (disallowedPrefixes ?? [
'new',
'class'
]),
Expand Down
2 changes: 1 addition & 1 deletion rules/no-unused-properties.js
Expand Up @@ -6,7 +6,7 @@ const messages = {
};

const getDeclaratorOrPropertyValue = declaratorOrProperty =>
declaratorOrProperty.init ||
declaratorOrProperty.init ??
declaratorOrProperty.value;

const isMemberExpressionCall = memberExpression =>
Expand Down
6 changes: 3 additions & 3 deletions rules/prefer-add-event-listener.js
Expand Up @@ -61,8 +61,8 @@ const isClearing = node => {
};

const create = context => {
const options = context.options[0] || {};
const excludedPackages = new Set(options.excludedPackages || ['koa', 'sax']);
const options = context.options[0] ?? {};
const excludedPackages = new Set(options.excludedPackages ?? ['koa', 'sax']);
let isDisabled;

const nodeReturnsSomething = new WeakMap();
Expand Down Expand Up @@ -95,7 +95,7 @@ const create = context => {
},

ReturnStatement(node) {
codePathInfo.returnsSomething = codePathInfo.returnsSomething || Boolean(node.argument);
codePathInfo.returnsSomething = codePathInfo.returnsSomething ?? Boolean(node.argument);
},

'AssignmentExpression:exit'(node) {
Expand Down
2 changes: 1 addition & 1 deletion rules/prefer-keyboard-event-key.js
Expand Up @@ -84,7 +84,7 @@ const fix = node => fixer => {
const isTestingEquality = operator === '==' || operator === '===';
const isRightValid = isTestingEquality && right.type === 'Literal' && typeof right.value === 'number';
// Either a meta key or a printable character
const keyCode = translateToKey[right.value] || String.fromCharCode(right.value);
const keyCode = translateToKey[right.value] ?? String.fromCharCode(right.value);
// And if we recognize the `.keyCode`
if (!isRightValid || !keyCode) {
return;
Expand Down
74 changes: 74 additions & 0 deletions rules/prefer-nullish-coalescing.js
@@ -0,0 +1,74 @@
'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) ||
isBooleanNode(node.left)
) {
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,
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,
fixable: 'code'
}
};
2 changes: 1 addition & 1 deletion rules/prefer-reflect-apply.js
Expand Up @@ -68,7 +68,7 @@ const create = context => {
return {
[selector]: node => {
const sourceCode = context.getSourceCode();
const fix = fixDirectApplyCall(node, sourceCode) || fixFunctionPrototypeCall(node, sourceCode);
const fix = fixDirectApplyCall(node, sourceCode) ?? fixFunctionPrototypeCall(node, sourceCode);
if (fix) {
return {
node,
Expand Down
4 changes: 2 additions & 2 deletions rules/prefer-spread.js
Expand Up @@ -134,7 +134,7 @@ function fixConcat(node, sourceCode, fixableArguments) {
text = `...${text}`;
}

return text || ' ';
return text ?? ' ';
})
.join(', ');

Expand Down Expand Up @@ -388,7 +388,7 @@ const create = context => {
fix: fixConcat(
node,
sourceCode,
node.arguments.map(node => getConcatArgumentSpreadable(node, scope) || {node, isSpreadable: true})
node.arguments.map(node => getConcatArgumentSpreadable(node, scope) ?? {node, isSpreadable: true})
)
});
}
Expand Down
2 changes: 1 addition & 1 deletion rules/prefer-string-slice.js
Expand Up @@ -37,7 +37,7 @@ const isLengthProperty = node => (
node.property.name === 'length'
);

const isLikelyNumeric = node => isLiteralNumber(node) || isLengthProperty(node);
const isLikelyNumeric = node => isLiteralNumber(node) ?? isLengthProperty(node);

const create = context => {
const sourceCode = context.getSourceCode();
Expand Down
2 changes: 1 addition & 1 deletion rules/prefer-switch.js
Expand Up @@ -33,7 +33,7 @@ function getEqualityComparisons(node) {

function getCommonReferences(expressions, candidates) {
for (const {left, right} of expressions) {
candidates = candidates.filter(node => isSame(node, left) || isSame(node, right));
candidates = candidates.filter(node => isSame(node, left) ?? isSame(node, right));

if (candidates.length === 0) {
break;
Expand Down
4 changes: 2 additions & 2 deletions rules/utils/avoid-capture.js
Expand Up @@ -26,7 +26,7 @@ const nameCollidesWithArgumentsSpecial = (name, scopes, isStrict) => {
return false;
}

return isStrict || scopes.some(scope => scopeHasArgumentsSpecial(scope));
return isStrict ?? scopes.some(scope => scopeHasArgumentsSpecial(scope));
};

/*
Expand All @@ -47,7 +47,7 @@ function unicorn() {
```
*/
const isUnresolvedName = (name, scopes) => scopes.some(scope =>
scope.references.some(reference => reference.identifier && reference.identifier.name === name && !reference.resolved) ||
scope.references.some(reference => reference.identifier && reference.identifier.name === name && !reference.resolved) ??
isUnresolvedName(name, scope.childScopes)
);

Expand Down