Skip to content

Narrow by comparisons to boolean literals #53714

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 19, 2023
Merged

Narrow by comparisons to boolean literals #53714

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
750fd41
Narrow by comparisons to boolean literals
Andarist Apr 9, 2023
2540cc4
few more tests
Andarist Apr 9, 2023
a27a5e6
Move `narrowTypeByEquality` before `narrowTypeByBooleanComparison`
Andarist Apr 10, 2023
586003b
Make `narrowTypeByBooleanComparison` last
Andarist Apr 10, 2023
d7f5a5a
Merge remote-tracking branch 'origin/main' into narrow-by-boolean-com…
Andarist Jun 13, 2023
302aded
Merge remote-tracking branch 'origin/main' into narrow-by-boolean-com…
Andarist Sep 11, 2023
9568e9e
add more issues-based tests
Andarist Sep 11, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ import {
isBlock,
isBlockOrCatchScoped,
IsBlockScopedContainer,
isBooleanLiteral,
isCallExpression,
isClassStaticBlockDeclaration,
isConditionalTypeNode,
Expand Down Expand Up @@ -1279,7 +1280,8 @@ function createBinder(): (file: SourceFile, options: CompilerOptions) => void {
case SyntaxKind.EqualsEqualsEqualsToken:
case SyntaxKind.ExclamationEqualsEqualsToken:
return isNarrowableOperand(expr.left) || isNarrowableOperand(expr.right) ||
isNarrowingTypeofOperands(expr.right, expr.left) || isNarrowingTypeofOperands(expr.left, expr.right);
isNarrowingTypeofOperands(expr.right, expr.left) || isNarrowingTypeofOperands(expr.left, expr.right) ||
(isBooleanLiteral(expr.right) && isNarrowingExpression(expr.left) || isBooleanLiteral(expr.left) && isNarrowingExpression(expr.right));
case SyntaxKind.InstanceOfKeyword:
return isNarrowableOperand(expr.left);
case SyntaxKind.InKeyword:
Expand Down
13 changes: 13 additions & 0 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
BigIntLiteral,
BigIntLiteralType,
BinaryExpression,
BinaryOperator,
BinaryOperatorToken,
binarySearch,
BindableObjectDefinePropertyCall,
Expand All @@ -44,6 +45,7 @@ import {
BindingPattern,
bindSourceFile,
Block,
BooleanLiteral,
BreakOrContinueStatement,
CallChain,
CallExpression,
Expand Down Expand Up @@ -27610,6 +27612,11 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return type;
}

function narrowTypeByBooleanComparison(type: Type, expr: Expression, bool: BooleanLiteral, operator: BinaryOperator, assumeTrue: boolean): Type {
assumeTrue = (assumeTrue !== (bool.kind === SyntaxKind.TrueKeyword)) !== (operator !== SyntaxKind.ExclamationEqualsEqualsToken && operator !== SyntaxKind.ExclamationEqualsToken);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my only comment on this bit at this point is just how unreadable this expression is; I honestly have no idea what I'm looking at here. Is there a more obvious way to represent this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iirc this is a double xor with booleans

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes me feel slightly better, but this is still like 3 nested equalities...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't disagree 😅 at first I started with some nested if statements - and believe me... it didn't look good either. Then I realized that what I writing there was really just a xor so I managed to fold it all into this one line. It didn't look that great to me either. I think it's a very nice way of doing this but the readability suffers here if one doesnt know what this is. I tried to give it some names but I don't have any good ones so I gave up. The best alternative that I can offer would be smth like:

const a = assumeTrue;
const b = bool.kind === SyntaxKind.TrueKeyword;
const c = operator !== SyntaxKind.ExclamationEqualsEqualsToken && operator !== SyntaxKind.ExclamationEqualsToken

// boolean xors
assumeTrue = (a !== b) !== c

return narrowType(type, expr, assumeTrue);
}

function narrowTypeByBinaryExpression(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
switch (expr.operatorToken.kind) {
case SyntaxKind.EqualsToken:
Expand Down Expand Up @@ -27658,6 +27665,12 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
if (isMatchingConstructorReference(right)) {
return narrowTypeByConstructor(type, operator, left, assumeTrue);
}
if (isBooleanLiteral(right)) {
return narrowTypeByBooleanComparison(type, left, right, operator, assumeTrue);
}
if (isBooleanLiteral(left)) {
return narrowTypeByBooleanComparison(type, right, left, operator, assumeTrue);
}
break;
case SyntaxKind.InstanceOfKeyword:
return narrowTypeByInstanceof(type, expr, assumeTrue);
Expand Down
264 changes: 264 additions & 0 deletions tests/baselines/reference/narrowByBooleanComparison.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,264 @@
//// [tests/cases/compiler/narrowByBooleanComparison.ts] ////

=== narrowByBooleanComparison.ts ===
type A = { type: "A" };
>A : Symbol(A, Decl(narrowByBooleanComparison.ts, 0, 0))
>type : Symbol(type, Decl(narrowByBooleanComparison.ts, 0, 10))

type B = { type: "B" };
>B : Symbol(B, Decl(narrowByBooleanComparison.ts, 0, 23))
>type : Symbol(type, Decl(narrowByBooleanComparison.ts, 1, 10))

type C = { type: "C" };
>C : Symbol(C, Decl(narrowByBooleanComparison.ts, 1, 23))
>type : Symbol(type, Decl(narrowByBooleanComparison.ts, 2, 10))

type MyUnion = A | B | C;
>MyUnion : Symbol(MyUnion, Decl(narrowByBooleanComparison.ts, 2, 23))
>A : Symbol(A, Decl(narrowByBooleanComparison.ts, 0, 0))
>B : Symbol(B, Decl(narrowByBooleanComparison.ts, 0, 23))
>C : Symbol(C, Decl(narrowByBooleanComparison.ts, 1, 23))

const isA = (x: MyUnion): x is A => x.type === "A";
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 5, 13))
>MyUnion : Symbol(MyUnion, Decl(narrowByBooleanComparison.ts, 2, 23))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 5, 13))
>A : Symbol(A, Decl(narrowByBooleanComparison.ts, 0, 0))
>x.type : Symbol(type, Decl(narrowByBooleanComparison.ts, 0, 10), Decl(narrowByBooleanComparison.ts, 1, 10), Decl(narrowByBooleanComparison.ts, 2, 10))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 5, 13))
>type : Symbol(type, Decl(narrowByBooleanComparison.ts, 0, 10), Decl(narrowByBooleanComparison.ts, 1, 10), Decl(narrowByBooleanComparison.ts, 2, 10))

function test1(x: MyUnion) {
>test1 : Symbol(test1, Decl(narrowByBooleanComparison.ts, 5, 51))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
>MyUnion : Symbol(MyUnion, Decl(narrowByBooleanComparison.ts, 2, 23))

if (isA(x) !== true) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (isA(x) !== false) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (isA(x) === false) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (isA(x) === true) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (isA(x) != true) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (isA(x) == true) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (true !== isA(x)) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}

if (true === isA(x)) {
>isA : Symbol(isA, Decl(narrowByBooleanComparison.ts, 5, 5))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 7, 15))
}
}

// https://github.com/microsoft/TypeScript/issues/53093
function test2(x: unknown) {
>test2 : Symbol(test2, Decl(narrowByBooleanComparison.ts, 39, 1))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 42, 15))

if (x instanceof Error === false) {
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 42, 15))
>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

return;
}
x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 42, 15))
}

// https://github.com/microsoft/TypeScript/issues/50712
function test3(foo: unknown) {
>test3 : Symbol(test3, Decl(narrowByBooleanComparison.ts, 47, 1))
>foo : Symbol(foo, Decl(narrowByBooleanComparison.ts, 50, 15))

if (typeof foo !== 'string' && Array.isArray(foo) === false) {
>foo : Symbol(foo, Decl(narrowByBooleanComparison.ts, 50, 15))
>Array.isArray : Symbol(ArrayConstructor.isArray, Decl(lib.es5.d.ts, --, --))
>Array : Symbol(Array, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
>isArray : Symbol(ArrayConstructor.isArray, Decl(lib.es5.d.ts, --, --))
>foo : Symbol(foo, Decl(narrowByBooleanComparison.ts, 50, 15))

throw new Error('Not a string or an array');
>Error : Symbol(Error, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))
}
foo;
>foo : Symbol(foo, Decl(narrowByBooleanComparison.ts, 50, 15))
}

// https://github.com/microsoft/TypeScript/issues/55395
class WebError extends URIError {
>WebError : Symbol(WebError, Decl(narrowByBooleanComparison.ts, 55, 1))
>URIError : Symbol(URIError, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

status?: number;
>status : Symbol(WebError.status, Decl(narrowByBooleanComparison.ts, 58, 33))
}
function test4() {
>test4 : Symbol(test4, Decl(narrowByBooleanComparison.ts, 60, 1))

try {
// make a request
} catch (err) {
>err : Symbol(err, Decl(narrowByBooleanComparison.ts, 64, 13))

if (err instanceof WebError === false || err.status != 401) {
>err : Symbol(err, Decl(narrowByBooleanComparison.ts, 64, 13))
>WebError : Symbol(WebError, Decl(narrowByBooleanComparison.ts, 55, 1))
>err.status : Symbol(WebError.status, Decl(narrowByBooleanComparison.ts, 58, 33))
>err : Symbol(err, Decl(narrowByBooleanComparison.ts, 64, 13))
>status : Symbol(WebError.status, Decl(narrowByBooleanComparison.ts, 58, 33))

console.error(err);
>console.error : Symbol(Console.error, Decl(lib.dom.d.ts, --, --))
>console : Symbol(console, Decl(lib.dom.d.ts, --, --))
>error : Symbol(Console.error, Decl(lib.dom.d.ts, --, --))
>err : Symbol(err, Decl(narrowByBooleanComparison.ts, 64, 13))
}
}
}

// https://github.com/microsoft/TypeScript/issues/44366
interface Entity {
>Entity : Symbol(Entity, Decl(narrowByBooleanComparison.ts, 69, 1))

type: string;
>type : Symbol(Entity.type, Decl(narrowByBooleanComparison.ts, 72, 18))
}
const ACTOR_TYPE = "actor";
>ACTOR_TYPE : Symbol(ACTOR_TYPE, Decl(narrowByBooleanComparison.ts, 75, 5))

interface Actor extends Entity {
>Actor : Symbol(Actor, Decl(narrowByBooleanComparison.ts, 75, 27))
>Entity : Symbol(Entity, Decl(narrowByBooleanComparison.ts, 69, 1))

type: typeof ACTOR_TYPE;
>type : Symbol(Actor.type, Decl(narrowByBooleanComparison.ts, 76, 32))
>ACTOR_TYPE : Symbol(ACTOR_TYPE, Decl(narrowByBooleanComparison.ts, 75, 5))
}
function isActor(entity: Entity): entity is Actor {
>isActor : Symbol(isActor, Decl(narrowByBooleanComparison.ts, 78, 1))
>entity : Symbol(entity, Decl(narrowByBooleanComparison.ts, 79, 17))
>Entity : Symbol(Entity, Decl(narrowByBooleanComparison.ts, 69, 1))
>entity : Symbol(entity, Decl(narrowByBooleanComparison.ts, 79, 17))
>Actor : Symbol(Actor, Decl(narrowByBooleanComparison.ts, 75, 27))

return entity.type === ACTOR_TYPE;
>entity.type : Symbol(Entity.type, Decl(narrowByBooleanComparison.ts, 72, 18))
>entity : Symbol(entity, Decl(narrowByBooleanComparison.ts, 79, 17))
>type : Symbol(Entity.type, Decl(narrowByBooleanComparison.ts, 72, 18))
>ACTOR_TYPE : Symbol(ACTOR_TYPE, Decl(narrowByBooleanComparison.ts, 75, 5))
}
function test5(bin: Entity) {
>test5 : Symbol(test5, Decl(narrowByBooleanComparison.ts, 81, 1))
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 82, 15))
>Entity : Symbol(Entity, Decl(narrowByBooleanComparison.ts, 69, 1))

if (isActor(bin) === false) {
>isActor : Symbol(isActor, Decl(narrowByBooleanComparison.ts, 78, 1))
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 82, 15))

bin;
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 82, 15))

} else {
bin;
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 82, 15))
}
}
function test6(bin: Entity) {
>test6 : Symbol(test6, Decl(narrowByBooleanComparison.ts, 88, 1))
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 89, 15))
>Entity : Symbol(Entity, Decl(narrowByBooleanComparison.ts, 69, 1))

if (isActor(bin) == false) {
>isActor : Symbol(isActor, Decl(narrowByBooleanComparison.ts, 78, 1))
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 89, 15))

bin;
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 89, 15))

} else {
bin;
>bin : Symbol(bin, Decl(narrowByBooleanComparison.ts, 89, 15))
}
}

// https://github.com/microsoft/TypeScript/issues/53005
function isFunction(x: unknown): x is Function {
>isFunction : Symbol(isFunction, Decl(narrowByBooleanComparison.ts, 95, 1))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 98, 20))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 98, 20))
>Function : Symbol(Function, Decl(lib.es5.d.ts, --, --), Decl(lib.es5.d.ts, --, --))

return typeof x === "function";
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 98, 20))
}

function test7(x: unknown) {
>test7 : Symbol(test7, Decl(narrowByBooleanComparison.ts, 100, 1))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 102, 15))

if (isFunction(x) !== false) {
>isFunction : Symbol(isFunction, Decl(narrowByBooleanComparison.ts, 95, 1))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 102, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 102, 15))
}
if (isFunction(x) === true) {
>isFunction : Symbol(isFunction, Decl(narrowByBooleanComparison.ts, 95, 1))
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 102, 15))

x;
>x : Symbol(x, Decl(narrowByBooleanComparison.ts, 102, 15))
}
}

Loading