From 5d4a24c44c7b7ed5b416656e0e0956cc7df20278 Mon Sep 17 00:00:00 2001 From: GlassBricks <24237065+GlassBricks@users.noreply.github.com> Date: Sun, 19 Dec 2021 10:33:15 -0800 Subject: [PATCH 1/2] Fix nullish coalescing and conditionals when using generics --- src/transformation/utils/typescript/types.ts | 52 +++++++++++++++---- .../visitors/binary-expression/index.ts | 7 +-- src/transformation/visitors/conditional.ts | 24 +-------- test/unit/conditionals.spec.ts | 13 +++++ test/unit/nullishCoalescing.spec.ts | 11 +++- 5 files changed, 69 insertions(+), 38 deletions(-) diff --git a/src/transformation/utils/typescript/types.ts b/src/transformation/utils/typescript/types.ts index 74e41db39..cb1af0824 100644 --- a/src/transformation/utils/typescript/types.ts +++ b/src/transformation/utils/typescript/types.ts @@ -2,15 +2,7 @@ import * as ts from "typescript"; import { TransformationContext } from "../../context"; export function isTypeWithFlags(context: TransformationContext, type: ts.Type, flags: ts.TypeFlags): boolean { - const predicate = (type: ts.Type) => { - if (type.symbol) { - const baseConstraint = context.checker.getBaseConstraintOfType(type); - if (baseConstraint && baseConstraint !== type) { - return isTypeWithFlags(context, baseConstraint, flags); - } - } - return (type.flags & flags) !== 0; - }; + const predicate = (type: ts.Type) => (type.flags & flags) !== 0; return typeAlwaysSatisfies(context, type, predicate); } @@ -20,6 +12,11 @@ export function typeAlwaysSatisfies( type: ts.Type, predicate: (type: ts.Type) => boolean ): boolean { + const baseConstraint = context.checker.getBaseConstraintOfType(type); + if (baseConstraint) { + type = baseConstraint; + } + if (predicate(type)) { return true; } @@ -40,6 +37,14 @@ export function typeCanSatisfy( type: ts.Type, predicate: (type: ts.Type) => boolean ): boolean { + const baseConstraint = context.checker.getBaseConstraintOfType(type); + if (!baseConstraint) { + // type parameter with no constraint can be anything, assume it might satisfy predicate + if (type.isTypeParameter()) return true; + } else { + type = baseConstraint; + } + if (predicate(type)) { return true; } @@ -110,3 +115,32 @@ export function isArrayType(context: TransformationContext, type: ts.Type): bool export function isFunctionType(type: ts.Type): boolean { return type.getCallSignatures().length > 0; } + +export function canBeFalsy(context: TransformationContext, type: ts.Type): boolean { + const strictNullChecks = context.options.strict === true || context.options.strictNullChecks === true; + const falsyFlags = + ts.TypeFlags.Boolean | + ts.TypeFlags.BooleanLiteral | + ts.TypeFlags.Never | + ts.TypeFlags.Void | + ts.TypeFlags.Unknown | + ts.TypeFlags.Any | + ts.TypeFlags.Undefined | + ts.TypeFlags.Null; + return typeCanSatisfy( + context, + type, + type => (type.flags & falsyFlags) !== 0 || (!strictNullChecks && !type.isLiteral()) + ); +} + +export function canBeFalsyWhenNotNull(context: TransformationContext, type: ts.Type): boolean { + const falsyFlags = + ts.TypeFlags.Boolean | + ts.TypeFlags.BooleanLiteral | + ts.TypeFlags.Never | + ts.TypeFlags.Void | + ts.TypeFlags.Unknown | + ts.TypeFlags.Any; + return typeCanSatisfy(context, type, type => (type.flags & falsyFlags) !== 0); +} diff --git a/src/transformation/visitors/binary-expression/index.ts b/src/transformation/visitors/binary-expression/index.ts index 330f51e7d..ba292d126 100644 --- a/src/transformation/visitors/binary-expression/index.ts +++ b/src/transformation/visitors/binary-expression/index.ts @@ -3,7 +3,7 @@ import * as lua from "../../../LuaAST"; import { FunctionVisitor, TransformationContext } from "../../context"; import { wrapInToStringForConcat } from "../../utils/lua-ast"; import { LuaLibFeature, transformLuaLibFunction } from "../../utils/lualib"; -import { isStandardLibraryType, isStringType, typeCanSatisfy } from "../../utils/typescript"; +import { canBeFalsyWhenNotNull, isStandardLibraryType, isStringType } from "../../utils/typescript"; import { transformTypeOfBinaryExpression } from "../typeof"; import { transformAssignmentExpression, transformAssignmentStatement } from "./assignments"; import { BitOperator, isBitOperator, transformBinaryBitOperation } from "./bit"; @@ -269,10 +269,7 @@ function transformNullishCoalescingOperationNoPrecedingStatements( const lhsType = context.checker.getTypeAtLocation(node.left); // Check if we can take a shortcut to 'lhs or rhs' if the left-hand side cannot be 'false'. - const typeCanBeFalse = (type: ts.Type) => - (type.flags & (ts.TypeFlags.Any | ts.TypeFlags.Unknown | ts.TypeFlags.Boolean)) !== 0 || - (type.flags & ts.TypeFlags.BooleanLiteral & ts.TypeFlags.PossiblyFalsy) !== 0; - if (typeCanSatisfy(context, lhsType, typeCanBeFalse)) { + if (canBeFalsyWhenNotNull(context, lhsType)) { // reuse logic from case with preceding statements const [precedingStatements, result] = createShortCircuitBinaryExpressionPrecedingStatements( context, diff --git a/src/transformation/visitors/conditional.ts b/src/transformation/visitors/conditional.ts index f83837a2d..518f658e5 100644 --- a/src/transformation/visitors/conditional.ts +++ b/src/transformation/visitors/conditional.ts @@ -4,29 +4,7 @@ import { FunctionVisitor, TransformationContext } from "../context"; import { transformInPrecedingStatementScope } from "../utils/preceding-statements"; import { performHoisting, popScope, pushScope, ScopeType } from "../utils/scope"; import { transformBlockOrStatement } from "./block"; - -function canBeFalsy(context: TransformationContext, type: ts.Type): boolean { - const strictNullChecks = context.options.strict === true || context.options.strictNullChecks === true; - - const falsyFlags = - ts.TypeFlags.Boolean | - ts.TypeFlags.BooleanLiteral | - ts.TypeFlags.Undefined | - ts.TypeFlags.Null | - ts.TypeFlags.Never | - ts.TypeFlags.Void | - ts.TypeFlags.Any; - - if (type.flags & falsyFlags) { - return true; - } else if (!strictNullChecks && !type.isLiteral()) { - return true; - } else if (type.isUnion()) { - return type.types.some(subType => canBeFalsy(context, subType)); - } else { - return false; - } -} +import { canBeFalsy } from "../utils/typescript"; function transformProtectedConditionalExpression( context: TransformationContext, diff --git a/test/unit/conditionals.spec.ts b/test/unit/conditionals.spec.ts index c9717d0c0..ad5d7cf05 100644 --- a/test/unit/conditionals.spec.ts +++ b/test/unit/conditionals.spec.ts @@ -99,3 +99,16 @@ test.each(["true", "false", "a < 4", "a == 8"])("Ternary Conditional Delayed (%p return delay(); `.expectToMatchJsResult(); }); + +test.each([false, true, null])("Ternary conditional with generic whenTrue branch (%p)", trueVal => { + util.testFunction` + function ternary(a: boolean, b: B, c: C) { + return a ? b : c + } + return ternary(true, ${trueVal}, "wasFalse") + ` + .setOptions({ + strict: true, + }) + .expectToMatchJsResult(); +}); diff --git a/test/unit/nullishCoalescing.spec.ts b/test/unit/nullishCoalescing.spec.ts index 309e43732..3728f2815 100644 --- a/test/unit/nullishCoalescing.spec.ts +++ b/test/unit/nullishCoalescing.spec.ts @@ -43,7 +43,7 @@ test("nullish-coalescing operator with side effect rhs", () => { test("nullish-coalescing operator with vararg", () => { util.testFunction` - + function foo(...args: any[]){ return args } @@ -54,3 +54,12 @@ test("nullish-coalescing operator with vararg", () => { return bar(1, 2) `.expectToMatchJsResult(); }); + +test.each([true, false, null])("nullish-coalescing with generic lhs (%p)", lhs => { + util.testFunction` + function coalesce(a: A, b: B) { + return a ?? b + } + return coalesce(${lhs}, "wasNull") + `.expectToMatchJsResult(); +}); From 063d5e551585048a8a98d30f61bee63a20406394 Mon Sep 17 00:00:00 2001 From: GlassBricks <24237065+GlassBricks@users.noreply.github.com> Date: Mon, 20 Dec 2021 15:33:39 -0800 Subject: [PATCH 2/2] Clarify strictNullChecks option in test case --- test/unit/conditionals.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/conditionals.spec.ts b/test/unit/conditionals.spec.ts index ad5d7cf05..738ecacc1 100644 --- a/test/unit/conditionals.spec.ts +++ b/test/unit/conditionals.spec.ts @@ -108,7 +108,7 @@ test.each([false, true, null])("Ternary conditional with generic whenTrue branch return ternary(true, ${trueVal}, "wasFalse") ` .setOptions({ - strict: true, + strictNullChecks: true, }) .expectToMatchJsResult(); });