Rethink restrictions around redundant implementations and mixins

[munificent]

Working on augmentations raises some questions around which compile errors are defined on a single syntactic declaration or the resulting semantic declaration after all augmentations are merged. For most errors, the distinction doesn't matter. But digging into this has revealed some corners of the language that feel inconsistent and somewhat arbitrary. So this issue to see if we want to make some changes to the language to be more consistent.

abstract class I {}

class C implements I, I {}

Today, this is an error:

Error: 'I' can only be implemented once.

However, this is fine:

abstract class I {}

class B implements I
class C extends B implements I {}

So you can implement the same interface twice if you go through a level of indirection. That makes sense because it's harmless to do so and would otherwise mean that adding an implements clause to a class could be a breaking change if a subclass already implemented the same interface.

Also, this is fine:

mixin M {}

class C with M, M {}

I suppose there is maybe an argument that allowing applying the same mixin multiple times is potentially useful for a mixin with certain carefully crafted methods and uses of super().

This is also allowed:

mixin class M {}

class C extends M with M {}

But this is an error:

class B {}

class C extends B implements B {}

Yields:

Error: 'B' can't be used in both 'extends' and 'implements' clauses.

But again a layer of indirection permits it:

class B {}

class C extends B {}

class D extends C implements B {}

No error here.

Most curiously, this is allowed:

mixin M {}

class C with M implements M {}

I can think of absolutely no reason to allow this while disallowing extends M implements C. I assume it was just an oversight.

With augmentations

Adding in augmentations complicates this. Consider:

abstract class I {}

class C implements I {}

augment class C implements I {}

Is that an error?

• If not, then we'll have to explicitly specify that redundant interfaces in implements are restricted to a single syntactic declaration before augmentations are applied. But that may be hard because augmentations are generally applied early in the compilation process before resolution and type checking has happened.

• If it is an error, then a code generator has to be careful to omit a redundant implements clause if the hand-authored class happens to already specify implementing the same interface.

No redundant implements errors

I think a better approach is to eliminate these errors completely. So these classes would all be valid:

mixin class M {}

class C implements M, M {}
class D extends M implements M {}
class E with M implements M {}

The third one is already valid. This would make the first two consistent.

Yes, it means you can write something that's a little redundant and pointless. But... we have empty statements in the language. You can often write code that doesn't do anything. Making this specific flavor of redundancy an error seems to carry little positive value and makes augmentations more complicated.

[lrhn]

+1

An extra unnecessary implements is harmless. It's originally an error because it's probably an author mistake to write implements I, I so close that the author is expected to be able to see that it's unnecessary, but that could just be a warning. There is no need to have it in the language.

That also avoids any issues when a desugaring introduces an implements clause that no user ever wrote.

I went through the declarations to see who does what:

class C1 extends M {}
class C2 with M {}
class C3 implements M {}
class C4 extends M implements M {} // Both error - not both extends and implements
class C5 extends M with M {}  // Analyzer error - not both extends and with
class C7 with M, M {}
class C8 implements M, M {}  // Both error - implemented twice
class C9 extends M with M implements M {}  // Both error - not both extends and implements
                                           // + analyzer - not both extends and with

class D1 = M with O1;
class D2 = O1 with M;
class D3 = O1 with O2 implements M;
class D4 = M with O1 implements M; // Both error - not both extends and implements
class D5 = M with M;  // Analyzer error - not both extends and with
class D6 = O1 with M implements M;
class D7 = O1 with M, M;
class D8 = O1 with O2 implements M, M; // Both error - implemented twice
class D9 = M with M implements M;  // Both error - not both extends and implements
                                   // + analyzer - not both extends and with

mixin G1 on M {}
mixin G2 on M, M {} // Both error - implemented twice
mixin G3 on M implements M {} // CFE error - "M can only be implemented once"
mixin G4 implements M, M {} // Both error - implemented twice

enum E1 with M {e}
enum E2 implements M {e}
enum E3 with M, M {e}
enum E4 with M implements M {e}
enum E5 implements M, M {e} // Both error - implemented twice

extension type F1(M _) implements M {}
extension type F2(M _) implements M, M {} // Both error - implemented twice

// ----

mixin class M {}
mixin class O1 {} // Other
mixin class O2 {} // Other

void main() {}

The analyzer has an extra error for "extends and implements same class". I filed a bug for that.
(Was also a VM bug: dart-lang/sdk#33596 (comment))

The CFE has one error of its own, for mixin ... on M implements M {}.
The on type is an interface, and a super-interface, but not a declared superinterface in the implements clause.
It shouldn't be an error. (But likely the spec is vague enough that it could be read as included in "not implement the same type twice".)

For extends S implements S being an error, see also dart-lang/sdk#52859