Skip to content

Commit 22f7f25

Browse files
kirkwaiblingerkirkwaiblinger
authored
fix(eslint-plugin): [no-unnecessary-condition] improve error message for literal comparisons (typescript-eslint#10194)
* no-unnecessary-condition: Improve error message for literal comparisons * lintfix * cov * ts-api-utils bug workaround * cov * update todo with ts-api-utils issue * merge cleanup
1 parent e2e9ffc commit 22f7f25

File tree

2 files changed

+449
-168
lines changed

2 files changed

+449
-168
lines changed

‎packages/eslint-plugin/src/rules/no-unnecessary-condition.ts

+103-28
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,11 @@ const valueIsPseudoBigInt = (
3232
return typeof value === 'object';
3333
};
3434

35-
const getValue = (type: ts.LiteralType): bigint | number | string => {
35+
const getValueOfLiteralType = (
36+
type: ts.LiteralType,
37+
): bigint | number | string => {
3638
if (valueIsPseudoBigInt(type.value)) {
37-
return BigInt((type.value.negative ? '-' : '') + type.value.base10Value);
39+
return pseudoBigIntToBigInt(type.value);
3840
}
3941
return type.value;
4042
};
@@ -43,13 +45,12 @@ const isFalsyBigInt = (type: ts.Type): boolean => {
4345
return (
4446
tsutils.isLiteralType(type) &&
4547
valueIsPseudoBigInt(type.value) &&
46-
!getValue(type)
48+
!getValueOfLiteralType(type)
4749
);
4850
};
4951
const isTruthyLiteral = (type: ts.Type): boolean =>
5052
tsutils.isTrueLiteralType(type) ||
51-
// || type.
52-
(type.isLiteral() && !!getValue(type));
53+
(type.isLiteral() && !!getValueOfLiteralType(type));
5354

5455
const isPossiblyFalsy = (type: ts.Type): boolean =>
5556
tsutils
@@ -89,13 +90,83 @@ const isPossiblyNullish = (type: ts.Type): boolean =>
8990
const isAlwaysNullish = (type: ts.Type): boolean =>
9091
tsutils.unionTypeParts(type).every(isNullishType);
9192

92-
// isLiteralType only covers numbers and strings, this is a more exhaustive check.
93-
const isLiteral = (type: ts.Type): boolean =>
94-
tsutils.isBooleanLiteralType(type) ||
95-
type.flags === ts.TypeFlags.Undefined ||
96-
type.flags === ts.TypeFlags.Null ||
97-
type.flags === ts.TypeFlags.Void ||
98-
type.isLiteral();
93+
function toStaticValue(
94+
type: ts.Type,
95+
):
96+
| { value: bigint | boolean | number | string | null | undefined }
97+
| undefined {
98+
// type.isLiteral() only covers numbers/bigints and strings, hence the rest of the branches.
99+
if (tsutils.isBooleanLiteralType(type)) {
100+
// Using `type.intrinsicName` instead of `type.value` because `type.value`
101+
// is `undefined`, contrary to what the type guard tells us.
102+
// See https://github.com/JoshuaKGoldberg/ts-api-utils/issues/528
103+
return { value: type.intrinsicName === 'true' };
104+
}
105+
if (type.flags === ts.TypeFlags.Undefined) {
106+
return { value: undefined };
107+
}
108+
if (type.flags === ts.TypeFlags.Null) {
109+
return { value: null };
110+
}
111+
if (type.isLiteral()) {
112+
return { value: getValueOfLiteralType(type) };
113+
}
114+
115+
return undefined;
116+
}
117+
118+
function pseudoBigIntToBigInt(value: ts.PseudoBigInt): bigint {
119+
return BigInt((value.negative ? '-' : '') + value.base10Value);
120+
}
121+
122+
const BOOL_OPERATORS = new Set([
123+
'<',
124+
'>',
125+
'<=',
126+
'>=',
127+
'==',
128+
'===',
129+
'!=',
130+
'!==',
131+
] as const);
132+
133+
type BoolOperator = typeof BOOL_OPERATORS extends Set<infer T> ? T : never;
134+
135+
function isBoolOperator(operator: string): operator is BoolOperator {
136+
return (BOOL_OPERATORS as Set<string>).has(operator);
137+
}
138+
139+
function booleanComparison(
140+
left: unknown,
141+
operator: BoolOperator,
142+
right: unknown,
143+
): boolean {
144+
switch (operator) {
145+
case '!=':
146+
// eslint-disable-next-line eqeqeq -- intentionally comparing with loose equality
147+
return left != right;
148+
case '!==':
149+
return left !== right;
150+
case '<':
151+
// @ts-expect-error: we don't care if the comparison seems unintentional.
152+
return left < right;
153+
case '<=':
154+
// @ts-expect-error: we don't care if the comparison seems unintentional.
155+
return left <= right;
156+
case '==':
157+
// eslint-disable-next-line eqeqeq -- intentionally comparing with loose equality
158+
return left == right;
159+
case '===':
160+
return left === right;
161+
case '>':
162+
// @ts-expect-error: we don't care if the comparison seems unintentional.
163+
return left > right;
164+
case '>=':
165+
// @ts-expect-error: we don't care if the comparison seems unintentional.
166+
return left >= right;
167+
}
168+
}
169+
99170
// #endregion
100171

101172
export type Options = [
@@ -141,7 +212,7 @@ export default createRule<Options, MessageId>({
141212
alwaysTruthyFunc:
142213
'This callback should return a conditional, but return is always truthy.',
143214
literalBooleanExpression:
144-
'Unnecessary conditional, both sides of the expression are literal values.',
215+
'Unnecessary conditional, comparison is always {{trueOrFalse}}. Both sides of the comparison always have a literal type.',
145216
never: 'Unnecessary conditional, value is `never`.',
146217
neverNullish:
147218
'Unnecessary conditional, expected left-hand side of `??` operator to be possibly null or undefined.',
@@ -397,19 +468,6 @@ export default createRule<Options, MessageId>({
397468
* - https://github.com/microsoft/TypeScript/issues/32627
398469
* - https://github.com/microsoft/TypeScript/issues/37160 (handled)
399470
*/
400-
const BOOL_OPERATORS = new Set([
401-
'<',
402-
'>',
403-
'<=',
404-
'>=',
405-
'==',
406-
'===',
407-
'!=',
408-
'!==',
409-
] as const);
410-
type BoolOperator = Parameters<typeof BOOL_OPERATORS.has>[0];
411-
const isBoolOperator = (operator: string): operator is BoolOperator =>
412-
(BOOL_OPERATORS as Set<string>).has(operator);
413471
function checkIfBoolExpressionIsNecessaryConditional(
414472
node: TSESTree.Node,
415473
left: TSESTree.Node,
@@ -418,10 +476,27 @@ export default createRule<Options, MessageId>({
418476
): void {
419477
const leftType = getConstrainedTypeAtLocation(services, left);
420478
const rightType = getConstrainedTypeAtLocation(services, right);
421-
if (isLiteral(leftType) && isLiteral(rightType)) {
422-
context.report({ node, messageId: 'literalBooleanExpression' });
479+
480+
const leftStaticValue = toStaticValue(leftType);
481+
const rightStaticValue = toStaticValue(rightType);
482+
483+
if (leftStaticValue != null && rightStaticValue != null) {
484+
const conditionIsTrue = booleanComparison(
485+
leftStaticValue.value,
486+
operator,
487+
rightStaticValue.value,
488+
);
489+
490+
context.report({
491+
node,
492+
messageId: 'literalBooleanExpression',
493+
data: {
494+
trueOrFalse: conditionIsTrue ? 'true' : 'false',
495+
},
496+
});
423497
return;
424498
}
499+
425500
// Workaround for https://github.com/microsoft/TypeScript/issues/37160
426501
if (isStrictNullChecks) {
427502
const UNDEFINED = ts.TypeFlags.Undefined;

0 commit comments

Comments
 (0)