feat: handle use-before-assigned case

typescript-eslint · May 16, 2021 · 9379caa · 9379caa
1 parent 84946c5
commit 9379caa
Show file tree

Hide file tree

Showing 3 changed files with 180 additions and 1 deletion.
diff --git a/packages/eslint-plugin/docs/rules/no-non-null-asserted-nullish-coalescing.md b/packages/eslint-plugin/docs/rules/no-non-null-asserted-nullish-coalescing.md
@@ -14,6 +14,13 @@ foo! ?? bar;
 foo.bazz! ?? bar;
 foo!.bazz! ?? bar;
 foo()! ?? bar;
+
+let x!: string;
+x! ?? '';
+
+let x: string;
+x = foo();
+x! ?? '';
 ```
 
 Examples of **correct** code for this rule:
@@ -26,6 +33,10 @@ foo ?? bar!;
 foo!.bazz ?? bar;
 foo!.bazz ?? bar!;
 foo() ?? bar;
+
+// This is considered correct code because because there's no way for the user to satisfy it.
+let x: string;
+x! ?? '';
 ```
 
 ## When Not To Use It

diff --git a/packages/eslint-plugin/src/rules/no-non-null-asserted-nullish-coalescing.ts b/packages/eslint-plugin/src/rules/no-non-null-asserted-nullish-coalescing.ts
@@ -1,6 +1,35 @@
 import { TSESTree, TSESLint } from '@typescript-eslint/experimental-utils';
+import {
+  Definition,
+  DefinitionType,
+  Variable,
+} from '@typescript-eslint/scope-manager';
 import * as util from '../util';
 
+function hasAssignmentBeforeNode(
+  variable: Variable,
+  node: TSESTree.Node,
+): boolean {
+  return (
+    variable.references.some(
+      ref => ref.isWrite() && ref.identifier.range[1] < node.range[1],
+    ) ||
+    variable.defs.some(
+      def =>
+        isDefinitionWithAssignment(def) && def.node.range[1] < node.range[1],
+    )
+  );
+}
+
+function isDefinitionWithAssignment(definition: Definition): boolean {
+  if (definition.type !== DefinitionType.Variable) {
+    return false;
+  }
+
+  const variableDeclarator = definition.node;
+  return variableDeclarator.definite ?? variableDeclarator.init !== null;
+}
+
 export default util.createRule({
   name: 'no-non-null-asserted-nullish-coalescing',
   meta: {
@@ -25,10 +54,31 @@ export default util.createRule({
       'LogicalExpression[operator = "??"] > TSNonNullExpression.left'(
         node: TSESTree.TSNonNullExpression,
       ): void {
+        if (node.expression.type === TSESTree.AST_NODE_TYPES.Identifier) {
+          const scope = context.getScope();
+          const identifier = node.expression;
+          const variable = scope.set.get(identifier.name);
+          if (variable && !hasAssignmentBeforeNode(variable, node)) {
+            return;
+          }
+        }
+
         context.report({
           node,
           messageId: 'noNonNullAssertedNullishCoalescing',
-          // use a suggestion instead of a fixer, because this can obviously break type checks
+          /*
+          Use a suggestion instead of a fixer, because this can break type checks.
+          The resulting type of the nullish coalesce is only influenced by the right operand if the left operand can be `null` or `undefined`.
+          After removing the non-null assertion the type of the left operand might contain `null` or `undefined` and then the type of the right operand
+          might change the resulting type of the nullish coalesce.
+          See the following example:
+
+          function test(x?: string): string {
+            const bar = x! ?? false; // type analysis reports `bar` has type `string`
+            //          x  ?? false; // type analysis reports `bar` has type `string | false`
+            return bar;
+          }
+          */
           suggest: [
             {
               messageId: 'suggestRemovingNonNull',

diff --git a/packages/eslint-plugin/tests/rules/no-non-null-asserted-nullish-coalescing.test.ts b/packages/eslint-plugin/tests/rules/no-non-null-asserted-nullish-coalescing.test.ts
@@ -16,6 +16,38 @@ ruleTester.run('no-non-null-asserted-nullish-coalescing', rule, {
     'foo() ?? bar;',
     'foo() ?? bar!;',
     '(foo ?? bar)!;',
+    `
+      let x: string;
+      x! ?? '';
+    `,
+    `
+      let x: string;
+      x ?? '';
+    `,
+    `
+      let x!: string;
+      x ?? '';
+    `,
+    `
+      let x: string;
+      foo(x);
+      x! ?? '';
+    `,
+    `
+      let x: string;
+      x! ?? '';
+      x = foo();
+    `,
+    `
+      let x: string;
+      foo(x);
+      x! ?? '';
+      x = foo();
+    `,
+    `
+      let x = foo();
+      x ?? '';
+    `,
   ],
   invalid: [
     {
@@ -130,5 +162,91 @@ ruleTester.run('no-non-null-asserted-nullish-coalescing', rule, {
         },
       ],
     },
+    {
+      code: `
+let x!: string;
+x! ?? '';
+      `.trimRight(),
+      errors: [
+        {
+          messageId: 'noNonNullAssertedNullishCoalescing',
+          suggestions: [
+            {
+              messageId: 'suggestRemovingNonNull',
+              output: `
+let x!: string;
+x ?? '';
+              `.trimRight(),
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+let x: string;
+x = foo();
+x! ?? '';
+      `.trimRight(),
+      errors: [
+        {
+          messageId: 'noNonNullAssertedNullishCoalescing',
+          suggestions: [
+            {
+              messageId: 'suggestRemovingNonNull',
+              output: `
+let x: string;
+x = foo();
+x ?? '';
+              `.trimRight(),
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+let x: string;
+x = foo();
+x! ?? '';
+x = foo();
+      `.trimRight(),
+      errors: [
+        {
+          messageId: 'noNonNullAssertedNullishCoalescing',
+          suggestions: [
+            {
+              messageId: 'suggestRemovingNonNull',
+              output: `
+let x: string;
+x = foo();
+x ?? '';
+x = foo();
+              `.trimRight(),
+            },
+          ],
+        },
+      ],
+    },
+    {
+      code: `
+let x = foo();
+x! ?? '';
+      `.trimRight(),
+      errors: [
+        {
+          messageId: 'noNonNullAssertedNullishCoalescing',
+          suggestions: [
+            {
+              messageId: 'suggestRemovingNonNull',
+              output: `
+let x = foo();
+x ?? '';
+              `.trimRight(),
+            },
+          ],
+        },
+      ],
+    },
   ],
 });