Prepare partial properties feature for merge (#73773)

Co-authored-by: Shen Chen <Cosifne@users.noreply.github.com>

src/Analyzers/CSharp/CodeFixes/Nullable/CSharpDeclareAsNullableCodeFixProvider.cs

@@ -234,6 +234,7 @@ SyntaxKind.IndexerDeclaration or
             var symbol = model.GetSymbolInfo(invocation.Expression, cancellationToken).Symbol;
             if (symbol is not IMethodSymbol method || method.PartialImplementationPart is not null)
             {
+                // https://github.com/dotnet/roslyn/issues/73772: should we also bail out on a partial property?
                 // We don't handle partial methods yet
                 return null;
             }

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

@@ -2310,18 +2310,6 @@ internal enum ErrorCode
         ERR_InterceptsLocationDataInvalidPosition = 9235,
         INF_TooManyBoundLambdas = 9236,
 
-        // PROTOTYPE(partial-properties): pack
-        ERR_PartialPropertyMissingImplementation = 9300,
-        ERR_PartialPropertyMissingDefinition = 9301,
-        ERR_PartialPropertyDuplicateDefinition = 9302,
-        ERR_PartialPropertyDuplicateImplementation = 9303,
-        ERR_PartialPropertyMissingAccessor = 9304,
-        ERR_PartialPropertyUnexpectedAccessor = 9305,
-        ERR_PartialPropertyInitMismatch = 9306,
-        ERR_PartialPropertyTypeDifference = 9307,
-        WRN_PartialPropertySignatureDifference = 9308,
-        ERR_PartialPropertyRequiredDifference = 9309,
-
         #endregion
 
         WRN_BadYieldInLock = 9237,
@@ -2337,6 +2325,17 @@ internal enum ErrorCode
         ERR_BadNonVirtualInterfaceMemberAccessOnAllowsRefLike = 9246,
         ERR_BadAllowByRefLikeEnumerator = 9247,
 
+        ERR_PartialPropertyMissingImplementation = 9248,
+        ERR_PartialPropertyMissingDefinition = 9249,
+        ERR_PartialPropertyDuplicateDefinition = 9250,
+        ERR_PartialPropertyDuplicateImplementation = 9251,
+        ERR_PartialPropertyMissingAccessor = 9252,
+        ERR_PartialPropertyUnexpectedAccessor = 9253,
+        ERR_PartialPropertyInitMismatch = 9254,
+        ERR_PartialPropertyTypeDifference = 9255,
+        WRN_PartialPropertySignatureDifference = 9256,
+        ERR_PartialPropertyRequiredDifference = 9257,
+
         // Note: you will need to do the following after adding errors:
         //  1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)
 

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

@@ -287,8 +287,7 @@ internal enum MessageID
 
         IDS_FeatureRefStructInterfaces = MessageBase + 12844,
 
-        // PROTOTYPE(partial-properties): pack
-        IDS_FeaturePartialProperties = MessageBase + 13000,
+        IDS_FeaturePartialProperties = MessageBase + 12845,
     }
 
     // Message IDs may refer to strings that need to be localized.

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.Variables.cs

@@ -396,6 +396,7 @@ internal Variables GetRootScope()
 
             internal Variables? GetVariablesForMethodScope(MethodSymbol method)
             {
+                // https://github.com/dotnet/roslyn/issues/73772: is this needed if we also delete the weird cascading in EnterParameters?
                 method = method.PartialImplementationPart ?? method;
                 var variables = this;
                 while (true)

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -2764,6 +2764,7 @@ private void EnterParameters()
             }
 
             // The partial definition part may include optional parameters whose default values we want to simulate assigning at the beginning of the method
+            // https://github.com/dotnet/roslyn/issues/73772: is this actually used/meaningful?
             methodSymbol = methodSymbol.PartialDefinitionPart ?? methodSymbol;
 
             var methodParameters = methodSymbol.Parameters;

src/Compilers/CSharp/Portable/Symbols/PublicModel/MethodSymbol.cs

@@ -298,8 +298,6 @@ IMethodSymbol IMethodSymbol.PartialDefinitionPart
             }
         }
 
-        // PROTOTYPE(partial-properties): Perhaps this API should be implemented as '_underlying.OriginalDefinition.IsPartialDefinition()' instead.
-        // However, this would be a behavior change. Callers may have been assuming that if the API returned true, then the receiver is an original definition symbol.
         bool IMethodSymbol.IsPartialDefinition => _underlying.IsDefinition && _underlying.IsPartialDefinition();
 
         INamedTypeSymbol IMethodSymbol.AssociatedAnonymousDelegate

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

@@ -14,7 +14,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Symbols
 {
     internal sealed class SourcePropertySymbol : SourcePropertySymbolBase
     {
-        // PROTOTYPE(partial-properties): Verify that the increase in memory consumption from this is acceptable
         private SourcePropertySymbol? _otherPartOfPartial;
 
         internal static SourcePropertySymbol Create(SourceMemberContainerTypeSymbol containingType, Binder bodyBinder, PropertyDeclarationSyntax syntax, BindingDiagnosticBag diagnostics)
@@ -175,7 +174,10 @@ public override OneOrMany<SyntaxList<AttributeListSyntax>> GetAttributeDeclarati
         {
             // Attributes on partial properties are owned by the definition part.
             // If this symbol has a non-null PartialDefinitionPart, we should have accessed this method through that definition symbol instead
-            Debug.Assert(PartialDefinitionPart is null);
+            Debug.Assert(PartialDefinitionPart is null
+                // We might still get here when asking for the attributes on a backing field.
+                // This is an error scenario (requires using a property initializer and field-targeted attributes on partial property implementation part).
+                || this.BackingField is not null);
 
             if (PartialImplementationPart is { } implementationPart)
             {
@@ -610,6 +612,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions,
             if (IsPartialDefinition && OtherPartOfPartial is { } implementation)
             {
                 PartialPropertyChecks(implementation, diagnostics);
+                implementation.CheckInitializerIfNeeded(diagnostics);
             }
         }
 

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

@@ -95,7 +95,7 @@ protected SourcePropertySymbolBase(
         {
             Debug.Assert(!isExpressionBodied || !isAutoProperty);
             Debug.Assert(!isExpressionBodied || !hasInitializer);
-            Debug.Assert(!isExpressionBodied || accessorsHaveImplementation); // PROTOTYPE(partial-properties): further adjust asserts?
+            Debug.Assert(!isExpressionBodied || accessorsHaveImplementation);
             Debug.Assert((modifiers & DeclarationModifiers.Required) == 0 || this is SourcePropertySymbol);
 
             _syntaxRef = syntax.GetReference();
@@ -279,20 +279,20 @@ explicitlyImplementedProperty is null ?
         internal bool IsExpressionBodied
             => (_propertyFlags & Flags.IsExpressionBodied) != 0;
 
-        private void CheckInitializer(
-            bool isAutoProperty,
-            bool isInterface,
-            bool isStatic,
-            Location location,
-            BindingDiagnosticBag diagnostics)
+        protected void CheckInitializerIfNeeded(BindingDiagnosticBag diagnostics)
         {
-            if (isInterface && !isStatic)
+            if ((_propertyFlags & Flags.HasInitializer) == 0)
             {
-                diagnostics.Add(ErrorCode.ERR_InstancePropertyInitializerInInterface, location);
+                return;
             }
-            else if (!isAutoProperty)
+
+            if (ContainingType.IsInterface && !IsStatic)
             {
-                diagnostics.Add(ErrorCode.ERR_InitializerOnNonAutoProperty, location);
+                diagnostics.Add(ErrorCode.ERR_InstancePropertyInitializerInInterface, Location);
+            }
+            else if (!IsAutoProperty)
+            {
+                diagnostics.Add(ErrorCode.ERR_InitializerOnNonAutoProperty, Location);
             }
         }
 
@@ -692,11 +692,7 @@ internal override void AfterAddingTypeMembersChecks(ConversionsBase conversions,
             this.CheckAccessibility(Location, diagnostics, isExplicitInterfaceImplementation);
             this.CheckModifiers(isExplicitInterfaceImplementation, Location, IsIndexer, diagnostics);
 
-            bool hasInitializer = (_propertyFlags & Flags.HasInitializer) != 0;
-            if (hasInitializer)
-            {
-                CheckInitializer(IsAutoProperty, ContainingType.IsInterface, IsStatic, Location, diagnostics);
-            }
+            CheckInitializerIfNeeded(diagnostics);
 
             if (RefKind != RefKind.None && IsRequired)
             {
@@ -1109,20 +1105,23 @@ private CustomAttributesBag<CSharpAttributeData> GetAttributesBag()
             // prevent infinite recursion:
             Debug.Assert(!ReferenceEquals(copyFrom, this));
 
+            // The property is responsible for completion of the backing field
+            // NB: when the **field keyword feature** is implemented, it's possible that synthesized field symbols will also be merged or shared between partial property parts.
+            // If we do that then this check should possibly be moved, and asserts adjusted accordingly.
+            _ = BackingField?.GetAttributes();
+
             bool bagCreatedOnThisThread;
             if (copyFrom is not null)
             {
                 // When partial properties get the ability to have a backing field,
                 // the implementer will have to decide how the BackingField symbol works in 'copyFrom' scenarios.
-                Debug.Assert(BackingField is null);
+                Debug.Assert(!IsAutoProperty);
 
                 var attributesBag = copyFrom.GetAttributesBag();
                 bagCreatedOnThisThread = Interlocked.CompareExchange(ref _lazyCustomAttributesBag, attributesBag, null) == null;
             }
             else
             {
-                // The property is responsible for completion of the backing field
-                _ = BackingField?.GetAttributes();
                 bagCreatedOnThisThread = LoadAndValidateAttributes(GetAttributeDeclarations(), ref _lazyCustomAttributesBag);
             }
 

src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_WellKnownAttributes.cs

@@ -11220,6 +11220,45 @@ void local1()
             Assert.True(verifier.HasLocalsInit("C.<get_PropNoAttribute>g__local1|3_0"));
         }
 
+        [Theory]
+        [InlineData("[SkipLocalsInit]", "")]
+        [InlineData("", "[SkipLocalsInit]")]
+        public void SkipLocalsInit_PartialPropertyAccessor_ContainsLocalFunction(string defAttrs, string implAttrs)
+        {
+            // SkipLocalsInit applied to either part affects the property and nested functions
+            var source = $$"""
+using System.Runtime.CompilerServices;
+
+public partial class C
+{
+    {{defAttrs}}
+    partial int PropWithAttribute { get; }
+
+    {{implAttrs}}
+    partial int PropWithAttribute
+    {
+        get
+        {
+            int w = 1;
+            w = w + w + w + w;
+
+            void local1()
+            {
+                int x = 1;
+                x = x + x + x + x;
+            }
+
+            return 0;
+        }
+    }
+}
+""";
+
+            var verifier = CompileAndVerifyWithSkipLocalsInit(source);
+            Assert.False(verifier.HasLocalsInit("C.PropWithAttribute.get"));
+            Assert.False(verifier.HasLocalsInit("C.<get_PropWithAttribute>g__local1|1_0"));
+        }
+
         [Fact]
         public void SkipLocalsInit_EventAccessor_ContainsLocalFunction()
         {

src/Compilers/CSharp/Test/Symbol/DocumentationComments/DocumentationCommentCompilerTests.cs

@@ -1972,7 +1972,7 @@ void verify(CSharpTestSource source)
                     Assert.Equal("p2", indexer.Parameters.Single().Name);
                 });
                 verifier.VerifyDiagnostics(
-                    // (5,24): warning CS9308: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
+                    // (5,24): warning CS9256: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
                     //     public partial int this[int p1] => 42;
                     Diagnostic(ErrorCode.WRN_PartialPropertySignatureDifference, "this").WithArguments("int C.this[int p2]", "int C.this[int p1]").WithLocation(5, 24));
 
@@ -2033,7 +2033,7 @@ void verify(CSharpTestSource source)
                     // (4,42): warning CS1734: XML comment on 'C.this[int]' has a paramref tag for 'p2', but there is no parameter by that name
                     //     /** <summary>Accepts <paramref name="p2"/>.</summary> */
                     Diagnostic(ErrorCode.WRN_UnmatchedParamRefTag, "p2").WithArguments("p2", "C.this[int]").WithLocation(4, 42),
-                    // (5,24): warning CS9308: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
+                    // (5,24): warning CS9256: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
                     //     public partial int this[int p1] => 42;
                     Diagnostic(ErrorCode.WRN_PartialPropertySignatureDifference, "this").WithArguments("int C.this[int p2]", "int C.this[int p1]").WithLocation(5, 24));
 
@@ -2094,7 +2094,7 @@ void verify(CSharpTestSource source)
                     Assert.Equal("p2", indexer.Parameters.Single().Name);
                 });
                 verifier.VerifyDiagnostics(
-                    // (4,24): warning CS9308: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
+                    // (4,24): warning CS9256: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
                     //     public partial int this[int p1] => 42;
                     Diagnostic(ErrorCode.WRN_PartialPropertySignatureDifference, "this").WithArguments("int C.this[int p2]", "int C.this[int p1]").WithLocation(4, 24),
                     // (4,42): warning CS1734: XML comment on 'C.this[int]' has a paramref tag for 'p1', but there is no parameter by that name
@@ -2158,7 +2158,7 @@ void verify(CSharpTestSource source)
                     Assert.Equal("p2", indexer.Parameters.Single().Name);
                 });
                 verifier.VerifyDiagnostics(
-                    // (4,24): warning CS9308: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
+                    // (4,24): warning CS9256: Partial property declarations 'int C.this[int p2]' and 'int C.this[int p1]' have signature differences.
                     //     public partial int this[int p1] => 42;
                     Diagnostic(ErrorCode.WRN_PartialPropertySignatureDifference, "this").WithArguments("int C.this[int p2]", "int C.this[int p1]").WithLocation(4, 24));