Implementation Issue for SuperMixins: Linter

[JekCharlsonYu]

What: Issue for implementation for Linter

ETA: 10/15 in Dart Repo.

For: SuperMixins Implementation and strawman documentation

Meta Issue: This is subsumed under the Super Mixins Meta Issue in the Language Repo

Comments: Feel free to break this further into further tasks and issues, or use as a meta issue for all the tasks in the implementation plan

[devoncarew]

From the new super mixins impl plan:

"Implement a lint that fires when an old-school super mixin is used."

I.e., we'll want clients to be able to opt-into a lint that validates that no new code gets checked in that uses the older style super mixins, once they've moved over to the newer style.

/cc @pq @a14n

[leafpetersen]

This is to track implementation of a lint which warns when a class declaration is used as a mixin (suggesting to prefer using explicit mixin declarations instead).

cc @pq

[bwilkerson]

Are there any cases where an existing class that is valid (under super-in-mixins) cannot be converted into the new syntax?

We should also have a quick fix for this lint.

[leafpetersen]

Yes. If you also use the mixin as a class, then you need to refactor/rename appropriately. e.g.

class M {}

class A extends M {}

class B extends Object with M {}

void main() {
 new M();
}

Would need to become something like:

mixin MMixin {}
class M with MMixin {}

class A extends M {}

class B extends Object with MMixin {}
// Or shorter, class B with MMixin {}

void main() {
 new M();
}

[bwilkerson]

That appears to require global knowledge, which is something that analyzer never assumes that it has.

[leafpetersen]

Yes, I wouldn't expect this as a quick fix. My thinking was that the lint would fire if you mixin a class, the quick fix will convert the class to a mixin, but the result may not be valid if you were also using the mixed-in-class as a class. If there's a preference that quick fixes never invalidate code we'd need to re-evaluate that.

[a14n]

@bwilkerson does analyzer lib support the new syntax? I can't find any MixinDeclaration class in analyzer-0.32.5.

[bwilkerson]

The syntax is supported in bleeding edge. We're still implementing some of the semantics, but it's getting close.

That said, so far it isn't supported by any of the runtimes, so we wouldn't want users to enable this line yet, because converting to the new syntax would break their code.

[devoncarew]

so we wouldn't want users to enable this line yet, because converting to the new syntax would break their code

Yup, but we do want to get this lint in and available soonest; there are a lot of moving parts here, and once the implementations are available, not having this lint would block Flutter from moving over to the new super mixins.

[bwilkerson]

I don't understand why it would block them, but OK.

Note that this will require publishing a breaking change version of the analyzer package.

[bwilkerson]

Minor point, but if we have an automated way to convert from the old syntax (a class) to the new syntax (a mixin), we should implement it as a quick-assist so that it's available to users that don't enable the lint.

[leafpetersen]

Is anyone looking at this? We should try to get this done for next week.

[bwilkerson]

I'll tackle it unless someone else starts on it first.

[a14n]

This is blocked until an analyzer version supporting the new mixin syntax is released.

[bwilkerson]

@leafpetersen Just to clarify, we want the lint to fire every time a class is referenced in a with clause, whether or not the class contains a reference to super. Is that true?

[leafpetersen]

Yes, that is correct.

[bwilkerson]

Lint landed: dart-archive/linter#1165. Working on getting the linter published and pulled into the sdk.

[pq]

Linter published and landed in the SDK as of cd26b88.

[bwilkerson]

The lint has now been pulled into the SDK (https://dart-review.googlesource.com/c/sdk/+/74880). Let us know if you see any issues with it.