Can we temporarily remove "cannot extend mixins" limitation?

[yjbanov]

(branched off #31 (comment))

Goal

Migrate all mixins (not only super-mixins) to the new mixin syntax.

Background

Classes that were meant to be super-mixins already had restrictions matching those of the new mixin keyword. This makes the migration from class to mixin for super-mixins simple. That's not the case for non-super-mixins. Those classes are extended, implemented, and mixed in in the wild. Therefore changing what previously was declared as a class to a mixin breaks classes that used extends instead of with.

Proposal: temporarily allow extending mixins

• It does not require migration of any code that uses mixins, only code that defines mixins
• extends and with have no semantic differences observable by the developer
• It does not conflict with the new mixin semantics

Beyond that, this allows us to remove the ability to derive a mixin as a non-breaking change. To do that we will have to overload what extends means.

Imagine this declaration:

class Foo extends Bar

The compiler first checks whether Bar is a mixin or a class.

If Bar is a mixin, then the compiler desugars the above declaration into:

class Foo extends Object with Bar

More generally, class Foo extends M1 with M2, ..., Mk desugars into class Foo extends Object with M1, M2, ..., Mk.

If Bar is a class, then it's a normal derivation. The lint prefer_mixin then can be used to help authors find classes used as mixins and convert them to mixin syntax without breaking their users.

This approach will allow converting the vast majority of mixins present in the Dart ecosystem to the new mixin syntax without breaking existing user code. For example, Flutter can update abstract class WidgetsBindingObserver to mixin WidgetsBindingObserver as a non-breaking change.

Changing from class to mixin still comes with mixin restrictions, just like new mixin proposal wants, such as:

• mixins cannot extend other classes
• mixins cannot have constructors and factories
• mixins cannot be instantiated directly

These properties will protect mixin authors from accidentally breaking their users.

Discussions elsewhere

This topic was discussed on these other threads:

• use WidgetsBindingObserver as a mixin flutter/plugins#797
• Enforce that WidgetsBindingObserver is only used as a mixin or interface flutter/flutter#22290
• Is mixin for non-supermixins a good idea? #33

Updates

• Added a "Goal" section
• Removed argument that A extends M is simpler syntactically due to existence of A with M
• Created a section with links to other discussions

@leafpetersen @lrhn @eernstg @Hixie

[Hixie]

Personally I have no objection to being able to use "with" with a true class. I would not want a lint that discourages such use.
I don't see much value in the "mixin" keyword. I'm ok with having it if it makes the language easier to process in the context of supermixins. I would not encourage people to move to that keyword except where required by the supermixin restrictions.
In particular, I would not change WidgetsBindingObserver to use the "mixin" keyword.

[yjbanov]

Yeah, I should have mentioned that @Hixie and I do not agree on everything mixin-related (e.g. I disagree with #32 (comment)). However, we both agree that the restriction on extends is excessive.

Update: added "do not" in a critical place of the previous paragraph! 😄

[matanlurey]

I think the language is/was relaxing with, i.e. this would be legal:

class Foo with Bar {}

... negating the need to use extends Bar to avoid the boiler-plate of extends Object with Bar.

[yjbanov]

@matanlurey this is mentioned as a future possible feature, but that's not what's launching in Dart 2.1.

The second trouble with with is that it still requires that we update all the call-sites. Therefore, migrating from class to mixin is no longer limited to mixin declarations. The difference in scope is several orders of magnitude.

[matanlurey]

@yjbanov:

but that's not what's launching in Dart 2.1.

🤕

The second trouble with with is that it still requires that we update all the call-sites.

It seems reasonable that, for Dart >= 2.1.0 < 3.0.0, a mixin M should allow C extends M. In say, 2.2.0, when a C with M is allowed, a compile-time hint could be issued to suggest users refactor C extends M to C with M.

To be honest though, are there concrete reasons to prevent C extends M? I've seen classes that are intended to be used a mix-in, but I don't recall ones that are intended to be used as a mix-in but are prohibited to be used as a base class.

(I don't know if the new mixin M on T syntax makes C extends M impossible)

[yjbanov]

FYI, I filed #33 to discuss whether the mixin keyword is good for non-supermixins. Please post your thoughts there. I'd like this issue to stay focused on the extends keyword.

[yjbanov]

To be honest though, are there concrete reasons to prevent C extends M? I've seen classes that are intended to be used a mix-in, but I don't recall ones that are intended to be used as a mix-in but are prohibited to be used as a base class.

In this issue I suggest that extends changes its meaning depending on whether you are extending a class or a mixin, which removes the reasons for having the restriction. In the vast majority of cases the difference is not observable by developers, so there's no need to teach them about extra concepts.

[lrhn]

This is mentioned as a future possible feature, but that's not what's launching in Dart 2.1.

I do believe class C with M {} is planned for Dart 2.1, to be released at the same time as mixin declarations. Both the front-end parser and the analyzer already support the syntax.
(It's just equivalent to class C extends Object with M {}, so it's not a big feature to implement).

[lrhn]

Let's be specific.
@yjbanov You are suggesting that the following should be valid:

mixin M {}
class B {}
class C1 extends M {}
class C2 with M {}
class C3 extends B {}
class C4 with B {}

Here the line extends M changes meaning to really mean with M (aka. extends Object with M), and the rest keep their behavior.

Should we also allow:

mixin M2 {}
class C5 extends M with M2 {}
class C6 extends M, M2 {}

?
I'd argue vehemently against the latter, and merely strongly against the former.
The latter is very, very confusing, because you can only do that with mixins, not with classes, so the user still needs to know the difference. (I believe they always need to know, assuming you can use Dart correctly without knowing the difference will not end well).

The former would mean that we are not just reinterpreting extends M as with M, we are rather reinterpreting it as extends _Object$M where we declare class _Object$M = Object with M; implicitly (which isn't a bad definition of it).

So, yes, we can definitely allow extends Mixin as equivalent to extends _Object_with_Mixin.
We chose not to because it requires users to keep mixins and classes separate.

[yjbanov]

@lrhn I agree that, ignoring existing Dart code for a moment, class C extends M doesn't make sense given that class C with M exists (which I didn't know until now!). However, we can't ignore existing Dart code. It will make the migration much easier if we continued supporting class C extends M for some time by rewriting it to class C with M in the compiler. We do have Flutter non-supermixins that are extended by users. I also saw dart: mixins extended in the wild, such as extends ListMixin. Given how easy it was to find these real world occurrences, we must have a lot of these in the wild.

We could have an analyzer hint suggesting that users replace extends with with for, say, 6 months, before enforcing it.

Because this is mostly for migration (I agree that class C with M is as simple as class C extends M), I do not suggest that we support class C6 extends M, M2. Such code does not exist in the wild.

class C5 extends M with M2 has a decent chance of existing in the wild. However, I have not seen examples of that. I would say let's support it and rewrite it to class C5 with M, M2, because we don't want to be surprised later? I'm not super sure about it.

[leafpetersen]

I would not encourage people to move to that keyword except where required by the supermixin restrictions.
In particular, I would not change WidgetsBindingObserver to use the "mixin" keyword.

This comment and surrounding discussion had led me to think your desire was to migrate all mixins to the new syntax. If this is not the case, we should just change the lint to match up what you want. My main goal is to get the super mixins migrated over.

[leafpetersen]

The former would mean that we are not just reinterpreting extends M as with M, we are rather reinterpreting it as extends _Object$M

@lrhn can you elaborate on why? Maybe it's just late, but I'm not sure I'm seeing how you can observe the difference in the non-super-mixin case.

[leafpetersen]

My take on this:

• I don't want to allow class C extends M when M has a superclass constraint
  • this really changes the feature in bad ways, and I'd much rather just have flutter keep using "classes as mixins" for the non-super-mixins than change the feature in this way.
• I could see allowing class C extends M when M has no superclass constraint
  • subject to the interpretation that this means class C extends Object with M, rather than class C extends _Object$M where _Object$M is the canonical application of M.
  • I don't love this - it provides two ways to do the same thing

[Hixie]

Yeah, looks like I was wrong in my comment last August, there are classes that make sense to use as both a superclass and a mixin. My bad.

I don't see much value in being able to extend a mixin.

[yjbanov]

there are classes that make sense to use as both a superclass and a mixin

@Hixie what properties of the class keyword are you looking for in a mixin that are not provided by the mixin keyword? Please, also provide example classes in Flutter that would benefit from these properties.

[yjbanov]

@Hixie

I don't see much value in being able to extend a mixin.

One value is that we can change non-supermixin-classes-meant-to-be-mixins to the mixin syntax, which brings benefits explained in the the original description of this issue, as a non-breaking change.

Making everything that's meant to be a mixin use the mixin syntax makes things simpler to the user. If we don't do this, we will end up with the following weirdness:

• mixin means super-mixin.
• class can mean mixin, class, or interface, but you'll have to read the source code and documentation to figure out which is it.

A simpler approach is:

• mixin means mixin (all kinds, super or otherwise).
• class means class or interface.

[yjbanov]

Update: updated the description of the issue to reflect the discussion.

[Hixie]

@Hixie what properties of the class keyword are you looking for in a mixin that are not provided by the mixin keyword?

The word "class" itself. WidgetsBindingObserver is the main example I have in mind.

I don't think "class can mean mixin, class, or interface, but you'll have to read the source code and documentation to figure out which is it" is that big a deal. That's always been the way it is in Dart 1, and I'm not aware of it causing any particularly dire problems.

The actual alternatives as I see them are:

Either:

• mixin means a mixin. The on clause determines what interfaces you have to subclass to use it.
• class means a class, which can be used as an interface, and which (if it doesn't have a superclass) can also be used as a mixin. You have to read the documentation to determine if using it as an interface is likely to work (it might not, e.g. if it uses privates).

or:

• mixin means a mixin, which (if it doesn't have an on clause) can also be used as a superclass. The on clause determines what interfaces you have to subclass to use it.
• class means a class, which can be used as an interface. You have to read the documentation to determine if using it as an interface is likely to work (it might not, e.g. if it uses privates).

[lrhn]

@lrhn can you elaborate on why? Maybe it's just late, but I'm not sure I'm seeing how you can observe the difference in the non-super-mixin case.

Purely being literal here. If we interpret extends M, where M denotes a mixin declaration, as with M, then class C extends M1 with M2 {} would become class C with M1 with M2 {}. We would have to rewrite that to class C with M1, M2 {}, which is obviously simple to do, as long as we are aware of it. So, nothing deep, it's just not a completely local rewrite.

The rule would be to rewrite extends M1 with M2, .., Mn where M1 denotes a mixin declaration, into with M1, M2, ..., Mn. Still simple and uncontroversial, so not really an issue, but it suggests that the user is not aware of the distinction between classes and mixins, and we probably want to make them aware.

[lrhn]

My main worry about introducing a temporary permission to extend a mixin is that I don't see a good way to pick a duration for it, or a simple way to phase it out again.

It's a feature that we don't actually want. Adding it temporarily is extra work, so that work had better pay off, but not so well that we get stuck with the feature. So, we want to incorporate an inconvenient feature.

If we allow extending mixins, we will immediately make it cause a warning to do so. The warning will be a "deprecation" warning, so people are encouraged to migrate their code. That still doesn't tell us when the deprecation/migration period ends. The "correct" choice would likely be Dart 3.0 - it is a breaking change to remove the feature. We don't have a schedule for reaching 3.0. It's just a number, we can pick it as a version number any time we feel like we have a big enough change (I'd probably use it for non-null by default, if we get that feature).

Another option is to make it a "migration only" feature, and remove it after a fixed period, say six months.
We would have to consider which situations would then be able to block us from removing the feature after that time. It will be breaking for all unmigrated code.

We can "encourage" migration by making it increasingly more inconvenient to use the feature.
Say, we start out by requiring a flag --deprecated-extend-mixin=6. It only works if the value is 6. Each month we decrement the number you have to put there to enable the feature, so you will be repeatedly reminded that you are opting in to deprecated behavior.

The issue here is that the person building is not necessarily the person who needs to migrate the offending package. It would put indirect some pressure on package writers to update their code (from extends M to with M), but an unmaintained package will never be updated. (That might be a good sign that you should find an alternative, though).

[yjbanov]

@Hixie thanks for the details. So, there's no loss in technical generality from moving from class to mixin that we're worried about. You prefer class as a matter of personal preference, which is fine and should be taken into account by the language team.

@lrhn I think Dart 3.0 timeline would work for the temporary extends M1 with M2 => with M1, M2 rewrite + prefer_mixin lint. It will give a long enough period both for stability and migration. I believe the prefer_mixin lint is sufficient for steering the ecosystem towards the desired outcome. Perhaps by Dart 2.2, if we observe that old-style mixins are rarely used in poorly maintained packages (e.g. using pub's scoring system), the warning can be elevated from a lint to a compiler warning to accelerate the process.

My own position is still that we should convert all mixins to the mixin syntax. As a personal preference, I think it gives developers a much simpler framework (you don't have to explain to anyone that a class may also be a mixin in certain situations). As a technical reason, I like a lot the idea that the compiler can use the mixin keyword to enforce mixin invariants, making sure mixin authors do not break users accidentally. This is not possible with classes.

At the end of the day, this is the language team's call. Please let us know how to proceed. The only issue I'm currently struggling with is that I'm technically prevented from converting non-supermixins to the new mixin syntax because it breaks the usages of extends, and therefore I'm asking for a softer transition strategy.

[lrhn]

I understand the pain, and I'd personally love to see classes used as mixins go the way of the dodo. However, we are not in a position to make such a change at this time. The features for Dart 2.1 are being finalized, and we don't have the time and available man-power to make a change at this point, even one as small as this (and things are never as small as they seem). For now, if you want to have something that can both be extended and mixed in, it will have to stay as a class. /L

…

On Tue, Oct 2, 2018 at 8:18 PM Yegor ***@***.***> wrote: @Hixie <https://github.com/Hixie> thanks for the details. So, there's no loss in technical generality from moving from class to mixin that we're worried about. You prefer class as a matter of personal preference, which is fine and should be taken into account by the language team. @lrhn <https://github.com/lrhn> I think Dart 3.0 timeline would work for the temporary extends M1 with M2 => with M1, M2 rewrite + prefer_mixin lint. It will give a long enough period both for stability and migration. I believe the prefer_mixin lint is sufficient for steering the ecosystem towards the desired outcome. Perhaps by Dart 2.2, if we observe that old-style mixins are rarely used in poorly maintained packages (e.g. using pub's scoring system), the warning can be elevated from a lint to a compiler warning to accelerate the process. My own position is still that we should convert all mixins to the mixin syntax. As a personal preference, I think it gives developers a much simpler framework (you don't have to explain to anyone that a class may also be a mixin in certain situations). As a technical reason, I like a lot the idea that the compiler can use the mixin keyword to enforce mixin invariants, making sure mixin authors do not break users accidentally. This is not possible with classes. At the end of the day, this is the language team's call. Please let us know how to proceed. The only issue I'm currently struggling with is that I'm *technically* prevented from converting *non-supermixins* to the new mixin syntax because it breaks the usages of extends, and therefore I'm asking for a softer transition strategy. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#32 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AEo9B3DjZ0EuVins5wiriyHaKADgci43ks5ug64NgaJpZM4W9p2i> .

-- Lasse R.H. Nielsen - lrn@google.com 'Faith without judgement merely degrades the spirit divine' Google Denmark ApS - Frederiksborggade 20B, 1 sal - 1360 København K - Denmark - CVR nr. 28 86 69 84

[yjbanov]

Leaving them as classes then. I'll instead file an issue for prefer_mixin to check for super calls so we can use that lint.