Generic type-guard with param narrows to 'never' on reachable code

[aaronbeall]

TypeScript Version: 2.0.10

Code

interface Type { type: string; }
function isOfType<T extends Type>(target: Type, type: string): target is T {
  return target.type === type;
}

interface A extends Type { a: string; }
interface B extends Type { b: string; }
const TYPE_A = "a", TYPE_B = "b", TYPE_X = "x";

function test(target: Type) {
  if (isOfType<A>(target, TYPE_A)) {
    return target.a;
  }
  
  if (isOfType<A>(target, TYPE_A)) {
    return target.a; // No error?
  }
  
  if (isOfType(target, TYPE_X)) {
    return target;
  }
  
  if (isOfType<B>(target, TYPE_B)) {
    return target.b; // Error: TS2339:Property 'b' does not exist on type 'never'.
  }
}

Expected behavior:

Error on the second target.a because that code is not reachable.

No error on the last target.b because that code is reachable.

[aluanhaddad]

The second error occurs because

if (isOfType(target, TYPE_X)) {
  return target;
}

infers the type of target to be Type and will always succeed, it has nothing to do with the value of the second argument.

I think there should be an error generated by the second if as you say.

[aaronbeall]

Yep, that's my understanding. So is the assumption that a type-guard given different parameters should not change the outcome of the guarded value?

[aaronbeall]

@aluanhaddad You thumbed up my comment, but it was actually a question. :) Is that the assumption the compiler makes? (That different other arguments in a type-guard function should not effect the outcome of the guarded argument.) If so, then I take it my isOfType() function is invalid; it would have been helpful if the compiler could have made this clear to me somehow, perhaps by disallowing the use of other parameters within the guard expression, or disallow other parameters altogether.

[aluanhaddad]

@aaronbeall I wasn't sure what you meant. Your assumption is not correct.
For example, you can have a function like this

function is<T>(value, condition: boolean | (()=> boolean)): value is T {
  return typeof condition === 'function'
    ? condition()
    : condition;
}

which works like a charm.

[aluanhaddad]

The only thing that is surprising is in your example code is that the second if is not considered unreachable.

[aaronbeall]

Yeah, that's basically the same behavior as my example. I guess my point is that the compiler is assuming that if a type-guard will return true for a type it will never let that type through because of the second argument being false... which seems like a logical assumption for the compiler to make but in that case neither my original isOfType or your is example seem to be very safe implementations. In my case I have working code that breaks the compiler because of this assumption. (And the second case seems to contradict that assumption, but I guess that's a legit bug.)

[aluanhaddad]

The safety of user defined type guard functions is entirely up to the author. We are free to write functions like

function is<T>(x): x is T {
  return true;
}

and thereby shoot ourselves in the foot but I think the point is to provide the ability to gain more control over the typechecker in scenarios where there is no way it could possible infer a type. A common scenario is dealing with external input. My example is just meant as a strawman.

[aaronbeall]

Got it. I certainly shot myself in the foot here. :)

[aluanhaddad]

I still think you have found a legitimate bug here. The second if statement should probably be an error since it is unreachable

[TylerBrinkley]

Yeah, I just hit this bug as well. Below is my test case. Note, the else if case is reachable and actually runs for this example.

class Base {
    private readonly $type: string
    constructor($type: string = null) { this.$type = $type || Base.GetType(); }
    static GetType() { return 'Base'; }
    static Is<T extends Base>(v: Base, t: { new(): T; GetType(): string; }): v is T { return v.$type === t.GetType(); }
}
class Derived extends Base {
    days = 1;
    constructor() { super(Derived.GetType()); }
    static GetType() { return 'Derived'; }
}

let value: Base = new Derived();
if (Base.Is(value, Base)) {
    alert('value is Base');
}
else if (Base.Is(value, Derived)) {
    alert(`value is Derived and days is ${value.days.toString()}`); // Error: Property 'days' does not exist on type 'never'.
}

[RyanCavanaugh]

These are all working as designed. If a type guard appears to be true for all inputs, its operand will become never in the false arm of the if. Type guards which return the wrong value can cause problems and you shouldn't write them!