Skip to content

Can we allow mixin on <SealedClass>? #3215

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
jakemac53 opened this issue Jul 13, 2023 · 6 comments
Open

Can we allow mixin on <SealedClass>? #3215

jakemac53 opened this issue Jul 13, 2023 · 6 comments
Labels
request Requests to resolve a particular developer problem

Comments

@jakemac53
Copy link
Contributor

jakemac53 commented Jul 13, 2023

There might be a better way to do this, if so please let me know :).

Consider a sealed class hierarchy, with private *Impl classes, and the *Impl classes also implement another shared interface.

Internally to my code, I want to only deal in the *Impl classes, but I can't declare a type which enforces something to be both the sealed class plus the additional private interface. I end up having to choose one or the other.

my_public_library.dart

sealed class Choice {}

abstract interface class ChoiceA implements Choice {}

abstract interface class ChoiceB implements Choice {}

my_private_library.dart

import 'my_public_library.dart';

class WithId {
  final int id;
  
  WithId(this.id);
}

/// Not allowed, but **might** be safe on its own?
///
/// I believe you would only be able to actually mix this into one of the impl classes.
mixin ChoiceImpl on Choice implements WithId  {}

class ChoiceAImpl extends WithId implements ChoiceA with ChoiceImpl {
  ChoiceAImpl(super.id);
}

class ChoiceBImpl extends WithId implements ChoiceB with ChoiceImpl {
  ChoiceBImpl(super.id);
}

internal_usage.dart

import 'my_private_library.dart';

class ThingWithChoice {
  /// This gives me access to both the `Choice` and `WithId` interfaces, while maintaining
  /// the sealed nature of `Choice` and hiding the `WithId` iterface from users.
  final ChoiceImpl choice;
}
@jakemac53 jakemac53 added the request Requests to resolve a particular developer problem label Jul 13, 2023
@jakemac53
Copy link
Contributor Author

jakemac53 commented Jul 13, 2023

To my horror, I have just realized we allow you to implement mixin types. That would mean this mixin does introduce a new real subtype of the sealed class, so it isn't safe.

Do we really want you to be able to implement a mixin type? Is that something people do? If we blocked that, then would we be able to allow this?

@lrhn
Copy link
Member

lrhn commented Jul 13, 2023

This was part of one of my designs along the way.
The trick is to require the mixin to be base-or-more and not allow ignoring that even inside the same library. Same thing we do when extending a base class from another library, so nothing new.

Then we can ensure that no instance of a subtype of a sealed class is not also an instance of one of the known direct subclasses, because the only way to create an instance implementering the mixin type, is to apply it to a real subclass of the on-type.

And if you don't want people implementing your mixin type, make it base.

@jakemac53
Copy link
Contributor Author

The trick is to require the mixin to be base-or-more and not allow ignoring that even inside the same library. Same thing we do when extending a base class from another library, so nothing new.

Yeah, I was discussing this at lunch with others and the idea of using base comes up but of course then I realized you can violate that within your own library today 😭 .

@jakemac53
Copy link
Contributor Author

jakemac53 commented Jul 13, 2023

The trick is to require the mixin to be base-or-more and not allow ignoring that even inside the same library. Same thing we do when extending a base class from another library, so nothing new.

Would it be reasonable to do this? Essentially say that you can't violate the normal rules (even in your own library) for a base mixin if the on type is a sealed type from another library? That sounds a bit complicated/special case for this but it would enable what I want to do here.

@lrhn
Copy link
Member

lrhn commented Jul 13, 2023

I already specified it, and fully believe it should be sound and safe.
We decided not to allow it because it was considered too confusing to allow a direct subtype of a sealed class in another library, not because it wasn't safe.

We would have to prevent everyone from implementing the mixin, in the same library or other libraries, even if the mixin is declared in a non-feature library. That's extra special, because they cannot write base on the mixin declaration.
The base is for your protection, and documentation, the constraint applies anyway.
Also transitively to mixins declared on the first mixin.

So it does get complicated.

@jakemac53
Copy link
Contributor Author

jakemac53 commented Jul 13, 2023

Ok, the juice might not be worth the squeeze then in this case. It does feel like a workaround, I was probably trying to be a bit too fancy.

What I really want here is intersection types anyways :).

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

2 participants