From 96ef1e7afd429b98c63b3ed3b83cc5ff5be0e194 Mon Sep 17 00:00:00 2001 From: Retsam Date: Sun, 1 Dec 2019 17:38:18 -0500 Subject: [PATCH] feat(eslint-plugin): [no-unnec-cond] support nullish coalescing (#1148) Co-authored-by: Brad Zacher --- .../src/rules/no-unnecessary-condition.ts | 56 +++++++++++++++++-- .../rules/no-unnecessary-condition.test.ts | 47 +++++++++++++++- 2 files changed, 96 insertions(+), 7 deletions(-) diff --git a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts index 420271a23b8..1ee2c262980 100644 --- a/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts +++ b/packages/eslint-plugin/src/rules/no-unnecessary-condition.ts @@ -31,6 +31,17 @@ const isPossiblyFalsy = (type: ts.Type): boolean => const isPossiblyTruthy = (type: ts.Type): boolean => unionTypeParts(type).some(type => !isFalsyType(type)); +// Nullish utilities +const nullishFlag = ts.TypeFlags.Undefined | ts.TypeFlags.Null; +const isNullishType = (type: ts.Type): boolean => + isTypeFlagSet(type, nullishFlag); + +const isPossiblyNullish = (type: ts.Type): boolean => + unionTypeParts(type).some(isNullishType); + +const isAlwaysNullish = (type: ts.Type): boolean => + unionTypeParts(type).every(isNullishType); + // isLiteralType only covers numbers and strings, this is a more exhaustive check. const isLiteral = (type: ts.Type): boolean => isBooleanLiteralType(type, true) || @@ -51,6 +62,8 @@ export type Options = [ export type MessageId = | 'alwaysTruthy' | 'alwaysFalsy' + | 'neverNullish' + | 'alwaysNullish' | 'literalBooleanExpression' | 'never'; export default createRule({ @@ -81,6 +94,10 @@ export default createRule({ messages: { alwaysTruthy: 'Unnecessary conditional, value is always truthy.', alwaysFalsy: 'Unnecessary conditional, value is always falsy.', + neverNullish: + 'Unnecessary conditional, expected left-hand side of `??` operator to be possibly null or undefined.', + alwaysNullish: + 'Unnecessary conditional, left-hand side of `??` operator is always `null` or `undefined`', literalBooleanExpression: 'Unnecessary conditional, both sides of the expression are literal values', never: 'Unnecessary conditional, value is `never`', @@ -120,12 +137,35 @@ export default createRule({ ) { return; } - if (isTypeFlagSet(type, TypeFlags.Never)) { - context.report({ node, messageId: 'never' }); - } else if (!isPossiblyTruthy(type)) { - context.report({ node, messageId: 'alwaysFalsy' }); - } else if (!isPossiblyFalsy(type)) { - context.report({ node, messageId: 'alwaysTruthy' }); + const messageId = isTypeFlagSet(type, TypeFlags.Never) + ? 'never' + : !isPossiblyTruthy(type) + ? 'alwaysFalsy' + : !isPossiblyFalsy(type) + ? 'alwaysTruthy' + : undefined; + + if (messageId) { + context.report({ node, messageId }); + } + } + + function checkNodeForNullish(node: TSESTree.Node): void { + const type = getNodeType(node); + // Conditional is always necessary if it involves `any` or `unknown` + if (isTypeFlagSet(type, TypeFlags.Any | TypeFlags.Unknown)) { + return; + } + const messageId = isTypeFlagSet(type, TypeFlags.Never) + ? 'never' + : !isPossiblyNullish(type) + ? 'neverNullish' + : isAlwaysNullish(type) + ? 'alwaysNullish' + : undefined; + + if (messageId) { + context.report({ node, messageId }); } } @@ -178,6 +218,10 @@ export default createRule({ function checkLogicalExpressionForUnnecessaryConditionals( node: TSESTree.LogicalExpression, ): void { + if (node.operator === '??') { + checkNodeForNullish(node.left); + return; + } checkNode(node.left); if (!ignoreRhs) { checkNode(node.right); diff --git a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts index 30e511ac119..0fe1326811c 100644 --- a/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts +++ b/packages/eslint-plugin/tests/rules/no-unnecessary-condition.test.ts @@ -88,7 +88,23 @@ function test(t: T | []) { function test(a: string) { return a === "a" }`, - + // Nullish coalescing operator + ` +function test(a: string | null) { + return a ?? "default"; +}`, + ` +function test(a: string | undefined) { + return a ?? "default"; +}`, + ` +function test(a: string | null | undefined) { + return a ?? "default"; +}`, + ` +function test(a: unknown) { + return a ?? "default"; +}`, // Supports ignoring the RHS { code: ` @@ -187,6 +203,35 @@ if (x === Foo.a) {} `, errors: [ruleError(8, 5, 'literalBooleanExpression')], }, + // Nullish coalescing operator + { + code: ` +function test(a: string) { + return a ?? 'default'; +}`, + errors: [ruleError(3, 10, 'neverNullish')], + }, + { + code: ` +function test(a: string | false) { + return a ?? 'default'; +}`, + errors: [ruleError(3, 10, 'neverNullish')], + }, + { + code: ` +function test(a: null) { + return a ?? 'default'; +}`, + errors: [ruleError(3, 10, 'alwaysNullish')], + }, + { + code: ` +function test(a: never) { + return a ?? 'default'; +}`, + errors: [ruleError(3, 10, 'never')], + }, // Still errors on in the expected locations when ignoring RHS {