Support public, internal, private, static, virtual, sealed, abstract,…

… extern and async modifiers with interface methods. (#18519)

docs/features/DefaultInterfaceImplementation.md

@@ -31,5 +31,7 @@ class Test1 : I1
 
 - Supplying an implementation along with declaration of an event and recognizing that implementation as default implementation for the event when a type implements the interface. 
 
+- Using **public**, **internal**, **private**, **static**, **virtual**, **sealed**, **abstract**, **extern** and **async** modifiers with interface methods.
+
 
 **Open issues and work items** are tracked in https://github.com/dotnet/roslyn/issues/17952.

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -1497,7 +1497,7 @@ If such a class is used as a base class and if the deriving class defines a dest
     <value>'{0}' in explicit interface declaration is not an interface</value>
   </data>
   <data name="ERR_InterfaceMemberNotFound" xml:space="preserve">
-    <value>'{0}' in explicit interface declaration is not a member of interface</value>
+    <value>'{0}' in explicit interface declaration is not found among members of the interface that can be implemented</value>
   </data>
   <data name="ERR_ClassDoesntImplementInterface" xml:space="preserve">
     <value>'{0}': containing type does not implement interface '{1}'</value>
@@ -5047,4 +5047,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_RuntimeDoesNotSupportDefaultInterfaceImplementationForMember" xml:space="preserve">
     <value>'{0}' cannot implement interface member '{1}' in type '{2}' because the target runtime doesn't support default interface implementation.</value>
   </data>
+  <data name="ERR_DefaultInterfaceImplementationModifier" xml:space="preserve">
+    <value>The modifier '{0}' is not valid for this item in C# {1}. Please use language version {2} or greater.</value>
+  </data>
 </root>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -1475,5 +1475,9 @@ internal enum ErrorCode
 
         ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation = 8501,
         ERR_RuntimeDoesNotSupportDefaultInterfaceImplementationForMember = 8502,
+
+        // PROTOTYPE(DefaultInterfaceImplementation): Should add this error code to the list monitored by the UpgradeProject code fixer.
+        // This PR can be used to find the relevant code and test files: https://github.com/dotnet/roslyn/pull/18045.
+        ERR_DefaultInterfaceImplementationModifier = 8503,
     }
 }

src/Compilers/CSharp/Portable/Symbols/Source/ExplicitInterfaceHelpers.cs

@@ -217,10 +217,11 @@ private static Symbol FindExplicitlyImplementedMember(
 
             foreach (Symbol interfaceMember in explicitInterfaceNamedType.GetMembers(interfaceMemberName))
             {
-                // At this point, we know that explicitInterfaceNamedType is an interface, so candidate must be public
-                // and, therefore, accessible.  So we don't need to check that.
+                // At this point, we know that explicitInterfaceNamedType is an interface.
                 // However, metadata interface members can be static - we ignore them, as does Dev10.
-                if (interfaceMember.Kind != implementingMember.Kind || interfaceMember.IsStatic)
+                // PROTOTYPE(DefaultInterfaceImplementation): We might want to check accessibility of the members as well, since  
+                //                                            they don't have to be public any more.
+                if (interfaceMember.Kind != implementingMember.Kind || !interfaceMember.IsImplementableInterfaceMember())
                 {
                     continue;
                 }

src/Compilers/CSharp/Portable/Symbols/Source/ModifierUtils.cs

@@ -62,6 +62,29 @@ internal static DeclarationModifiers CheckModifiers(
             return result;
         }
 
+        internal static void ReportDefaultInterfaceImplementationModifiers(
+            LanguageVersion availableVersion,
+            LanguageVersion requiredVersion,
+            DeclarationModifiers modifiers,
+            DeclarationModifiers defaultInterfaceImplementationModifiers,
+            Location errorLocation,
+            DiagnosticBag diagnostics)
+        {
+            DeclarationModifiers errorModifiers = modifiers & defaultInterfaceImplementationModifiers;
+            var requiredVersionArgument = new CSharpRequiredLanguageVersion(requiredVersion);
+            var availableVersionArgument = availableVersion.ToDisplayString();
+            while (errorModifiers != DeclarationModifiers.None)
+            {
+                DeclarationModifiers oneError = errorModifiers & ~(errorModifiers - 1);
+                Debug.Assert(oneError != DeclarationModifiers.None);
+                errorModifiers = errorModifiers & ~oneError;
+                diagnostics.Add(ErrorCode.ERR_DefaultInterfaceImplementationModifier, errorLocation,
+                                ConvertSingleModifierToSyntaxText(oneError),
+                                availableVersionArgument,
+                                requiredVersionArgument);
+            }
+        }
+
         private static string ConvertSingleModifierToSyntaxText(DeclarationModifiers modifier)
         {
             switch (modifier)

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberContainerSymbol_ImplementationChecks.cs

@@ -123,7 +123,7 @@ private ImmutableArray<SynthesizedExplicitImplementationForwardingMethod> Comput
                         case SymbolKind.Method:
                         case SymbolKind.Property:
                         case SymbolKind.Event:
-                            if (interfaceMember.IsStatic)
+                            if (!interfaceMember.IsImplementableInterfaceMember())
                             {
                                 continue;
                             }

src/Compilers/CSharp/Portable/Symbols/Source/SourceMemberMethodSymbol.cs

@@ -769,21 +769,39 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind
 
             // Check that the set of modifiers is allowed
             var allowedModifiers = DeclarationModifiers.Partial | DeclarationModifiers.Unsafe;
+            var defaultInterfaceImplementationModifiers = DeclarationModifiers.None;
 
             if (methodKind != MethodKind.ExplicitInterfaceImplementation)
             {
-                allowedModifiers |= DeclarationModifiers.New;
+                allowedModifiers |= DeclarationModifiers.New |
+                                    DeclarationModifiers.Sealed |
+                                    DeclarationModifiers.Abstract |
+                                    DeclarationModifiers.Static |
+                                    DeclarationModifiers.Virtual;
 
                 if (!isInterface)
                 {
                     allowedModifiers |=
                         DeclarationModifiers.AccessibilityMask |
-                        DeclarationModifiers.Sealed |
-                        DeclarationModifiers.Abstract |
-                        DeclarationModifiers.Static |
-                        DeclarationModifiers.Virtual |
                         DeclarationModifiers.Override;
                 }
+                else
+                {
+                    const DeclarationModifiers allowedAccess = (DeclarationModifiers.AccessibilityMask & ~(DeclarationModifiers.Protected | DeclarationModifiers.ProtectedInternal));
+
+                    // This is needed to make sure we can detect 'public' modifier specified explicitly and
+                    // check it agains language version below.
+                    defaultAccess = DeclarationModifiers.None;
+
+                    allowedModifiers |= allowedAccess | DeclarationModifiers.Extern | DeclarationModifiers.Async;
+                    defaultInterfaceImplementationModifiers |= DeclarationModifiers.Sealed |
+                                                               DeclarationModifiers.Abstract |
+                                                               DeclarationModifiers.Static |
+                                                               DeclarationModifiers.Virtual |
+                                                               DeclarationModifiers.Extern | 
+                                                               DeclarationModifiers.Async |
+                                                               allowedAccess;
+                }
             }
 
             if (!isInterface)
@@ -796,6 +814,20 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind
 
             this.CheckUnsafeModifier(mods, diagnostics);
 
+            if (!hasBody && (mods & defaultInterfaceImplementationModifiers) != 0)
+            {
+                LanguageVersion availableVersion = ((CSharpParseOptions)location.SourceTree.Options).LanguageVersion;
+                LanguageVersion requiredVersion = MessageID.IDS_DefaultInterfaceImplementation.RequiredVersion();
+                if (availableVersion < requiredVersion)
+                {
+                    ModifierUtils.ReportDefaultInterfaceImplementationModifiers(availableVersion, requiredVersion, mods, 
+                                                                                defaultInterfaceImplementationModifiers, 
+                                                                                location, diagnostics);
+                }
+
+                // PROTOTYPE(DefaultInterfaceImplementation): Should we also check runtime support for some of the modifiers?
+            }
+
             mods = AddImpliedModifiers(mods, isInterface, methodKind, hasBody);
             return mods;
         }
@@ -806,8 +838,29 @@ private static DeclarationModifiers AddImpliedModifiers(DeclarationModifiers mod
             // Proper errors must have been reported by now.
             if (containingTypeIsInterface)
             {
-                mods = (mods & ~DeclarationModifiers.AccessibilityMask) | DeclarationModifiers.Public | 
-                       (hasBody ? DeclarationModifiers.Virtual : DeclarationModifiers.Abstract);
+                if ((mods & (DeclarationModifiers.Static | DeclarationModifiers.Private | DeclarationModifiers.Virtual | DeclarationModifiers.Abstract)) == 0)
+                {
+                    if (hasBody || (mods & DeclarationModifiers.Extern) != 0)
+                    {
+                        if ((mods & DeclarationModifiers.Sealed) == 0)
+                        {
+                            mods |= DeclarationModifiers.Virtual;
+                        }
+                        else
+                        {
+                            mods &= ~DeclarationModifiers.Sealed;
+                        }
+                    }
+                    else
+                    {
+                        mods |= DeclarationModifiers.Abstract;
+                    }
+                }
+
+                if ((mods & DeclarationModifiers.AccessibilityMask) == 0)
+                {
+                    mods |= DeclarationModifiers.Public;
+                }
             }
             else if (methodKind == MethodKind.ExplicitInterfaceImplementation)
             {

src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbol.cs

@@ -1566,10 +1566,13 @@ protected void CheckModifiersForBody(SyntaxNode declarationSyntax, Location loca
 
                 if (!ContainingAssembly.RuntimeSupportsDefaultInterfaceImplementation)
                 {
+                    // PROTOTYPE(DefaultInterfaceImplementation): It looks like some flavor of static members was supported by
+                    //                                            legacy runtimes. Should we suppress this error for them?
                     diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportDefaultInterfaceImplementation, location);
                 }
             }
-            else if (IsExtern && !IsAbstract)
+
+            if (IsExtern && !IsAbstract)
             {
                 diagnostics.Add(ErrorCode.ERR_ExternHasBody, location, this);
             }

src/Compilers/CSharp/Portable/Symbols/SymbolExtensions.cs

@@ -386,5 +386,10 @@ internal static void GetTypeOrReturnType(this Symbol symbol, out RefKind refKind
                     throw ExceptionUtilities.UnexpectedValue(symbol.Kind);
             }
         }
+
+        internal static bool IsImplementableInterfaceMember(this Symbol symbol)
+        {
+            return !symbol.IsStatic && (symbol.IsAbstract || symbol.IsVirtual) && (symbol.ContainingType?.IsInterface ?? false); 
+        }
     }
 }

src/Compilers/CSharp/Portable/Symbols/TypeSymbol.cs

@@ -466,6 +466,11 @@ public Symbol FindImplementationForInterfaceMember(Symbol interfaceMember)
                 throw new ArgumentNullException(nameof(interfaceMember));
             }
 
+            if (!interfaceMember.IsImplementableInterfaceMember())
+            {
+                return null;
+            }
+
             return FindImplementationForInterfaceMemberWithDiagnostics(interfaceMember).Symbol;
         }
 
@@ -777,6 +782,7 @@ private SymbolAndDiagnostics ComputeImplementationAndDiagnosticsForInterfaceMemb
         private static Symbol ComputeImplementationForInterfaceMember(Symbol interfaceMember, TypeSymbol implementingType, DiagnosticBag diagnostics)
         {
             Debug.Assert(interfaceMember.Kind == SymbolKind.Method || interfaceMember.Kind == SymbolKind.Property || interfaceMember.Kind == SymbolKind.Event);
+            Debug.Assert(interfaceMember.IsImplementableInterfaceMember());
 
             NamedTypeSymbol interfaceType = interfaceMember.ContainingType;
             Debug.Assert((object)interfaceType != null && interfaceType.IsInterface);

src/Compilers/CSharp/Test/Semantic/Semantics/BindingAsyncTests.cs

@@ -1214,10 +1214,14 @@ interface IInterface
 {
     async void F(); 
 }";
-            CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
-                // (4,16): error CS0106: The modifier 'async' is not valid for this item
+            CreateCompilationWithMscorlib45(source, parseOptions:TestOptions.Regular7).VerifyDiagnostics(
+                // (4,16): error CS8503: The modifier 'async' is not valid for this item in C# 7. Please use language version 7.1 or greater.
+                //     async void F(); 
+                Diagnostic(ErrorCode.ERR_DefaultInterfaceImplementationModifier, "F").WithArguments("async", "7", "7.1").WithLocation(4, 16),
+                // (4,16): error CS1994: The 'async' modifier can only be used in methods that have a body.
                 //     async void F(); 
-                Diagnostic(ErrorCode.ERR_BadMemberFlag, "F").WithArguments("async"));
+                Diagnostic(ErrorCode.ERR_BadAsyncLacksBody, "F").WithLocation(4, 16)
+                );
         }
 
         [Fact]