fix(eslint-plugin): [NUTA] false positive for null assign to undefined (

#536) Fixes #529
typescript-eslint · Jun 7, 2019 · b16409a · b16409a
1 parent 6489293
commit b16409a
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 23 deletions.
diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts
@@ -188,18 +188,43 @@ export default util.createRule<Options, MessageIds>({
         } else {
           // we know it's a nullable type
           // so figure out if the variable is used in a place that accepts nullable types
+
           const contextualType = getContextualType(checker, originalNode);
-          if (contextualType && util.isNullableType(contextualType)) {
-            context.report({
-              node,
-              messageId: 'contextuallyUnnecessary',
-              fix(fixer) {
-                return fixer.removeRange([
-                  originalNode.expression.end,
-                  originalNode.end,
-                ]);
-              },
-            });
+          if (contextualType) {
+            // in strict mode you can't assign null to undefined, so we have to make sure that
+            // the two types share a nullable type
+            const typeIncludesUndefined = util.isTypeFlagSet(
+              type,
+              ts.TypeFlags.Undefined,
+            );
+            const typeIncludesNull = util.isTypeFlagSet(
+              type,
+              ts.TypeFlags.Null,
+            );
+
+            const contextualTypeIncludesUndefined = util.isTypeFlagSet(
+              contextualType,
+              ts.TypeFlags.Undefined,
+            );
+            const contextualTypeIncludesNull = util.isTypeFlagSet(
+              contextualType,
+              ts.TypeFlags.Null,
+            );
+            if (
+              (typeIncludesUndefined && contextualTypeIncludesUndefined) ||
+              (typeIncludesNull && contextualTypeIncludesNull)
+            ) {
+              context.report({
+                node,
+                messageId: 'contextuallyUnnecessary',
+                fix(fixer) {
+                  return fixer.removeRange([
+                    originalNode.expression.end,
+                    originalNode.end,
+                  ]);
+                },
+              });
+            }
           }
         }
       },

diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts
@@ -1,5 +1,4 @@
 import {
-  isTypeFlagSet,
   isTypeReference,
   isUnionOrIntersectionType,
   unionTypeParts,
@@ -115,18 +114,24 @@ export function getConstrainedTypeAtLocation(
  * Checks if the given type is (or accepts) nullable
  * @param isReceiver true if the type is a receiving type (i.e. the type of a called function's parameter)
  */
-export function isNullableType(type: ts.Type, isReceiver?: boolean): boolean {
-  let flags: ts.TypeFlags = 0;
-  for (const t of unionTypeParts(type)) {
-    flags |= t.flags;
-  }
+export function isNullableType(
+  type: ts.Type,
+  {
+    isReceiver = false,
+    allowUndefined = true,
+  }: { isReceiver?: boolean; allowUndefined?: boolean } = {},
+): boolean {
+  const flags = getTypeFlags(type);
 
-  flags =
-    isReceiver && flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)
-      ? -1
-      : flags;
+  if (isReceiver && flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
+    return true;
+  }
 
-  return (flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) !== 0;
+  if (allowUndefined) {
+    return (flags & (ts.TypeFlags.Null | ts.TypeFlags.Undefined)) !== 0;
+  } else {
+    return (flags & ts.TypeFlags.Null) !== 0;
+  }
 }
 
 /**
@@ -138,3 +143,32 @@ export function getDeclaration(
 ): ts.Declaration {
   return checker.getSymbolAtLocation(node)!.declarations![0];
 }
+
+/**
+ * Gets all of the type flags in a type, iterating through unions automatically
+ */
+export function getTypeFlags(type: ts.Type): ts.TypeFlags {
+  let flags: ts.TypeFlags = 0;
+  for (const t of unionTypeParts(type)) {
+    flags |= t.flags;
+  }
+  return flags;
+}
+
+/**
+ * Checks if the given type is (or accepts) the given flags
+ * @param isReceiver true if the type is a receiving type (i.e. the type of a called function's parameter)
+ */
+export function isTypeFlagSet(
+  type: ts.Type,
+  flagsToCheck: ts.TypeFlags,
+  isReceiver?: boolean,
+): boolean {
+  const flags = getTypeFlags(type);
+
+  if (isReceiver && flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown)) {
+    return true;
+  }
+
+  return (flags & flagsToCheck) !== 0;
+}
diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-type-assertion.test.ts
@@ -2,7 +2,7 @@ import path from 'path';
 import rule from '../../src/rules/no-unnecessary-type-assertion';
 import { RuleTester } from '../RuleTester';
 
-const rootDir = path.join(process.cwd(), 'tests/fixtures');
+const rootDir = path.resolve(__dirname, '../fixtures/');
 const ruleTester = new RuleTester({
   parserOptions: {
     ecmaVersion: 2015,
@@ -88,6 +88,13 @@ class Foo {
   prop: number = x!;
 }
     `,
+    // https://github.com/typescript-eslint/typescript-eslint/issues/529
+    `
+declare function foo(str?: string): void;
+declare const str: string | null;
+
+foo(str!);
+    `,
   ],
 
   invalid: [