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

noUncheckedIndexedAccess with enums could be type narrowed #47508

Closed
Stono opened this issue Jan 19, 2022 · 9 comments · Fixed by #49912
Closed

noUncheckedIndexedAccess with enums could be type narrowed #47508

Stono opened this issue Jan 19, 2022 · 9 comments · Fixed by #49912
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Help Wanted You can do this
Milestone

Comments

@Stono
Copy link

Stono commented Jan 19, 2022

Bug Report

Hi, we're trying to tighten up some of our code by using noUncheckedIndexedAccess. I know there are caveats to this compiler option which we've found by searching, but this one feels more like a bug?

🔎 Search Terms

Searched a variety of issues, the most relevant being: #13778

🕗 Version & Regression Information

4.5.4

⏯ Playground Link

💻 Code

    enum Meat {
      Sausage,
      Bacon
    }
    const sausage = Meat.Sausage
    const value = Meat[sausage] // string | undefined

🙁 Actual behavior

value in the above example can't be undefined

🙂 Expected behavior

value to be typed as string, not string | undefined

@Stono
Copy link
Author

Stono commented Jan 19, 2022

Can be worked around with cont value = Meat[sausage]!

@MartinJohns
Copy link
Contributor

Your example is very narrow. Here's another example where it wouldn't work:

const sausage: Meat = 123
const value = Meat[sausage] // string | undefined

@Stono
Copy link
Author

Stono commented Jan 19, 2022

Yip it was intentionally narrow to demonstrate a scenario where I believe it shouldn't produce an undefined?

@MartinJohns
Copy link
Contributor

MartinJohns commented Jan 19, 2022

And I demonstrated why it should produce undefined. :-) Enums are not a union of literal types, so the type Meat.Sausage is the same as Meat.

@jcalz
Copy link
Contributor

jcalz commented Jan 19, 2022

I think the issue here is that numeric enums' reverse mappings are not strongly typed. One could imagine a stronger typing where you wouldn't care about index signatures at all (why would you want to index into an enum with an arbitrary number anyway? It's not like TS implements reverse bit flags):

⚙🛠
const _Meat = { Sausage: 0, Bacon: 1 } as const;

function withReverse<T extends Record<keyof T, PropertyKey>>(
    obj: T): T & { [K in keyof T as T[K]]: K } {
    return Object.assign({}, obj,
        Object.fromEntries(Object.entries(obj)
            .map(([k, v]) => ([v, k]))));
}

const Meat = withReverse(_Meat);
namespace Meat {
    export type Sausage = typeof Meat.Sausage;
    export type Bacon = typeof Meat.Bacon;
}
type Meat = typeof Meat[keyof typeof _Meat]
const sausage = Meat.Sausage // 0
const value = Meat[sausage] // "Sausage"

Playground link

As such this feels like it's strongly related to #38806

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Jan 19, 2022
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jan 19, 2022
@RyanCavanaugh
Copy link
Member

In this case sausage has the specific literal type Meat.Sausage and in that case we can be sure it won't be undefined. Any such access where all possible indices are specific literals of the enum (as opposed to the containing enum type Meat) can be safely narrowed to string.

@fatcerberus
Copy link

fatcerberus commented Jan 20, 2022

@RyanCavanaugh Counterpoint:

enum Meat {
    Sausage,
    Bacon
}
const sausage: Meat.Sausage = 42;
const value = Meat[sausage] // string | undefined
console.log(value);  // indeed, undefined

Numeric enums are weird.

@RyanCavanaugh
Copy link
Member

Sure, but all the unsoundness is packed into the assignment from 42, which already has an exploitable hole

const sausage: Meat.Sausage = 42;
const zero: 0 = sausage;
const tup = [1, 1] as const;
const n: number = tup[zero];

@fatcerberus
Copy link

Huh, that’s bizarre. So Meat.Sausage is treated like a literal 0 type (its actual value) but allows assignment from any number…

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants