Merge "extended partial methods" feature to 16.7-p2

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -2112,14 +2112,11 @@ If such a class is used as a base class and if the deriving class defines a dest
     <value>Inconsistent lambda parameter usage; parameter types must be all explicit or all implicit</value>
   </data>
   <data name="ERR_PartialMethodInvalidModifier" xml:space="preserve">
-    <value>A partial method cannot have access modifiers or the virtual, abstract, override, new, sealed, or extern modifiers</value>
+    <value>A partial method cannot have the 'abstract' modifier</value>
   </data>
   <data name="ERR_PartialMethodOnlyInPartialClass" xml:space="preserve">
     <value>A partial method must be declared within a partial class, partial struct, or partial interface</value>
   </data>
-  <data name="ERR_PartialMethodCannotHaveOutParameters" xml:space="preserve">
-    <value>A partial method cannot have out parameters</value>
-  </data>
   <data name="ERR_PartialMethodNotExplicit" xml:space="preserve">
     <value>A partial method may not explicitly implement an interface method</value>
   </data>
@@ -2156,9 +2153,6 @@ If such a class is used as a base class and if the deriving class defines a dest
   <data name="ERR_PartialMethodInExpressionTree" xml:space="preserve">
     <value>Partial methods with only a defining declaration or removed conditional methods cannot be used in expression trees</value>
   </data>
-  <data name="ERR_PartialMethodMustReturnVoid" xml:space="preserve">
-    <value>Partial methods must have a void return type</value>
-  </data>
   <data name="WRN_ObsoleteOverridingNonObsolete" xml:space="preserve">
     <value>Obsolete member '{0}' overrides non-obsolete member '{1}'</value>
   </data>
@@ -6106,4 +6100,25 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_RelationalPatternWithNaN" xml:space="preserve">
     <value>Relational patterns may not be used for a floating-point NaN.</value>
   </data>
+  <data name="IDS_FeatureExtendedPartialMethods" xml:space="preserve">
+    <value>extended partial methods</value>
+  </data>
+  <data name="ERR_PartialMethodWithNonVoidReturnMustHaveAccessMods" xml:space="preserve">
+    <value>Partial method '{0}' must have accessibility modifiers because it has a non-void return type.</value>
+  </data>
+  <data name="ERR_PartialMethodWithOutParamMustHaveAccessMods" xml:space="preserve">
+    <value>Partial method '{0}' must have accessibility modifiers because it has 'out' parameters.</value>
+  </data>
+  <data name="ERR_PartialMethodWithAccessibilityModsMustHaveImplementation" xml:space="preserve">
+    <value>Partial method '{0}' must have an implementation part because it has accessibility modifiers.</value>
+  </data>
+  <data name="ERR_PartialMethodWithExtendedModMustHaveAccessMods" xml:space="preserve">
+    <value>Partial method '{0}' must have accessibility modifiers because it has a 'virtual', 'override', 'sealed', 'new', or 'extern' modifier.</value>
+  </data>
+  <data name="ERR_PartialMethodAccessibilityDifference" xml:space="preserve">
+    <value>Both partial method declarations must have identical accessibility modifiers.</value>
+  </data>
+  <data name="ERR_PartialMethodExtendedModDifference" xml:space="preserve">
+    <value>Both partial method declarations must have identical combinations of 'virtual', 'override', 'sealed', and 'new' modifiers.</value>
+  </data>
 </root>

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

@@ -531,7 +531,7 @@ internal enum ErrorCode
         ERR_InconsistentLambdaParameterUsage = 748,
         ERR_PartialMethodInvalidModifier = 750,
         ERR_PartialMethodOnlyInPartialClass = 751,
-        ERR_PartialMethodCannotHaveOutParameters = 752,
+        // ERR_PartialMethodCannotHaveOutParameters = 752, Removed as part of 'extended partial methods' feature
         // ERR_PartialMethodOnlyMethods = 753, Removed as it is subsumed by ERR_PartialMisplaced
         ERR_PartialMethodNotExplicit = 754,
         ERR_PartialMethodExtensionDifference = 755,
@@ -544,7 +544,7 @@ internal enum ErrorCode
         ERR_PartialMethodStaticDifference = 763,
         ERR_PartialMethodUnsafeDifference = 764,
         ERR_PartialMethodInExpressionTree = 765,
-        ERR_PartialMethodMustReturnVoid = 766,
+        // ERR_PartialMethodMustReturnVoid = 766, Removed as part of 'extended partial methods' feature
         ERR_ExplicitImplCollisionOnRefOut = 767,
         ERR_IndirectRecursiveConstructorCall = 768,
 
@@ -1788,6 +1788,17 @@ internal enum ErrorCode
 
         #endregion diagnostics introduced for C# 9.0
 
+        #region diagnostics introduced for C# 9
+
+        ERR_PartialMethodWithAccessibilityModsMustHaveImplementation = 8795,
+        ERR_PartialMethodWithNonVoidReturnMustHaveAccessMods = 8796,
+        ERR_PartialMethodWithOutParamMustHaveAccessMods = 8797,
+        ERR_PartialMethodWithExtendedModMustHaveAccessMods = 8798,
+        ERR_PartialMethodAccessibilityDifference = 8799,
+        ERR_PartialMethodExtendedModDifference = 8800,
+
+        #endregion
+
         // Note: you will need to re-generate compiler code after adding warnings (eng\generate-compiler-code.cmd)
     }
 }

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

@@ -198,6 +198,7 @@ internal enum MessageID
         IDS_FeatureAndPattern = MessageBase + 12774,
         IDS_FeatureNotPattern = MessageBase + 12775,
         IDS_FeatureRelationalPattern = MessageBase + 12776,
+        IDS_FeatureExtendedPartialMethods = MessageBase + 12777,
     }
 
     // Message IDs may refer to strings that need to be localized.
@@ -317,6 +318,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
                 case MessageID.IDS_FeatureTypePattern:
                 case MessageID.IDS_FeatureRelationalPattern:
                 case MessageID.IDS_FeatureNativeInt:
+                case MessageID.IDS_FeatureExtendedPartialMethods: // semantic check
                     return LanguageVersion.Preview;
 
                 // C# 8.0 features.

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

@@ -61,13 +61,6 @@ internal static DeclarationModifiers CheckModifiers(
                 modifierErrors = true;
             }
 
-            bool isMethod = (allowedModifiers & (DeclarationModifiers.Partial | DeclarationModifiers.Virtual)) == (DeclarationModifiers.Partial | DeclarationModifiers.Virtual);
-            if (isMethod && ((result & (DeclarationModifiers.Partial | DeclarationModifiers.Private)) == (DeclarationModifiers.Partial | DeclarationModifiers.Private)))
-            {
-                diagnostics.Add(ErrorCode.ERR_PartialMethodInvalidModifier, errorLocation);
-                modifierErrors = true;
-            }
-
             if ((result & DeclarationModifiers.PrivateProtected) != 0)
             {
                 modifierErrors |= !Binder.CheckFeatureAvailability(errorLocation.SourceTree, MessageID.IDS_FeaturePrivateProtected, diagnostics, errorLocation);

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

@@ -1683,10 +1683,13 @@ private void CheckMemberNameConflicts(DiagnosticBag diagnostics)
         // UNDONE: Consider adding a secondary location pointing to the second method.
         private void ReportMethodSignatureCollision(DiagnosticBag diagnostics, SourceMemberMethodSymbol method1, SourceMemberMethodSymbol method2)
         {
-            // Partial methods are allowed to collide by signature.
-            if (method1.IsPartial && method2.IsPartial)
+            switch (method1, method2)
             {
-                return;
+                case (SourceOrdinaryMethodSymbol { IsPartialDefinition: true }, SourceOrdinaryMethodSymbol { IsPartialImplementation: true }):
+                case (SourceOrdinaryMethodSymbol { IsPartialImplementation: true }, SourceOrdinaryMethodSymbol { IsPartialDefinition: true }):
+                    // these could be 2 parts of the same partial method.
+                    // Partial methods are allowed to collide by signature.
+                    return;
             }
 
             // If method1 is a constructor only because its return type is missing, then
@@ -2567,6 +2570,10 @@ private static void MergePartialMembers(
                     {
                         diagnostics.Add(ErrorCode.ERR_PartialMethodInconsistentTupleNames, method.Locations[0], method, method.OtherPartOfPartial);
                     }
+                    else if (method is { IsPartialDefinition: true, OtherPartOfPartial: null, HasExplicitAccessModifier: true })
+                    {
+                        diagnostics.Add(ErrorCode.ERR_PartialMethodWithAccessibilityModsMustHaveImplementation, method.Locations[0], method);
+                    }
                 }
             }
         }

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

@@ -827,7 +827,7 @@ private static void CheckOverrideMember(Symbol overridingMember, OverriddenOrHid
                         diagnostics.Add(ErrorCode.ERR_CantOverrideSealed, overridingMemberLocation, overridingMember, overriddenMember);
                         suppressAccessors = true;
                     }
-                    else if (!overridingMember.IsPartialMethod() && !OverrideHasCorrectAccessibility(overriddenMember, overridingMember))
+                    else if (!OverrideHasCorrectAccessibility(overriddenMember, overridingMember))
                     {
                         var accessibility = SyntaxFacts.GetText(overriddenMember.DeclaredAccessibility);
                         diagnostics.Add(ErrorCode.ERR_CantChangeAccessOnOverride, overridingMemberLocation, overridingMember, accessibility, overriddenMember);

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

@@ -441,14 +441,22 @@ public override Accessibility DeclaredAccessibility
             }
         }
 
-        public sealed override bool IsExtern
+        internal bool HasExternModifier
         {
             get
             {
                 return (this.DeclarationModifiers & DeclarationModifiers.Extern) != 0;
             }
         }
 
+        public override bool IsExtern
+        {
+            get
+            {
+                return HasExternModifier;
+            }
+        }
+
         public sealed override bool IsSealed
         {
             get

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

@@ -642,7 +642,8 @@ private void DecodeDllImportAttribute(ref DecodeWellKnownAttributeArguments<Attr
             Debug.Assert(!attribute.HasErrors);
             bool hasErrors = false;
 
-            if (!this.IsExtern || !this.IsStatic)
+            var implementationPart = this.PartialImplementationPart ?? this;
+            if (!implementationPart.IsExtern || !implementationPart.IsStatic)
             {
                 arguments.Diagnostics.Add(ErrorCode.ERR_DllImportOnInvalidMethod, arguments.AttributeSyntaxOpt.Name.Location);
                 hasErrors = true;

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

@@ -100,7 +100,8 @@ private SourceOrdinaryMethodSymbol(
             _hasAnyBody = hasBody;
 
             bool modifierErrors;
-            var declarationModifiers = this.MakeModifiers(modifiers, methodKind, hasBody, location, diagnostics, out modifierErrors);
+            DeclarationModifiers declarationModifiers;
+            (declarationModifiers, HasExplicitAccessModifier, modifierErrors) = this.MakeModifiers(modifiers, methodKind, hasBody, location, diagnostics);
 
             var isMetadataVirtualIgnoringModifiers = (object)explicitInterfaceType != null; //explicit impls must be marked metadata virtual
 
@@ -289,16 +290,6 @@ private void MethodChecks(MethodDeclarationSyntax syntax, Binder withTypeParamsB
 
             if (IsPartial)
             {
-                // check that there are no out parameters in a partial
-                foreach (var p in this.Parameters)
-                {
-                    if (p.RefKind == RefKind.Out)
-                    {
-                        diagnostics.Add(ErrorCode.ERR_PartialMethodCannotHaveOutParameters, location);
-                        break;
-                    }
-                }
-
                 if (MethodKind == MethodKind.ExplicitInterfaceImplementation)
                 {
                     diagnostics.Add(ErrorCode.ERR_PartialMethodNotExplicit, location);
@@ -611,7 +602,7 @@ internal bool IsPartialDefinition
         {
             get
             {
-                return this.IsPartial && !_hasAnyBody;
+                return this.IsPartial && !_hasAnyBody && !HasExternModifier;
             }
         }
 
@@ -622,7 +613,7 @@ internal bool IsPartialImplementation
         {
             get
             {
-                return this.IsPartial && _hasAnyBody;
+                return this.IsPartial && (_hasAnyBody || HasExternModifier);
             }
         }
 
@@ -677,6 +668,16 @@ public override MethodSymbol PartialImplementationPart
             }
         }
 
+        public sealed override bool IsExtern
+        {
+            get
+            {
+                return IsPartialDefinition
+                    ? _otherPartOfPartial?.IsExtern ?? false
+                    : HasExternModifier;
+            }
+        }
+
         public override string GetDocumentationCommentXml(CultureInfo preferredCulture = null, bool expandIncludes = false, CancellationToken cancellationToken = default(CancellationToken))
         {
             ref var lazyDocComment = ref expandIncludes ? ref this.lazyExpandedDocComment : ref this.lazyDocComment;
@@ -756,7 +757,9 @@ internal override bool IsExpressionBodied
             get { return _isExpressionBodied; }
         }
 
-        private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind methodKind, bool hasBody, Location location, DiagnosticBag diagnostics, out bool modifierErrors)
+        internal bool HasExplicitAccessModifier { get; }
+
+        private (DeclarationModifiers mods, bool hasExplicitAccessMod, bool modifierErrors) MakeModifiers(SyntaxTokenList modifiers, MethodKind methodKind, bool hasBody, Location location, DiagnosticBag diagnostics)
         {
             bool isInterface = this.ContainingType.IsInterface;
             bool isExplicitInterfaceImplementation = methodKind == MethodKind.ExplicitInterfaceImplementation;
@@ -808,7 +811,19 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind
                 allowedModifiers |= DeclarationModifiers.ReadOnly;
             }
 
-            var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(modifiers, defaultAccess, allowedModifiers, location, diagnostics, out modifierErrors);
+            // In order to detect whether explicit accessibility mods were provided, we pass the default value
+            // for 'defaultAccess' and manually add in the 'defaultAccess' flags after the call.
+            bool hasExplicitAccessMod;
+            var mods = ModifierUtils.MakeAndCheckNontypeMemberModifiers(modifiers, defaultAccess: DeclarationModifiers.None, allowedModifiers, location, diagnostics, out bool modifierErrors);
+            if ((mods & DeclarationModifiers.AccessibilityMask) == 0)
+            {
+                hasExplicitAccessMod = false;
+                mods |= defaultAccess;
+            }
+            else
+            {
+                hasExplicitAccessMod = true;
+            }
 
             this.CheckUnsafeModifier(mods, diagnostics);
 
@@ -817,7 +832,7 @@ private DeclarationModifiers MakeModifiers(SyntaxTokenList modifiers, MethodKind
                                                                         location, diagnostics);
 
             mods = AddImpliedModifiers(mods, isInterface, methodKind, hasBody);
-            return mods;
+            return (mods, hasExplicitAccessMod, modifierErrors);
         }
 
         private static DeclarationModifiers AddImpliedModifiers(DeclarationModifiers mods, bool containingTypeIsInterface, MethodKind methodKind, bool hasBody)
@@ -905,26 +920,40 @@ private ImmutableArray<TypeParameterSymbol> MakeTypeParameters(MethodDeclaration
             return result.ToImmutableAndFree();
         }
 
+        private const DeclarationModifiers PartialMethodExtendedModifierMask =
+            DeclarationModifiers.Virtual |
+            DeclarationModifiers.Override |
+            DeclarationModifiers.New |
+            DeclarationModifiers.Sealed |
+            DeclarationModifiers.Extern;
+
+        internal bool HasExtendedPartialModifier => (DeclarationModifiers & PartialMethodExtendedModifierMask) != 0;
+
         private void CheckModifiers(bool isExplicitInterfaceImplementation, bool hasBody, Location location, DiagnosticBag diagnostics)
         {
-            const DeclarationModifiers partialMethodInvalidModifierMask = (DeclarationModifiers.AccessibilityMask & ~DeclarationModifiers.Private) |
-                     DeclarationModifiers.Virtual |
-                     DeclarationModifiers.Abstract |
-                     DeclarationModifiers.Override |
-                     DeclarationModifiers.New |
-                     DeclarationModifiers.Sealed |
-                     DeclarationModifiers.Extern;
-
             bool isExplicitInterfaceImplementationInInterface = isExplicitInterfaceImplementation && ContainingType.IsInterface;
 
-            if (IsPartial && !ReturnsVoid)
+            if (IsPartial && HasExplicitAccessModifier)
             {
-                diagnostics.Add(ErrorCode.ERR_PartialMethodMustReturnVoid, location);
+                Binder.CheckFeatureAvailability(SyntaxNode, MessageID.IDS_FeatureExtendedPartialMethods, diagnostics, location);
             }
-            else if (IsPartial && (DeclarationModifiers & partialMethodInvalidModifierMask) != 0)
+
+            if (IsPartial && IsAbstract)
             {
                 diagnostics.Add(ErrorCode.ERR_PartialMethodInvalidModifier, location);
             }
+            else if (IsPartial && !HasExplicitAccessModifier && !ReturnsVoid)
+            {
+                diagnostics.Add(ErrorCode.ERR_PartialMethodWithNonVoidReturnMustHaveAccessMods, location, this);
+            }
+            else if (IsPartial && !HasExplicitAccessModifier && HasExtendedPartialModifier)
+            {
+                diagnostics.Add(ErrorCode.ERR_PartialMethodWithExtendedModMustHaveAccessMods, location, this);
+            }
+            else if (IsPartial && !HasExplicitAccessModifier && Parameters.Any(p => p.RefKind == RefKind.Out))
+            {
+                diagnostics.Add(ErrorCode.ERR_PartialMethodWithOutParamMustHaveAccessMods, location, this);
+            }
             else if (this.DeclaredAccessibility == Accessibility.Private && (IsVirtual || (IsAbstract && !isExplicitInterfaceImplementationInInterface) || IsOverride))
             {
                 diagnostics.Add(ErrorCode.ERR_VirtualPrivate, location, this);
@@ -1144,6 +1173,20 @@ private static void PartialMethodChecks(SourceOrdinaryMethodSymbol definition, S
                 diagnostics.Add(ErrorCode.ERR_PartialMethodParamsDifference, implementation.Locations[0]);
             }
 
+            if (definition.HasExplicitAccessModifier != implementation.HasExplicitAccessModifier
+                || definition.DeclaredAccessibility != implementation.DeclaredAccessibility)
+            {
+                diagnostics.Add(ErrorCode.ERR_PartialMethodAccessibilityDifference, implementation.Locations[0]);
+            }
+
+            if (definition.IsVirtual != implementation.IsVirtual
+                || definition.IsOverride != implementation.IsOverride
+                || definition.IsSealed != implementation.IsSealed
+                || definition.IsNew != implementation.IsNew)
+            {
+                diagnostics.Add(ErrorCode.ERR_PartialMethodExtendedModDifference, implementation.Locations[0]);
+            }
+
             PartialMethodConstraintsChecks(definition, implementation, diagnostics);
 
             SourceMemberContainerTypeSymbol.CheckValidNullableMethodOverride(