Take a detour via a legacy library in order to break class modifier rules?

[eernstg]

I think the following situation might not be fully clarified in the feature spec about class modifiers.

[Edit: The example should use extends, does that now.]

The current situation

// ----- Library 'legacy.dart'.
// @dart = 2.19

// `Runes` is just an example, so we rename it.
typedef FinalPlatformClass = Runes;

// OK, due to special platform/legacy exception.
class C extends FinalPlatformClass {
  C(): super('');
  noSuchMethod(Invocation _) => throw 0;
}

// ----- Library 'main.dart'.
import 'legacy.dart';

final class D extends C {} // OK or not?

void main() {}

The modifier final on D satisfies even the strongest requirements on D itself, which means that we can concentrate on the permission to declare D at all, noting that it is a subtype of FinalPlatformClass in a post-feature library.

Currently, no tools report an error for the declaration of D.

Analysis

The fact that legacy.dart exists and has @dart = 2.19 is a necessary precondition for the existence of a class which is a subtype of FinalPlatformClass, given that FinalPlatformClass is final and no subtypes are declared in the same library. However, it is not so obvious that we should allow further subtypes of FinalPlatformClass to be declared in new code (given that they could not be declared at all without a legacy library like legacy.dart).

Note that we do insist that indirect violations of other rules must be flagged as compile-time errors:

// ----- Library 'legacy2.dart'.
// @dart = 2.19
import 'dart:collection';

// OK, cannot have `base`, and we accept that.
class C2<E extends LinkedListEntry<E>> extends LinkedList<E> {}

// ----- Library 'main2.dart', new code.
import 'dart:collection';
import 'legacy2.dart';

// Error, new code _must_ preserve `base`. (This error is specified, but not yet implemented.)
class D2<E extends LinkedListEntry<E>> extends C2<E> {}

What to do?

We could of course say that the declaration of class D is not an error, which is how the tools currently behave.

The rationale would be that it makes no real difference whether or not we can have instances of C whose run-time type is a subtype of FinalPlatformClass, or we can have instances of C as well as D. In any case, no code can assume that the subtype graph under FinalPlatformClass is just the root FinalPlatformClass itself.

However, a counterargument could be that if we allow a legacy library to introduce a loophole in new code then we're essentially turning parts of the new code into legacy code, with no explicit heads-up to the author of the code at all. This could make it harder to assess the effort associated with a migration of any legacy libraries.

One approach to get these errors on classes like D could be the following: Whenever a class like C is analyzed or compiled, it gets a flag saying that this class is an errorButForLegacy, and it would then be a compile-time error to use that class as a superinterface in new libraries.

[lrhn]

I think the following situation might not be fully clarified ...

I'll disagree on that 😉

final class D implements C {} // OK or not?

Not OK, specification is completely clear on that.

Whether we want to reconsider is another issue, but it's definitely what is specified, and intentionally so.

The problem is not the subclassing, but using implements with a declaration which has a base or final super-declaration in another library.

Doing final class D extends C {} would be valid. That declaration has to be base, final or sealed, because it has a super-declaration marked final, but it's allowed to exist.

The rule that the D declaration violates is to implement a declaration which has a base or final super-declaration in another library.
Here D is implementing C which has super-declaration pragma, which is final and in a library other than that of D.

The only exception to that rule is if the implementing library is a pre-feature library, and all the base or final super-declarations are in platform libraries. In this case the implementing library is not pre-feature, so the exception does not apply.

Currently, no tools report an error for the declaration of D.

See: dart-lang/sdk#52078

[eernstg]

Not OK, specification is completely clear on that.

Ah, of course. I did not get any errors, but we do have the transitive requirement about base and implements. Changed the example to use extends such that this is not an issue.

Doing final class D extends C {} would be valid.

Assuming the pre-edit version of the example, I would expect the same rule to kick in: This is an error because we had class C implements FinalPlatformClass. It is allowed for a legacy class to have the implements relation to a final platform class, but I would still expect that implements clause to make D an error, even with extends C.

But now I've edited the example, and it uses extends from D to C and also from C to FinalPlatformClass.

That's the actual intended topic in this issue. We have the following in the feature specification:

Going through a pre-feature library does not remove transitive restrictions for code in post-feature libraries.

The notion of transitive restrictions is only mentioned in the non-normative section Transitive restrictions, so it's not quite clear what that would mean.

For example, it may or may not imply that the following is an error:

// ----- Library 'legacy.dart'.
// @dart = 2.19

// `Runes` is just an example, so we rename it.
typedef FinalPlatformClass = Runes;

// OK, due to special platform/legacy exception.
class C implements FinalPlatformClass {
  C(): super('');
  noSuchMethod(Invocation _) => throw 0;
}

// ----- Library 'main.dart'.
import 'legacy.dart';

final class D extends C {}

void main() {}

[lrhn]

There is no error in the new example.

The final class D extends C {} does not fall awry of any of the specified restrictions, so it is allowed.

The class it directly extends is not declared interface, final or sealed. (It cannot be, it's from a pre-feature library.)

Going through a pre-feature library does not remove transitive restrictions for code in post-feature libraries.

That quote is itself non-normative. It has to be because it doesn't actually say anything.
It basically just emphasizes that there are no exceptions other than the exceptions that are explicitly mentioned. You don't need to look further.

The "transitive restrictions" are the ones that are not just about direct super-dependences: The one about implements, which does not apply, and the one which says that if you have a super-declaration marked final or base, you must yourself be marked final, base or sealed, which D satisfies by being final.

The four rules in the specification which can cause a compile-time error due to modifiers are:

It's a compile-time error if:

• A declaration depends directly on a sealed declaration from another library.
• A class extends or mixes in a declaration marked interface or final from another library (with some exceptions for platform libraries).
• A declaration implements another declaration, and the other declaration itself, or any of its super-declarations, are marked base or final and are not from the first declaration's library (with some exceptions for platform libraries).
• A declaration has a base or final superdeclaration, and is not itself marked base, final or sealed.

The last two of those are "transitive", the first two only relates to immediate dependencies.

The remaining errors are about mixin class declarations and mixing in non-mixins.

None of these four rules apply to D:

• It only depends directly on C which is not marked sealed.
• It extends only C which is not marked interface or final.
• It does not implement any declaration.
• It is itself marked final.

So completely specified.

There is no rule that you cannot extend a class in a second library which implements a base/final class from a third library.
We could add such a rule, but it can only apply if the second library is a pre-feature library and the third library is a platform library, and it's unlikely to be worth the effort.

The pre-feature library will have to make a (possibly breaking) change when it updates to 3.0. At that point, down-stream code will have to adapt. Until then, everything is exactly as unsafe as the pre-feature library itself implementing a platform library final class, and the SDK must be prepared for that to happen.

It's more likely to be actually useful to remove the "must be base or more" rule when going through an implements clause from a pre-feature library to a platform library. Again, the damage is already done by the pre-feature library, so forcing the down-stream library to add base, when it's not actually helping anything, and when the pre-feature library will likely change to not require the base when it migrates, somehow, to 3.0, isn't helping anyone. (But it is hinting that something will likely change when the pre-feature library migrates.)

[munificent]

Can we close this? It sounds like the spec is where we intend it to be?

[lrhn]

Yes.

The worry is that we have a path through a pre-feature library that allows something in a post-feature library that it couldn't do directly.

The answer is that the pre-feature library does something it cannot keep doing when it migrates to 3.0, so that problem will go away.
We could detect the issue, and prevent the post-feature library from extending/implementing the pre-feature library class, but we don't know what the pre-feature library fix will be. Maybe it will allow the post-feature library extending/implementing, and preventing that now isn't helping anyone.

The only way to be absolutely safe is to do dependency order migration. As always.
We don't know what it is

[eernstg]

Yes, it's OK to close it.

The main reason why it is no big deal after all is that this is only about platform classes, and most of those have very special rules (like num, int and other "primitive" types, and like other very special types such as FutureOr, Function, Never, and so on). Those special rules already prevent almost all the class modifier errors that legacy classes have special permission to have. Most of the remaining platform classes are protected in other ways; for instance, many of them do not have a public generative constructor, so nobody can create a subclass of them anyway.

The main reason why I was worried was that the special permission for legacy classes to break these rules would allow new (Dart 3.0) code to exist, based on legacy classes, and those Dart 3.0 classes would be "secretly legacy". This means that it could be quite difficult to assess the amount of migration work needed in the situation where a million line package has even just a few lines of pre-3.0 code left. It seemed safer to me if we would make it an error for new code to be a subtype of those "bad" legacy classes.

However, there's no way we can do anything about it right now. Also, we can introduce a lint to flag those "secretly legacy" classes in Dart 3.0 code.

If we don't do anything then we may end up with a lot of packages including just a few lines of legacy code, because that's a loophole that allows the rest of the package, ostensibly "Dart 3.0", to continue to break the rules about class modifiers, which is particularly unfortunate if it is done by accident.

But, as I said above, it's only a very small number of platform classes, and it's probably never going to be a big issue.