Skip to content

Commit 4bdbe67

Browse files
JoshuaKGoldbergbradzacher
JoshuaKGoldberg
authored and
bradzacher
committed
feat(eslint-plugin): [prefer-nullish-coalescing]: add support for assignment expressions (#5234)
BREAKING CHANGE: Adds an additional class of checks to the rule
1 parent af41b7f commit 4bdbe67

File tree

3 files changed

+254
-208
lines changed

3 files changed

+254
-208
lines changed

‎packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
---
2-
description: 'Enforce using the nullish coalescing operator instead of logical chaining.'
2+
description: 'Enforce using the nullish coalescing operator instead of logical assignments or chaining.'
33
---
44

55
> 🛑 This file is source code, not the primary documentation location! 🛑
@@ -9,7 +9,10 @@ description: 'Enforce using the nullish coalescing operator instead of logical c
99
The `??` nullish coalescing runtime operator allows providing a default value when dealing with `null` or `undefined`.
1010
Because the nullish coalescing operator _only_ coalesces when the original value is `null` or `undefined`, it is much safer than relying upon logical OR operator chaining `||`, which coalesces on any _falsy_ value.
1111

12-
This rule reports when an `||` operator can be safely replaced with a `??`.
12+
This rule reports when you can safely replace:
13+
14+
- An `||` operator with `??`
15+
- An `||=` operator with `??=`
1316

1417
:::caution
1518
This rule will not work as expected if [`strictNullChecks`](https://www.typescriptlang.org/tsconfig#strictNullChecks) is not enabled.
@@ -73,7 +76,10 @@ declare const b: string | null;
7376

7477
if (a || b) {
7578
}
79+
if ((a ||= b)) {
80+
}
7681
while (a || b) {}
82+
while ((a ||= b)) {}
7783
do {} while (a || b);
7884
for (let i = 0; a || b; i += 1) {}
7985
a || b ? true : false;
@@ -87,7 +93,10 @@ declare const b: string | null;
8793

8894
if (a ?? b) {
8995
}
96+
if ((a ??= b)) {
97+
}
9098
while (a ?? b) {}
99+
while ((a ??= b)) {}
91100
do {} while (a ?? b);
92101
for (let i = 0; a ?? b; i += 1) {}
93102
a ?? b ? true : false;
@@ -110,6 +119,7 @@ declare const c: string | null;
110119
declare const d: string | null;
111120

112121
a || (b && c);
122+
a ||= b && c;
113123
(a && b) || c || d;
114124
a || (b && c) || d;
115125
a || (b && c && d);
@@ -124,6 +134,7 @@ declare const c: string | null;
124134
declare const d: string | null;
125135

126136
a ?? (b && c);
137+
a ??= b && c;
127138
(a && b) ?? c ?? d;
128139
a ?? (b && c) ?? d;
129140
a ?? (b && c && d);

‎packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts

Lines changed: 87 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,17 @@ export default util.createRule<Options, MessageIds>({
2323
type: 'suggestion',
2424
docs: {
2525
description:
26-
'Enforce using the nullish coalescing operator instead of logical chaining',
26+
'Enforce using the nullish coalescing operator instead of logical assignments or chaining',
2727
recommended: 'strict',
2828
requiresTypeChecking: true,
2929
},
3030
hasSuggestions: true,
3131
messages: {
3232
preferNullishOverOr:
33-
'Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.',
33+
'Prefer using nullish coalescing operator (`??{{ equals }}`) instead of a logical {{ description }} (`||{{ equals }}`), as it is a safer operator.',
3434
preferNullishOverTernary:
35-
'Prefer using nullish coalescing operator (`??`) instead of a ternary expression, as it is simpler to read.',
36-
suggestNullish: 'Fix to nullish coalescing operator (`??`).',
35+
'Prefer using nullish coalescing operator (`??{{ equals }}`) instead of a ternary expression, as it is simpler to read.',
36+
suggestNullish: 'Fix to nullish coalescing operator (`??{{ equals }}`).',
3737
},
3838
schema: [
3939
{
@@ -74,6 +74,75 @@ export default util.createRule<Options, MessageIds>({
7474
const sourceCode = context.getSourceCode();
7575
const checker = parserServices.program.getTypeChecker();
7676

77+
// todo: rename to something more specific?
78+
function checkAssignmentOrLogicalExpression(
79+
node: TSESTree.AssignmentExpression | TSESTree.LogicalExpression,
80+
description: string,
81+
equals: string,
82+
): void {
83+
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
84+
const type = checker.getTypeAtLocation(tsNode.left);
85+
const isNullish = util.isNullableType(type, { allowUndefined: true });
86+
if (!isNullish) {
87+
return;
88+
}
89+
90+
if (ignoreConditionalTests === true && isConditionalTest(node)) {
91+
return;
92+
}
93+
94+
if (
95+
ignoreMixedLogicalExpressions === true &&
96+
isMixedLogicalExpression(node)
97+
) {
98+
return;
99+
}
100+
101+
const barBarOperator = util.nullThrows(
102+
sourceCode.getTokenAfter(
103+
node.left,
104+
token =>
105+
token.type === AST_TOKEN_TYPES.Punctuator &&
106+
token.value === node.operator,
107+
),
108+
util.NullThrowsReasons.MissingToken('operator', node.type),
109+
);
110+
111+
function* fix(
112+
fixer: TSESLint.RuleFixer,
113+
): IterableIterator<TSESLint.RuleFix> {
114+
if (node.parent && util.isLogicalOrOperator(node.parent)) {
115+
// '&&' and '??' operations cannot be mixed without parentheses (e.g. a && b ?? c)
116+
if (
117+
node.left.type === AST_NODE_TYPES.LogicalExpression &&
118+
!util.isLogicalOrOperator(node.left.left)
119+
) {
120+
yield fixer.insertTextBefore(node.left.right, '(');
121+
} else {
122+
yield fixer.insertTextBefore(node.left, '(');
123+
}
124+
yield fixer.insertTextAfter(node.right, ')');
125+
}
126+
yield fixer.replaceText(
127+
barBarOperator,
128+
node.operator.replace('||', '??'),
129+
);
130+
}
131+
132+
context.report({
133+
data: { equals, description },
134+
node: barBarOperator,
135+
messageId: 'preferNullishOverOr',
136+
suggest: [
137+
{
138+
data: { equals },
139+
messageId: 'suggestNullish',
140+
fix,
141+
},
142+
],
143+
});
144+
}
145+
77146
return {
78147
ConditionalExpression(node: TSESTree.ConditionalExpression): void {
79148
if (ignoreTernaryTests) {
@@ -103,7 +172,7 @@ export default util.createRule<Options, MessageIds>({
103172
node.test.right.left,
104173
node.test.right.right,
105174
];
106-
if (node.test.operator === '||') {
175+
if (['||', '||='].includes(node.test.operator)) {
107176
if (
108177
node.test.left.operator === '===' &&
109178
node.test.right.operator === '==='
@@ -205,10 +274,13 @@ export default util.createRule<Options, MessageIds>({
205274

206275
if (isFixable) {
207276
context.report({
277+
// TODO: also account for = in the ternary clause
278+
data: { equals: '' },
208279
node,
209280
messageId: 'preferNullishOverTernary',
210281
suggest: [
211282
{
283+
data: { equals: '' },
212284
messageId: 'suggestNullish',
213285
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
214286
const [left, right] =
@@ -231,64 +303,15 @@ export default util.createRule<Options, MessageIds>({
231303
});
232304
}
233305
},
234-
306+
'AssignmentExpression[operator = "||="]'(
307+
node: TSESTree.AssignmentExpression,
308+
): void {
309+
checkAssignmentOrLogicalExpression(node, 'assignment', '=');
310+
},
235311
'LogicalExpression[operator = "||"]'(
236312
node: TSESTree.LogicalExpression,
237313
): void {
238-
const tsNode = parserServices.esTreeNodeToTSNodeMap.get(node);
239-
const type = checker.getTypeAtLocation(tsNode.left);
240-
const isNullish = util.isNullableType(type, { allowUndefined: true });
241-
if (!isNullish) {
242-
return;
243-
}
244-
245-
if (ignoreConditionalTests === true && isConditionalTest(node)) {
246-
return;
247-
}
248-
249-
const isMixedLogical = isMixedLogicalExpression(node);
250-
if (ignoreMixedLogicalExpressions === true && isMixedLogical) {
251-
return;
252-
}
253-
254-
const barBarOperator = util.nullThrows(
255-
sourceCode.getTokenAfter(
256-
node.left,
257-
token =>
258-
token.type === AST_TOKEN_TYPES.Punctuator &&
259-
token.value === node.operator,
260-
),
261-
util.NullThrowsReasons.MissingToken('operator', node.type),
262-
);
263-
264-
function* fix(
265-
fixer: TSESLint.RuleFixer,
266-
): IterableIterator<TSESLint.RuleFix> {
267-
if (node.parent && util.isLogicalOrOperator(node.parent)) {
268-
// '&&' and '??' operations cannot be mixed without parentheses (e.g. a && b ?? c)
269-
if (
270-
node.left.type === AST_NODE_TYPES.LogicalExpression &&
271-
!util.isLogicalOrOperator(node.left.left)
272-
) {
273-
yield fixer.insertTextBefore(node.left.right, '(');
274-
} else {
275-
yield fixer.insertTextBefore(node.left, '(');
276-
}
277-
yield fixer.insertTextAfter(node.right, ')');
278-
}
279-
yield fixer.replaceText(barBarOperator, '??');
280-
}
281-
282-
context.report({
283-
node: barBarOperator,
284-
messageId: 'preferNullishOverOr',
285-
suggest: [
286-
{
287-
messageId: 'suggestNullish',
288-
fix,
289-
},
290-
],
291-
});
314+
checkAssignmentOrLogicalExpression(node, 'or', '');
292315
},
293316
};
294317
},
@@ -331,7 +354,9 @@ function isConditionalTest(node: TSESTree.Node): boolean {
331354
return false;
332355
}
333356

334-
function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean {
357+
function isMixedLogicalExpression(
358+
node: TSESTree.AssignmentExpression | TSESTree.LogicalExpression,
359+
): boolean {
335360
const seen = new Set<TSESTree.Node | undefined>();
336361
const queue = [node.parent, node.left, node.right];
337362
for (const current of queue) {
@@ -343,7 +368,7 @@ function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean {
343368
if (current && current.type === AST_NODE_TYPES.LogicalExpression) {
344369
if (current.operator === '&&') {
345370
return true;
346-
} else if (current.operator === '||') {
371+
} else if (['||', '||='].includes(current.operator)) {
347372
// check the pieces of the node to catch cases like `a || b || c && d`
348373
queue.push(current.parent, current.left, current.right);
349374
}

0 commit comments

Comments
 (0)