TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Confusing type checker error message for invalid discriminated union

Open davimiku opened this issue 2 years ago • 7 comments

Bug Report

🔎 Search Terms

  • Tagged union
  • Discriminated union
  • Confusing/unclear error message

🕗 Version & Regression Information

This issue occurs on the oldest TS available on the playground (v3.3) and the current version (v4.9)

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about discriminated unions, tagged unions

⏯ Playground Link

Playground link with relevant code (v4.9.4)

💻 Code

type DiscriminatedUnion =
    | { tag: 'a', data: string }
    | { tag: 'b', data: boolean }
    | { tag: 'c', data: number }

const myUnion: DiscriminatedUnion = {
    tag: 'a',
    data: true,  // <-- wrong, should be string
}

🙁 Actual behavior

Error from the type checker:

Type '{ tag: "a"; data: true; }' is not assignable to type 'DiscriminatedUnion'.
  Type '{ tag: "a"; data: true; }' is not assignable to type '{ tag: "c"; data: number; }'.
    Types of property 'tag' are incompatible.
      Type '"a"' is not assignable to type '"c"'.

🙂 Expected behavior

There should be an error from the type checker, but not that Type '"a"' is not assignable to type '"c"'.. In this example, "c" doesn't have anything to do with the issue, the problem is that data should have been a string, but it was actually a boolean.

I'm not sure what the exact error message should be in this case, something along the lines of `Type 'boolean' is not assignable to type 'string'.

davimiku avatar Jan 22 '23 20:01 davimiku

tag is a field with three possible types, and data is a field with three possible types. In terms of determining which is the "real" discriminant, there's not really a principled way to pick one over the other. The code to find this picks the first one it finds that looks like a real discriminant, and I don't think that it'd be worthwhile to turn this into a ranking algorithm vs a straightforward filter.

RyanCavanaugh avatar Jan 23 '23 16:01 RyanCavanaugh

In terms of determining which is the "real" discriminant, there's not really a principled way to pick one over the other.

Shouldn't it always pick tag as the discriminant in this case, since data is not literal-typed? And in the case that it picks data because boolean is true | false, then it seems weird that it's picking as a candidate the one where data is a number

fatcerberus avatar Jan 23 '23 17:01 fatcerberus

then it seems weird that it's picking as a candidate the one where data is a number

Yeah, that part is weird. I missed that part.

What I was trying to get at is that either message has equal standing to claim to be the correct one:

  • The discriminant is wrong based on the data type, so 'a' should be 'b'
  • The data type is wrong based on the discriminant, so the boolean should be a string

So in this case, referring to c/number is bad.

RyanCavanaugh avatar Jan 23 '23 18:01 RyanCavanaugh

@fatcerberus I guess what the type checker is doing is:

Is it '{ tag: 'a', data: string }'? No
Is it '{ tag: 'b', data: boolean }'? No
Is it '{ tag: 'c', data: number }'? No - show error to user

So it looks like the error is shown with "c" because that happens to be the final case that is checked.

If I understand @RyanCavanaugh correctly, it may not be possible (or practical) to find a heuristic to know which of the fields are the discriminant. Since the discriminant/tag is "implicit" as compared to F#/OCaml/whatever, it's not clear to the compiler which field is actually the tag. There might be a combinatorial explosion of possibilities if multiple fields could be discriminants (is that possible?)

Ideally, the compiler work flow is something like:

  1. Determine if the union type is a discriminated union
  2. Match the discriminant with the type being checked
  3. If discriminant was matched, type check the rest of the fields

But maybe this is not a practical/possible workflow depending on the possibilities of what would need to be checked.

davimiku avatar Jan 23 '23 18:01 davimiku

There's logic to find a discriminant property for error reporting (and other) purposes, e.g. this code will give you a different error depending on the tag

type DiscriminatedUnion =
    | { tag: 'a', alpha: string }
    | { tag: 'b', beta: boolean }
    | { tag: 'c', gamma: number }

const myUnion: DiscriminatedUnion = {
    tag: 'b'
}

but (per user feedback!) discriminants need not be only literal types:

type DiscriminatedUnion =
    | { tag: string, astring: string }
    | { tag: number, anumber: boolean }
    | { tag: boolean, aboolean: number }

const myUnion: DiscriminatedUnion = {
    tag: 42
}

So both data and tag are considered discriminants in the OP

RyanCavanaugh avatar Jan 23 '23 18:01 RyanCavanaugh

Here's what's going on: The checker classifies both tag and data as potential discriminant properties. The reason data is considered a discriminant is because type boolean is a union of two literal types (we just require the presence of one literal type or union thereof). Upon discovering more than one candidate discriminant we decide not to discriminate at all and pick the last type in the union for error elaboration. We probably should just pick the first candidate discriminant since that'll at least report something more meaningful.

ahejlsberg avatar Jan 23 '23 21:01 ahejlsberg

This is fixed in 5.1.6. The error is now:

Type '{ tag: "a"; data: boolean; }' is not assignable to type 'DiscriminatedUnion'.
  Types of property 'data' are incompatible.
    Type 'boolean' is not assignable to type 'string'.

rotu avatar Jun 30 '24 03:06 rotu

As stated above, this is resolved: Playground Link

davimiku avatar Nov 16 '24 00:11 davimiku