Skip to content
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

Do not allow mixing in a class, with migration path #1643

Closed
lrhn opened this issue May 20, 2021 · 1 comment
Closed

Do not allow mixing in a class, with migration path #1643

lrhn opened this issue May 20, 2021 · 1 comment
Labels
feature Proposed language feature that solves one or more problems smaller-dart

Comments

@lrhn
Copy link
Member

lrhn commented May 20, 2021

Dart currently allows you to mix in (mixins derived from) class declarations, as long as they have Object as superclass and no generative constructors declared. That's not desirable, and we have wanted to disallow that for a long time.

The main problem with disallowing it is that some projects, Flutter included, expose class declarations which are currently used both as superclasses and as mixins. If they migrate those declarations to be mixin declarations, existing uses of extends ThatThing will become invalid until migrated to with ThatThing . It's a breaking change.

This is a proposal for a way to force the change without breaking existing code, by using language versioning.

Prior proposal

One prior proposal was to allow extending a mixin, extends Mixin, temporarily until migration of all classes to mixins had completed, then force those to change to with Mixin, and finally remove the ability to write extends Mixin. This is a somewhat circuitous route with many steps, and no good way to force each step.

This proposal instead does everything in one language version increment, and relies on the general motivation to catch up with language features as the motivation to migrate.

Proposal

We change the behavior of mixin application and superclass declarations in a new language version. In this document we'll just say "new code" about code opted into the new language version and "legacy code" about code that has not yet been migrated.

We then define the behavior of cross version mixin applications (where the mixin application and the mixin declarations, or class and superclass declarations, are not both new code or legacy code) such that migration of existing code becomes non-breaking, and as painless as possible.

All existing restrictions still apply, so where we allow mixing in a class, it must still be a class with Object as superclass and no declared generative constructors, and the mixed in members must be valid overrides of existing members with the same name. We then add further restrictions, and one extra allowance.

Mixin applications

It is a compile-time error if a mixin application mixes in a class declared in new code. New-code classes are never intended as mixins.

Optional: If a mixin application occurs in new code, it's a compile-time error if it mixes in a class declared in legacy code. If enabled, this rule will force migration in dependency order. If not, it's possible to migrate ahead of your dependencies, and assume (correctly or incorrectly) that the dependency will migrate a class to a mixin.

Mixin applications in legacy code can mix-in legacy class declarations, and both new and legacy mixin declarations.

Class extension

If a class declaration occurs in new code, it's a compile-time error if it extends a mixin declaration. New code must use with Mixin to get Object with Mixin as superclass.

If a class declaration occurs in legacy code, it's a compile-time error if it extends a mixin declared in legacy code.

A legacy class declaration can extend a mixin declared in new code, but only if its only on type is Object. If so, extends Mixin is treated as with Mixin, aka., extends Object with Mixin. This allows code migration to new code to change a class declaration to a mixin declaration without breaking legacy code.

Summary

Legacy class
extends
Legacy class
mixes in
New class
extends
New class
mixes in
Legacy class (✓)
Legacy mixin
New class
New mixin 1

1: If the mixin has Object as its only on type, which is the default if no on type is written.

A new or legacy mixin is definitely intended as a mixin, and can be mixed in.

A new mixin might have been a legacy class, now migrated, so we allow legacy classes to extend it, but not new classes, they know it's a mixin.

A new class is definitely not intended as a mixin, and cannot be mixed in by anybody.

A legacy class is potentially intended as a mixin and can be mixed in (optionally not in new code, if we want to force migration order.)

The top-left quadrant is the current behavior, the bottom-right quadrant is the post-migration behavior, and these differ exactly in that post migration, you cannot mix-in a class.

Migration

The only changes necessary to migrate to the new language version is:

  • to change class declarations intended as mixins to be mixin declarations, and
  • to change superclass declarations of new mixins (or legacy classes intended to be mixins, if allowed) to be mixin applications.

If a library does not declare any classes intended to be mixins, and does not extend any class intended to be a mixin, it doesn't need to change.

With these rules implemented, it is possible to migrate a class declaration, which is currently being used as a mixin, to a mixin declaration when opting in to the new language version. This won't break any legacy code depending on the declaration.

If the class is intended to be used as a mixin, there'll likely be tests of that behavior, and updating the language version will break those tests until the class declaration is changed to a mixin. As such, migration from class-as-mixin to actual mixin is forced when you want the new language version.

If the class was not intended to be used as a mixin, but someone else did so anyway, that will stop working. It was not intended to work, and now it doesn't. The general recommendation is to not use a class as a mixin unless it's documented as intending that use.

Legacy code will keep being able to extend Mixin that mixin declaration (it's implicitly converted to extends Object with Mixin). When they update their code to the new language version, that code stops working and must be changed to with Mixin. We can give warnings or hints immediately when a class extends a mixin, it's code which must eventually be changed.

The biggest migration risks are:

  • A library has a class that is being used as a mixin, but is not intended as such, and is migrated before the libraries depending on it.
    • This will immediately break the code using the class as a mixin, since you cannot mix in a new-code class. Unless the original author can be convinced to change it to a mixin anyway (now a breaking change if they already updated their language version), the client code will have no clear migration path. They may be stuck on an older version of the library.
  • A library has a class that is being used as a mixin, but is not intended as such, and is migrated after the libraries depending on it.
    • The client code will have assumed that the class was intended as a mixin, and have migrated to code which mixes it in. This works temporarily.
    • When the library migrates to new code, the client code stops working, but the client code authors thought they were long past migration, and are not expecting any problems now.
    • With the optional rule, the client code would not be able to migrate first.

If no class is mistakenly used as a mixin by a different package, migration should be safe and relatively painless.

@lrhn
Copy link
Member Author

lrhn commented Jun 4, 2024

Done, just not like this.

@lrhn lrhn closed this as completed Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Proposed language feature that solves one or more problems smaller-dart
Projects
None yet
Development

No branches or pull requests

2 participants