Flow: Flow doesn't always check for incompatible enum comparisons

Created on 31 Jul 2018  Â·  2Comments  Â·  Source: facebook/flow

Flow's "string literal is incompatible with enum" error doesn't always apply to object properties.

If I access an enum through the normal object.property syntax or through variable destructuring, flow infers the enum type correctly but does not complain about comparisons with incompatible strings.

If I access the enum through a type-annotated parameter or variable destructuring, flow helpfully provides an error if I accidently compare it with an incompatible value.

Examples:

// @flow

type SampleEnum = 'FOO' | 'BAR';
type Props = { a: SampleEnum };

function Component1({ a }: Props) {
  (a: SampleEnum);

  if (a === 'invalid') { // error
  }
}

function Component2(props: Props) {
  (props.a: SampleEnum);

  if (props.a === 'invalid') { // no error
  }
}

function Component3(props: Props) {
  const { a } = props;

  (a: SampleEnum);

  if (a === 'invalid') { // no error
  }
}

function Component4(props: Props) {
  const { a }: Props = props;

  (a: SampleEnum);

  if (a === 'invalid') { // error
  }
}

This came up in a React project, where Flow caught my typos in functional components (where I usually destructure props), but not in class-based components (where props must be accessed through this.props).

My workaround for now is to add a duplicate Props type annotation like the fourth example, but I would expect Flow to detect an error based on the type of the props parameter alone.

unionintersections bug

Most helpful comment

I encourage you to structure your code such that you do not rely on
incompatible enum comparisons raising a type error, for reasons that
I’ll describe below. There are two techniques that you can use to avoid
this.

The first is to use empty-assertions, which give you a much stronger
guarantee in the cases where they make sense. In your SampleEnum
example, you can write:

// @flow

type SampleEnum = "FOO" | "BAR";
type Props = { a: SampleEnum };

function Component2(props: Props) {
  switch (props.a) {
    case "FOO":
      // do something
      break;
    case "BAR":
      // do something
      break;
    default:
      throw new Error("unexpected enum value: " + (props.a: empty));
  }
}

Note the assertion (props.a: empty) in the last clause. This asks Flow
to prove that there are no values that props.a can legally take
on—i.e., that this case is impossible. This code typechecks because Flow
can verify that case "FOO" and case "BAR" cover all the cases for a
value of type SampleEnum.

If you accidentally write case "FOOLISH" instead of case "FOO", then
you will get a helpful type error in the default case:

Cannot cast props.a to empty because string literal FOO [1] is
incompatible with empty [2].

Here, Flow tells you exactly what case you have forgotten.

This approach is powerful because it is robust not only to typos, but
also to new enum cases added later. If tomorrow you decide to add a
"BAZ" case to the definition of type SampleEnum, then you’re going
to have to find and update all the code that depends on there only being
two values for SampleEnum. If you consistently use empty-assertions,
then this daunting task becomes trivial: just add the "BAZ" case to
the type, run Flow, and fix all the errors that Flow tells you about.

Often, when people reach for incompatible enum comparisons to raise an
error, what they really want is an exhaustive switch over the
potential values of the enum.

The second technique applies in cases where an empty-assertion really
isn’t what you need. You can explicitly ask Flow to verify that the
right-hand side is of enum type. In your example, this looks like either
of the following:

// @flow

type SampleEnum = "FOO" | "BAR";
type Props = { a: SampleEnum };

function f1(props: Props) {
  (props.a: SampleEnum);
  if (props.a === ("invalid": SampleEnum)); // error (good)
  if (props.a === ("invalid": typeof props.a)); // error (good)
}

function f2(props: Props) {
  const { a } = props;
  if (a === ("invalid": SampleEnum)); // error (good)
  if (a === ("invalid": typeof a)); // error (good)
}

I will note, however, that in developing a 20KLOC project typed entirely
with Flow, I’ve used empty-assertions many, many times, and I’ve not
needed the latter approach even once.

The reason that you should avoid relying on incompatible enum
comparisons is that this behavior is a fundamentally broken aspect of
Flow’s type system. It violates Liskov substitutability: you have a type
T = "FOO" | "BAR" that is a subtype of string, and yet not all
operations on string are valid on T. This means that you’ll always
be fighting against type inference, which will insert valid upcasts in
cases where you want there to be an error. It also means that it is not
possible for Flow to define useful semantics that handle all cases
consistently, so these kinds of problems will never go away.

Happy to answer any questions; let me know if something is unclear.

All 2 comments

I encourage you to structure your code such that you do not rely on
incompatible enum comparisons raising a type error, for reasons that
I’ll describe below. There are two techniques that you can use to avoid
this.

The first is to use empty-assertions, which give you a much stronger
guarantee in the cases where they make sense. In your SampleEnum
example, you can write:

// @flow

type SampleEnum = "FOO" | "BAR";
type Props = { a: SampleEnum };

function Component2(props: Props) {
  switch (props.a) {
    case "FOO":
      // do something
      break;
    case "BAR":
      // do something
      break;
    default:
      throw new Error("unexpected enum value: " + (props.a: empty));
  }
}

Note the assertion (props.a: empty) in the last clause. This asks Flow
to prove that there are no values that props.a can legally take
on—i.e., that this case is impossible. This code typechecks because Flow
can verify that case "FOO" and case "BAR" cover all the cases for a
value of type SampleEnum.

If you accidentally write case "FOOLISH" instead of case "FOO", then
you will get a helpful type error in the default case:

Cannot cast props.a to empty because string literal FOO [1] is
incompatible with empty [2].

Here, Flow tells you exactly what case you have forgotten.

This approach is powerful because it is robust not only to typos, but
also to new enum cases added later. If tomorrow you decide to add a
"BAZ" case to the definition of type SampleEnum, then you’re going
to have to find and update all the code that depends on there only being
two values for SampleEnum. If you consistently use empty-assertions,
then this daunting task becomes trivial: just add the "BAZ" case to
the type, run Flow, and fix all the errors that Flow tells you about.

Often, when people reach for incompatible enum comparisons to raise an
error, what they really want is an exhaustive switch over the
potential values of the enum.

The second technique applies in cases where an empty-assertion really
isn’t what you need. You can explicitly ask Flow to verify that the
right-hand side is of enum type. In your example, this looks like either
of the following:

// @flow

type SampleEnum = "FOO" | "BAR";
type Props = { a: SampleEnum };

function f1(props: Props) {
  (props.a: SampleEnum);
  if (props.a === ("invalid": SampleEnum)); // error (good)
  if (props.a === ("invalid": typeof props.a)); // error (good)
}

function f2(props: Props) {
  const { a } = props;
  if (a === ("invalid": SampleEnum)); // error (good)
  if (a === ("invalid": typeof a)); // error (good)
}

I will note, however, that in developing a 20KLOC project typed entirely
with Flow, I’ve used empty-assertions many, many times, and I’ve not
needed the latter approach even once.

The reason that you should avoid relying on incompatible enum
comparisons is that this behavior is a fundamentally broken aspect of
Flow’s type system. It violates Liskov substitutability: you have a type
T = "FOO" | "BAR" that is a subtype of string, and yet not all
operations on string are valid on T. This means that you’ll always
be fighting against type inference, which will insert valid upcasts in
cases where you want there to be an error. It also means that it is not
possible for Flow to define useful semantics that handle all cases
consistently, so these kinds of problems will never go away.

Happy to answer any questions; let me know if something is unclear.

@wchargin thanks, this is quite enlightening and explains a lot. The original examples seem to throw error since 0.84 (without plugging in empty to help flow with exhaustive enum check) — https://flow.org/try/#0C4TwDgpgBAyghgWzAGwgUQHYFcFQLxQDkAYgPKmFQA+RAQgIIBKhA3AFCiRQAKATgPZgAzvigBvKHABcsRCnTZcAX3ZsAZlgwBjYAEt+GKAGF+SAxAzAATAAowA4TL6ChASnFsoUOw6EA6aVkkVEwcV3YvTyhdNW97FwD8PAJCXQwANzhkXQATQncxKKU2JSA

Was this page helpful?
0 / 5 - 0 ratings

Related issues

cubika picture cubika  Â·  3Comments

ghost picture ghost  Â·  3Comments

bennoleslie picture bennoleslie  Â·  3Comments

damncabbage picture damncabbage  Â·  3Comments

jamiebuilds picture jamiebuilds  Â·  3Comments