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

User-defined type guards don't work for all types #4432

Closed
sophiajt opened this issue Aug 24, 2015 · 14 comments
Closed

User-defined type guards don't work for all types #4432

sophiajt opened this issue Aug 24, 2015 · 14 comments
Assignees
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript

Comments

@sophiajt
Copy link
Contributor

If the user defines a type guard for a boolean, as in this example:

function isBool(a: any): a is boolean {
    return (typeof a === "boolean");
}

function bob() {
    var z;

    if (isBool(z)) {
        z; // type of z is still 'any'
    }
}

...then z is not properly refined to 'boolean'. It stays 'any'. I think the expected behavior here is for the type guard to refine to boolean inside of the if check.

@danquirk danquirk added the Bug A bug in TypeScript label Aug 24, 2015
@tinganho
Copy link
Contributor

I did this intentionally to mimic the instanceof check in:
https://github.com/Microsoft/TypeScript/blob/master/src/compiler/checker.ts#L6285-L6287

let z;
class A {}
if(z instanceof A) {
    z; // any
}

Should I remove it @JsonFreeman ?

@JsonFreeman
Copy link
Contributor

We've gone back and forth on the narrowing of any. I think the philosophy here (like instanceof) is that narrowing should give the user access to more properties than if narrowing had not occurred. In the case where the type being narrowed is any, narrowing actually gives you access to fewer properties. So that is the reason for not narrowing.

I think it's fine in principle to change the rule for both type predicates and instanceof, but it depends on the impact to actual code. I do think that any is a weird beast because unlike other types, everything is allowed, but nothing is encouraged. So I think having a different rule for any with respect to narrowing does make sense.

In other words, I think it makes the most sense as it is, but a case could be made for changing it.

@sophiajt
Copy link
Contributor Author

I think we can't necessarily treat it as instanceof. For example:

function bob() {
    var z;

    if (typeof z === "boolean") {
        z; // type of z is 'boolean'
    }
}

But abstracting into a user-defined type guard in a well-typed way actually changes what we know:

function bob() {
    var z;

    if (isBool(z)) {
        z; // type of z is still 'any'
    }
}

I think the reasonable assumption here is that abstraction would not lose the capability to refine the type, and that a user-defined type guard could be used instead.

Not to say that you'll be making up your own booleans... but I guess I'm arguing for a solution that's general.

@JsonFreeman
Copy link
Contributor

@jonathandturner I see your point. The question is, when we see a type predicate of the form x is T, how do we know if it corresponds to a typeof style type guard versus an instanceof style type guard? Sounds like you want to narrow for the typeof case, but not the instanceof case?

One idea is that if T is a primitive, we assume it's a typeof style type guard. If T is anything else, it's instanceof. Not sure if that's a good rule, but it's an idea.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Bug A bug in TypeScript labels Aug 25, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Aug 25, 2015

The behavior here matches the intended design. it does not match the user intuition though. we need to consider this for both instance of and user defined type guards.

@JsonFreeman
Copy link
Contributor

Whether or not we decide to narrow, I do think it makes sense to keep the behavior consistent between the type predicate form and the instanceof form.

@sophiajt sophiajt added Bug A bug in TypeScript and removed In Discussion Not yet reached consensus labels Aug 31, 2015
@sophiajt
Copy link
Contributor Author

We talked about this at the design meeting and agreed this is a bug.

@JsonFreeman
Copy link
Contributor

Is it also a bug for instanceof?

@sophiajt
Copy link
Contributor Author

Doesn't this already work for instanceof? I was thinking the problem was that the ideas worked for instanceof-style checks but not typeof.

@mhegazy
Copy link
Contributor

mhegazy commented Aug 31, 2015

@JsonFreeman we will need to change both user defined type guard functions and instance of.

@mhegazy mhegazy removed the Suggestion An idea for TypeScript label Aug 31, 2015
@JsonFreeman
Copy link
Contributor

Right, so to summarize, the intended behavior for type predicates and instanceof, is that they should narrow when the initial type is any, which will be a breaking change.

@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Aug 31, 2015
@mhegazy
Copy link
Contributor

mhegazy commented Aug 31, 2015

correct. adding the breaking change label.

@dsherret
Copy link
Contributor

👍 for narrowing any. Good decision.

@mhegazy
Copy link
Contributor

mhegazy commented Oct 6, 2015

We have a discussion about this and narrowing any a. is a breaking change, b. is not clear that there is any value added, and more importantly, c. there is no way to opt-out unless cast is used inside the blocks.

with union types available now, the user specified any explicitly (assuming noImplicitAny is on) and i would take that as an indication that she does not want more compiler checks. reverting this again because of a type guard seems more annoying than helpful.

@mhegazy mhegazy closed this as completed Oct 6, 2015
@mhegazy mhegazy added Suggestion An idea for TypeScript and removed Breaking Change Would introduce errors in existing code Bug A bug in TypeScript labels Nov 12, 2015
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Declined The issue was declined as something which matches the TypeScript vision Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants