From 574e8bc6f1adcb0740c4cf3ef3dcced588114f53 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 16 May 2019 10:24:49 -0700 Subject: [PATCH 1/3] fix(eslint-plugin): [NUTA] false positive for null assign to undefined 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: [ From 87ffc49884ff174905b781b53891a7df4ad02978 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 16 May 2019 10:42:30 -0700 Subject: [PATCH 2/3] fix(eslint-plugin): [NUTA] handle missing declarations Fixes #532 --- .../rules/no-unnecessary-type-assertion.ts | 5 +++ packages/eslint-plugin/src/util/types.ts | 13 +++++- .../no-unnecessary-type-assertion.test.ts | 40 +++++++++++++++++++ 3 files changed, 56 insertions(+), 2 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 54bb2d6cc92..6dd69974ca2 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts @@ -134,6 +134,11 @@ export default util.createRule({ */ function isPossiblyUsedBeforeAssigned(node: ts.Expression): boolean { const declaration = util.getDeclaration(checker, node); + if (!declaration) { + // don't know what the declaration is for some reason, so just assume the worst + return true; + } + if ( // non-strict mode doesn't care about used before assigned errors isStrictCompilerOptionEnabled(compilerOptions, 'strictNullChecks') && diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index 98ae09b02c4..04eb720126a 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -140,8 +140,17 @@ export function isNullableType( export function getDeclaration( checker: ts.TypeChecker, node: ts.Expression, -): ts.Declaration { - return checker.getSymbolAtLocation(node)!.declarations![0]; +): ts.Declaration | null { + const symbol = checker.getSymbolAtLocation(node); + if (!symbol) { + return null; + } + const declarations = symbol.declarations; + if (!declarations) { + return null; + } + + return declarations[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 7ed69a441f4..4c06e95a042 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 @@ -95,6 +95,21 @@ declare const str: string | null; foo(str!); `, + // https://github.com/typescript-eslint/typescript-eslint/issues/532 + ` +declare function a(a: string): any; +declare const b: string | null; +class Mx { + @a(b!) + private prop = 1; +} + `, + ` +class Mx { + @a(b!) + private prop = 1; +} + `, ], invalid: [ @@ -280,5 +295,30 @@ class Foo { }, ], }, + // https://github.com/typescript-eslint/typescript-eslint/issues/532 + { + code: ` +declare function a(a: string): any; +const b = 'asdf'; +class Mx { + @a(b!) + private prop = 1; +} + `, + output: ` +declare function a(a: string): any; +const b = 'asdf'; +class Mx { + @a(b) + private prop = 1; +} + `, + errors: [ + { + messageId: 'unnecessaryAssertion', + line: 5, + }, + ], + }, ], }); From 1871854e32c87aa62d4dcaaa1e03d67bfcf66784 Mon Sep 17 00:00:00 2001 From: Brad Zacher Date: Thu, 6 Jun 2019 22:24:42 -0700 Subject: [PATCH 3/3] fix: shonky master merge --- packages/eslint-plugin/src/util/types.ts | 29 ------------------------ 1 file changed, 29 deletions(-) diff --git a/packages/eslint-plugin/src/util/types.ts b/packages/eslint-plugin/src/util/types.ts index 843798c0d99..04eb720126a 100644 --- a/packages/eslint-plugin/src/util/types.ts +++ b/packages/eslint-plugin/src/util/types.ts @@ -181,32 +181,3 @@ export function isTypeFlagSet( return (flags & flagsToCheck) !== 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; -}