All inherited interface members should be implemented on an interface marked with DynamicInterfaceCastableImplementationAttribute

[jkoritzinsky]

An interface marked with a System.Runtime.InteropServices.DynamicInterfaceCastableImplementationAttribute attribute should provide a default interface implementation of all inherited interface methods and should not provide any new non-private interface methods without a default implementation.

interface Foo
{
      void Frob();
}

[DynamicInterfaceCastableImplementation]
interface Bar : Foo
{
	void Baz();
}

Category: Usage
Default: Enabled
Severity: Warning

When a method is not implemented, a warning will be emitted on the attributed interface name. A code fix will be offered to implement the method with a default implementation that throws a NotImplementedException.

When a method is defined on the implementation interface, a warning will be emitted on the method name. If the method has a body, a code fix will be offered to make the method public sealed. Otherwise, a code fix will be offered to make the method sealed and add a body that throws a NotImplementedException.

cc @terrajobst @stephentoub @AaronRobinsonMSFT @elinor-fung

[Mrnikbobjeff]

Does this actually mean that the interface has to have implementations for all methods but not provide them itself?
Specifically I would like to know whether
public interface IA { string Foo() => "Test"; } public interface IB : IA { }
IB could fulfill the attribute contract

[jkoritzinsky]

Yes, your example would fulfill the attribute contract.

[jkoritzinsky]

Marking as blocking so we codify that we'll start the schedule with this issue on Friday.

[KevinCathcart]

Is there a hard rule that no inbox analyzers are allowed to default to error severity? If not, then I pose this question: If this is not a good candidate for error severity, then what would qualify? Consider that if hypothetically the development of this feature had been driven by the compiler team rather than by COMWrappers (perhaps one could imaging the compiler team inventing it as part of an effort to refresh and improving dynamic and the DLR), then the compiler absolutely would already be emitting an error here.

When you apply this attribute you are making the statement that the interface is effectively concrete (regardless of the fact that the metadata says otherwise since it is an interface). Thus logically speaking an interface that has this attribute but which has failed to have a concrete implementation of all methods has a direct analogy in. That would be a concrete class failing to implement all abstract methods of a base class (or to fail to implement all non-DIM methods of some interface it claims to implement).

The compiler treats those an as error. Other than a rule that no in-box analyzers are allowed to default to error, it seems to me that any argument that this should be only a warning, would be equally an argument that the compiler out to downgrade the class cases to a warning. I guess there is an argument that we don't want error level diagnostics to suddenly appear in code that previously compiled (modulo people using the warnings as errors feature, but they are literally asking for it). My counter argument would be that given how extremely obscure this feature still is at the moment, I'm not sure that is really a big deal [0]. Obviously if somebody really needed to compile code that would error with this (for example there is probably a unit test that verifies the behavior in this scenario), they could suppress the diagnostic, or change the default level of the diagnostic.

Footnote [0]

The complete list of public GitHub repos with source using this attribute as of time of writing:

• dotnet/runtime
• microsoft/CsWinRT
• dotnet/samples
• mono/SkiaSharp (has CsWinRT generated source for a COM wrapper)
• SteamTools-Team/SteamTools (has CsWinRT generated source for a COM wrapper)
• Silver-Fang/WindowsContracts.Net (repo is just CsWinRT generated source for Windows.Foundation.UniversalApiContract, Windows.Foundation.FoundationContract, and Windows.Networking.Connectivity.WwanContract)
• A few other repos that are just forks of one of the above (that GitHub does not know are forks), or that include an unzipped copy of all the samples.

So basically only Microsoft/.NET Foundation have written public code using this that is not CsWinRT generated. So while I'm sure there are a handful of private projects using this, I'd not be shocked if it was only 100 or fewer. Or to put it another way, the legacy Desktop Framework made regularly made breaking changes that were far more impactful than adding an error here.

All that said, I'm sure the underlying feature will only become more popular to use in hand written code over time. Many developers simply only use the LTS releases for example.

[bartonjs]

Video

The analyzer looks good as proposed.

Regarding Error vs Warning: @terrajobst can you chime in with what the heuristics are? If there's a mode where we can be an on-by-default Error we think that might be the right answer.

[jkoritzinsky]

Blocked on dotnet/roslyn#53605

[Youssef1313]

@jkoritzinsky If you're working on this, you can temporarily use SyntaxFactory instead of SyntaxGenerator. The fix to SyntaxGenerator (when done) might take some time to flow into roslyn-analyzers repository (the version in roslyn-analyzers isn't updated frequently).

[jkoritzinsky]

@Youssef1313 that worked. It's ugly, but it'll do.

[sharwell]

Is there a hard rule that no inbox analyzers are allowed to default to error severity?

I always argue for the position that error severity is reserved for cases where the language specification fails to provide unambiguous semantics allowing the production of binary code for source in the specified form. Warnings are for cases where the binary code is unambiguous according to the language specification, but may not be what the user wanted. On this definition, analyzers cannot produce error diagnostics, but source generators might.

Use of Treat Warnings as Errors is highly encouraged for CI scenarios, which forces action be taken to address warnings one step removed from the innermost developer loop.

[Youssef1313]

@sharwell FYI there is an analyzer with error default severity dotnet/roslyn-analyzers#5155.

[sharwell]

@Youssef1313 I don't see any analyzers using that value, so I'll just update the comment for clarity. dotnet/roslyn-analyzers#5303

[jkoritzinsky]

Fixed by dotnet/roslyn-analyzers#5129