[semi-auto-props]: Require overriding all accessors

src/Compilers/CSharp/Portable/Binder/Binder.cs

@@ -194,10 +194,10 @@ internal virtual ImmutableArray<LocalFunctionSymbol> GetDeclaredLocalFunctionsFo
             return this.Next.GetDeclaredLocalFunctionsForScope(scopeDesignator);
         }
 
-        internal virtual FieldSymbol? GetSymbolForPossibleFieldKeyword()
+        internal virtual FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
         {
             RoslynDebug.Assert(Next is object);
-            return this.Next.GetSymbolForPossibleFieldKeyword();
+            return this.Next.GetSymbolForPossibleFieldKeyword(diagnostics);
         }
 
         /// <summary>

src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs

@@ -1566,7 +1566,7 @@ private BoundExpression BindIdentifier(
                     else if (node.Identifier.ContextualKind() == SyntaxKind.FieldKeyword &&
                         ContainingMember().CanHaveFieldKeywordBackingField())
                     {
-                        if (GetSymbolForPossibleFieldKeyword() is { } backingField)
+                        if (GetSymbolForPossibleFieldKeyword(diagnostics) is { } backingField)
                         {
                             expression = BindNonMethod(node, backingField, diagnostics, LookupResultKind.Viable, indexed: false, isError: false);
                             if (IsInsideNameof)

src/Compilers/CSharp/Portable/Binder/Binder_Statements.cs

@@ -1709,8 +1709,6 @@ private static bool IsPropertyAssignedThroughBackingField(BoundExpression receiv
                     // PROTOTYPE(semi-auto-props): Consider `public int P { get => field; set; }`
                     sourceProperty.GetMethod is SourcePropertyAccessorSymbol { IsEquivalentToBackingFieldAccess: true } &&
                     // To be assigned through backing field, either SetMethod is null, or it's equivalent to backing field write
-                    // PROTOTYPE(semi-auto-props): TODO: Do we need to use `GetOwnOrInheritedSetMethod` instead of `SetMethod`?
-                    // Legacy auto-properties are required to override all accessors. If this is not the case with semi auto props, we may need to use GetOwnOrInheritedSetMethod
                     sourceProperty.SetMethod is null or SourcePropertyAccessorSymbol { IsEquivalentToBackingFieldAccess: true } &&
                     TypeSymbol.Equals(sourceProperty.ContainingType, fromMember.ContainingType, TypeCompareKind.ConsiderEverything2) &&
                     IsConstructorOrField(fromMember, isStatic: sourceProperty.IsStatic) &&

src/Compilers/CSharp/Portable/Binder/BuckStopsHereBinder.cs

@@ -241,7 +241,7 @@ internal override BoundStatement BindLockStatementParts(BindingDiagnosticBag dia
             throw ExceptionUtilities.Unreachable;
         }
 
-        internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
+        internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
         {
             // There's supposed to either be a FieldKeywordBinder or a SpeculativeFieldKEywordBinder in the chain.
             throw ExceptionUtilities.Unreachable;

src/Compilers/CSharp/Portable/Binder/FieldKeywordBinder.cs

@@ -19,9 +19,9 @@ internal FieldKeywordBinder(SourcePropertyAccessorSymbol accessor, Binder next)
             _accessor = accessor;
         }
 
-        internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
+        internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
         {
-            return _accessor.Property.GetOrCreateBackingFieldForFieldKeyword();
+            return _accessor.Property.GetOrCreateBackingFieldForFieldKeyword(diagnostics);
         }
     }
 }

src/Compilers/CSharp/Portable/Binder/SpeculativeFieldKeywordBinder.cs

@@ -1,4 +1,4 @@
-// Licensed to the .NET Foundation under one or more agreements.
+// Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
@@ -19,7 +19,7 @@ internal SpeculativeFieldKeywordBinder(SourcePropertyAccessorSymbol accessor, Bi
             _accessor = accessor;
         }
 
-        internal override FieldSymbol? GetSymbolForPossibleFieldKeyword()
+        internal override FieldSymbol? GetSymbolForPossibleFieldKeyword(BindingDiagnosticBag diagnostics)
         {
             // field in the speculative model does not bind to a backing field if the original location was not a semi-auto property
             return _accessor.Property.FieldKeywordBackingField;

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

@@ -185,7 +185,7 @@ protected SourcePropertySymbolBase(
                 Debug.Assert(!IsIndexer);
                 // PROTOTYPE(semi-auto-props): Make sure that TestSemiAutoPropertyWithInitializer (when enabled back) is affected by this.
                 // That is, if we removed "hasInitializer", the test should fail, or any other test should get affected.
-                GetOrCreateBackingField(isCreatedForFieldKeyword: hasInitializer && !isAutoProperty, isEarlyConstructed: true);
+                GetOrCreateBackingField(isCreatedForFieldKeyword: hasInitializer && !isAutoProperty, isEarlyConstructed: true, diagnostics);
             }
 
             if (hasGetAccessor)
@@ -198,7 +198,7 @@ protected SourcePropertySymbolBase(
             }
         }
 
-        private SynthesizedBackingFieldSymbol? GetOrCreateBackingField(bool isCreatedForFieldKeyword, bool isEarlyConstructed)
+        private SynthesizedBackingFieldSymbol? GetOrCreateBackingField(bool isCreatedForFieldKeyword, bool isEarlyConstructed, BindingDiagnosticBag diagnostics)
         {
             Debug.Assert(!IsIndexer);
             if (_lazyBackingFieldSymbol == _lazyBackingFieldSymbolSentinel)
@@ -210,15 +210,24 @@ protected SourcePropertySymbolBase(
                                               hasInitializer: (_propertyFlags & Flags.HasInitializer) != 0,
                                               isCreatedForFieldKeyword: isCreatedForFieldKeyword,
                                               isEarlyConstructed: isEarlyConstructed);
-                Interlocked.CompareExchange(ref _lazyBackingFieldSymbol, backingField, _lazyBackingFieldSymbolSentinel);
+                if (Interlocked.CompareExchange(ref _lazyBackingFieldSymbol, backingField, _lazyBackingFieldSymbolSentinel) == _lazyBackingFieldSymbolSentinel &&
+                    isCreatedForFieldKeyword)
+                {
+                    // semi auto property should override all accessors.
+                    if ((!HasSetAccessor && !this.IsReadOnly) ||
+                        (!HasGetAccessor && !this.IsWriteOnly))
+                    {
+                        diagnostics.Add(ErrorCode.ERR_AutoPropertyMustOverrideSet, Location, this);
+                    }
+                }
             }
 
             return (SynthesizedBackingFieldSymbol?)_lazyBackingFieldSymbol;
         }
 
-        internal SynthesizedBackingFieldSymbol? GetOrCreateBackingFieldForFieldKeyword()
+        internal SynthesizedBackingFieldSymbol? GetOrCreateBackingFieldForFieldKeyword(BindingDiagnosticBag diagnostics)
         {
-            return GetOrCreateBackingField(isCreatedForFieldKeyword: true, isEarlyConstructed: false);
+            return GetOrCreateBackingField(isCreatedForFieldKeyword: true, isEarlyConstructed: false, diagnostics);
         }
 
         private void EnsureSignatureGuarded(BindingDiagnosticBag diagnostics)

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

@@ -39,13 +39,6 @@ namespace Microsoft.CodeAnalysis.CSharp.Semantic.UnitTests.Semantics
     // both expression body and block body. We should confirm that SemanticModel doesn't bind an expression body in presence of a block body.
 
     // PROTOTYPE(semi-auto-props): Add ENC tests.
-
-    // PROTOTYPE(semi-auto-props): Add a test where a virtual `get; set;` auto property is overridden by a sealed `get => 0;`.
-    // A synthesized sealed accessor is expected to be produced.
-
-    // PROTOTYPE(semi-auto-props): Add a test where a virtual property is `public virtual int P6 { get => 0; set { } }`, and
-    // it's overridden by `public override int P6 { get => field; }`
-    // The assignment of the property in constructor should use the base setter.
     public class PropertyFieldKeywordTests : CompilingTestBase
     {
         /// <summary>
@@ -87,6 +80,63 @@ private void VerifyTypeIL(CSharpCompilation compilation, string typeName, string
             CompileAndVerify(compilation).VerifyTypeIL(typeName, expected);
         }
 
+        [Fact]
+        public void TestVirtualPropertyOverride()
+        {
+            var comp = CreateCompilation(@"
+public class Base
+{
+    public virtual int P1 { get; set; }
+    public virtual int P2 { get => 0; set { } }
+}
+
+public class Derived1 : Base
+{
+    public override int P1 { get => field; }
+    public override int P2 { get => field; }
+}
+
+public class Derived2 : Base
+{
+    public override int P1 { set => _ = field; }
+    public override int P2 { set => _ = field; }
+}
+
+public class Derived3 : Base
+{
+    // PROTOTYPE(semi-auto-props):
+    // This should produce ERR_AutoPropertyMustOverrideSet ""Auto-implemented properties must override all accessors of the overridden property.""
+    // instead of ERR_AutoPropertyMustHaveGetAccessor, unless https://github.com/dotnet/csharplang/issues/6089 is accepted.
+    public override int P1 { set; }
+}
+
+public class Derived4 : Base
+{
+    public override int P1 { get => field; set => field = value; }
+}
+");
+            var accessorBindingData = new SourcePropertySymbolBase.AccessorBindingData();
+            comp.TestOnlyCompilationData = accessorBindingData;
+            comp.VerifyDiagnostics(
+                // (10,25): error CS8080: Auto-implemented properties must override all accessors of the overridden property.
+                //     public override int P1 { get => field; } // PROTOTYPE(semi-auto-props): This should error
+                Diagnostic(ErrorCode.ERR_AutoPropertyMustOverrideSet, "P1").WithArguments("Derived1.P1").WithLocation(10, 25),
+                // (11,25): error CS8080: Auto-implemented properties must override all accessors of the overridden property.
+                //     public override int P2 { get => field; } // PROTOTYPE(semi-auto-props): This should error
+                Diagnostic(ErrorCode.ERR_AutoPropertyMustOverrideSet, "P2").WithArguments("Derived1.P2").WithLocation(11, 25),
+                // (16,25): error CS8080: Auto-implemented properties must override all accessors of the overridden property.
+                //     public override int P1 { set => _ = field; } // PROTOTYPE(semi-auto-props): This should error
+                Diagnostic(ErrorCode.ERR_AutoPropertyMustOverrideSet, "P1").WithArguments("Derived2.P1").WithLocation(16, 25),
+                // (17,25): error CS8080: Auto-implemented properties must override all accessors of the overridden property.
+                //     public override int P2 { set => _ = field; } // PROTOTYPE(semi-auto-props): This should error
+                Diagnostic(ErrorCode.ERR_AutoPropertyMustOverrideSet, "P2").WithArguments("Derived2.P2").WithLocation(17, 25),
+                // (25,30): error CS8051: Auto-implemented properties must have get accessors.
+                //     public override int P1 { set; }
+                Diagnostic(ErrorCode.ERR_AutoPropertyMustHaveGetAccessor, "set").WithArguments("Derived3.P1.set").WithLocation(25, 30)
+                );
+            Assert.Equal(0, accessorBindingData.NumberOfPerformedAccessorBinding);
+        }
+
         [Fact]
         public void TestInInterface()
         {