Allow non-unit types in union discriminants

[ahejlsberg]

With this PR we permit non-unit types in union type discriminants. Specifically, we now consider a property of a union type to be a discriminant property if it has a union type containing at least one unit type and no instantiable types. For example:

type Result<T> = { error?: undefined, value: T } | { error: Error };

function test(x: Result<number>) {
    if (!x.error) {
        x.value;  // number
    }
    else {
        x.error.message;  // string
    }
}

test({ value: 10 });
test({ error: new Error("boom") });

Inspired by #27631 which was closed in favor of this PR.

Fixes #24193.

[jack-williams]

I mentioned it in the old PR, but I think it is worth recording in the PR that gets merged:

This change will have the side-effect of enabling excess property checking for certain union types that were previous not subject to the checks. Unions types are only checked if they have a discriminant property, and this change distinguishes more discriminant types. Technically this is a breaking change.

[jack-williams]

Could it be worth adding a test that includes constrained generics? My reasoning being that in the future it might be worth considering discriminants that have constrained parameters because they are comparable to literals in some form. Having a test that would flag any changes might be helpful. Something like:

function f21<T extends number>(x: { a: undefined; b: { x: number } } | { a: T, b: { y: number } }) {
    if (x.a === undefined) {
        x.b = { y: 42 };
    }
}

[weswigham]

function f(x: { kind: true, a: string } | { kind: "b" } | { kind: string, c: string }) {
    if (x.kind === true) {
        x.a;
    }
    else if (x.kind !== "b") {
        x.c; // Should be `{ kind: string, c: string }`
    }
    else {
        x; // should be `{ kind: "b" } | { kind: string, c: string }`
    }
}

doesn't seem to work yet with this PR.

[jack-williams]

@weswigham
EDIT: Correction, the reduction is irrelevant in this case. The problem is that the literal is comparable to the string.

Think the issue with your example is that literal reduction loses type "b", so when it comes to narrowing by inequality it does nothing. What would be the impact of disabling literal reduction when getting the property of a union type?

[ahejlsberg]

@jack-williams You're actually right about type string absorbing type "b" in a union type. All of the narrowing logic in the control flow analyzer uses union type operations so in those operations we really only can discriminate on true | string. We do have the ability to construct non-reduced union types such as "b" | string, but that capability is used only to make contextual types a bit more precise and isn't actually observable in types that the user might declare or operate on.

I think it is fine to document that discriminants must be truly disjoint types, and that string and number can't be used to define "default" cases. If we really wanted to support such a default pattern in a meaningful way, we'd need to verify that the specialized cases are subtypes of their corresponding default cases, and we'd need to narrow to the specialized cases consistently. But that's beyond the scope of this PR and I don't even think we want to go there.

[jack-williams]

@ahejlsberg Thanks for the explanation. I managed to confuse myself when I experimented with turning off union reduction and finding that it did not have an effect in this example.

But that's beyond the scope of this PR and I don't even think we want to go there.

Yes, I agree with your assessment here. The asymmetry brought about by trying to narrow types like "b" | string that are influenced by inequality, but not equality, leads to code that can be quite subtle.

[weswigham]

Fair enough. I just wanted to point out that we still don't narrow all the things we look like we could because of how the broader string type and the specific literal type are related.

[weswigham]

In any case, this definitely looks like an improvement, and we can always adjust it to recognize more types as discriminable in the future if need be.

[bmingles]

Any chance this will allow using tuples as discriminants?

e.g.

type RouteA = {
  tokens: ['a', string]
};

type RouteB = {
  tokens: ['b', string, string]
};

type Route =
  | RouteA
  | RouteB;

const route: Route = ...

if(route.tokens[0] === 'a') {
   // compiler knows this is RouteA
}

[ksaldana1]

@bmingles I believe your example will narrow correctly since v3.1.