Use control flow analysis for type guards

[jednano]

TS 1.4

Type guard fails to narrow the union type when returning early.

function foo(x: number | string) {
    if (typeof x === 'string') {
        return;
    }
    // x should now be a number
    if (2 % x !== 0) {
        // do something
    }
}

Unfortunately, I get the following compiler error:

The right-hand side of an arithmetic operation must be of type 'any', 'number' or an enum type.

Clearly, the compiler should know at this point that it can't possibly be dealing with a string, as that scenario has already been returned.

[RyanCavanaugh]

This would be nice. We should probably have a single issue for "Use control flow analysis for type guards" to unify all the various permutations here.

[vilicvane]

+1 for this feature.

[jednano]

@RyanCavanaugh I've updated the issue title, as you suggested. Happy to discuss it here.

[zspitz]

+1
Suggestion: an if block containing a throw statement should be treated the same way:

function foo(x: number | string) {
    if (typeof x === 'string') { throw 'No strings attached';}
    return x * 2;
}

[blakeembrey]

👍 I prefer writing code without using else so this would be pretty amazing

[zpdDG4gta8XKpMCd]

👍

[ghost]

👍

[falsandtru]

+1 It is a basic coding style.

[mjohnsonengr]

👍 this would be really awesome to have. +1 as well for the same with a throw.

[alvivi]

Right now I am using type coercion to avoid falling into a pyramid of doom, so +1.

[ivogabe]

I've opened a PR at #6959.

[TheTedAdams]

Just ran in to this issue today and had to wrap my code in an else (dirty). Would love to see this implemented!

[paul-go]

Is it within scope to make this feature also operate on type intersections?

class A { a: string; }
class B { b: string; }

function fn(param: A & B)
{
    if (param instanceof A)
        return;

    param.a; // Generate a type error here?
}

[RyanCavanaugh]

@paul-go what's the intent of the code here? I would expect param: A | B given the instanceof check. It seems impossible for a A & B to actually exist in a codebase where there was checking with instanceof

[ghost]

@RyanCavanaugh You can get an A & B when both A and B are classes.

class AB extends A implements B {
  b = 'b';
}

Of course instanceof gets unreliable when you implement classes rather than extend them. This seems like a bad idea in general, but possible.

The same problem exists with interfaces, but then you can't use instanceof.

This also raises that instanceof can tell you that something is a kind of thing, but not that it is not that kind of thing. I'm not sure how this relates to type guards, or if it is a scenario that should be supported.

[jods4]

@paul-go The code is not a type error, since param: A & B means that param.a has to (always) be valid or your typing is flaky.

Maybe it's dead code if you think you can never fail the param instanceof A test, but that's another issue.

[paul-go]

@jods4 By "Generate a type error here?", I actually meant "Can we generate a type error here?"

[paul-go]

@RyanCavanaugh: This is going to be controversial, but I think you may be making an assumption that all usages of & use Object the underlying constructor. This isn't the case. As a library designer, we've generally favored using & over |, because | requires an assertion before the members can be discovered. While on many occasions | technically more correct, we've found this to create tediousness. Most notably, in cases where a function is used widely throughout a code base, but the developer rarely wants to handle the other side of the |.

And not that we should be striving to appease the completeness gods, it also seems like a pretty gapping completeness hole :-)

[RyanCavanaugh]

FWIW I don't have strong beliefs about constructors.

In the example you have there, though, I don't understand why we would want to generate an error there. If something is A & B then it doesn't matter what it's instanceof; that type concretely always has an a member.

[paul-go]

We use & as an overloading mechanism that the user doesn't need to assert before gaining access to members. If you think of it like that, then it makes sense that when the user guards with an instanceof, the other side should be culled. But I guess the intents of | and & don't exactly fit this use case.

[Arnavion]

@paul-go The point is that this is valid in TS:

class Foo { bar: string }
var foo: Foo = { bar: "a" };
console.log(foo instanceof Foo); // false even though foo is a Foo according to the type system

Thus in your example even when a is not instanceof A, it can still be an A for the purpose of type-checking. Only the inverse is true - if a is instanceof A then it must be an A for the purpose of type-checking.

[RyanCavanaugh]

If you changed the A & B to A | B, then (once the PR goes in) inside the if you'd see only the members of A, and after the if block you'd see only the members of B. That seems 100% optimal for your use case

[Arnavion]

@RyanCavanaugh

after the if block you'd see only the members of B

Just B ? Not A | B ?

[paul-go]

Our use case is to show the union of both member lists in the unguarded case, and a single member list in the guarded case. As it stands, the intersection of both member lists are shown in the unguarded case:

[ghost]

I think @paul-go is using type intersections to model what are more accurately type unions, because he feels that type unions are too cumbersome to use properly. I don't think "modelling it incorrectly because the correct way is inconvenient" is a good use-case to support.

[jods4]

@paul-go from your description I think that you don't use intersection types the way they are intended... And I have the impression that your request is twisting & to match your perception/usage of the feature, which doesn't seem correct.

let's take your last example:

// or is it A & B ??
function fn(param: A | B) {
}

You have to decide if param always has members of both A and B (which is the meaning of A & B) or if it can be either a A or a B (which is the meaning of A | B).

Accessing members of both in an unguarded way would be a bug if the object is sometimes a A, sometimes a B. It is only correct if param always satisfies both contracts. And if it does, type guards are meaningless, because the object satisfies all contracts all the time!

Accessing members behind a guard kind of implies that param is not always both types. This would be A | B and TS guards are well adapted for consuming this object in a safe, simple way without casts.

If you actually have A & B and can access all members in an unguarded way but want to reduce IntelliSense for some weird reason, you can always use a cast:

let param: A & B;
param.a = 1; // ok
param.b = 2; // ok
let restricted = <A>param;
restricted.a = 3; // ok
restricted.b = 4; // type error, no b on A. (actually ok at runtime, though)

[paul-go]

Yes, I'm aware of how type guards work. But all of this is pointless banter as TS isn't going to support our unfortunately bizarre use case.

[RyanCavanaugh]

@paul-go Reading between the lines here, it seems like you'd like possibly-present members of union types to show up in the completion list. We already have this functionality for JavaScript code and could think about moving it to TypeScript if that'd be useful. One caveat would be that obviously these maybe-present members wouldn't actually work without a type guard of some sort, but it seems like you're writing that code anyway (just later). As in JS, we could show those with a sort of 'warning' formatting connotation. Thoughts?

[ghost]

@RyanCavanaugh Is what you're proposing similar to generating an implicit interface that is a supertype of all types in the union, with the non-common properties marked optional?

[paul-go]

@RyanCavanaugh By "wouldn't actually work", I'm assuming you mean "will emit a compiler error when used"? If that's the case, unless I'm misunderstanding, I'm not sure if it's helpful to display provably-faulty members (at least as far as TS is concerned) in a completion list.

[jods4]

@RyanCavanaugh Doesn't seem like a high-priority to me, but would probably fall into the "nice to have" bucket to me.

If the IntelliSense presentation is well-done: dimmed member (because it's not legal to use at this point) with the containing types on the right (to provide insight as to which guard is required) would give an interesting glance at the underlying types. Better than, say, a completely empty IntelliSense popup.

[DanielRosenwasser]

I'm not sure if it's helpful to display provably-faulty members (at least as far as TS is concerned) in a completion list

This is actually why we don't display all the members from union types. We don't know what type you have, so until you find a way to tell us, we're going to have to give an error if you use any member that the other types don't have. That's basically the issue.

For the record, there was a lot of discussion on the completion list scenario when we were first thinking of union types. Exploratory coding is really helpful, and keeping friction low to leverage the IDE to do so makes users happy. Check #5334 out for something similar. Maybe this could be part of that suggestion.

But if this is something we'd like to see, let's open another issue since the conversation has moved into something orthogonal to the main suggestion here.

[JsonFreeman]

There's room for debate on this question, but I have a feeling that showing the extra properties (even grayed out), and then erroring on them is a bad experience. Though I suppose an argument could be made for showing the properties in the list (along with the associated type), but refusing to auto-complete them, but I think this would also be odd.

[ahejlsberg]

@ivogabe I just pushed some changes to the controlFlowTypes branch that break && and || operators down to their actual control flows. This should fix the issue you pointed out. Also, I have done more work on destructuring assignments. I expect to put up the official pull request for all of this next week.