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

TypeScript silently omits enum assignment type error between two enums with the same name #55915

Closed
evanw opened this issue Sep 29, 2023 · 2 comments · Fixed by #55924
Closed
Assignees
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Experimentation Needed Someone needs to try this out to see what happens Fix Available A PR has been opened for this issue

Comments

@evanw
Copy link
Contributor

evanw commented Sep 29, 2023

🔎 Search Terms

I'm not exactly sure what keywords to use for this because it's kind of a strange problem. Some ideas: enum assignment same name error missing

🕗 Version & Regression Information

  • This is the behavior in every version I tried (both TypeScript 3.3 and TypeScript 5.2)

⏯ Playground Link

No response

💻 Code

I couldn't do a playground link for this because it requires two separate files:

  • types.ts

    export enum DiagnosticCategory {
        Warning,
        Error,
        Suggestion,
        Message,
    }
    
    export enum DiagnosticCategory2 {
        Warning,
        Error,
        Suggestion,
        Message,
    }
  • diagnosticInformationMap.generated.ts

    import { DiagnosticCategory as DiagnosticCategory2 } from "./types";
    // import { DiagnosticCategory2 as DiagnosticCategory2 } from "./types";
    
    enum DiagnosticCategory {
      Warning = 'Warning',
      Error = 'Error',
      Suggestion = 'Suggestion',
      Message = 'Message',
    }
    
    function diag2(category1: DiagnosticCategory, category2: DiagnosticCategory2) {
      category1 = category2;
      category2 = category1;
    }

🙁 Actual behavior

There is no error if you import DiagnosticCategory as DiagnosticCategory2 but there is an error if you DiagnosticCategory2 as DiagnosticCategory2. My tsconfig.json for this experiment:

{
  "compilerOptions": {
    "target": "ESNext",
    "module": "ESNext",
    "strict": true
  }
}

Demo:

$ node_modules/.bin/tsc -version
Version 5.2.2

$ head -n 2 diagnosticInformationMap.generated.ts 
import { DiagnosticCategory as DiagnosticCategory2 } from "./types";
// import { DiagnosticCategory2 as DiagnosticCategory2 } from "./types";

$ node_modules/.bin/tsc -p .
(no errors)

$ head -n 2 diagnosticInformationMap.generated.ts 
// import { DiagnosticCategory as DiagnosticCategory2 } from "./types";
import { DiagnosticCategory2 as DiagnosticCategory2 } from "./types";

$ node_modules/.bin/tsc -p .
diagnosticInformationMap.generated.ts:12:3 - error TS2322: Type 'DiagnosticCategory2' is not assignable to type 'DiagnosticCategory'.

12   category1 = category2;
     ~~~~~~~~~

diagnosticInformationMap.generated.ts:13:3 - error TS2322: Type 'DiagnosticCategory' is not assignable to type 'DiagnosticCategory2'.

13   category2 = category1;
     ~~~~~~~~~


Found 2 errors in the same file, starting at: diagnosticInformationMap.generated.ts:12

🙂 Expected behavior

I expected importing DiagnosticCategory as DiagnosticCategory2 to cause the same error that importing DiagnosticCategory2 as DiagnosticCategory2 does. My rationale is that the two type definitions in types.ts appear to be identical to me, so my intuition tells me to expect them to behave identically. It seems to me like TypeScript is perhaps silently swallowing the error about assignment compatibility if the two types are named the same thing even though the names refer to two separate incompatible things from two separate files.

Additional information about the issue

I discovered this confusing behavior while helping @jakebailey on #55894. It seemed like a bug to me so I'm reporting it here. Feel free to close this issue if this is expected behavior.

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Sep 29, 2023

What an enormous WAT.

Here is a single-file example:

namespace a {
    export enum DiagnosticCategory {
        Warning,
        Error,
        Suggestion,
        Message,
    }

    export enum DiagnosticCategory2 {
        Warning,
        Error,
        Suggestion,
        Message,
    }
}

namespace b {
    export enum DiagnosticCategory {
        Warning = "Warning",
        Error = "Error",
        Suggestion = "Suggestion",
        Message = "Message",
    }
}

function f(x: a.DiagnosticCategory, y: b.DiagnosticCategory) {
    x = y;
    y = x;
}

function g(x: a.DiagnosticCategory2, y: b.DiagnosticCategory) {
    x = y;
    y = x;
}

and here is one that doesn't use namespace declarations at all:

export enum DiagnosticCategory {
    Warning,
    Error,
    Suggestion,
    Message,
}
export let x: DiagnosticCategory

{
    enum DiagnosticCategory {
        Warning = "Warning",
        Error = "Error",
        Suggestion = "Suggestion",
        Message = "Message",
    }
    function f(y: DiagnosticCategory) {
        x = y;
        y = x;
    }
}

So the context is that ever since TypeScript 1.8(?), a source enum is are compatible with a target enum if its names (the enum declaration and its members) are all identical.

I think the weird thing here is that we never checked whether the enum values even though that was explicitly mentioned in the design notes at #5943. I don't know if @sandersn knows why that is though.

Part of the issue I think was that people got frustrated over mismatches in numeric enum values. Adding a new member can shift an entire range of enums.

Still, I don't know if I find that all that convincing. Maybe it's a thing about bitflags? But ignoring the values seems extremely wrong.

@fatcerberus
Copy link

a source enum is are compatible with a target enum if its names (the enum declaration and its members) are all identical

tl;dr someone took the term "nominal typing" too literally

@DanielRosenwasser DanielRosenwasser added Bug A bug in TypeScript Experimentation Needed Someone needs to try this out to see what happens labels Oct 2, 2023
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 5.4.0 milestone Oct 2, 2023
@DanielRosenwasser DanielRosenwasser self-assigned this Oct 2, 2023
@DanielRosenwasser DanielRosenwasser added the Breaking Change Would introduce errors in existing code label Nov 10, 2023
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change Would introduce errors in existing code Bug A bug in TypeScript Experimentation Needed Someone needs to try this out to see what happens Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants