feat(eslint-plugin): add prefer-nullish-coalescing (#1069)

Author: bradzacher

Diff for: packages/eslint-plugin/README.md

@@ -194,6 +194,7 @@ Then you should add `airbnb` (or `airbnb-base`) to your `extends` section of `.e
 | [`@typescript-eslint/prefer-function-type`](./docs/rules/prefer-function-type.md)                         | Use function types instead of interfaces with call signatures                                                                                       |                    | :wrench: |                   |
 | [`@typescript-eslint/prefer-includes`](./docs/rules/prefer-includes.md)                                   | Enforce `includes` method over `indexOf` method                                                                                                     | :heavy_check_mark: | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/prefer-namespace-keyword`](./docs/rules/prefer-namespace-keyword.md)                 | Require the use of the `namespace` keyword instead of the `module` keyword to declare custom TypeScript modules                                     | :heavy_check_mark: | :wrench: |                   |
+| [`@typescript-eslint/prefer-nullish-coalescing`](./docs/rules/prefer-nullish-coalescing.md)               | Enforce the usage of the nullish coalescing operator instead of logical chaining                                                                    |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/prefer-optional-chain`](./docs/rules/prefer-optional-chain.md)                       | Prefer using concise optional chain expressions instead of chained logical ands                                                                     |                    | :wrench: |                   |
 | [`@typescript-eslint/prefer-readonly`](./docs/rules/prefer-readonly.md)                                   | Requires that private members are marked as `readonly` if they're never modified outside of the constructor                                         |                    | :wrench: | :thought_balloon: |
 | [`@typescript-eslint/prefer-regexp-exec`](./docs/rules/prefer-regexp-exec.md)                             | Prefer RegExp#exec() over String#match() if no global flag is provided                                                                              | :heavy_check_mark: |          | :thought_balloon: |

Diff for: packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md

@@ -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/)

Diff for: packages/eslint-plugin/src/configs/all.json

@@ -67,6 +67,7 @@
     "@typescript-eslint/prefer-function-type": "error",
     "@typescript-eslint/prefer-includes": "error",
     "@typescript-eslint/prefer-namespace-keyword": "error",
+    "@typescript-eslint/prefer-nullish-coalescing": "error",
     "@typescript-eslint/prefer-optional-chain": "error",
     "@typescript-eslint/prefer-readonly": "error",
     "@typescript-eslint/prefer-regexp-exec": "error",

Diff for: packages/eslint-plugin/src/rules/index.ts

@@ -52,6 +52,7 @@ import preferForOf from './prefer-for-of';
 import preferFunctionType from './prefer-function-type';
 import preferIncludes from './prefer-includes';
 import preferNamespaceKeyword from './prefer-namespace-keyword';
+import preferNullishCoalescing from './prefer-nullish-coalescing';
 import preferOptionalChain from './prefer-optional-chain';
 import preferReadonly from './prefer-readonly';
 import preferRegexpExec from './prefer-regexp-exec';
@@ -127,6 +128,7 @@ export default {
   'prefer-function-type': preferFunctionType,
   'prefer-includes': preferIncludes,
   'prefer-namespace-keyword': preferNamespaceKeyword,
+  'prefer-nullish-coalescing': preferNullishCoalescing,
   'prefer-optional-chain': preferOptionalChain,
   'prefer-readonly': preferReadonly,
   'prefer-regexp-exec': preferRegexpExec,

Diff for: packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts

@@ -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;
+}