Skip to content

Commit

Permalink
feat(eslint-plugin): [no-unnec-cond] support nullish coalescing (#1148)
Browse files Browse the repository at this point in the history

Co-authored-by: Brad Zacher <brad.zacher@gmail.com>
  • Loading branch information
Retsam and bradzacher committed Dec 1, 2019
1 parent e350a21 commit 96ef1e7
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 7 deletions.
56 changes: 50 additions & 6 deletions packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
Expand Up @@ -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) ||
Expand All @@ -51,6 +62,8 @@ export type Options = [
export type MessageId =
| 'alwaysTruthy'
| 'alwaysFalsy'
| 'neverNullish'
| 'alwaysNullish'
| 'literalBooleanExpression'
| 'never';
export default createRule<Options, MessageId>({
Expand Down Expand Up @@ -81,6 +94,10 @@ export default createRule<Options, MessageId>({
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`',
Expand Down Expand Up @@ -120,12 +137,35 @@ export default createRule<Options, MessageId>({
) {
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 });
}
}

Expand Down Expand Up @@ -178,6 +218,10 @@ export default createRule<Options, MessageId>({
function checkLogicalExpressionForUnnecessaryConditionals(
node: TSESTree.LogicalExpression,
): void {
if (node.operator === '??') {
checkNodeForNullish(node.left);
return;
}
checkNode(node.left);
if (!ignoreRhs) {
checkNode(node.right);
Expand Down
Expand Up @@ -88,7 +88,23 @@ function test<T>(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: `
Expand Down Expand Up @@ -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
{
Expand Down

0 comments on commit 96ef1e7

Please sign in to comment.