Enum indexer return type should be `string|undefined` for --strictNullChecks

[ulrichb]

TypeScript Version: 2.0.3

Code

enum SomeEnum { A = 1, B = 2 }

function lookUp(index: number) {
    return SomeEnum[index].toLowerCase();
}

console.log(lookUp(1)); // => "a"
console.log(lookUp(2)); // => "b"
console.log(lookUp(3)); // => runtime error: "SomeEnum[index] is undefined"

Expected behavior:
When using --strictNullChecks, TSC should emit a compile-time error in lookUp() because there is no undefined-check. => Indexer return type should be string|undefined instead of just string.

[ulrichb]

I've fixed this in a local TSC build, and would love to propose a PR.

[Arnavion]

IIRC it's the same as why array indexing also returns T instead of T | undefined - it's inconvenient to write ! all the time when the user knows the indexing is safe.

[RyanCavanaugh]

Hard to square this with the fact that people continuously want SomeEnum[someStr] to always be of type number even though it can also produce a string

[ulrichb]

@RyanCavanaugh Thank you for your reply. Note that the string-indexer for enums already behaves differently. It has a number return type iff the argument is a matching and constant value, otherwise the return type is any. See the following example.

enum SomeEnum { A = 1 }

SomeEnum["A"].toFixed(2);        // => '1.00'
SomeEnum["A"]._invalid_;         // Error: "Property '_invalid_' does not exist on type 'SomeEnum'"
SomeEnum["A".trim()]._invalid_;  // No error because this indexer-result type is `any`; will evaluate to `undefined` at run-time
SomeEnum["X"]._invalid_;         // No error because this indexer-result type is `any`; will throw "Cannot read property '_invalid_' of undefined" at run-time

Further note that for the last two accesses (SomeEnum["A".trim()] and SomeEnum["X"]), TSC will warn us with activated --noImplicitAny ("Element implicitly has an 'any' type because index expression is not of type 'number'"). => With enabled --noImplicitAny this indexer-overload is safe in contrast to the number-indexer (as described in the issue text above).

@Arnavion Yes, the same applies to the array-indexer, which I think should also be discussed. I wanted to start with the enum "lookup" indexer because I think in this case the chance of a missing undefined-check is higher than for missing undefined- or bounds-checks for array-indexer-accesses.

[MartinJohns]

I fell over this issue today. We're using objects as maps and define an interface for it, something like this:

interface IMyMap {
    [code: string]: ResultType;
}

let myMap: IMyMap = {};

Now when accessing myMap['invalidCode'] I would have expected that the type is ResultType | undefined, but to my surprise it's actually just ResultType. At runtime it's clearly undefined, as this property was never initialized.

I understand it might be annoying for people to write ! in cases where they know that it's not null/undefined. But isn't the whole point of the strict null checks to prevent potential undefined accesses? That's kinda undermined with this.

This is the same case with arrays: myArray[9999] could return undefined in arrays that are shorter.

I wish that strictNullChecks would actually be strict. Right now (while still being a great feature and a benefit) it seems rather half-hearted.

[Arnavion]

You could write [code: string]: ResultType | undefined and have a tslint rule to ban indexers without | undefined (I don't think one already exists). That still doesn't help with array access of course.

[RyanCavanaugh]

Duplicate #9235

[ulrichb]

@RyanCavanaugh I don't think that this issue is a duplicate. The referenced issue is more general and concerns all indexer (return) types. This issue just concerns the number-indexer of enums and can also be fixed just for that (did this already in a local TS build).

Further note that the main argument in #9235 ("break the very common case of arrays") does not apply here.