Enum value comparison fails to compile if performed twice

[GeofG]

TypeScript Version: 2.0.3

Code

enum E { A, B, C }
class C {
     e: E;
}
function f(c: C): boolean {
    if (E.B == c.e)
        return true;
    if (E.B == c.e)
        return true;
}

Expected behavior:
Code compiles.

Actual behavior:
t.ts(8,6): error TS2365: Operator '==' cannot be applied to types 'E.B' and 'E.A | E.C'.

If the second if statement is removed, there is no error. If the first return statement is replaced with 1==1, there is no error. If the second test is for a different value (e.g. E.A), there is no error. The problem remains even if intervening code changes the value of c.e.

[ahejlsberg]

This is working as intended. Following the first if statement the control flow analyzer refines the type of c.e to E.A | E.C (because the function returns for E.B). The error in the second if statement is the compiler's way of pointing out (and explaining why) the test will never be true. If you insert an intervening assignment to c.e it does indeed work, but of course only if you assign something that could be E.B, because the compiler still knows that the function returned for E.B. For example, this compiles with no error:

function f(c: C): boolean {
    if (E.B == c.e)
        return true;
    c.e = Math.random() > 0.5 ? E.A : E.B;
    if (E.B == c.e)
        return true;
}

[RyanCavanaugh]

I would add that this logic has found multiple bugs in existing codebases.

[GeofG]

That makes sense, and I can see that it might catch many bugs. But unless I'm missing something, this prevents information hiding. The validity of the code in f dependent on the implementation of whatever happens to c in between, even if that code is someone else's responsibility. In a more general case, closer to my actual code:

function f(c: C, x: number): boolean {
    if (E.B == c.e)
        return true;
    c.e = g(x);
    if (E.B == c.e)
        return true;
}

Is there a way to have information hiding between f and g, so that f cares only about the specification of g, not its implementation? (I tried interposing an interface; it didn't help.)

In my actual code, you are correct that the second test is redundant. But I don't want to remove it: I want to preserve the ignorance of f about the implementation of g. In future, g may be designed differently and return different E values. (The problem would also arise for stubs.)

The spec for g is that it can return E.B. In this particular instance it does not, but in future it might. If I remove the second test now, such a change to g will require a change in f. (It will also introduce an unexpected bug, though that can be caught with a runtime assertion.) To prevent the implementation of f from depending on the implementation of g, I could replace the enum with numeric constants: but that seems inappropriate and results in weaker type checking.

[RyanCavanaugh]

If the intent of g is that it could actually return any E value at some point in the future, just add a return type annotation to its declaration explicitly saying that.

Consider a separate case: If the spec for g is that it returns string, it'd be illegal to compare its return value to number. There's no intrinsic way to pretend that you don't know that, apart from a type assertion.

[GeofG]

You are correct about the return type. But if (as is actually the case; I seem to have oversimplified) c.e is changed as a side-effect of calling g, the problem remains:

function g(c: C, x: number): void {
    c.e = E.A;
}
function f(c: C, x: number): boolean {
    if (E.B == c.e)
        return true;
    g(c, x);
    if (E.B == c.e)
        return true;
}

I am not aware of a way to annotate g to indicate which side-effects on c are valid. I could return a complex type, but that's ugly, and doesn't actually make sense for a state machine, which is what my actual c is.

[RyanCavanaugh]

#9998 covers the discussion we had around this. There doesn't seem to be any good general solution, unfortunately.

[GeofG]

For the record, the solution for my actual code appears to be to create an access method for c.e:

enum E { A, B, C }
class C {
    e: E;
    g(x: number): void {
        this.e = E.A;
    }
    getE(): E {
        return this.e;
    }
}

function f(c: C, x: number): boolean {
    if (E.B == c.getE())
        return true;
    c.g(x);
    if (E.B == c.getE())
        return true;
}

I can live with this. I guess for me the biggest problem was figuring out why this was an error, and what I could do about it. I don't know if it's practical, but it would be nice to give a hint of the possibility in the error message (maybe indicating it might be an issue with side-effects, thereby providing a useful search term), and some mention of side-effects in the documentation might help the next person who runs into this.

[RyanCavanaugh]

#10991 (comment) tracks issuing a better error message in cases like this