-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Type guards as assertions #8548
Conversation
what about the idea of not using types from |
@mhegazy The idea of ignoring types from In aggregate I think this new approach is much better. We only produce |
I think this is generally a pretty good direction, though I'll probably need to think more on it. I still think it would be worth investigating flagging spots where a condition narrows to |
👍 |
Yes, if a branch is inferred to be |
Latest commits add the "revert to declared type" behavior to the |
This seems arbitrary: function func(x: number | string) {
x; // 'x' has type 'number | string' here
if (typeof x !== "number" && typeof x !== "string") {
x; // why does 'x' get type 'number' here?
// why not, say, 'string', 'number | string', or 'nothing' instead?
}
x; // 'x' has type 'number | string' here
} (tested on |
Yes, it does seem a bit arbitrary that if (typeof x !== "number") {
x; // string (we're fine here)
if (typeof x !== "string") {
x; // number (we switched back to declared type)
}
} The behavior above doesn't seem quite as arbitrary, but from a control flow perspective there's really no difference. All in all, we're talking about the best possible compromise here. None of this happens as long as you're writing meaningful code and the alternative of |
First, I think one confusing part here is why you referred to I feel that in order to get a good perspective on this I would need to observe many examples and variations or become familiar with the actual, precise logic that is being used at this point. Anyway, I tried two more variations and compared them: Expected: function func1(x: number | string | boolean) {
x; // (1) 'x' has type 'number | string | boolean'
if (typeof x !== "number") {
x; // (2) 'x' has type 'string | boolean'
if (typeof x !== "string") {
x; // (3) 'x' has type 'boolean'
}
}
} Unexpected: function func2(x: number | string | boolean) {
x = "hi";
x; // (1) 'x' has type 'string'
if (typeof x !== "number") {
x; // (2) 'x' still has type 'string'
if (typeof x !== "string") {
x; // (3) 'x' has type 'number | boolean'
}
}
} So it seems that in the second example, the 'surprising' negative assertion for a |
OK, I think I got the idea, it reverts back to the 'declared type' but then applies the negative assertion. So it is more like the I feel that's a very simple (simplistic?) way to approach this scenario. The original intention here seemed to have been to mostly address cases with positive assertions through guards, which seem much simpler. Negative assertions appear less trivial. I'm trying to think about alternative solutions, hopefully not ones that would require more sophisticated techniques like backtracking etc. - though it may be unavoidable here, I'm not sure at this point. |
I think I could isolate the issue here in the following way: function func2(x: number | string | boolean) {
x = "hi";
// (1)
if (typeof x !== "number") {
// (2)
if (typeof x !== "string") {
x; // (3) 'x' has type 'number | boolean'
}
}
} With the new logic, the compiler now makes the 'worst-case' assumption that the mysterious event that caused So after looking at it like this, surprisingly I'm starting to think this is somewhat reasonable after all as a rough fallback, for these types of cases. However the original one, there wasn't any side effect that could (reasonably) possibly happen between the two negative assertions, so I believe it would be better for it to fall back to either function func(x: number | string) {
x; // 'x' has type 'number | string' here
if (typeof x !== "number" && typeof x !== "string") {
x; // why does 'x' get type 'number' here?
}
x; // 'x' has type 'number | string' here
} (it would be extremely unusual if |
I'll try to 'reboot' once again from a more practical perspective: function func(x: number | string) {
if (typeof x !== "number" && typeof x !== "string") {
// 'x' gets type 'number' here
//
// However the programmer's intention was most likely to check if it was
// null, undefined, or some other type that's not 'number' or 'string'
// ..
}
}
func(<any> undefined); // <any> cast not needed if non-nullability is disabled The reasoning for this behavior was that it is somehow consistent with nested guards and that the compiler isn't currently 'smart' enough to differentiate between conjunct and nested guards. However, that's not really a satisfying argument - that just explains why it happens, but doesn't show why is that a reasonable outcome for this relatively common scenario (based on objective measures, it most likely isn't). |
I had some initial conception of an alternative approach, that's still quite 'rough', but provides somewhat more precise 'awareness' for the soundness, or 'confidence' levels for types. I felt that I'm not in a position to evaluate how useful or suitable something like this would be to augment the current analysis, but I wanted to share it here anyway, in case it does turn out to be of some use (perhaps for even other things like literal types etc.). Idea sketch: soundness metrics for flow analysis What if the compiler had implicit traits for analyzed type
This includes a granular trait for each individual component of a union. E.g. a union like
Some general rules:
Simple example: let x: number | string; // x starts as (unsound number, unsound string)
x = 1; // x is (sound number, sound not string)
function mysteriousFunction() {
// ..
}
mysteriousFunction(); // all 'sound' types of variables possibly captured by the
// function are degraded to 'unsound'
x; // x is (unsound number, unsound not string)
if (typeof x !== "number") {
x; // 'x' falls back to (sound not number, unsound not string)
// which is effectively seen as 'string' for the programmer
} Another example: let x: number | string; // x starts as (unsound number, unsound string)
x = 1; // x is (sound number, sound not string)
if (typeof x !== "number") {
x; // x changes to (sound not number, sound not string)
// which is effectively seen as 'nothing' to the programmer
} More complex example: function func(x: number | string | boolean) {
// x is (unsound number, unsound string, unsound boolean)
x = "hi";
// x is (sound not number, sound string, sound not boolean)
function mysteriousFunction() {
// ..
}
mysteriousFunction();
// x is (unsound not number, unsound string, unsound not boolean)
if (typeof x !== "number") {
// type of 'x' is (sound not number, unsound string, unsound not boolean)
if (typeof x !== "string") {
// type of 'x' is (sound not number, sound not string, unsound not boolean)
// the type shown to the programmer is 'boolean' - despite the fact it is negated,
// the negation isn't considered sound so that's the only remaining option.
}
}
} I cannot predict, at this point, and with my limited knowledge, if, when and how exactly these metrics would/should be applied, though (I mean, actually.. in practice rather than just as an example). I guess it feels too early for that. [Edits: fixed many small mistakes and inaccuracies] |
Just wanted to mention I'm aware of this possibility (as well as other cases like a function being indirectly called using function callTheFunctionInTheBox(box: { secret: Function }) {
box.secret();
}
function example() {
let x: number | undefined = 1;
function imNotCalledDirectly() {
x = undefined;
}
let secretBox = { secret: imNotCalledDirectly };
callTheFunctionInTheBox(secretBox);
// x is now undefined
} However that's not really something that cannot be accounted for as well. Simply assume the worst possible case and always consider a local variable's type as |
This PR fixes several of the issues we've had around
nothing
types in control flows. We previously took the view that when the current control flow based type of a variable is disjoint with the type implied by a type guard, we produce typenothing
. Effectively, we'd trust that the control flow based type is right and therefore conclude that the type guard must be wrong.This PR changes the reasoning in favor of the type guard instead. We now say that if narrowing the current control flow type produces
nothing
, but narrowing the declared type would produce an actual type, then the type guard must be there for a reason (and consequently we must have missed something in the reasoning about the control flow). Therefore, instead of producingnothing
, we revert to the declared type and narrow that. Effectively we treat the type guard as an assertion that the condition could be true and respect it if it actually could be true according to the declared type.Things that could cause the control flow reasoning to disagree with the type guard include an invalid argument passed for a parameter or an assignment to local variable by a nested function. For example:
Fixes #8513.
Fixes #8429.