Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(eslint-plugin): [no-unnecessary-type-assertion] false positive for null assign to undefined #536

Merged
merged 2 commits into from Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 36 additions & 11 deletions packages/eslint-plugin/src/rules/no-unnecessary-type-assertion.ts
Expand Up @@ -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,
]);
},
});
}
}
}
},
Expand Down
56 changes: 45 additions & 11 deletions packages/eslint-plugin/src/util/types.ts
@@ -1,5 +1,4 @@
import {
isTypeFlagSet,
isTypeReference,
isUnionOrIntersectionType,
unionTypeParts,
Expand Down Expand Up @@ -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;
}
}

/**
Expand All @@ -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;
}
Expand Up @@ -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,
Expand Down Expand Up @@ -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: [
Expand Down