Skip to content

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

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

Open
ds84182 opened this issue May 20, 2023 · 8 comments
Labels
request Requests to resolve a particular developer problem

Comments

@ds84182
Copy link

ds84182 commented May 20, 2023

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
Copy link
Author

ds84182 commented May 20, 2023

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
Copy link
Author

ds84182 commented May 20, 2023

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
Copy link
Member

lrhn commented May 21, 2023

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.

@lrhn lrhn closed this as completed May 21, 2023
@ds84182
Copy link
Author

ds84182 commented May 21, 2023

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
Copy link
Member

lrhn commented May 23, 2023

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.

@lrhn lrhn reopened this May 23, 2023
@lrhn lrhn transferred this issue from dart-lang/sdk May 23, 2023
@lrhn lrhn changed the title [analyzer][cfe] base mixin that implements a sealed class counts towards exhaustiveness (& related oddities) base mixin that implements a sealed class counts towards exhaustiveness. Maybe allow sealed mixin. May 23, 2023
@lrhn lrhn added the request Requests to resolve a particular developer problem label May 23, 2023
@mateusfccp
Copy link
Contributor

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
Copy link
Member

lrhn commented Oct 23, 2023

Another case of this: dart-lang/sdk#53809

@TekExplorer
Copy link

it is considered too confusing to have something visibly marked mixin which cannot be mixed in.

That doesnt make sense to me. if its sealed, which is the first thing you see, then you already know it cant be used at all by your classes, so it being a mixin or not is completely irrelevant, and should be considered an implementation detail.

that implies that sealed class is confusing because its a class you cant extend. perhaps its not exactly the same, but its similar enough.

I often find myself wanting to use sealed mixins so that I can take advantage of mixin features while achieving exhaustiveness.

I see no reason why sealed would not be allowed on any implementable type, mixin, class, or any combination thereof.

hell, perhaps the analyzer should just not show anything other than sealed in hints, because thats all consumers should care about.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests

4 participants