Skip to content

Narrowing doesn't recognize string constant as truthy #33878

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.

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

Open
nmain opened this issue Oct 8, 2019 · 5 comments
Open

Narrowing doesn't recognize string constant as truthy #33878

nmain opened this issue Oct 8, 2019 · 5 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@nmain
Copy link

nmain commented Oct 8, 2019

TypeScript Version: 3.7-beta

Search Terms: cfa narrowing string truthy

Code

declare var a: { b: number } | null

!a || a.b > 3; // works
!a && true || a.b > 3 && true; // works
!a && "a was not defined" || a.b > 3 && "a.b was too big"; // error "Object is possibly null"

Expected behavior:

The narrowings all work

Actual behavior:

The third narrowing does not work and produces an error

Playground Link:
Playground

Related Issues:
I suspect this is a duplicate, as most CFA narrowing bugs are. But I searched as best I could and didn't find any other bugs that matched what was going on here. It seems like typescript doesn't understand that a non-empty string-literal is true in this situation?

@poseidonCore
Copy link

For context, this error occurs when strictNullChecks is on, but not when it is off.

However, note that here !a && "a was not defined" || a.b > 3 && "a.b was too big" is comprehended as a string, not a boolean even when strictNullChecks is off.

The problem lies in that !null && "whatever" technically evaluates as a string (because of short-circuit evaluation: true && "whatever" -> "whatever"), and null is a possibility for a ... and so the third evaluation collapses to non-empty string or boolean, which is then comprehended as string.

TS Playground

@poseidonCore
Copy link

Although, strangely, in the case where a is not null, the comprehension is still string, but the evaluation is boolean: eg !{b:1} && "whatever" is false because of short-circuiting.

TS Playground

@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Oct 9, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.8.0 milestone Oct 9, 2019
@RyanCavanaugh
Copy link
Member

Probably a bug in how we treat the short-circuiting operators when they're nested

@poseidonCore
Copy link

As I pointed out, there is an error even without nesting, which then may be feeding back into the nesting logic.

var f = !{b:1}; // :boolean=false (correct)
var m = false && "whatever"; // :boolean=false (correct)
var n = !{b:1} && "whatever"; // :string (wrong)

@ahejlsberg
Copy link
Member

This is related to our unreachable code analysis in the binder. We consider false branch of true and the true branch of false to be unreachable, but we don't extend this to other literals. So, from a control flow analysis perspective, a string literal is simply a string expression that could be truthy or falsy.

It's not particularly hard to change this, but the question is where to stop. We'd of course have to check string, number, and bigint literals, but also unary - applied to a numeric literal (and, for symmetry, unary +). And, I guess, we should recognize that {...} and [...] literals are always truthy. But then there's unary ! applied to a literal, which would break a bunch of tests because we currently rely on !!true not being recognized as definitely truthy.

I'm not opposed to the change, but also don't think it is particularly high yield.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

4 participants