new annotation to identify classes safe for use as mixins

[pq]

More context in #58543.

TL;DR: some classes are intended to be mixed in and it would be great if an annotation could teach the prefer_mixin lint about them.

[pq]

Spitballing names:

• @classMixin?

[bwilkerson]

I think it's worth considering some of the alternatives to creating an annotation before we do so, but if we do create an annotation I suspect that finding a good name will be a challenge. The one listed above seems a bit repetitive:

@classMixin class ChangeNotifier { ... }

We could shorten it

@mixin class ChangeNotifier { ... }

but that seems potentially confusing because there are differences between a class that's used as a mixin and an actual mixin. I'm not coming up with a suggestion I really like at the moment, though I did think of a minor riff on a another suggestion you made:

@mixable class ChangeNotifier { ... }

Still not great.

[pq]

See also dart-lang/language#1529 and dart-lang/language#1643.

Also fyi: @lrhn, @Hixie.

Assuming we're allowing a class to be used as both a class and a mixin, I'm inclined towards @mixable.

[lrhn]

We're currently allowing it, and it would be nice to mark those classes that should really be mixins as exceptions to a rule of not using classes as mixins. (And recommending you change extends to with for them).

How about just using mixin?

const mixin = pragma("analyzer:mixin_class");

@mixin
class JumpingJehoshaphat {
  void jump() {
    print("How high?");
  }
}

The word mixin is only a built-in identifier, not a reserved word, so you can declare variables with that name.

Also, it reads as "I really mean this to be a mixin".

(I'd use a pragma constant for the marker, because then we can create an identical constant inside the platform libraries. Honestly, I'd use pragma constants for all analyzer-understood annotations, instead of inventing new classes.)

[pq]

I believe we've arrived at a near consensus that mixin is the way to go with a leaning towards an annotation over a pragma. IIRC one unresolved issue with consuming this in the SDK is how it would be documented in the APIs and that @lrhn would prefer not to see a mixin annotation show up in the SDK API docs. Does that sound right?

/fyi @jcollins-g

[lrhn]

Not sure what the difference between an annotation and a pragma is, pragmas are annotations.

I'm perfectly fine with annotating platform mixin-classes with @pragma("analyzer:mixin-class") if it keeps the annotation from showing up anywhere. It's only going to be a handful of them, and we don't plant to ever introduce any more.

That would require the analyzer to recognize both that pragma and whichever mixin constant is introduced into package:meta, but that should be doable?

Also, we are currently considering allowing you to write extends Mixin as shorthand for extends Object with Mixin for mixins which can apply to Object. That will allow us to migrate all those "mixin classes" to proper mixin declarations, non-breakingly, and remove the need for the annotation. That won't be any time before 2.16, though, so until then, an annotation can work.

[jcollins-g]

Accidentally submitted comment instead of canceling it. Sorry for the confusion.

[leafpetersen]

I'm perfectly fine with annotating platform mixin-classes with @pragma("analyzer:mixin-class") if it keeps the annotation from showing up anywhere. It's only going to be a handful of them, and we don't plant to ever introduce any more.

@lrhn I don't quite follow this. I may have missed some of the conversation in another venue, but why do you not want this to show up anywhere (by which I presume you mean something like API docs)? This seems relevant to users to know (that this class is expected to continue to be valid to use as a mixin, unlike other classes).

[lrhn]

I don't want it to show up in documentation, because then I also have to document that annotation in the platform libraries, otherwise they won't be stand-alone readable.

The classes are named SomethingMixin. and they are documented as being mix-in-able, so I don't worry about making it clear that they can be mixed in. You don't, and shouldn't, need to read the annotation for that information.

The annotation is for tools. It's there to avoid the analyzer giving a warning if someone does mix the classes in. The success criteria is not getting a warning, and a hidden annotation does that perfectly (so would a hard-coded list, which is what we have today, and I'd also be happy to keep that. Whichever is easier, as long as we don't start getting warnings.)

It's also a short-term thing, I hope to make all the classes into mixins by 2.16, if we manage to land the "allow extending mixins" change (which I'll try to do).

[bwilkerson]

Personally, I'd rather leave the hard coded list in analyzer for now because it will be easier to remove the hardcoded exceptions than it will be to remove code that handles pragma (because an external user might start using pragma and then removing support for it becomes a breaking change).

[pq]

Personally, I'd rather leave the hard coded list in analyzer for now because it will be easier to remove the hardcoded exceptions than it will be to remove code that handles pragma

This is an interesting point. Looking ahead, do we want to consider supporting pragma advice for the analyzer? If not, this would be wasted effort for sure. (Note I have a parallel question for the Flutter SDK, about which the analyzer platform is intimately aware -- not in a good way!)

because an external user might start using pragma and then removing support for it becomes a breaking change

This would indeed be really unfortunate. My thinking is that regardless of what we do for the SDK (if anything) we might still want something in package:meta to support Flutter (#58543) and my sincere hope is that external folks would prefer to use that.

It's also a short-term thing, I hope to make all the classes into mixins by 2.16, if we manage to land the "allow extending mixins" change (which I'll try to do).

Given this, the point may be moot for this specific use case and maybe adding @mixin to package:meta is sufficient. I guess I still think the general issue is interesting and worth considering if putting advice for the analyzer into the SDK is better or worse in the long run but maybe that can wait until the next motivating use case? 🤷‍♂️

[bwilkerson]

Looking ahead, do we want to consider supporting pragma advice for the analyzer?

I have no objection to adding support for some forms of pragma if doing so would help users. My understanding is that currently all of the uses of pragma in the SDK are instructions to the compilers that don't impact analysis so there's no need for it.

And if pragma('okToBeUsedAsAMixin') were something that we wanted to support long term, then I'd have no problem supporting that particular form of pragma. What I don't want is to be stuck supporting a form of pragma that users outside the SDK started using when we already know that we'd remove its use from the SDK relatively soon.

Note I have a parallel question for the Flutter SDK, about which the analyzer platform is intimately aware -- not in a good way!

I doubt that the Flutter SDK is going to want to use pragma for anything other than the compilation directives that it's already used for because (a) they can import meta and (b) they are far less likely to want the annotation to be hidden from their docs.

I agree that we want to decouple the analyzer from the Flutter SDK, but I think we can do so by introducing new annotations that have the desired meaning. However, if we need to support some form of pragma in order to do so, I'm not opposed.

... we might still want something in package:meta to support Flutter ...

Absolutely. I wasn't suggesting that we not add an annotation to meta, only that we don't also build in support for pragmas.

I still think the general issue is interesting and worth considering if putting advice for the analyzer into the SDK is better or worse in the long run ...

Agreed. I didn't intend to say that we should never support pragma if that's useful in the long run, I just don't want a short-lived patch that we might not be able to get rid of. It's all about the time frame.

We could also prevent that from happening by supporting pragma('okToBeUsedAsAMixin') and having a hint that it can't be used outside the SDK. I just don't see the upside when the SDK code is already special-cased.

[pq]

Thanks Brian!

I agree that we want to decouple the analyzer from the Flutter SDK, but I think we can do so by introducing new annotations that have the desired meaning.

I hope this is true. My biggest concern is how this would scale. I wouldn't want package:meta to get overwhelmed with annotations that are overly-specific to capturing semantics that are really particular to Flutter. Here the bug that pragmas aren't discoverable becomes a feature (but all the downsides persist).

(Anyway, this is a bigger conversation for another thread!)

I wasn't suggesting that we not add an annotation to meta, only that we don't also build in support for pragmas.

Ideally we wouldn't but this depends on whether we want to allow the SDK APIs to advise analysis themselves and if we do, if it's reasonable for the SDK to use the same mechanism for specifying these semantics as external users (e.g., annotations shared w/ package:meta) or, if not, if we want to allow a special internal pragma-based solution that mirrors the external one. Alternatively we can keep maintaining a separate model in the analyzer.

[bwilkerson]

... annotations that are overly-specific to capturing semantics that are really particular to Flutter.

I think you already know that I agree :-), but it's worth mentioning anyway. We should always work toward more widely applicable solutions.

... whether we want to allow the SDK APIs to advise analysis themselves ...

In general, I think we do want that.

... if it's reasonable for the SDK to use the same mechanism for specifying these semantics as external users ...

I think it's clear that the SDK can't use annotations defined in meta, but we've discussed a couple of ways to share the declaration and I'm confident we can choose one of them.

Alternatively we can keep maintaining a separate model in the analyzer.

In general, I'd prefer we didn't do that. If it weren't for the fact that

• we already have an implementation (so we're not talking about adding special-case code), and
• we expect the need for this support to be relatively short lived,

I'd be in favor of supporting a more general solution in this case too.

[pq]

😄

In general, I'd prefer we didn't do that. If it weren't for the fact that

• we already have an implementation (so we're not talking about adding special-case code), and
• we expect the need for this support to be relatively short lived,

💯

For this case in particular I think you're right that the solution would be short-lived. That said, if this case motivated a solution that would get us pointed in a desired direction then maybe that's moot. I take it the rub is getting agreement on to what extent we want to aspire to SDK APIs that advise analysis themselves and that's I think a bigger question for the language and analyzer teams to hash out.

Related to this is the Flutter use-case that motivated this conversation in the first place. What's interesting there is that Flutter has an aspiration to support forks (which I think suggests that our tools should too). Our current Flutter-awareness is too brittle to do that reliably or at all (since it depends fundamentally on hardwired classnames and URIs). Portable Flutter analysis is a different requirement than the aspiration to improve analyzer hygiene and maintainability but it's interesting to note the overlaps.

Thanks all for the thoughtful responses!

[parlough]

With the addition of the mixin class modifier as part of Dart 3's class modifier feature, it seems the use cases discussed here are handled.

As for the pragma/package:meta discussion, if anyone would like to continue that, please open a separate issue. Thanks all!