Double mixin leads to lost super.

[polina-c]

In the example dispose of Bar is lost. The fix is to remove ‘with Disposable’ from Bar, as Disposable does not make sense for Bar, because it is already mixed into ChangeNotifier.

But the problem is that double mixin of Disposable is not noticed by compiler.

There should be either compile time error or the second mixin should be just ignored.

[polina-c]

cc @goderbauer

[polina-c]

cc @lrhn

[lrhn]

Disallowing the same mixin being applied twice in the same inheritance chain has been discussed before. Fx. https://github.com/dart-lang/sdk/issues/28794

From the language perspective, there is no problem with allowing it, the semantics are well defined, so if someone wants to do it, for any reason, Dart is generally permissive. It doesn't prevent something unless there is a technical problem with it.

That said, there is a restriction on applying the same mixin twice, if it has a private instance variable. That's not a language problem, but it was an implementation problem. It made it so much harder to optimize those variables, that we disallowed it. Which is inconsistent and potentially surprising to users from other libraries, because they can be hit by a restriction about a variable they can't even see. (If your public mixin has a private variable, and it gets applied twice.)

Maybe it's time to revisit that issue. Not because it's a language problem, but because it's a user stumbling block. The question is how breaking it would be.

[bwilkerson]

If it proves to be undesirable to solve it as a language change, then we could consider adding a lint to flag the situation.

[polina-c]

It would be better to ignore the second mixin of the same mixin.

If we fail on double mixin this reasonable pattern will result in breaking change:

1. Application author mixes in Disposable to class Child that derives from Parent
2. Package author mixes in Disposable to Parent
3. Application author upgrades the package and gets error about about double mixin

[lrhn]

Ignoring something the author writes is unlikely. Even if we do that, there'll almost certainly be an analyzer warning, or recommended lint, about "unnecessary mixin", that many projects will treat as an error anyway.

It's very specific to treat the repeated application of one mixin as a problem, but not, fx, the case where you have two mutually exclusive mixins, and apply both. (Think "DisposableSync" and "DisposableAsync", where you must choose only one.)

Imagine a mixin feature where you could declare a mixin to be non-repeatable (error if applied twice) or idempotent (ignored if applied again), or nothing (current behavior). And which allows composite mixins, so the two disposables could both extend the same unrepeatable mixin.

(And constructors in mixins, because that'd be nice, while we are there.)

[bwilkerson]

Imagine a mixin feature where you could declare a mixin to be ...

Unsurprisingly, I like that idea. The 'idempotent' case would require language support, but 'non-repeatable' could be handled by an annotation. An annotation would also allow users to express mutually exclusive mixins, though it might require putting the information on all members of the set of mutually exclusive mixins.

[eernstg]

I'd still prefer to make repeated application of the same mixin a compile-time error.

Mutual exclusion for a set of mixins can already be expressed:

interface class _FooExclusion<T> {} // Support mutual exclusion for `Foo...` mixins.

mixin FooSync implements _FooExclusion<FooSync> {}
mixin FooAsync implements _FooExclusion<FooAsync> {}

class CantHaveBoth extends Object with FooSync, FooAsync {} // Compile-time error.

[gnprice]

I think the core of the issue here can be seen as a hole in the semantics of @mustCallSuper: it doesn't currently catch when a method gets overridden by a mixin application.

In the original example, there should be an analyzer warning at extends Bar with Disposable, because that overrides Bar.dispose with an implementation that isn't going to call Bar.dispose, and Bar.dispose is annotated with mustCallSuper. (It has the annotation implicitly, because it overrides Disposable.dispose which has that annotation.)

So that seems like a bug that can be fixed.

---

The issue isn't specific to applying a mixin twice. It can be reproduced with only a single mixin application:

import 'package:meta/meta.dart';

class Base {
  @mustCallSuper
  void f() => print("Base.f");
}

mixin Mixin {
  void f() {}
}

class Derived extends Base with Mixin {
  // This would produce a warning because of `mustCallSuper`.
  // But when the same override comes via `with Mixin`, it doesn't.
  // void f() {} 
}

void main() {
  final x = Derived();
  assert(x is Base);
  x.f();           // doesn't call Base.f
  (x as Base).f(); // still doesn't call Base.f
}

---

There are versions of the original example that would be confusing even in the absence of @mustCallSuper. In general I think there's some risk of confusion any time a mixin application overrides a member that was already in the base class — it's a lot less explicit and easier to miss than an individual member declaration with @override would be.

But I think what really drives the original example and makes it feel concerning is that it's a way to accidentally fail to call a superclass's implementation of a method. (Following the example's method name dispose: you might have some complex hierarchy of classes, each responsible for disposing of the resources they manage, and this could lead to leaking some of those resources.) When that's a concern, one should probably be using @mustCallSuper anyway. And then this is a problem only because it's a way to accidentally bypass the effect of @mustCallSuper.

[lrhn]

If we want to catch this at the application point, maybe the thing to focus on is that Disposable.dispose does not have an @override annotation, and it overrides a @mustCallSuper-annotated method.

If it doesn't have an @override, it cannot be assumed to know that there is a super to call. Or even, if it has an @override, but the signature it actually overrides isn't @mustCallSuper-annotated.

[gnprice]

I think the issue can reproduce just as well when the overriding implementation does have @override. For example:

import 'package:meta/meta.dart';

class Base {
  @mustCallSuper
  void f() => print("Base.f");
}

mixin HasF {
  void f();
}

mixin Mixin on HasF {
  @override
  void f() => print("Mixin.f");
}

class Derived extends Base with HasF, Mixin {}

void main() {
  final x = Derived();
  assert(x is Base); // ignore: unnecessary_type_check
  x.f();           // doesn't call Base.f
  (x as Base).f(); // still doesn't call Base.f
}

Maybe that's the sort of situation you had in mind in the second of these conditions:

If it doesn't have an @override, it cannot be assumed to know that there is a super to call. Or even, if it has an @override, but the signature it actually overrides isn't @mustCallSuper-annotated.

---

Actually, experimenting a bit with those, it looks like there's a case that escapes both of those conditions — the overriding implementation does have an @override, and the signature it overrides (at its declaration point, up on the mixin) does have @mustCallSuper, but that's a different method from the @mustCallSuper-annotated method that's getting overridden by the mixin application. Like this:

import 'package:meta/meta.dart';

class Base {
  @mustCallSuper
  void f() => print("Base.f");
}

mixin HasF {
  @mustCallSuper
  void f();
}

mixin Mixin on HasF {
  @override
  void f() { print("Mixin.f"); }
}

class Derived extends Base with HasF, Mixin {}

void main() {
  final x = Derived();
  assert(x is Base); // ignore: unnecessary_type_check
  x.f();           // doesn't call Base.f
  (x as Base).f(); // still doesn't call Base.f
}

It's key to that example that the method declaration on the base mixin HasF is abstract; if it had an implementation, then Base with HasF would be the mixin application that was overriding Base.f, and it'd fall under your first condition.

This example might point to a different bug, though: it seems like @mustCallSuper on an abstract method isn't getting enforced on overrides. Specifically the @mustCallSuper on HasF.f isn't getting enforced when that method is overridden at Mixin.f; if you change HasF.f to have a body, then Mixin.f does get such a warning.

If that behavior were fixed, then there'd be a warning on Mixin.f; if the developer fixes that by adding super.f() in that method body, then that ends up calling Base.f and all is well.

[gnprice]

If that other bug(?) gets fixed, then I believe the two conditions in @lrhn's last comment should be enough to close this hole. In other words, if a mixin application overrides a @mustCallSuper-annotated method, then:

• the overriding implementation should have @override; and

• the implementation that the overriding implementation was overriding at its declaration point should have been annotated @mustCallSuper too.

  (The one it was overriding at its declaration point is the one that came via an on clause of the mixin declaration (or maybe some other possibilities), as contrasted with the one it's overriding at the mixin application, which came from an extends clause there or a previous item in the with clause.)

This works because if the overriding implementation meets those two conditions, then the @mustCallSuper check on its body should have already confirmed (subject to its normal caveats, including // ignore:) that it calls super.

---

Working through how that turns out on each of the examples:

The original example would get a warning at "Bar with Disposable", because of the first condition, because the overriding implementation is Disposable.dispose and as Lasse pointed out it has no @override. The developer can delete "Disposable, ", making it just "extends Bar with ChangeNotifier", and all will be well.

The example in my earlier comment https://github.com/dart-lang/sdk/issues/53682#issuecomment-2179676030 would get a warning at "Base with Mixin", because Mixin.f has no @override. If Mixin.f and Base.f are conceptually different implementations of the same thing, they can say mixin Mixin on Base and add @override; if not, they've learned of an accidental name collision and have to rename something.

And the first example in my last comment https://github.com/dart-lang/sdk/issues/53682#issuecomment-2182012434 would get a warning at "Base with …, Mixin" because of the second condition: Mixin.f does have @override, but at its declaration it was overriding HasF.f which lacks @mustCallSuper. Same story as the previous example.

(Finally, to repeat it here for completeness: my second example in that last comment would get a warning at Mixin.f, because the @mustCallSuper on HasF.f would be enforced there without regard to the fact that HasF.f is abstract.)

[lrhn]

Moving to area-devexp. The fix here seems to be better mustCallSuper handling, not changing mixins.