microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.02k stars 12.49k forks source link

Confusing enum reverse string lookup behaviour in strict mode #29094

Open jleclanche opened 5 years ago

jleclanche commented 5 years ago

Summary

Code below.

Typescript in --strict mode will reject enum reverse lookups using arbitrary strings with this very confusing error message:

Element implicitly has an 'any' type because index expression is not of type 'number'.

A workaround is to cast the string to a subset of all possible enum keys but that's impractical with large enums (or is there an easy way to say "any key of enum X" in TS?).

My real world use case: I have a react app with a <select> filter on various items, and a bunch of <option> values, all of which are part of an integer enum. I have to use the string representation in the dom, so the state of the filter has to be the string representation. When I check the item's enum value I already know it can only be one of those enum members.

This report is three-fold:

  1. Is this behaviour intended? There are cases where it can definitely catch a reverse lookup that can fail.
  2. If it is intended, then what is the correct way to declare that the lookup will always be valid? let typeFilter: "FOO" | "BAR" = "FOO" is impractical and increases risk of errors.
  3. Regardless of the above, the error message should be fixed. The element highlighted (typeFilter) does not in fact have an "any" type, and Type[typeFilter] will not be any either (in fact, Type[typeFilter] as Type does not fix it). An error such as "Reverse lookup on enum Type may fail because typeFilter is of type 'string'" is much clearer already.

Details

TypeScript Version: 3.2.2

Search Terms: enum reverse string lookup

Code

// test.ts
// Compile with `tsc --strict test.ts`
export enum Type {
    FOO = 0,
    BAR = 1,
}

const Test = () => {
    // Change the following line to `let typeFilter: "FOO" | "BAR" = "FOO";` and it will work
    let typeFilter = "FOO";

    if (Type[typeFilter] != Type.FOO) {
        return;
    }
};

export default Test;

Actual behavior:

test.ts:9:11 - error TS7015: Element implicitly has an 'any' type because index expression is not of type 'number'.

9  if (Type[typeFilter] != Type.FOO) {
            ~~~~~~~~~~

Found 1 error.

Playground Link: [Playground](http://www.typescriptlang.org/play/#src=export%20enum%20Type%20%7B%0D%0A%09FOO%20%3D%200%2C%0D%0A%09BAR%20%3D%201%2C%0D%0A%7D%0D%0A%0D%0Aconst%20Test%20%3D%20()%20%3D%3E%20%7B%0D%0A%09let%20typeFilter%20%3D%20%22FOO%22%3B%0D%0A%0D%0A%09if%20(Type%5BtypeFilter%5D%20!%3D%20Type.FOO)%20%7B%0D%0A%09%09return%3B%0D%0A%09%7D%0D%0A%7D%3B%0D%0A%0D%0Aexport%20default%20Test%3B)

Related Issues: Maybe #27297

jack-williams commented 5 years ago

Use const instead of let to get a string literal inference. Currently the inferred type is string, which is unsound and correctly flagged.

jleclanche commented 5 years ago

@jack-williams Yes that's intended for the purpose of reproducing the issue. The problem isn't that the inferred type is string (that's correct), it's that the behaviour when doing an arbitrary string reverse lookup is very confusing.

jack-williams commented 5 years ago

My apologies; misread the intent of the query. Yes I agree the error message could probably do with more elaboration. The problem is that indexing into the enum using a number returns a string, and therefore this code is being hit:

else if (getIndexTypeOfType(objectType, IndexKind.Number)) {
    error(accessExpression.argumentExpression, Diagnostics.Element_implicitly_has_an_any_type_because_index_expression_is_not_of_type_number);
}

I think before dropping into this case there should be some elaboration based on whether the target is an enum, and the index type is string. I think if the index type is a string and the target is an enum, it is more likely that you were trying to get a specific member, rather than trying to pull out the name for a value --- but I might be wrong on that assumption.

jleclanche commented 5 years ago

I think if the index type is a string and the target is an enum, it is more likely that you were trying to get a specific member, rather than trying to pull out the name for a value --- but I might be wrong on that assumption.

Right, that's what was confusing me as well. In this particular case I have the identifier and I want to get the member.

For my edification, what's the cleanest way to declare that a type is going to be one of an enum's identifiers? (Is there some syntax such as let typeFilter: "" | memberof Type = "";?)

jack-williams commented 5 years ago

let typeFilter: keyof typeof Type = "FOO";

I think

weswigham commented 5 years ago

The error message is correct, but in the case of enums, could stand to be improved (since the index signature is implicit). Something like

Element implicitly has an 'any' type because index expression is not an enum key or `number`.