Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
feat(eslint-plugin): add prefer-nullish-coalescing (#1069)
- Loading branch information
1 parent
b91b0ba
commit a9cd399
Showing
7 changed files
with
761 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
141 changes: 141 additions & 0 deletions
141
packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,141 @@ | ||
# Enforce the usage of the nullish coalescing operator instead of logical chaining (prefer-nullish-coalescing) | ||
|
||
TypeScript 3.7 added support for the nullish coalescing operator. | ||
This operator allows you to safely cascade a value when dealing with `null` or `undefined`. | ||
|
||
```ts | ||
function myFunc(foo: string | null) { | ||
return foo ?? 'a string'; | ||
} | ||
|
||
// is equivalent to | ||
|
||
function myFunc(foo: string | null) { | ||
return foo !== null && foo !== undefined ? foo : 'a string'; | ||
} | ||
``` | ||
|
||
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 _falsey_ value: | ||
|
||
```ts | ||
const emptyString = ''; | ||
|
||
const nullish1 = emptyString ?? 'unsafe'; | ||
const logical1 = emptyString || 'unsafe'; | ||
|
||
// nullish1 === '' | ||
// logical1 === 'unsafe' | ||
|
||
declare const nullString: string | null; | ||
|
||
const nullish2 = nullString ?? 'safe'; | ||
const logical2 = nullString || 'safe'; | ||
|
||
// nullish2 === 'safe' | ||
// logical2 === 'safe' | ||
``` | ||
|
||
## Rule Details | ||
|
||
This rule aims enforce the usage of the safer operator. | ||
|
||
## Options | ||
|
||
```ts | ||
type Options = [ | ||
{ | ||
ignoreConditionalTests?: boolean; | ||
ignoreMixedLogicalExpressions?: boolean; | ||
}, | ||
]; | ||
|
||
const defaultOptions = [ | ||
{ | ||
ignoreConditionalTests: true, | ||
ignoreMixedLogicalExpressions: true; | ||
}, | ||
]; | ||
``` | ||
|
||
### ignoreConditionalTests | ||
|
||
Setting this option to `true` (the default) will cause the rule to ignore any cases that are located within a conditional test. | ||
|
||
Generally expressions within conditional tests intentionally use the falsey fallthrough behaviour of the logical or operator, meaning that fixing the operator to the nullish coalesce operator could cause bugs. | ||
|
||
If you're looking to enforce stricter conditional tests, you should consider using the `strict-boolean-expressions` rule. | ||
|
||
Incorrect code for `ignoreConditionalTests: false`, and correct code for `ignoreConditionalTests: true`: | ||
|
||
```ts | ||
declare const a: string | null; | ||
declare const b: string | null; | ||
|
||
if (a || b) { | ||
} | ||
while (a || b) {} | ||
do {} while (a || b); | ||
for (let i = 0; a || b; i += 1) {} | ||
a || b ? true : false; | ||
``` | ||
|
||
Correct code for `ignoreConditionalTests: false`: | ||
|
||
```ts | ||
declare const a: string | null; | ||
declare const b: string | null; | ||
|
||
if (a ?? b) { | ||
} | ||
while (a ?? b) {} | ||
do {} while (a ?? b); | ||
for (let i = 0; a ?? b; i += 1) {} | ||
a ?? b ? true : false; | ||
``` | ||
|
||
### ignoreMixedLogicalExpressions | ||
|
||
Setting this option to `true` (the default) will cause the rule to ignore any logical or expressions thare are part of a mixed logical expression (with `&&`). | ||
|
||
Generally expressions within mixed logical expressions intentionally use the falsey fallthrough behaviour of the logical or operator, meaning that fixing the operator to the nullish coalesce operator could cause bugs. | ||
|
||
If you're looking to enforce stricter conditional tests, you should consider using the `strict-boolean-expressions` rule. | ||
|
||
Incorrect code for `ignoreMixedLogicalExpressions: false`, and correct code for `ignoreMixedLogicalExpressions: true`: | ||
|
||
```ts | ||
declare const a: string | null; | ||
declare const b: string | null; | ||
declare const c: string | null; | ||
declare const d: string | null; | ||
|
||
a || (b && c); | ||
(a && b) || c || d; | ||
a || (b && c) || d; | ||
a || (b && c && d); | ||
``` | ||
|
||
Correct code for `ignoreMixedLogicalExpressions: false`: | ||
|
||
```ts | ||
declare const a: string | null; | ||
declare const b: string | null; | ||
declare const c: string | null; | ||
declare const d: string | null; | ||
|
||
a ?? (b && c); | ||
(a && b) ?? c ?? d; | ||
a ?? (b && c) ?? d; | ||
a ?? (b && c && d); | ||
``` | ||
|
||
**_NOTE:_** Errors for this specific case will be presented as suggestions, instead of fixes. This is because it is not always safe to automatically convert `||` to `??` within a mixed logical expression, as we cannot tell the intended precedence of the operator. Note that by design, `??` requires parentheses when used with `&&` or `||` in the same expression. | ||
|
||
## When Not To Use It | ||
|
||
If you are not using TypeScript 3.7 (or greater), then you will not be able to use this rule, as the operator is not supported. | ||
|
||
## Further Reading | ||
|
||
- [TypeScript 3.7 Release Notes](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-3-7.html) | ||
- [Nullish Coalescing Operator Proposal](https://github.com/tc39/proposal-nullish-coalescing/) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
174 changes: 174 additions & 0 deletions
174
packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,174 @@ | ||
import { | ||
AST_NODE_TYPES, | ||
AST_TOKEN_TYPES, | ||
TSESLint, | ||
TSESTree, | ||
} from '@typescript-eslint/experimental-utils'; | ||
import ts from 'typescript'; | ||
import * as util from '../util'; | ||
|
||
export type Options = [ | ||
{ | ||
ignoreConditionalTests?: boolean; | ||
ignoreMixedLogicalExpressions?: boolean; | ||
}, | ||
]; | ||
export type MessageIds = 'preferNullish'; | ||
|
||
export default util.createRule<Options, MessageIds>({ | ||
name: 'prefer-nullish-coalescing', | ||
meta: { | ||
type: 'suggestion', | ||
docs: { | ||
description: | ||
'Enforce the usage of the nullish coalescing operator instead of logical chaining', | ||
category: 'Best Practices', | ||
recommended: false, | ||
requiresTypeChecking: true, | ||
}, | ||
fixable: 'code', | ||
messages: { | ||
preferNullish: | ||
'Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.', | ||
}, | ||
schema: [ | ||
{ | ||
type: 'object', | ||
properties: { | ||
ignoreConditionalTests: { | ||
type: 'boolean', | ||
}, | ||
ignoreMixedLogicalExpressions: { | ||
type: 'boolean', | ||
}, | ||
}, | ||
additionalProperties: false, | ||
}, | ||
], | ||
}, | ||
defaultOptions: [ | ||
{ | ||
ignoreConditionalTests: true, | ||
ignoreMixedLogicalExpressions: true, | ||
}, | ||
], | ||
create(context, [{ ignoreConditionalTests, ignoreMixedLogicalExpressions }]) { | ||
const parserServices = util.getParserServices(context); | ||
const sourceCode = context.getSourceCode(); | ||
const checker = parserServices.program.getTypeChecker(); | ||
|
||
return { | ||
'LogicalExpression[operator = "||"]'( | ||
node: TSESTree.LogicalExpression, | ||
): void { | ||
const tsNode = parserServices.esTreeNodeToTSNodeMap.get< | ||
ts.BinaryExpression | ||
>(node); | ||
const type = checker.getTypeAtLocation(tsNode.left); | ||
const isNullish = util.isNullableType(type, { allowUndefined: true }); | ||
if (!isNullish) { | ||
return; | ||
} | ||
|
||
if (ignoreConditionalTests === true && isConditionalTest(node)) { | ||
return; | ||
} | ||
|
||
const isMixedLogical = isMixedLogicalExpression(node); | ||
if (ignoreMixedLogicalExpressions === true && isMixedLogical) { | ||
return; | ||
} | ||
|
||
const barBarOperator = sourceCode.getTokenAfter( | ||
node.left, | ||
token => | ||
token.type === AST_TOKEN_TYPES.Punctuator && | ||
token.value === node.operator, | ||
)!; // there _must_ be an operator | ||
|
||
const fixer = isMixedLogical | ||
? // suggestion instead for cases where we aren't sure if the fixer is completely safe | ||
({ | ||
suggest: [ | ||
{ | ||
messageId: 'preferNullish', | ||
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { | ||
return fixer.replaceText(barBarOperator, '??'); | ||
}, | ||
}, | ||
], | ||
} as const) | ||
: { | ||
fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix { | ||
return fixer.replaceText(barBarOperator, '??'); | ||
}, | ||
}; | ||
|
||
context.report({ | ||
node: barBarOperator, | ||
messageId: 'preferNullish', | ||
...fixer, | ||
}); | ||
}, | ||
}; | ||
}, | ||
}); | ||
|
||
function isConditionalTest(node: TSESTree.Node): boolean { | ||
const parents = new Set<TSESTree.Node | null>([node]); | ||
let current = node.parent; | ||
while (current) { | ||
parents.add(current); | ||
|
||
if ( | ||
(current.type === AST_NODE_TYPES.ConditionalExpression || | ||
current.type === AST_NODE_TYPES.DoWhileStatement || | ||
current.type === AST_NODE_TYPES.IfStatement || | ||
current.type === AST_NODE_TYPES.ForStatement || | ||
current.type === AST_NODE_TYPES.WhileStatement) && | ||
parents.has(current.test) | ||
) { | ||
return true; | ||
} | ||
|
||
if ( | ||
[ | ||
AST_NODE_TYPES.ArrowFunctionExpression, | ||
AST_NODE_TYPES.FunctionExpression, | ||
].includes(current.type) | ||
) { | ||
/** | ||
* This is a weird situation like: | ||
* `if (() => a || b) {}` | ||
* `if (function () { return a || b }) {}` | ||
*/ | ||
return false; | ||
} | ||
|
||
current = current.parent; | ||
} | ||
|
||
return false; | ||
} | ||
|
||
function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean { | ||
const seen = new Set<TSESTree.Node | undefined>(); | ||
const queue = [node.parent, node.left, node.right]; | ||
for (const current of queue) { | ||
if (seen.has(current)) { | ||
continue; | ||
} | ||
seen.add(current); | ||
|
||
if (current && current.type === AST_NODE_TYPES.LogicalExpression) { | ||
if (current.operator === '&&') { | ||
return true; | ||
} else if (current.operator === '||') { | ||
// check the pieces of the node to catch cases like `a || b || c && d` | ||
queue.push(current.parent, current.left, current.right); | ||
} | ||
} | ||
} | ||
|
||
return false; | ||
} |
Oops, something went wrong.