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
Handle supermixins #288
Comments
Supermixins in the form used by Flutter will not be supported in Dart 2. We expect to introduce a separate notion of mixins. So we may have a |
That's good to know! Am I getting the right impression that it is not worth for us to enable supermixin in analysis, and aim for the new mixin format whenever it arrives? |
Absolutely. The VM will also not support super mixins in Dart 2. It will only exist under a flag so Flutter will have time to migrate to the new syntax that we will (hopefully) introduce in a Dart 2.1 update. |
But the VM will support the new syntax, and that syntax will provide feature parity with the current Flutter uses? /cc @Hixie |
Use cases we care about:
|
@Hixie these use cases are valid, and I believe @lrhn and @eernstg will be able to point you toward the work that will go into the tooling (e.g. Right now we are a bit too restrictive, as we are treating non-Flutter packages as they were not supporting super-mixins, which is true only for the web platform, while Dart VM could support them. Fixing that is possible, but it would be valid for only a short time period, and after that there would be different mixin syntax, rules (and appropriate @tvolkert: |
@Hixie Our planned feature should be able to handle those cases. Tentative syntax: mixin Mixin on A, B, C implements W {
/// body
foo() => super.methodFromA();
} and allowing The constructor forwarding issues are separate, but we still plan to fix them. |
Wonderful! |
@eernstg: what's the latest on the supermixin support in Dart2? Should we still wait with this in |
The plan is still as stated above: Dart 2 will not have support for extracting a mixin from a class whose superclass is different from You may be able to use So for a published package like |
I should have been more clear about the question: should If we should, what would be the best way for it? E.g. run |
Ah, sorry, I didn't establish enough context to be able to give a reasonable response. Flutter uses super-mixins so it would be unacceptably breaking to emit super-mixin warnings for everything that is intended to run on that platform. At some point, almost everything on Flutter will need to be adjusted in order to stop using super-mixins and start using the new So if you have already detected that a given package is intended to be used on Flutter only, then I'd recommend running For anything that might be used on some other platform than Flutter, I'd recommend running Otherwise there is this tricky case where super-mixins are actually used, and that's because the developers of that package want it so, but there is no concrete evidence that the given package is Flutter-only (it might be intended to be Flutter+VM, or it might be intended to be Flutter-only). Do you have any kind of options for I'm not quite sure what 'scan the parsed source codes for patterns' means, so I won't comment on that. |
Thanks, that's good insight, and I have one follow-up question that could influence what actions we shall take now: would the web platform ( |
Definitely yes. |
In that case the current lack of supermixin support in We could change the order of processing, and do the platform analysis before calling |
Are you generally running the analyses (language checks, lints, custom analyzers, whatever it is that you're running) in a maximally permissive configuration? That is, do you want to have as few diagnostic messages as possible, skipping silently over every issue that you could have chosen to flag, simply because there is a way (say, an option) to ignore it? If that's your policy then it makes sense to allow the use of supermixins in every situation where it could possibly work. However, there are several reasons why this might be a little too optimistic. Let's use F to denote is the feature that has been known as 'super-mixins' for a while, and let G denote the new Flutter and the VM have feature F and the web platform does not have F. In the future we will add feature G to all platforms and delete F. G has a different syntax and a different semantics than F, but we will strive to make the transition as smooth as possible. It is true that G can be used to express roughly the same thing as F, but there may be a need to introduce new entities (say, adding new classes along with the refactoring that changes some existing classes into
That would match the situation if we were simply planning to add F to the web platform, but it doesn't match the situation so well when F is going to die and G will be added to all platforms. The following sounds like a very useful approach:
.. because that would make it possible to detect Flutter-only packages (where supermixins must be allowed). However, the approach you mention is much more permissive:
.. that is, allow the use of super-mixins also in all packages that aren't exclusively for the web platform, e.g., every package that is intended to be used on more than one platform. So if you want to be as permissive as possible you can of course use this approach. But it's definitely not a good idea to actively support the use of super-mixins outside Flutter at this point, so I would prefer a mechanism where supermixins are not automatically accepted for anything except "Flutter-only" packages. |
Yeah, that's a good point. Even if we unblock the not-too-many packages that are using e.g.
This is exactly what we have today. Let's keep it the current way for now, I'm closing this issue. I have a few updates from the half-baked-PR related to this change that are worth to submit, regardless of the supermixin support, but will the main thing intact. |
Sounds good, thanks! |
Dart VM supports them, web platform does not.
We should either (a) allow it and disqualify
web
platform when its use is detected or (b) run two analysis (enabled/disabled), and if they differ, use the enabled one and disqualifyweb
.The text was updated successfully, but these errors were encountered: