Skip to content
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

Narrow string and number types in literal equality checks #11587

Merged
merged 5 commits into from
Oct 13, 2016

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Oct 13, 2016

With this PR we narrow the string and number types as appropriate when they are compared with values of literal types.

function move(direction: "up" | "down") {
    // ...
}

function do1(command: string) {
    if (command === "up" || command === "down") {
        move(command);  // Narrowed to type "up" | "down"
    }
}

function do2(command: string) {
    switch (command) {
        case "up":
        case "down":
            move(command);  // Narrowed to type "up" | "down"
            break;
    }
}

function f1(x: number, y: 1 | 2) {
    if (x === 0 || x === y) {
        x;  // Narrowed to type 0 | 1 | 2
    }
}

function f2(x: number | "foo" | "bar", y: 1 | 2 | string) {
    if (x === y) {
        x;  // Narrowed to type "foo" | "bar" | 1 | 2
        y;  // Narrowed to type "foo" | "bar" | 1 | 2
    }
}

Interestingly this found a bug in our scanner, pointing out that type 2 is not comparable to type 8.

Fixes #7447.
Fixes #10417.
Fixes #11306.

@ahejlsberg ahejlsberg merged commit 17c2ab2 into master Oct 13, 2016
@ahejlsberg ahejlsberg deleted the narrowStringAndNumber branch October 13, 2016 21:00
@zpdDG4gta8XKpMCd
Copy link

i am not at the computer to check, but would it fix #11433 as well?

@mhegazy
Copy link
Contributor

mhegazy commented Oct 14, 2016

i am not at the computer to check, but would it fix #11433 as well?

nope. that is a different issue.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants