feat(eslint-plugin): added ignoreTernaryTests option to prefer-nullis…

…h-coalescing rule
typescript-eslint · May 12, 2022 · ed7f382 · ed7f382
1 parent 77e15a9
commit ed7f382
Show file tree

Hide file tree

Showing 3 changed files with 385 additions and 3 deletions.
diff --git a/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md b/packages/eslint-plugin/docs/rules/prefer-nullish-coalescing.md
@@ -46,19 +46,57 @@ This rule aims enforce the usage of the safer operator.
 ```ts
 type Options = [
   {
+    ignoreTernaryTests?: boolean;
     ignoreConditionalTests?: boolean;
     ignoreMixedLogicalExpressions?: boolean;
   },
 ];
 
 const defaultOptions = [
   {
+    ignoreTernaryTests: true;
     ignoreConditionalTests: true,
     ignoreMixedLogicalExpressions: true,
   },
 ];
 ```
 
+### `ignoreTernaryTests`
+
+Setting this option to `true` (the default) will cause the rule to ignore any ternary expressions that could be simplified by using a nullish coalesce operator.
+
+Incorrect code for `ignoreTernaryTests: false`, and correct code for `ignoreTernaryTests: true`:
+
+```ts
+const foo: any = 'bar';
+foo !== undefined && foo !== null ? foo : 'a string';
+foo === undefined || foo === null ? 'a string' : foo;
+
+const foo: ?string = 'bar';
+foo !== undefined ? foo : 'a string';
+foo === undefined ? 'a string' : foo;
+
+const foo: string | null = 'bar';
+foo !== null ? foo : 'a string';
+foo === null ? 'a string' : foo;
+```
+
+Correct code for `ignoreTernaryTests: false`:
+
+```ts
+const foo: any = 'bar';
+foo ?? 'a string';
+foo ?? 'a string';
+
+const foo: ?string = 'bar';
+foo ?? 'a string';
+foo ?? 'a string';
+
+const foo: string | null = 'bar';
+foo ?? 'a string';
+foo ?? 'a string';
+```
+
 ### `ignoreConditionalTests`
 
 Setting this option to `true` (the default) will cause the rule to ignore any cases that are located within a conditional test.

diff --git a/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/prefer-nullish-coalescing.ts
@@ -5,14 +5,20 @@ import {
   TSESTree,
 } from '@typescript-eslint/utils';
 import * as util from '../util';
+import * as ts from 'typescript';
 
 export type Options = [
   {
     ignoreConditionalTests?: boolean;
+    ignoreTernaryTests?: boolean;
     ignoreMixedLogicalExpressions?: boolean;
   },
 ];
-export type MessageIds = 'preferNullish' | 'suggestNullish';
+
+export type MessageIds =
+  | 'preferNullish'
+  | 'preferNullishOverTernary'
+  | 'suggestNullish';
 
 export default util.createRule<Options, MessageIds>({
   name: 'prefer-nullish-coalescing',
@@ -29,6 +35,8 @@ export default util.createRule<Options, MessageIds>({
     messages: {
       preferNullish:
         'Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.',
+      preferNullishOverTernary:
+        'Prefer using nullish coalescing operator (`??`) instead of ternary expression, as it is simpler to read.',
       suggestNullish: 'Fix to nullish coalescing operator (`??`).',
     },
     schema: [
@@ -38,6 +46,9 @@ export default util.createRule<Options, MessageIds>({
           ignoreConditionalTests: {
             type: 'boolean',
           },
+          ignoreTernaryTests: {
+            type: 'boolean',
+          },
           ignoreMixedLogicalExpressions: {
             type: 'boolean',
           },
@@ -52,15 +63,87 @@ export default util.createRule<Options, MessageIds>({
   defaultOptions: [
     {
       ignoreConditionalTests: true,
+      ignoreTernaryTests: true,
       ignoreMixedLogicalExpressions: true,
     },
   ],
-  create(context, [{ ignoreConditionalTests, ignoreMixedLogicalExpressions }]) {
+  create(
+    context,
+    [
+      {
+        ignoreConditionalTests,
+        ignoreTernaryTests,
+        ignoreMixedLogicalExpressions,
+      },
+    ],
+  ) {
     const parserServices = util.getParserServices(context);
     const sourceCode = context.getSourceCode();
     const checker = parserServices.program.getTypeChecker();
 
     return {
+      ConditionalExpression(node: TSESTree.ConditionalExpression): void {
+        if (ignoreTernaryTests) {
+          return;
+        }
+
+        let identifier: TSESTree.Identifier;
+        let alternate: TSESTree.Expression;
+        let requiredOperator: '!==' | '===';
+        if (
+          node.consequent.type === AST_NODE_TYPES.Identifier &&
+          ((node.test.type === AST_NODE_TYPES.BinaryExpression &&
+            node.test.operator === '!==') ||
+            (node.test.type === AST_NODE_TYPES.LogicalExpression &&
+              node.test.left.type === AST_NODE_TYPES.BinaryExpression &&
+              node.test.left.operator === '!=='))
+        ) {
+          identifier = node.consequent;
+          alternate = node.alternate;
+          requiredOperator = '!==';
+        } else if (node.alternate.type === AST_NODE_TYPES.Identifier) {
+          identifier = node.alternate;
+          alternate = node.consequent;
+          requiredOperator = '===';
+        } else {
+          return;
+        }
+
+        if (
+          isFixableExplicitTernary({
+            requiredOperator,
+            identifier,
+            node,
+          }) ||
+          isFixableImplicitTernary({
+            parserServices,
+            checker,
+            requiredOperator,
+            identifier,
+            node,
+          })
+        ) {
+          context.report({
+            node,
+            messageId: 'preferNullishOverTernary',
+            suggest: [
+              {
+                messageId: 'suggestNullish',
+                fix(fixer: TSESLint.RuleFixer): TSESLint.RuleFix {
+                  return fixer.replaceText(
+                    node,
+                    `${identifier.name} ?? ${sourceCode.text.slice(
+                      alternate.range[0],
+                      alternate.range[1],
+                    )}`,
+                  );
+                },
+              },
+            ],
+          });
+        }
+      },
+
       'LogicalExpression[operator = "||"]'(
         node: TSESTree.LogicalExpression,
       ): void {
@@ -181,3 +264,142 @@ function isMixedLogicalExpression(node: TSESTree.LogicalExpression): boolean {
 
   return false;
 }
+
+/**
+ * This is for cases where we check both undefined and null.
+ * example: foo === undefined && foo === null ? 'a string' : foo
+ * output: foo ?? 'a string'
+ */
+function isFixableExplicitTernary({
+  requiredOperator,
+  identifier,
+  node,
+}: {
+  requiredOperator: string;
+  identifier: TSESTree.Identifier;
+  node: TSESTree.ConditionalExpression;
+}): boolean {
+  if (node.test.type !== AST_NODE_TYPES.LogicalExpression) {
+    return false;
+  }
+  if (requiredOperator === '===' && node.test.operator === '&&') {
+    return false;
+  }
+  const { left, right } = node.test;
+  if (
+    left.type !== AST_NODE_TYPES.BinaryExpression ||
+    right.type !== AST_NODE_TYPES.BinaryExpression
+  ) {
+    return false;
+  }
+
+  if (
+    left.operator !== requiredOperator ||
+    right.operator !== requiredOperator
+  ) {
+    return false;
+  }
+
+  const isIdentifier = (
+    i: TSESTree.Expression | TSESTree.PrivateIdentifier,
+  ): boolean =>
+    i.type === AST_NODE_TYPES.Identifier && i.name === identifier.name;
+
+  const hasUndefinedCheck =
+    (isIdentifier(left.left) && isUndefined(left.right)) ||
+    (isIdentifier(left.right) && isUndefined(left.left)) ||
+    (isIdentifier(right.left) && isUndefined(right.right)) ||
+    (isIdentifier(right.right) && isUndefined(right.left));
+
+  if (!hasUndefinedCheck) {
+    return false;
+  }
+
+  const hasNullCheck =
+    (isIdentifier(left.left) && isNull(left.right)) ||
+    (isIdentifier(left.right) && isNull(left.left)) ||
+    (isIdentifier(right.left) && isNull(right.right)) ||
+    (isIdentifier(right.right) && isNull(right.left));
+
+  if (!hasNullCheck) {
+    return false;
+  }
+
+  return true;
+}
+
+/**
+ * This is for cases where we check either undefined or null and fall back to
+ * using type information to ensure that our checks are correct.
+ * example: const foo:? string = 'bar';
+ *          foo !== undefined ? foo : 'a string';
+ * output: foo ?? 'a string'
+ */
+function isFixableImplicitTernary({
+  parserServices,
+  checker,
+  requiredOperator,
+  identifier,
+  node,
+}: {
+  parserServices: ReturnType<typeof util.getParserServices>;
+  checker: ts.TypeChecker;
+  requiredOperator: string;
+  identifier: TSESTree.Identifier;
+  node: TSESTree.ConditionalExpression;
+}): boolean {
+  if (node.test.type !== AST_NODE_TYPES.BinaryExpression) {
+    return false;
+  }
+  const { left, right, operator } = node.test;
+  if (operator !== requiredOperator) {
+    return false;
+  }
+  const isIdentifier = (
+    i: TSESTree.Expression | TSESTree.PrivateIdentifier,
+  ): boolean =>
+    i.type === AST_NODE_TYPES.Identifier && i.name === identifier.name;
+
+  const i = isIdentifier(left) ? left : isIdentifier(right) ? right : null;
+  if (!i) {
+    return false;
+  }
+
+  const tsNode = parserServices.esTreeNodeToTSNodeMap.get(i);
+  const type = checker.getTypeAtLocation(tsNode);
+  const flags = util.getTypeFlags(type);
+
+  if (flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
+    return false;
+  }
+
+  const hasNullType = (flags & ts.TypeFlags.Null) !== 0;
+  const hasUndefinedType = (flags & ts.TypeFlags.Undefined) !== 0;
+
+  if (
+    (hasUndefinedType && hasNullType) ||
+    (!hasUndefinedType && !hasNullType)
+  ) {
+    return false;
+  }
+
+  if (hasNullType && (isNull(right) || isNull(left))) {
+    return true;
+  }
+
+  if (hasUndefinedType && (isUndefined(right) || isUndefined(left))) {
+    return true;
+  }
+
+  return false;
+}
+
+function isUndefined(
+  i: TSESTree.Expression | TSESTree.PrivateIdentifier,
+): boolean {
+  return i.type === AST_NODE_TYPES.Identifier && i.name === 'undefined';
+}
+
+function isNull(i: TSESTree.Expression | TSESTree.PrivateIdentifier): boolean {
+  return i.type === AST_NODE_TYPES.Literal && i.value === null;
+}