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

Type guard failures that become an intersection type #9859

Closed
falsandtru opened this issue Jul 21, 2016 · 20 comments
Closed

Type guard failures that become an intersection type #9859

falsandtru opened this issue Jul 21, 2016 · 20 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@falsandtru
Copy link
Contributor

TypeScript Version: 2.0.0

Code

class A<T> {
  private _: T;
}
class B<T> {
  private _: T;
}
function f() {
  var o: A<void> | B<void> = new B<void>();
  if (o instanceof A) return o; // B<void> & A<any>, should be A<void>
}

Expected behavior:

A type of o narrowed by type guard is A<void>.

Actual behavior:

A type of o narrowed by type guard is B<void> & A<any>.

@yortus
Copy link
Contributor

yortus commented Jul 21, 2016

I'd say this follows from #8911/#9016.

o is narrowed to B<void> when you assign new B<void> to o. Then the compiler sees the o instanceof A as further narrowing and comes up with the intersection type.

I suspect the A & B type, which seems impossible in terms runtime behaviour, seems fine to the compiler which is just concerned about the structural intersection of the A and B types.

@yortus
Copy link
Contributor

yortus commented Jul 21, 2016

Note this comment in particular: #8911 (comment)

@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2016

then options here are either never or A & B. never since this code can not execute if A and B are indeed disjoint, or A & B if B extends A at runtime. The first is too restrictive.

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 21, 2016
@falsandtru
Copy link
Contributor Author

falsandtru commented Jul 21, 2016

This problem seems to caused by declared type internal overwriting. This behavior should be a bug because following code works correctly.

class A<T> {
  private _: T;
}
class B<T> {
  private _: T;
}
function f() {
  var o: A<void> | B<void>;
  if (o instanceof A) return o; // A<void>
}

@mhegazy
Copy link
Contributor

mhegazy commented Jul 21, 2016

I am not sure i understand what you meant there. but the important difference between the two samples is the assignment...

var o: A<void> | B<void> = new B<void>();

// here o can only be `B<void>` and nothing else. no assignments has happened to o since it got `new B<void>()`

if (o instanceof A) return o;  /// now how can we narrow B into A?

@falsandtru
Copy link
Contributor Author

@mhegazy why do you think "working as intended" about that difference? The two samples should be a same narrowed type by type guard.

@yortus
Copy link
Contributor

yortus commented Jul 21, 2016

@falsandtru assignments cause narrowing as per #8010. Then subsequent narrowing to an unrelated type narrows to the intersection of the two narrowed types as per #9031. According to those two rules, your example is indeed working as intended.

Your sample is a bit like the following, which produces the same intersection narrowed type:

class A<T> {
  private _: T;
}
class B<T> {
  private _: T;
}
function f() {
  var o: A<void> | B<void>;
  if (o instanceof B) {
      if (o instanceof A) return o; // B<void> & A<any>, should be A<void>
  }
}

@falsandtru
Copy link
Contributor Author

falsandtru commented Jul 22, 2016

I understood that behavior, but I'm not sure that behavior is a spec, or a bug.

@yortus
Copy link
Contributor

yortus commented Jul 22, 2016

Why would it be a bug? Control flow analysis can infer:

  1. That o is a B throughout the body of f, since a B is assigned to o and there are no subsequent assignments that could change it to something else.
  2. That the only way we could reach the guarded return o statement is if o is an A. And since o is already known to be a B, then o must be an A & B in that guarded clause.

@falsandtru
Copy link
Contributor Author

Merging #9861.

class A<T> {
  private _: T;
}
class B<T> {
  private _: T;
}
function f() {
  var o: A<void> | B<void> = new B<void>();
  if (!(o instanceof B)) return o; // B<void>, should be A<void>
  o; // B<any>, should be B<boid>
}

Merging #9862.

class A<T> {
  private _: T;
}
class B<T> {
  private _: T;
}
function f() {
  var o: A<void> | B<void> = new A<void>();
  o instanceof A || o instanceof B;
  o; // A<any>, should be A<void> | B<void>
}

@mhegazy
Copy link
Contributor

mhegazy commented Jul 22, 2016

the first case is again because of the initialization. there is no way for o to be any thing but B. so checking !(o instanceof B) is really not meaningful here. so you have B, you take out B, what are you left with..

The same of the second case, it is A, it can not be anything else.

@falsandtru
Copy link
Contributor Author

falsandtru commented Jul 22, 2016

I see, I think we can close this issue. However, a compiler option for detection of this assignment (as initialization) type mismatch is helpful to avoid these confusing. A declared type shouldn't be wider than an assigned type with initialization in these CFA behaviors.

@yortus
Copy link
Contributor

yortus commented Jul 22, 2016

A declared type shouldn't be wider than an assigned type

A wider declared type allows for re-assignment later on:

let a: string | number = 'foo';
a; // a is narrowed to string here
a = 42;
a; // a is narrowed to number here

@falsandtru
Copy link
Contributor Author

It case seems an anti-pattern.

@yortus
Copy link
Contributor

yortus commented Jul 22, 2016

@falsandtru why an anti-pattern?

@falsandtru
Copy link
Contributor Author

It should be

const a: string = 'foo';
const b: number = 42;

Also see #9886.

@yortus
Copy link
Contributor

yortus commented Jul 22, 2016

There's plenty of discussion about this in #8513. Note the realistic motivating example involves modelling an Optional<T> type as a union of Some<T> and None, and some control flow that initialises an Optional to None and then possibly reassigns it with a Some value.

I don't think you can simply say

It should be

const a: string = 'foo';
const b: number = 42;

in light of legitimate examples where people are trying to model runtime-valid code using TypeScript, where union variables with initializers occur naturally as a result.

@falsandtru
Copy link
Contributor Author

My example is based on immutable programing. So I suggested as optional in #9886.

@yortus
Copy link
Contributor

yortus commented Jul 22, 2016

OK, then if you make the variable in your first example immutable:

function f() {
  const o: A<void> | B<void> = new B<void>();
  if (o instanceof A) return o; // B<void> & A<any>, should be A<void>
}

...then it seems an even stronger case for the current compiler behaviour being correct, doesn't it? Or have I misunderstood what you mean by immutable?

@falsandtru
Copy link
Contributor Author

It example is a bad case. So I said "But this operation is a mistake, should be removed." in #9886. My suggestion is useful for finding mistakes like this.

@mhegazy mhegazy closed this as completed Apr 19, 2017
@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
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants