Skip to content

Commit 2429e26

Browse files
GlassBricksGlassBricks
authored
Fix nullish coalescing and conditionals when using generics (#1191)
* Fix nullish coalescing and conditionals when using generics * Clarify strictNullChecks option in test case
1 parent 976ed2f commit 2429e26

5 files changed

Lines changed: 69 additions & 38 deletions

File tree

‎src/transformation/utils/typescript/types.ts‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,7 @@ import * as ts from "typescript";
22
import { TransformationContext } from "../../context";
33

44
export function isTypeWithFlags(context: TransformationContext, type: ts.Type, flags: ts.TypeFlags): boolean {
5-
const predicate = (type: ts.Type) => {
6-
if (type.symbol) {
7-
const baseConstraint = context.checker.getBaseConstraintOfType(type);
8-
if (baseConstraint && baseConstraint !== type) {
9-
return isTypeWithFlags(context, baseConstraint, flags);
10-
}
11-
}
12-
return (type.flags & flags) !== 0;
13-
};
5+
const predicate = (type: ts.Type) => (type.flags & flags) !== 0;
146

157
return typeAlwaysSatisfies(context, type, predicate);
168
}
@@ -20,6 +12,11 @@ export function typeAlwaysSatisfies(
2012
type: ts.Type,
2113
predicate: (type: ts.Type) => boolean
2214
): boolean {
15+
const baseConstraint = context.checker.getBaseConstraintOfType(type);
16+
if (baseConstraint) {
17+
type = baseConstraint;
18+
}
19+
2320
if (predicate(type)) {
2421
return true;
2522
}
@@ -40,6 +37,14 @@ export function typeCanSatisfy(
4037
type: ts.Type,
4138
predicate: (type: ts.Type) => boolean
4239
): boolean {
40+
const baseConstraint = context.checker.getBaseConstraintOfType(type);
41+
if (!baseConstraint) {
42+
// type parameter with no constraint can be anything, assume it might satisfy predicate
43+
if (type.isTypeParameter()) return true;
44+
} else {
45+
type = baseConstraint;
46+
}
47+
4348
if (predicate(type)) {
4449
return true;
4550
}
@@ -110,3 +115,32 @@ export function isArrayType(context: TransformationContext, type: ts.Type): bool
110115
export function isFunctionType(type: ts.Type): boolean {
111116
return type.getCallSignatures().length > 0;
112117
}
118+
119+
export function canBeFalsy(context: TransformationContext, type: ts.Type): boolean {
120+
const strictNullChecks = context.options.strict === true || context.options.strictNullChecks === true;
121+
const falsyFlags =
122+
ts.TypeFlags.Boolean |
123+
ts.TypeFlags.BooleanLiteral |
124+
ts.TypeFlags.Never |
125+
ts.TypeFlags.Void |
126+
ts.TypeFlags.Unknown |
127+
ts.TypeFlags.Any |
128+
ts.TypeFlags.Undefined |
129+
ts.TypeFlags.Null;
130+
return typeCanSatisfy(
131+
context,
132+
type,
133+
type => (type.flags & falsyFlags) !== 0 || (!strictNullChecks && !type.isLiteral())
134+
);
135+
}
136+
137+
export function canBeFalsyWhenNotNull(context: TransformationContext, type: ts.Type): boolean {
138+
const falsyFlags =
139+
ts.TypeFlags.Boolean |
140+
ts.TypeFlags.BooleanLiteral |
141+
ts.TypeFlags.Never |
142+
ts.TypeFlags.Void |
143+
ts.TypeFlags.Unknown |
144+
ts.TypeFlags.Any;
145+
return typeCanSatisfy(context, type, type => (type.flags & falsyFlags) !== 0);
146+
}

‎src/transformation/visitors/binary-expression/index.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import * as lua from "../../../LuaAST";
33
import { FunctionVisitor, TransformationContext } from "../../context";
44
import { wrapInToStringForConcat } from "../../utils/lua-ast";
55
import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib";
6-
import { isStandardLibraryType, isStringType, typeCanSatisfy } from "../../utils/typescript";
6+
import { canBeFalsyWhenNotNull, isStandardLibraryType, isStringType } from "../../utils/typescript";
77
import { transformTypeOfBinaryExpression } from "../typeof";
88
import { transformAssignmentExpression, transformAssignmentStatement } from "./assignments";
99
import { BitOperator, isBitOperator, transformBinaryBitOperation } from "./bit";
@@ -269,10 +269,7 @@ function transformNullishCoalescingOperationNoPrecedingStatements(
269269
const lhsType = context.checker.getTypeAtLocation(node.left);
270270

271271
// Check if we can take a shortcut to 'lhs or rhs' if the left-hand side cannot be 'false'.
272-
const typeCanBeFalse = (type: ts.Type) =>
273-
(type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown | ts.TypeFlags.Boolean)) !== 0 ||
274-
(type.flags & ts.TypeFlags.BooleanLiteral & ts.TypeFlags.PossiblyFalsy) !== 0;
275-
if (typeCanSatisfy(context, lhsType, typeCanBeFalse)) {
272+
if (canBeFalsyWhenNotNull(context, lhsType)) {
276273
// reuse logic from case with preceding statements
277274
const [precedingStatements, result] = createShortCircuitBinaryExpressionPrecedingStatements(
278275
context,

‎src/transformation/visitors/conditional.ts‎

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,7 @@ import { FunctionVisitor, TransformationContext } from "../context";
44
import { transformInPrecedingStatementScope } from "../utils/preceding-statements";
55
import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope";
66
import { transformBlockOrStatement } from "./block";
7-
8-
function canBeFalsy(context: TransformationContext, type: ts.Type): boolean {
9-
const strictNullChecks = context.options.strict === true || context.options.strictNullChecks === true;
10-
11-
const falsyFlags =
12-
ts.TypeFlags.Boolean |
13-
ts.TypeFlags.BooleanLiteral |
14-
ts.TypeFlags.Undefined |
15-
ts.TypeFlags.Null |
16-
ts.TypeFlags.Never |
17-
ts.TypeFlags.Void |
18-
ts.TypeFlags.Any;
19-
20-
if (type.flags & falsyFlags) {
21-
return true;
22-
} else if (!strictNullChecks && !type.isLiteral()) {
23-
return true;
24-
} else if (type.isUnion()) {
25-
return type.types.some(subType => canBeFalsy(context, subType));
26-
} else {
27-
return false;
28-
}
29-
}
7+
import { canBeFalsy } from "../utils/typescript";
308

319
function transformProtectedConditionalExpression(
3210
context: TransformationContext,

‎test/unit/conditionals.spec.ts‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,3 +99,16 @@ test.each(["true", "false", "a < 4", "a == 8"])("Ternary Conditional Delayed (%p
9999
return delay();
100100
`.expectToMatchJsResult();
101101
});
102+
103+
test.each([false, true, null])("Ternary conditional with generic whenTrue branch (%p)", trueVal => {
104+
util.testFunction`
105+
function ternary<B, C>(a: boolean, b: B, c: C) {
106+
return a ? b : c
107+
}
108+
return ternary(true, ${trueVal}, "wasFalse")
109+
`
110+
.setOptions({
111+
strictNullChecks: true,
112+
})
113+
.expectToMatchJsResult();
114+
});

‎test/unit/nullishCoalescing.spec.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ test("nullish-coalescing operator with side effect rhs", () => {
4343

4444
test("nullish-coalescing operator with vararg", () => {
4545
util.testFunction`
46-
46+
4747
function foo(...args: any[]){
4848
return args
4949
}
@@ -54,3 +54,12 @@ test("nullish-coalescing operator with vararg", () => {
5454
return bar(1, 2)
5555
`.expectToMatchJsResult();
5656
});
57+
58+
test.each([true, false, null])("nullish-coalescing with generic lhs (%p)", lhs => {
59+
util.testFunction`
60+
function coalesce<A, B>(a: A, b: B) {
61+
return a ?? b
62+
}
63+
return coalesce(${lhs}, "wasNull")
64+
`.expectToMatchJsResult();
65+
});

0 commit comments

Comments
 (0)