`mixin class` hides super methods of other ascendants.

[polina-c]

This is follow up for previous discussion

There is a way to declare mixin class. Very popular example is ChangeNonifier in Flutter Framework.

I experimented with it and could not find a way use mixin class so that it does not hide methods with the same name of other super classes. Even if we make sure they are from the same interface. This means, if my class uses ChangeNonifier, it cannot meaningfully use any other disposable mixin or derive from a disposable class. And, it seems to be a problem.

Did i miss something?

import 'package:meta/meta.dart';
import 'package:test/test.dart';

mixin Disposable {
  @mustCallSuper
  void dispose() {
    print('i am Disposable');
  }
}

class Class1 with Disposable {
  @override
  void dispose() {
    print('i am Class1');
    super.dispose();
  }
}

mixin Mixin1 on Disposable {
  @override
  void dispose() {
    print('i am Mixin1');
    super.dispose();
  }
}

mixin class MixinClass1 implements Disposable {
  @override
  @mustCallSuper
  void dispose() {
    print(
        'i am MixinClass1, i will have to implement Disposable, because I cannot extend or be on non object');
  }
}

class SuperPosition1 extends Class1 with Mixin1, MixinClass1 {
  @override
  void dispose() {
    print('i am SuperPosition');
    super.dispose();
  }
}

void main() {
  test('dispose SuperPosition1', () {
    SuperPosition1().dispose();
    // Output shows dispose of Class1 and Mixin1 are missing:
    // i am SuperPosition
    // i am MixinClass1, i will have to implement Disposable, because I cannot extend or be on non Object
  });
}

[lrhn]

This correctly describes how mixins work.

A mixin applied to a superclass will create a subclass of the superclass. The application adds the mixin's instance members as if they were declared in the subclass, and if they have the same name as a superclass member, they will override the superclass member. And they must be valid overrides.

The problem here is not that behavior, it works precisely as designed. The problem is how it interacts with the annotation @mustCallSuper.

If a mixin has a member, like dispose, which usually must call super, then the mixin can either call super.dispose, in which case it must have an on type with a dispose method, or it can not call super.dispose, in which case it's not safe to mix in on top of something with a dispose that must be called. (And it is possible to warn you if you do that, because a mixin with no on type cannot possibly call super.dispose.)

There is no way to conditionally call super.dispose() if it's there. That does make it problematic to have a mixin class with a dispose method, because it cannot call super.dispose since it's superclass is Object, and if used as a mixin on something which is already Disposable, it won't call that super.dispose, and there is no easy way to do so from subclass itself.

The way I'd do that today is to have two different types: A mixin with an on type and a super-invocation and a base class that you can extend.

mixin ChangeNotifier on Disposable {
  // ...
  void dispose() {
    // ... do work ...
    super.dispose();
  }
}

class ChangeNotifierBase = Disposable with ChangeNotifier;

(Possibly, or something like that, haven't checked whether Disposable can be extended.)

Then someone mixing in ChangeNotifier will call super, and must do so on an existing disposable, and someone using it as the base superclass must use the class, which adds the superclass dispose for it to call.

Not as convenient as today, but what we have today doesn't actually work, and cannot work.

The one feature which could possibly help here would be generalized mixin-super-calls, where you can call not just super.dispose(), but SpecificSuperclass.super.dispose() as well. In that case you would be able to write:

class MyNotifyingThing extend SomethingDisposable with ChangeNotifier {
  // ...
  void dispose() {
    ChangeNotifier.super.dispose(); // Same as `super.dispose()`.
    SomethingDisposable.super.dispose(); // "`super.super.dispose()`".
  }
}

The issue with that approach is that it's harder to help you not forget. If you call just super.dispose(), then it looks like you're doing what you should. It requires the analyzer to recognize that ChangeNotifier is not itself calling super.dispose(), and that you are doing it, so that all superinterfaces with a @mustCallSuper void dispose() are actually called, and that without the SomethingDisposable.super.dispose() that isn't the case.

I think that might be possible for the analyzer, but it's not a given. If not, this code pattern may cause warnings that need to be ignored, or miss warnings that you're not calling the super methods correctly.

[polina-c]

Thanks, @Irhn

Yes, the language works as explained. And yes, the issue with ChangeNotifier, as well as with any other mixin class, is not that it is easy to forget, but that, if I use it, there is NO WAY to invoke dispose of other super.

So, you suggest to solve the problem for ChangeNotifier by separating class and mixin, i.e. by renaming class ChangeNotifier to ChangeNotifierBase, right? That would be a breaking change in Flutter Framework.

It turns out mixin class is a 'breaking change prone construction'. When people use it, they do not know about this side effect. When they discover it, it already means a breaking change is needed. Should we start alerting on mixin class with lint, explaining its side effect at the moment of declaration?

Some internal examples: https://docs.google.com/document/d/1Z1Wpyu58ck0X7l_gs90NWwCDYLIpoduvSAVTQMtWoFM/edit?resourcekey=0-6hiFkqJiPQlZrqqT7jXmMQ

[lrhn]

A mixin class, by itself, is perfectly reasonable as something that can be used both as a superclass and a mixin.

The problem is the @mustCallSuper annotation which interacts particularly with mixins in general.

If a mixin has an implementation of a mustCallSuper member, it must have an on supertype with that mixin, and call super.theMethod() in its implementation. Otherwise it won't be correct to mix it in on top of something which already implements that member.

That's independent of being a mixin class, you'd have the same problem with a plain mixin that not have the "Disposable" interface as on type, and doesn't call super.dispose().

The mustCallSuper can occur in two ways:

• Introducing, the base case which doesn't call super, because there is no super to call.
• Incrementing, on top of something which already has the must-call-super method, and which must call super.

The same method cannot do both, so each method must be writter for one of the two roles.

If you just declare a normal method, you usually know whether you need to call super, it depends on the superclass, which you know at the point of declaration.

Mixin methods must also be designed for one of the two roles. The problem here is that ChangeNotifier is designed for the "introducing" rule, and therefore it doesn't work for the "incrementing" role. You can still use the current ChangeNotifier to introduce a dispose method, but it doesn't work as an increment on an existing dispose method.

Again, that's entirely specific to @mustCallSuper and mixins, it's not about mixin class. It's just that a mixin class necessarily must have the "introducing" role, since it cannot call super, but it's just as much a problem for a non-class mixin with the same method.

[polina-c]

I disagree. There is still issue with mixin class. Normal mixin can switch from introducing to incrementing, without static breaking changes, while mixin class cannot, because it can be just for Object.

And, it is a common pattern to reorganize code by moving introduction of a role to a separate class/mixin, after initial implementation.

[polina-c]

• @pq

[pq]

/fyi @kallentu

[lrhn]

I agree that mixin class can have problems with @mustCallSuper in general. It can only be used to introduce a @mustCallSuper property. If you also need a mixin which adds an increment/propagation of the @mustCallSuper member, you'll probably be better off only having that mixin.

That is, instead of base class ChangeNotifierBase (introducing dispose) and mixin ChangeNotifier (propagating dispose), only have the latter. Then you'll have to write extends Disposable with ChangeNotifier to start disposables at the introducing member.

This is still very much an issue about @mustCallSuper. There could be a couple of other annotations with similar role-separations, but most other annotations are probably completely indifferent to whether the superclass has the same annotation or not. So anything we'd warn about should be about @mustCallSuper.

I'd suggest warning if mixing in a mixin which introduces a @mustCallSuper member, if the superclass already has a @mustCallSuper member with the same name. Here "introduces a @mustCallSuper member" is defined as having a concrete member the mixin declaration, which is either itself annotated as @mustCallSuper or which overrides a superinterface member which was annotated @mustCallSuper, but not having such a member in any superclass (on-type) interface.

(That's a property that can be computed for each mixin, once and for all: Which @mustCallSuper members does it introduce.)

Mixing such a mixin on top of a superclass which does have a @mustCallSuper member with the same name should give a warning of that mixin application not calling super.

[polina-c]

Yes, we need mixin Disposable, that introduces dispose, and it should be incremented by many other classes, not just ChangeNotifier. I am not sure what was reason to introduce ChangeNotifier as 'mixin class' and if it is ok not to have it as class now, and have it just as mixin.

I agree, both 'mixin class' and @mustCallSuper have issues.

[polina-c]

ChangeNotifier was a class initially and in March it was converted to 'mixin class'. So, making it just mixin will be a big breaking change.

[lrhn]

ChangeNotifier has not changed behavior, it has always behaved the same way, and had the same problems with @mustCallSuper.

Whatever change it's needed to address those problems, either in ChangeNotifier itself, or adding a warning when using it unsafely, is probably not so urgent it needs to be rushed.

[polina-c]

While, yes, this issue is very old, and, yes, it is not super urgent, it still important, because resources are not released in cases, when users made special effort to release them:

1. It is common case to use ChangeNodifier together with mixin like AutoDispose
2. Mixins like AutoDispose are created specifically for the goal to dispose resources.
3. If ChangeNodifier is not disposed, it will not release listeners, that can cause memory leaks by keeping the listeners in memory much longer than it is designed.
4. dispose of either ChangeNodifier or AutoDispose' is missed, while users are confident they are both invoked

b/301445398 shows details.

I will create design doc to address this by introducing Disposable and converting ChangeNotifier to pure mixin.

[polina-c]

@goderbauer