dart-lang / language

Design of the Dart language
Other
2.67k stars 205 forks source link

`base mixin` that implements a `sealed class` counts towards exhaustiveness. Maybe allow `sealed mixin`. #3103

Open ds84182 opened 1 year ago

ds84182 commented 1 year ago

Seeing this on 3.1.0-109.0.dev.

Only in Analyzer: Uncommenting Foo, like the enums, complains that the case will never match.

sealed class Sealed {}

enum Enum implements Sealed {
  value;
}

enum Enum2 implements Sealed {
  value;
}

final class Subclass extends Sealed {}

base mixin Foo implements Sealed {}

enum EnumFoo with Foo {
  value;
}

// The type 'Sealed' is not exhaustively matched by the switch cases since it doesn't match 'Foo()'.
//                   vvvvvv
int foo(Sealed s) => switch (s) {
      // The matched value type 'Sealed' can never match the required type 'EnumFoo'. Try using a different pattern.
      //vv
      Enum _ => 0,
      Enum2.value => 0,
      Subclass _ => 0,
      // The matched value type 'Sealed' can never match the required type 'EnumFoo'. Try using a different pattern.
      //vvvvv
      EnumFoo _ => 0,
      // The matched value type 'Sealed' can never match the required type 'Foo'. Try using a different pattern.
      // vvv
      // Foo _ => 0,
    };

void main() {
  foo(Enum.value);
}
ds84182 commented 1 year ago

I'm guessing exhaustiveness is not transitive through a base mixin using the implements clause? It works with an on clause but that can't be used with a mixin on an enum.

ds84182 commented 1 year ago

Ok, so according to the specification sealed mixin should be allowed but it isn't Because it isn't allowed you can't propagate exhaustiveness through a mixin, which wouldn't be an issue typically but because you can't use an extends clause on an enum you cannot actually have this class hierarchy.

https://github.com/dart-lang/language/blob/main/accepted/future-releases/sealed-types/feature-specification.md

lrhn commented 1 year ago

That is correct.

Exhaustiveness only propagates through sealed, and mixins cannot be sealed, because sealed prevents mixing in outside of the library, and it is considered too confusing to have something visibly marked mixin which cannot be mixed in.

What you can do is:

sealed class Sealed {}

enum Enum implements Sealed {
  value;
}

enum Enum2 implements Sealed {
  value;
}

final class Subclass extends Sealed {}

sealed class Foo implements Sealed {}

mixin _Foo {
  // Shared implementation of `Foo` which needs to be mixed in.
} 

enum EnumFoo with _Foo implements Foo {
  value;
}

int foo(Sealed s) => switch (s) {
      Enum _ => 0,
      Enum2.value => 0,
      Subclass _ => 0,
      EnumFoo _ => 0,
    };

void main() {
  foo(Enum.value);
}

That is, the type you want to expose doesn't have to be a mixin.

ds84182 commented 1 year ago

This does not actually work because using a private mixin breaks type inference within ternary and switch expressions. And the lack of implements on the mixin means that @override does not function (and actually lints, which is worse). This could all be mitigated if sealed classes could be used as mixins in the library they're defined in at the very least.

lrhn commented 1 year ago

Not sure how the private mixin breaks type inference, but I'm guessing it's our classic "UP is stupid" issue (which could be possibly be at least helped by not including declarations which are inaccessible when finding common superinterfaces). You can work around that by adding more classes, to push the ones you care about further from Object.

If you want to have @override on some of the abstract members of Foo, you can move those into a non-sealed superclass of Foo, which you can implement.

sealed class Sealed {}

enum Enum implements Sealed {
  value;
}

enum Enum2 implements Sealed {
  value;
}

final class Subclass extends Sealed {}

sealed class _UpIsStupid implements Sealed {}

abstract interface class _FooApi {
  int get foo;
}

sealed class Foo implements _UpIsStupid, _FooApi {}

mixin _Foo implements _FooApi {
  @override
  int get foo => 42;
} 

enum EnumFoo with _Foo implements Foo {
  value;
}

int foo(Sealed s) => switch (s) {
      Enum _ => 0,
      Enum2.value => 0,
      Subclass _ => 0,
      EnumFoo _ => 0,
    };

void main() {
  foo(Enum.value);
}

It's something of a workaround, just because you can't have a sealed mixin. I'll reopen the issue and move it to the language repository. It's not an implementation issue, the implementations are doing what the language specifies. You'll want to change the language to allow it.

mateusfccp commented 1 year ago

I agree that sealed mixin should be a thing. It should only prevent mixing outside of library, like sealed class do with extension. I don't think they are mutually exclusive and incompatible.

lrhn commented 1 year ago

Another case of this: https://github.com/dart-lang/sdk/issues/53809