From b16409ab8b532fca61a61dc0a0b6abcff512dad5 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 6 Jun 2019 21:26:16 -0700 Subject: [PATCH] fix(eslint-plugin): [NUTA] false positive for null assign to undefined (#536) Fixes #529 --- .../rules/no-unnecessary-type-assertion.ts | 47 ++++++++++++---- packages/eslint-plugin/src/util/types.ts | 56 +++++++++++++++---- .../no-unnecessary-type-assertion.test.ts | 9 ++- 3 files changed, 89 insertions(+), 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 index 765fd308653..54bb2d6cc92 100644 --- 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({ } 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 index 55567f00aea..98ae09b02c4 100644 --- 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 index 7106fc461b4..7ed69a441f4 100644 --- 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: [