Feature propsoal: Allow use of override keyword on interface method implementations.

[rerdavies]

Proposal:

Allow the use of the override method modifier on methods that implement interface methods.

Rationale:

The override modifier cannot be used on methods that implement an inteface method, but do not implement a virtual or abstract method in a base class.

The problem with this is that there is no way to detect orphaned implementations of interface methods at compile time.

Orphaned overrides of virtual or abstract methods generate a compiler error; not so for orphaned methods that used to implement an interface method.

If the override method modifier is permitted on methods that implement interface methods, developers can mark these methods with the override keyword, and be given an error message when the method no longer implements an interface method.

Java tooling, interestingly, optionally allows use of the @OverRide attribute on interface methods. By default it is not allowed. But when enabled, will generate a warning or error when a method does not override an existing base class method or interface method.

Since all methods are virtual in Java, but not in C#, there should be a mechanism to declare virtual and abstract implementations of interface methods.

Example:

public interface IStartable {
      void Start();
 }

public class StartableClass : IStartable {
    // currently generates an error. Would not under the current 
   // proposal.
      public override void Start() {  
               OnStart();
      }
     protected abstract void OnStart();
}

If the Start method in IStartable were later renamed to OnStart(), the declaration of StartableClass.Start wold generate an error since it no longer implements an interface method.

Startable class would then be rewritten as:

public class StartableClass : IStartable {
      public override abstract void OnStart();
 }

Details:

When a method overrides an existing method in a base class, regardless of whether it also implements an interface method, existing semantics apply as usual:

 override  - A virtual implementation of the base method, and any interface methods.

When a method does not override an existing method, the following semantics apply:

override - An error if the method does not implement an interface member
Otherwise, a non-virtual implementation of the interface method.

override virtual - An error if the method does not implement an interface member.
Otherwise a virtual implementation of the interface method.

override abstract - An error if the method does not implement an interface member;
otherwise, an abstract implementation of an interface method.

Use of the override modifier on interface method implementation is optional. If the override keyword is not present, current semantics apply.

Whether the method implements an interface or not is determined within the scope of the declaring class: the declaring class, or base classes of the declaring class must indicate that they implement the interface in question. The method may become an interface method implementation in a derived class by mixin; but must be declared without the override modifier in that case.

Impact:

The proposal is currently syntactically legal, but not semantically legal. It has no impact on emitted code, since it generates an error during semantic analysis of a class declaration. It is not backward compatible, because it allows code that would be semantically invalid in previous versions of C#.

[svick]

@rerdavies

The problem with this is that there is no way to detect orphaned implementations of interface methods at compile time.

There are some options:

1. Use explicit interface implementation.
2. Detect public instance methods with no callers.

If the Start method in IStartable were later renamed to OnStart(), the declaration of StartableClass.Start wold generate an error since it no longer implements an interface method.

It would generate an error even without the override, because the protected method OnStart can't implement the renamed IStartable.OnStart.

---

In general, I'm not convinced that this would be actually valuable. If an interface changes in your own code, you should update all its implementations at the same time, so this wouldn't help much. An interface changing like this in an external library should be quite rare and when it does happen, it's likely to cause some kind of compiler error. And even when it doesn't, the only harm is some unnecessary code. How is that worth changing the language?

[PathogenDavid]

The problem with this is that there is no way to detect orphaned implementations of interface methods at compile time.

I agree with this being a valuable premise. Unfortunately, I also think the ship has already sailed. You'll only detect the error in cases where the developer was aware of this benefit and used override on all implemented methods.

I know I'm beating a dead horse here, but I think this is a good use case for analyzers. Interfaces could be marked with attributes like [OldMethod("Start")] and any type implementing that interface with a method matching the attribute would get a warning from the analyzer.

That way the feature works with existing code without the breaking change of adding a new language feature that introduces a similar warning.

(Of course, in an alternative universe where this proposal is accepted, you could just as easily implement an analyzer that complains about any interface implementations not marked as override.)

[svick]

@PathogenDavid

Of course, in an alternative universe where this proposal is accepted, you could just as easily implement an analyzer that complains about any interface implementations not marked as override.

If you design the analyzer differently, I think you don't need an alternative universe: the analyzer could complain about any interface implementation not marked [Override].

[canton7]

I agree with this being a valuable premise. Unfortunately, I also think the ship has already sailed. You'll only detect the error in cases where the developer was aware of this benefit and used override on all implemented methods.

I don't think that's right?

The problem here (at least in my experience, and I've wanted this for a long time) is that you write a method to implement an interface method, then later that interface method is removed, but your implementation lives on, unnecessarily.

Therefore what you want is an error if a method is marked override but doesn't override anything. The ship hasn't sailed here: methods not marked override behave the same as before; methods newly marked override are now checked.

This is the same story as overriding base methods. The only reason that we have an override keyword is so that the compiler can tell us if we've made a mistake and we're not actually overriding anything. Java didn't have @Override originally, they added it for exactly this case.

Explicit interface implementations work here sometimes, but they have issues with inheritance (and FxCop will moan at you if you use one in a non-sealed class) and of course callers need cast you to the interface type first.

An analyser and [Override] attribute would work (the analyser would complain if it saw a method with this attribute which wasn't implementing an interface member), but it's ugly having both override and [Override] (or any attribute) for what is essentially the same meaning.

[PathogenDavid]

If you design the analyzer differently, I think you don't need an alternative universe: the analyzer could complain about any interface implementation not marked [Override].

Also a good idea, although I'd suggest the name [InterfaceImplementation] or something similar to make it easier to tell it's intent and avoid the issue @canton7 mentioned.

I don't think that's right?

You're right. I was only looking at it from the perspective of the interface author wanting errors to occur when things change rather than the interface implementer.

[gafter]

Interface overrides are specified in https://github.com/dotnet/csharplang/blob/master/proposals/covariant-returns.md

[rerdavies]

@svick

If an interface changes in your own code, you should update all its implementations at the same time

You should. And this is precisely the use-case for this feature. You delete a method from an interface; and you should also delete all implementations. But there is currently no way to find all the implementations of that deleted method. The designers of C# thought this use-case important enough for method overrides that they provided a keyword specifically for that purpose. But there is no equivalent tooling for interface methods..

In the case of deleted methods,there is no warning or error whatsoever.

Explicit interface implementation is not a realistic option (as previously pointed out). The problem with relying on warnings about unused public methods is that kind of warning works when code is stable, and ready to commit, and you're cleaning up all the last warnings and infos. But for code-in-progress, this is the kind of information you'd like when the interface gets changed, rather than when you're tidying up. It seems to me it's an error of a different priority than whether a member variable was unsued (for example).

[rerdavies]

@gafter

The current status of the covariant returns proposal: "Offline discussion toward a decision to support overriding of class methods only in C# 9.0.".

[CyrusNajmabadi]

You should. And this is precisely the use-case for this feature. You delete a method from an interface; and you should also delete all implementations.

Why?

Either the mtehod is public, in which case you expected people to call it, outside of the interface, in which case you need to keep it. Or it's an explicit-impl, in which case this will break.

It feels weird that you're trying to have it both ways. "this should only be called by people calling the interface, but i'm not going to explicit-impl it".

I'm struggling to find a case where i've ever wanted that, or where that would make sense.

[svick]

@rerdavies

But there is currently no way to find all the implementations of that deleted method. The designers of C# thought this use-case important enough for method overrides that they provided a keyword specifically for that purpose.

I think the reason override keyword exists is to make sure you're actually overriding the method.

Consider code like this:

class Base
{
    public virtual void SomeLongMethodName() {}
}

class Derived : Base
{
    public override void SomeLongMethodNam() {}
}

Thanks to override, the above doesn't compile, alerting you to the typo. Interfaces don't have the same problem, because the equivalent code there would complain that the class does not implement the method.

Except that's not actually true anymore, because of default interface methods: an interface can contain a virtual method and if you want to override it with a public method, there is no good way to let the compiler check the spelling for you.

Because of that, I think I am now slightly in favor of this feature.

[GeeLaw]

Suppose X is a public class/struct member eligible for implicit interface implementation, then we can think of what is happening as that the interface member is automatically explicitly implemented to call the public member X. (*)

The modifiers on X are for the class member, not the interface member. For this reason, I'm not in favor of this proposal, because the public /* override? */ void Foo() is not overriding any overridable class member.

An interface member is always virtual and never sealed, and whenever it is (re-)implemented, it is overridden. For the same reason why you cannot say const static, these modifiers should not appear for any interface member.

It's very helpful to avoiding many traps by disconnecting a public member and the interface member it implicitly implements and only retaining the "forwarding relationship".

To detect removed/misspelt interface members, just always use explicit implementation (and forward the call to a public member, if it was intended to be implicitly implemented).

(*) What actually happens is different. Implicit implementation without virtual will mark the public member as virtual final in IL, whereas a separate forwarding explicit implementation will not mark the public member without virtual as virtual at all. They're equivalent for all practical purposes, and isn't the latter actually better!

[HaloFour]

@GeeLaw

The modifiers on X are for the class member, not the interface member.

Yes, the override here would be applied to the class member.

For this reason, I'm not in favor of this proposal, because the public /* override? */ void Foo() is not overriding any overridable class member.

override doesn't specifically mean that the base member has to be a class member, it only means that the class member is overriding the virtual slot for any base member, including an interface member. Notably, this is exactly what you use in IL when you want to explicitly override an interface member, which you can see with explicit implementation. It's not required with implicit implementation because the runtime will automagickally wire up any compatible member, but it may be explicitly specified.

    .method public final hidebysig newslot virtual instance void Dispose () cil managed 
    {
        .override method instance void [System.Private.CoreLib]System.IDisposable::Dispose()
        .maxstack 1

        ret
    } 

This is also the convention in Java where the @Override annotation may be used to explicitly mark the implementing members:

public class C : AutoCloseable {
    @Override
    public void close() { }
}

This will cause a compiler error if you remove AutoCloseable from the list of interfaces implemented by the class.

[GeeLaw]

@HaloFour The magic is done by C# compiler, not the (common language) runtime.

Edit: Thanks to HaloFour for pointing out that CLR does participate in resolving interface mapping.

override doesn't specifically mean that the base member has to be a class member, it only means that the class member is overriding the virtual slot for any base member, including an interface member.

This is quite debatable in the world of C#. Let me elaborate on the philosophy of "implicit = forwarding explicit". Suppose we have the following code:

interface IFoo { void Foo(); }
class FooImpl : IFoo { public /* virtual? */ void Foo() { } }
class FooFwdExpl : IFoo { public /* virtual? */ void Foo() { } void IFoo.Foo() { Foo(); } }

My claim is that as far as C# language is concerned, there is no difference (*) between the two:

• If virtual is not specified, the class member Foo cannot be overridden in derived classes.
• If virtual is specified, the class member Foo can be overridden in derived classes. If the class member Foo is overridden, the interface member IFoo.Foo will also use the overriding version.
• The class member Foo can always be shadowed using new. If so, the interface member IFoo.Foo has nothing to do with the shadowing version.
• The interface member IFoo.Foo can always be re-implemented. If so, the re-implementing version of IFoo.Foo has nothing to do with the class member Foo.

(*) The emitted IL by today's C# compiler are different, but that's an implementation detail.

Using this methodology, one can regard any implicit implementation as being rewritten as follows:

  public <modifiers> TRet Method(<arglist>) { <body> }
=>
  public <modifiers> TRet Method(<arglist>) { <body> }
  TRet IIntf.Method(<arglist>) { return Method(<args>); }

The rule is simple and clear. The rule that all modifiers goes to the public member is what I meant by "the modifiers are for the class member, not the interface member".

If you agree with this methodology, it's clear that override should not be used in FooImpl, because the override will transfer to the class member Foo after rewriting, and it's incorrect to say public override void Foo() { } in FooFwdExpl.

---

The separation of interface and class members will be important in certain cases:

// Foo0.Foo = IFoo.Foo = 0
class Foo0 : IFoo { public void Foo() { Console.WriteLine("Foo0"); } }
// Foo1.Foo = 1, Foo0.Foo = IFoo.Foo = 0
class Foo1 : Foo0 { public new void Foo() { Console.WriteLine("Foo1"); } }
// Foo2.Foo = IFoo.Foo = 2, Foo0.Foo = 0
class Foo2 : Foo0, IFoo { public new void Foo() { Console.WriteLine("Foo2"); } }

// 1 0 0
(new Foo1()).Foo();
((Foo0)new Foo1()).Foo();
((IFoo)new Foo1()).Foo();
// 2 0 2
(new Foo2()).Foo();
((Foo0)new Foo2()).Foo();
((IFoo)new Foo2()).Foo();

Also, by orthogonality of features, if this proposal were to be accepted, override applied to interface members should not affect how new is applied to class members, but it would sound weird to say public new override void Foo() in Foo2 (which should be interpreted as "new" class member "override" interface member), right?

---

It's quite a weak justification for a counterpart to be in C# just because some feature exists in Java. The worst feature C# borrowed from Java is array covariance. Also, interfaces in Java are very different from those in C#:

• You can seal an interface method in Java, but you can't in C#.
• You cannot explicitly implement (hide) interface methods in Java, but you can in C#.

Interfaces are closer to abstract classes in Java than they are in C#.

[HaloFour]

@GeeLaw

The magic is done by C# compiler, not the (common language) runtime.

The magic is done by both. The language makes the member automatically virtual final, and runtime is what silently allows the member to implement the interface without an explicit .override modifier.

Interfaces are closer to abstract classes in Java than they are in C#.

This is very much not true. C# and Java interfaces are nearly identical, with the exception that C# interfaces are a bit more flexible due to the explicit .override modifier that allows an implementing member to have a differing name and accessibility and enables "explicit implementation".

You are correct that the distinction between the two, such as with explicit implementation, is a language concern. At the IL level they are nearly identical, and it's totally possible to have "explicit" overrides of virtual members on the base class. The point of this proposal is to redefine those terms in C# so that override is more broadly applicable, the fact that it doesn't mean that today isn't that relevant. And the reasons for doing so are very similar to why Java allows @Override on implemented interface members, so I think it's perfectly relevant.

[macsux]

I've just created a Roslyn analyzer to address this.
https://stakhov.pro/hidden-dangers-of-implicit-interface-implementations

[zvrba]

Yes, I also miss that compiler detects deleted interface members. After having read this discussion, I have another proposal: if an interface member is marked with [Obsolete("Obsoleted" /*, true*/)] then the compiler flags all members that implement the obsoleted interface method. As of today, it only flags uses of the method.

So you would mark the interface method as obsolete, clean up the implementations that the compiler complains about and finally remove the member from the interface.

[rerdavies]

It's quite a weak justification for a counterpart to be in C# just because some feature exists in Java.
The worst feature C# borrowed from Java is array covariance. Also, interfaces in Java are very different from those in C#:

The justification is not that Java has it. The justification is that it is useful, increases productivity, and prevents defects in production code. And that the current implementation of the existing "override" modifier feature in C# misses an important use-case and is therefore cognitively dissonant, and in many cases counterproductive.

The subtle distinction between virtual default method, pure virtual class method, interface method, and default interface method is too fine a distinction to be made over the keyboard while pounding out code. I just want a way to make sure that it will be an error if the method I am writing no longer overrides something without having to break flow by navigating through class hierarchy to find out whether it was originally declared as an interface method rather than a class method.

(Worth point out in passing that the feature request is NOT the same as the Java features, since THAT feature can trivially be implemented with existing C# features). And because Java doesn't have an existing "override" modifier.

[gaspardpetit]

If I can chip in, C++ also allows the override keyword against pure virtual methods, for the same reasons raised in this proposal. Overriding an interface method might cause conceptual discomfort, since semantically speaking, in the English language, 'override' suggests an alteration and there is obviously no alteration when used against an interface method.

But C# is not English. A strongly typed programming language like C# should be concerned about maintainability and refactoring use cases. Using override against an interface method is a simple but powerful safeguard against orphaning of methods. Apart from the conceptual argument, I see no reason not to allow it, and those who dislike it can simply chose to ignore feature.

[zvrba]

'override' suggests an alteration and there is obviously no alteration when used against an interface method.

With default interface methods, even alteration is on the table :)

[FaustVX]

@gaspardpetit @zvrba

override suggests an alteration and there is obviously no alteration when used against an interface method.

Also, we already use override over abstract methods, but there are still no alteration :)

[zvrba]

And another case with refactorings.

Given class C { public void M() { ... } }. Now add an interface with a default implementation and add it to the class:

interface ID { void M() { /* default impl */ } }
class C : ID { /* as before */ }

Now, just adding ID to C, unintentionally (? or not -- not possible to say without a keyword) overrode the default implementation present in ID. This is a silent override, which is 100% opposite of how overriding works with classes, where it's required to be doubly explicit (override and virtual keywords).

Anyway, I'd be most happy to just acknowledge that override is an instruction to replace a slot in a dispatch table. Therefore it should be allowed on interface method implementations and signal an error if no slot is found to be overwritten. The compiler should also signal a warning if a slot would be silently overwritten (as in the example).

To support existing code, there should be two warnings: 1) Class method silently defines a non-DIM interface method [turning it on would make it possible to fix existing code gradually], 2) loud warning if a class overwrites a vtable/itable slot without override keyword (the case illustrated above).

[333fred]

As this is not a championed proposal, I'm converting this to a discussion, as per our README.