Enhanced enum type parameters do not work well in functions with switch statements

[hacker1024]

Consider the following example:

enum TimestampValueType<T> {
  unknown<void>(),
  dateString<String>(),
  microseconds<int>(),
  milliseconds<int>(),
  seconds<int>();

  DateTime convert(T timestamp) {
    switch (this) {
      case TimestampValueType.unknown:
        throw ArgumentError('Timestamp scheme is unknown.');
      case TimestampValueType.dateString:
        return DateTime.parse(timestamp);
      case TimestampValueType.microseconds:
        return DateTime.fromMicrosecondsSinceEpoch(timestamp);
      case TimestampValueType.milliseconds:
        return DateTime.fromMillisecondsSinceEpoch(timestamp);
      case TimestampValueType.seconds:
        return DateTime.fromMillisecondsSinceEpoch(timestamp * Duration.millisecondsPerSecond);
    }
  }
}

I'd expect to be able to implement the convert function in this way, where the timestamp argument type is known in each switch branch based on the matched enum value.

At the moment, though, this code does not even compile, even when casting timestamp usages.

Every case statement results in an error like the following, where X is the type argument given in the relevant enum declaration:

Error: Type 'TimestampValueType<X>' of the case expression is not a subtype of type 'TimestampValueType<T>' of this switch expression.

[lrhn]

You do not get promotion from identical checks in Dart. That includes those done by switch cases. So, the type of this doesn't change just because it happens to be identical to something you know the type of.

Your class is generic. That means that it treats a family of types, those accepted as type arguments, generically. You would have the same issues if it wasn't an enum. You can promote this internally if you want to, like: if (this is TimestampValueType<int>) ... but that allows you to treat this that way, not T timestamp. To promote that, you still need to do (timestamp as int).

[hacker1024]

Ah, right. The casting issue isn't a problem then, but the switch statement itself is also broken. Even with nothing inside the case blocks (as written below), the error mentioned above is still generated.

enum TimestampValueType<T> {
  unknown<void>(),
  dateString<String>(),
  microseconds<int>(),
  milliseconds<int>(),
  seconds<int>();

  void test() {
    switch (this) {
      case TimestampValueType.unknown:
      case TimestampValueType.dateString:
      case TimestampValueType.microseconds:
      case TimestampValueType.milliseconds:
      case TimestampValueType.seconds:
      default:
        return;
    }
  }
}

Upcasting this with switch(this as TimestampValueType<Object?>) works around this issue, but surely that shouldn't be necessary? The linter even marks it as an unnecessary cast. Explicitly declaring the enum as enum TimestampValueType<T extends Object?> doesn't help either.

[lrhn]

It perhaps shouldn't be necessary. Good point. That's a language issue, implementations are acting as currently specified. Moving this to the language repo.

We used to require that the type of case expressions of switch should be instances of the same class as the switch expression. We changed that for Null Safety to require it being a subtype.

That fails when switching on this inside a generic class, because the switch expression has type C<T> and the constants have type, say, C<int>. There is no subtype relation between the two. It's not just for enums, you can get the same problem for any generic class:

class C<T> {
  const C();
  static const C<int> x = C<int>();
  static const C<double> y = C<double>();
  static const C<String> z = C<String>();
  
  void m() {
    switch (this) {
      case C.x: print("x"); break; // The switch case expression type 'C<int>' must be a subtype of the switch expression type 'C<T>'.
      case C.y: print("y"); break; // The switch case expression type 'C<int>' must be a subtype of the switch expression type 'C<T>'.
      case C.z: print("z"); break; // The switch case expression type 'C<int>' must be a subtype of the switch expression type 'C<T>'.
    }
  }
}

So, this is working as intended. It might just be a corner we hadn't considered.

Should we change the "subtype of the static type of e" to be "subtype of the static type of e with type variables instantiated to bound"?

[hacker1024]

I'm glad that this is working as intended, and that an improvement is being considered. In the meantime, should another issue be filed for the analyzer/linter? It marks the cast in switch(this as TimestampValueType<Object?>) as unnecessary, even though its removal results in compilation errors.

[srawlins]

We unfortunately have a few "false positive" bugs filed for unnecessary_cast (and related: unnecessary_type_check, I think). See this search.

We haven't given them too much priority, but I would personally like to close the loop on these issues. You could look through and see if an issue is filed for your case here and file a new one if it is not represented.

[eernstg]

We'd want to avoid switch case expressions whose type implies that there could never be a match, that is, we should probably change the rule such that it is an error unless such case expressions have a type which cannot be a subtype of the static type of the scrutinee for any possible value of type variables in scope.

This means that it is concerned with cases where the type of the scrutinee has type variables from an enclosing generic function, or from an enclosing generic class/mixin/extension.

This is not necessarily the same thing as using values for the type variables which are obtained by instantiation to bound. The following example illustrates that we may encounter a scrutinee whose value for X is a supertype of the declared bound (because that makes the given object a subtype), as well as cases where it is a subtype of the declared bound.

class C<X> {
  final X x;
  const C(this.x);
}

void gDyn(dynamic _) {}
void gNum(num _) {}
void gInt(int _) {}
void gNever(Never _) {}

void f<X extends num>(C<void Function(X)> c) {
  switch (c) {
    case C(gDyn): break; // OK.
    case C(gNum): break; // OK.
    case C<void Function(int)>(gInt): break; // Error reported, should be accepted.
    case C<void Function(Never)>(gNever): break; // Error reported, should be accepted.
  }
}

void main() {
  f(const C(gDyn)); // The type argument of `f` is `num`.
  f(const C(gNum)); // .. is `num`.
  f(const C(gInt)); // .. is `int`.
  f(const C(gNever)); // .. is `Never`.
}

The example also illustrates that we may wish to use a different context type for each case expression (the actual type arguments for the third and fourth cases are required in order to get the specified value and make the expression type correct).

[lrhn]

That's a good way to put it: In a switch case expression, we want to ensure that the expression (constant) value can be the same as the switch expression value.

In other situations, like variable assignments, we want to make sure that the assigned value can validly be used at the variable's type. We know that the value's runtime type is a subtype of its static type. Pessimistically we allow the assignment only if all possible instances of that static type satisfies the requirement. With generics involved, we want it to work for all instantiations of the type parameters.

With switch, the case expression's static type being a subtype of the switch expressions static type seems to catch what we want, but it fails if the switch expression type is generic. In that case, we want to accept the expression if there is (optimistically) any valid instantiations of the generic type that is a supertype of the case expression's type. We're not trying to ensure that it's always right. It doesn't have to, since cases can fail to match. We're trying to ensure that it's not always wrong, because then the switch case is dead and unnecessary.

I suggested instantiate to bounds, because it seemed like a useful approximation, but the precise test would be to infer types for free type variables in the switch expression's static type, based on the type of the case expression (and bounds if necessary), and if that succeeds, then the case is accepted. We would do that individually for each case expression (if several cases have the same static type, we can reuse the result).

As function the example above shows, we can't just infer one non-generic type and hope it works for every case expression. There can be invariantly occurring type variables that has to be matched to the precise type of each case expression individually.

[munificent]

We'd want to avoid switch case expressions whose type implies that there could never be a match, that is, we should probably change the rule such that it is an error unless such case expressions have a type which cannot be a subtype of the static type of the scrutinee for any possible value of type variables in scope.

Ah, good insight!

the precise test would be to infer types for free type variables in the switch expression's static type, based on the type of the case expression (and bounds if necessary), and if that succeeds, then the case is accepted. We would do that individually for each case expression (if several cases have the same static type, we can reuse the result).

That sounds right to me too.

It might make sense to roll these changes into pattern matching since that will also change a lot of behavior around switches.