Skip to content

Compiler allows 'number' to be used where enum is expected #48296

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

Closed
AbakumovAlexandr opened this issue Mar 16, 2022 · 20 comments
Closed

Compiler allows 'number' to be used where enum is expected #48296

AbakumovAlexandr opened this issue Mar 16, 2022 · 20 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@AbakumovAlexandr
Copy link

Bug Report

Compiler shouldn't allow number to be used where enum is expected.

🔎 Search Terms

number where enum expected

🕗 Version & Regression Information

  • This is a crash
  • This changed between versions ______ and _______
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about enums_and_function_params
  • I was unable to test this on prior versions because _______

⏯ Playground Link

Playground link with relevant code

💻 Code

export enum StatusEnum {
    Cancelled = 4,
    Paid = 6
}

export interface Summary {
    readonly status: number;
}

export class TripSummary {
    private readonly status: StatusEnum;

    public constructor(dto: Summary, status: number) {
        this.status = status; // status is number, but allowed to be assigned to an enum field
        
        this.setFullStatus(dto.status); // dto.status is number, but allowed to be passed as an enum param
    }

    private setFullStatus(status: StatusEnum): void {
        console.log(status);
    }
}

🙁 Actual behavior

Compiler allows number to be used where enum is expected. It's incorrect because narrowing conversion is done implicitly while the compiler cannot make sure that a value passed as the enum param is within a range of the enum values.

🙂 Expected behavior

Implicit conversions from number to enum should generate an error requiring an explicit conversion to be performed (for exampe, using as)

@fatcerberus
Copy link

This is intentional because numeric enums are often used as bitfields, e.g. FooEnum.Foo | FooEnum.Bar is typed as number because it’s not explicitly part of the enum. You can use string-valued enums for stricter checks.

@RyanCavanaugh RyanCavanaugh added the Working as Intended The behavior described is the intended behavior; this is not a bug label Mar 16, 2022
@AbakumovAlexandr
Copy link
Author

This is intentional because numeric enums are often used as bitfields, e.g. FooEnum.Foo | FooEnum.Bar is typed as number because it’s not explicitly part of the enum. You can use string-valued enums for stricter checks.

Can you provide a full line of code to demostrate what you've meant? I'm not sure how this is related to allowing implicitly assigning numbers into enum typed members.

@AbakumovAlexandr
Copy link
Author

@RyanCavanaugh What is the justification for 'Working as Intended'? Especially in 'strict' mode?

@fatcerberus
Copy link

fatcerberus commented Mar 16, 2022

let computedFlags = FlagsEnum.One | FlagsEnum.Two;
obj.flagsEnum = computedFlags;

computedFlags should ideally be assignable back to the enum type, if obj is using it as a bitfield. However it’s currently typed as number because its actual value is not (necessarily) part of FlagsEnum. Thus why this type hole is present.

@AbakumovAlexandr
Copy link
Author

AbakumovAlexandr commented Mar 16, 2022

let computedFlags = FlagsEnum.One | FlagsEnum.Two;
obj.flagsEnum = computedFlags;

computedFlags should ideally be assignable back to the enum type, if obj is using it as a bitfield. However it’s currently typed as number because its actual value is not (necessarily) part of FlagsEnum. Thus why this type hole is present.

If you needed this type 'abuse' (in my opinion, sorry if too harsh :) ), you always could shut up the compiler explicitly:

let computedFlags: FlagsEnum = (FlagsEnum.One | FlagsEnum.Two) as FlagsEnum;
obj.flagsEnum = computedFlags;

So, fixing this TS compiler bug doesn't affect your use case and can't be the justification for the missing type safety related to enums in 'strict' mode.

@RyanCavanaugh
Copy link
Member

@AbakumovAlexandr because we did this behavior on purpose

@fatcerberus
Copy link

I’m not making any value judgments here, just telling you why the behavior is the way it is, and why it’s intentional. It’s also unlikely to change—at least, not without an additional opt-in compiler flag—as it would be pretty disruptive to a lot of code in the wild.

@AbakumovAlexandr
Copy link
Author

@AbakumovAlexandr because we did this behavior on purpose

Can you elaborate: what is the purpose?

@fatcerberus
Copy link

Can you elaborate: what is the purpose?

🤦‍♂️

@RyanCavanaugh
Copy link
Member

So that you can use bitflag-style enums without getting an error, as described above.

@AbakumovAlexandr
Copy link
Author

So that you can use bitflag-style enums without getting an error, as described above.

So, what should I do to ensure that the value in an enum-typed member is actually the one I've defined in my enum without any runtime code?

@RyanCavanaugh
Copy link
Member

There currently isn't a way to do that. The suggestion tracking adding a way to do so is #32690

@fatcerberus
Copy link

Use a string-valued enum.

@RyanCavanaugh
Copy link
Member

You could also use a const-style enum

const StatusEnum = {
    Cancelled: 4,
    Paid: 6
} as const;
type StatusEnum = (typeof StatusEnum)[keyof typeof StatusEnum];

from there the code sample works as desired

@AbakumovAlexandr
Copy link
Author

AbakumovAlexandr commented Mar 16, 2022

There currently isn't a way to do that. The suggestion tracking adding a way to do so is #32690

it's 3 years old :(

TypeScript is intended to bring static type safety. At the same time, its default behavior is to implicitly allow (-Infinity, +Infinity) where (1, 2, 3) only are explicitly expected including the 'strict' mode.

If someone wants to assign a value into an enum member which doesn't exist in its definition (bit flags), doesn't it look like it is him who should explicitly tell it to the compiler? Or what's even the point of enumerating possible values of an enum, then?

I just can't remember any other language with static types implicitly allowing this by default.

@fatcerberus
Copy link

This is how enums work in C++ and C#, too, fwiw.

@AbakumovAlexandr
Copy link
Author

AbakumovAlexandr commented Mar 16, 2022

This is how enums work in C++ and C#, too, fwiw.

Of course, it's not true (https://dotnetfiddle.net/VnMaVJ), since a wider type can't be auto-converted into a narrower type in any statically-typed language:
image

Otherwise as I said, there would be no point in narrow types (enums) if they would allow random non-existing in its definition values to be assigned. This behavior is simply implicit type breakage.

@fatcerberus
Copy link

Re: Statically-typed languages and correctness, at this point I'm just going to leave this here:
https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

Non-goals
3. Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.

@AbakumovAlexandr
Copy link
Author

AbakumovAlexandr commented Mar 17, 2022

Re: Statically-typed languages and correctness, at this point I'm just going to leave this here: https://github.com/Microsoft/TypeScript/wiki/TypeScript-Design-Goals

Non-goals
3. Apply a sound or "provably correct" type system. Instead, strike a balance between correctness and productivity.

So what do you want to say by this exactly? 'The ability to assign the result of bit flagging with enums IMPLICITLY (when there's no problem to use as at all) is more productive than completely messing the primitive type system rules and breaking the MOST COMMON use case of enums'? Really?

Are you aware that such breakage leads to massive and undetectable at compile time issues with the whole type system? Which is much more counter-productive(!) than the above 'inconvenience'.

How about this one?

export enum StatusEnum {
  Cancelled = 4,
  Paid = 6
}

export interface Summary {
  readonly status: number;
}

export class TripSummary {
  private readonly status: StatusEnum;

  public constructor(status: number) {
      this.status = status; // status is number, but allowed to be assigned to an enum field
      console.log(this.status);
      
      const s: string = this.setFullStatus(100500); // dto.status is number, but allowed to be passed as an enum param
      
      if (s !== undefined)
        console.log("TypeScript rocks!");
      else
        console.log("Really? 'undefined' from the method which return type is 'string' (NOT 'string | undefined')???");
    }

  private setFullStatus(status: StatusEnum): string {
      switch(status) {
          case StatusEnum.Cancelled:
              return "Cancelled";
          case StatusEnum.Paid:
              return "Paid";
      }
  }
}

const ts: TripSummary = new TripSummary(200500);

image

@RyanCavanaugh
Copy link
Member

This doesn't seem like a productive discussion. A trade-off was intentionally made; if you don't agree with the trade-off that's fine but there's no value to be found in reiterating all the same discussion points that occurred at the point of the decision.

@microsoft microsoft locked as resolved and limited conversation to collaborators Mar 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

3 participants