Proposal: readonly and sealed interfaces

[leonidumanskiy]

Quick summary
Introduce new usage for keywords "readonly" and "sealed":

// Interface can not be inherited
public sealed interface IService { } 

// Interface can only be implemented within current assembly
public readonly interface IService { } 

Motivation

• Prevent consumers of your library/nuget package from re-implementing your interfaces.
• Guarantee that adding new methods to existing interfaces will not break existing consumer code.
• Classes already have that mechanism (make all constructors private/internal). There is no such mechanism for interfaces.

Motivation (longer explanation)
When declaring an interface within a library, one could have two possible intentions:

1. Interface is intended to be implemented by library/package consumers. Adding new methods to that interface is considered a breaking change.
2. Interface is implemented within the library and it is not intended for client code to re-implement it. When new methods are added to that interface, it is not considered a breaking change.

Unfortunately, there is no way to make it clear whether consumers should or should not implement a given interface. As a result, it is not possible to add new methods to interfaces and guarantee that updating the library will not break consumer code.

As a result, when it comes to selecting a versioning strategy for your library you have two options:

1. Consider every interface change a breaking change. Increment a major version of your library / nuget package every time you add new functionality to an interface.
2. Rely on the documentation to describe what interfaces are not intended to be implemented by consumers. When adding new methods to your interfaces, consider it non-breaking change and only increment minor version.

While first option guarantees that adding new functionality will not break consumer code, this means that every time new methods are added to existing classes and interfaces, you must increment major version of your nuget package.

Proposed solution solves this by making a mechanism to prevent unintended interface usage. When it's guaranteed that no other code except your assembly implements an interface, you can safely add new methods to that interface and it is guaranteed that doing so will not break consumer code.

[ufcpp]

InternalImplementationOnly?

[leonidumanskiy]

@ufcpp I think attribute would also work. I don't know why this discussion didn't go anywhere tho (discussion issue was closed in 2015).

[CyrusNajmabadi]

@leonidumanskiy Can you link the discussion you're referring to?

[leonidumanskiy]

@CyrusNajmabadi It is what @ufcpp linked, plus discussion thread on these notes

[AustinBryan]

sealed and readonly are misleading because that means it can't be inherited, which defeats the point of interfaces, and it has nothing to do with any concept of readonly. If the real goal is to make it so that it can only be implemented by internal, then the name should reflect that.

Also, the whole motiviation is outdated now. C# 8 is adding default-method implementation (#52) so, you can safely add new methods, and, if the new methods have a default implementation (even if it's empty) it won't break existing code not using it. Hooray!

[JustNrik]

sealed interfaces can make sense, I think you are confusing inheriting with implementing, they are completely different things. A sealed interface would just prevent another interface inherithing it, it doesn't prevent the interface being implemented in a class or struct.

[leonidumanskiy]

@AustinBryan

1. I don't agree that making interface sealed defeats the point of interfaces, if sealed means interface can not be inherited from (but it can be implemented). Having readonly interfaces without sealed might allow users to inherit from an interface first, and then implement a new one:

AssemblyA:
public readonly interface IService { }

AssemblyB:
public interface IMyService : IService { }
public class MyImplementation : IMyService { }

I agree that "readonly" accessor keyword might not be the best fit, however I could not come up with a better one. If attribute is used instead of an accessor, "InternalImplementationOnly" might be a good option.

2. I don't see how "C# 8 is adding default-method implementation". What you linked is a proposal that still might get rejected. Please correct me if I am wrong. I would be happy to close this issue if "default method implementations" is 100% decided.

[yaakov-h]

@leonidumanskiy DIM has been prototyped and discussed amongst the LDM for a long time, and is currently scheduled for C# 8.0. AFAICT it's unlikely that it would be intentionally cut unless they run out of time and require more time to implement/improve.

[brabebhin]

Why not make the interfaces internal or private if you don't want users to implement them? The way I see it, an interface shouldn't be part of a public API surface if your users can't implement it. The only instances in which this does happen is COM interop/winRT, but there is a good reason interfaces are used there as API surface as opposed to concrete classes.

I really dont see a place where such a feature would be useful.

[Unknown6656]

I can see some usage cases for sealed interface ...., but I agree with @mcosmin222 on the readonly-issue: why not make it private or internal?

[theunrepentantgeek]

In the past, when I've had the requirement to prevent additional implementations outside the original assembly, I've been able to achieve the desired result with an abstract class.

Prior to C# 7.2, this involved declaring a public abstract class with a private constructor; all the implementations had to be nested within that class in order to access the private constructor.

From C# 7.2 it gets easier - the constructor can be declared as private protected, allowing the implementations to be in regular classes instead of being nested.

In either case, other implementations are impossible because they can't access the constructor on the abstract class.

So, I believe that this is already a solved problem, not one that requires modification of the language.

[RusKnyaz]

You can only inherit from one abstract class. So this is not always the solution to the problem.

[stuartstein777]

@mcosmin222 We use public interfaces in a nuget that is used internally. The interface exists so they can mock it, but the nuget also provides a concrete implementation.

e.g.

ISomeDbThing
{
    List<Foo> GetAllTheFoos();
}

SomeDbThing : ISomeDbThing
{
   List<Foo> GetAllTheFoos() => // do something to get the foos.
}

That way our developers can write testable code that they write against ISomeDbThing, but then use the concrete implementation at runtime, which is resolved through what IoC they use. I do't want them to really implement ISomeDbThing though.

Does this proposal solves this though?

[brabebhin]

I see your point, but I don't think we should have language features aimed at unit testing. Having better unit test tools more widely available should be the goal.

Interfaces for tests, especially in the case of db objects, is a case of overengineering. You can mock any unsealed class.

As for mocking more complex objects, use the right tools for the job. There are plenty around (like Microsoft Fakes), although many are not free. APIs should be designed for human users, not for working around test framework limitations. Don't let the tools dictate your design. The interface should be there to either provide DI support or multiple inheritance.

Anyway, I don't think the proposal fixes this. Your mocking object will be considered outside of his dll, so it can't implement the interface. It kind of defeats the whole purpose you are trying to achieve.

[jnm2]

@leonidumanskiy

Motivation (longer explanation)
When declaring an interface within a library, one could have two possible intentions:

1. Interface is intended to be implemented by library/package consumers. Adding new methods to that interface is considered a breaking change.
2. Interface is implemented within the library and it is not intended for client code to re-implement it. When new methods are added to that interface, it is not considered a breaking change.

Either way, why not allow consumers to create their own interface that inherits from yours? How can that possibly hurt anything? Since you're still allowing consumers to implement the interface, they'll still break just the same if you add (non-defaulting) members to your interface.

[dklompmaker]

Defining The Problem
The problem stems around communication of potentially breaking changes. If you need to increment a major version of your library every time you add a method signature to interfaces, then it quickly becomes heavy handed to make any sweeping changes.

Responsibility
Although it is the library owner's responsibility to communicate breaking changes with the consumers, I don't believe it's the library owner's responsibility to prevent consumers from their own implementations.

If a consumer chooses to implement your interface, it is on them to ensure it compiles. If the consumer chooses to upgrade your library, it is on them to make the changes necessary.

DIM
Default interface methods may support your request, but I don't really like the concept personally. We might as well do away with interfaces and simply use abstract classes in the future. Alas, the community has voted.

Summary
It seems like many library creators view adding method signatures to their interfaces as a minor change; when in reality it creates a breaking compiler change. Thus, this problem falls on the definition of Major/Minor changes. It seems like the library creators need to constrain their standards to follow the versioning rules they impose on their library.

Perhaps a new versioning scheme for this use-case could be: {libraryVersion}.{majorVersion}.{minorVersion}.{revision}.

This would keep the standard 4 number version. Conceptually the libraryVersion represents the whole of the system. Major would be used for any break change (interface additions included); so on and so forth.

[PathogenDavid]

The LDM notes linked by @ufcpp earlier allude to it, but if anyone wants a real-world example, the IOperation API in Roslyn is a good one. (It's also an example of one that was recently broken by a change in Roslyn 2.9.0.)

That attribute is enforced by Microsoft.CodeAnalysis.Analyzers via RS1009. (By the way those LDM notes are worded, it wasn't enforced by an analyzer back then.)

---

Personally, I think an attribute+analyzer enforcement is the way to go right now with DIM being the long-term proper solution.

No matter what I think there should always be a way for the developer to say "I appreciate the concern, but I don't care." For a project I'm working on right now, it's acceptable for us to be pinned to a specific version of Roslyn with no regard for forward compatibility, so we disable RS1009 in a handful of very narrow cases where the alternative is making the design significantly more complicated. (Without getting too far into it, we mock a couple IOperation interfaces for the purposes of rewriting certain IOperation subtrees to help with the way things are processed internally.)

[AustinBryan]

I don't agree that making interface sealed defeats the point of interfaces, if sealed means interface can not be inherited from (but it can be implemented).

1. By that's the problem, sealed doesn't mean "interface cannot be inherited from (but it can be implemented" it means "class cannot be inherited from. Now, what would be really confusing is if we reuse sealed again, because, unliked Java, the C# syntax doesn't differentiate between inheritance and implementation. Java has class A extends class B and class A implements IB whereas C# only has class A : B and class A : IB.

And, right now, sealed makes it so class A : B does not work. That's what we expect to happen. It would be really confusing and inconsistent if class A : IB does work, when IB is sealed. That goes against what intuition, I feel. I mean, you would have this:

sealed class B { }
sealed interface IB { }

class A : B        // does not work
class A : IB       // works
interface IA : IB  // does not work

That's inconsistent and confusing and involves redefining what sealed has meant for 16 years (in C#, at least).

Why not just use a different keyword to do the same exact functionality, but call it closed or something else so there's no confusion and redefinition?

2. Yeah, I'd say it's pretty much confirmed