Skip to content

Commit

Permalink
fix(eslint-plugin): [no-unnecessary-condition] false positives with b…
Browse files Browse the repository at this point in the history
…randed types (#7466)

* fix(eslint-plugin): [no-unnecessary-condition] false positives with branded types (#7293)

* Improve test coverage and handle branded types in 'isPossiblyTruthy'
  • Loading branch information
MBuchalik committed Aug 19, 2023
1 parent 8172456 commit b52658f
Show file tree
Hide file tree
Showing 2 changed files with 72 additions and 1 deletion.
12 changes: 11 additions & 1 deletion packages/eslint-plugin/src/rules/no-unnecessary-condition.ts
Expand Up @@ -28,13 +28,23 @@ const isTruthyLiteral = (type: ts.Type): boolean =>
const isPossiblyFalsy = (type: ts.Type): boolean =>
tsutils
.unionTypeParts(type)
// Intersections like `string & {}` can also be possibly falsy,
// requiring us to look into the intersection.
.flatMap(type => tsutils.intersectionTypeParts(type))
// PossiblyFalsy flag includes literal values, so exclude ones that
// are definitely truthy
.filter(t => !isTruthyLiteral(t))
.some(type => isTypeFlagSet(type, ts.TypeFlags.PossiblyFalsy));

const isPossiblyTruthy = (type: ts.Type): boolean =>
tsutils.unionTypeParts(type).some(type => !tsutils.isFalsyType(type));
tsutils
.unionTypeParts(type)
.map(type => tsutils.intersectionTypeParts(type))
.some(intersectionParts =>
// It is possible to define intersections that are always falsy,
// like `"" & { __brand: string }`.
intersectionParts.every(type => !tsutils.isFalsyType(type)),
);

// Nullish utilities
const nullishFlag = ts.TypeFlags.Undefined | ts.TypeFlags.Null;
Expand Down
Expand Up @@ -83,6 +83,34 @@ const result2 = foo() == null;
necessaryConditionTest('null | object'),
necessaryConditionTest('undefined | true'),
necessaryConditionTest('void | true'),
// "branded" type
necessaryConditionTest('string & {}'),
necessaryConditionTest('string & { __brand: string }'),
necessaryConditionTest('number & { __brand: string }'),
necessaryConditionTest('boolean & { __brand: string }'),
necessaryConditionTest('bigint & { __brand: string }'),
necessaryConditionTest('string & {} & { __brand: string }'),
necessaryConditionTest(
'string & { __brandA: string } & { __brandB: string }',
),
necessaryConditionTest('string & { __brand: string } | number'),
necessaryConditionTest('(string | number) & { __brand: string }'),
necessaryConditionTest('string & ({ __brand: string } | number)'),
necessaryConditionTest('("" | "foo") & { __brand: string }'),
necessaryConditionTest(
'(string & { __brandA: string }) | (number & { __brandB: string })',
),
necessaryConditionTest(
'((string & { __brandA: string }) | (number & { __brandB: string }) & ("" | "foo"))',
),
necessaryConditionTest(
'{ __brandA: string} & (({ __brandB: string } & string) | ({ __brandC: string } & number))',
),
necessaryConditionTest(
'(string | number) & ("foo" | 123 | { __brandA: string })',
),

necessaryConditionTest('string & string'),

necessaryConditionTest('any'), // any
necessaryConditionTest('unknown'), // unknown
Expand Down Expand Up @@ -645,6 +673,7 @@ const t1 = b2 && b1 ? 'yes' : 'no';
unnecessaryConditionTest('null', 'alwaysFalsy'),
unnecessaryConditionTest('void', 'alwaysFalsy'),
unnecessaryConditionTest('never', 'never'),
unnecessaryConditionTest('string & number', 'never'),

// More complex logical expressions
{
Expand Down Expand Up @@ -1821,5 +1850,37 @@ foo &&= null;
},
],
},

// "branded" types
unnecessaryConditionTest('"" & {}', 'alwaysFalsy'),
unnecessaryConditionTest('"" & { __brand: string }', 'alwaysFalsy'),
unnecessaryConditionTest(
'("" | false) & { __brand: string }',
'alwaysFalsy',
),
unnecessaryConditionTest(
'((string & { __brandA: string }) | (number & { __brandB: string })) & ""',
'alwaysFalsy',
),
unnecessaryConditionTest(
'("foo" | "bar") & { __brand: string }',
'alwaysTruthy',
),
unnecessaryConditionTest(
'(123 | true) & { __brand: string }',
'alwaysTruthy',
),
unnecessaryConditionTest(
'(string | number) & ("foo" | 123) & { __brand: string }',
'alwaysTruthy',
),
unnecessaryConditionTest(
'((string & { __brandA: string }) | (number & { __brandB: string })) & "foo"',
'alwaysTruthy',
),
unnecessaryConditionTest(
'((string & { __brandA: string }) | (number & { __brandB: string })) & ("foo" | 123)',
'alwaysTruthy',
),
],
});

0 comments on commit b52658f

Please sign in to comment.