Emit ObsoleteAttribute for required member constructors (#61005)

Support emitting ObsoleteAttribute on the constructor of types with required members. Specification: https://github.com/dotnet/csharplang/blob/main/proposals/required-members.md Test plan: #57046
dotnet · Apr 28, 2022 · c803634 · c803634
1 parent dbca19f
commit c803634
Show file tree

Hide file tree

Showing 14 changed files with 219 additions and 21 deletions.
diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEEventSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEEventSymbol.cs
@@ -478,7 +478,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
         {
             get
             {
-                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref _lazyObsoleteAttributeData, _handle, (PEModuleSymbol)(this.ContainingModule), ignoreByRefLikeMarker: false);
+                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref _lazyObsoleteAttributeData, _handle, (PEModuleSymbol)(this.ContainingModule), ignoreByRefLikeMarker: false, ignoreRequiredMemberMarker: false);
                 return _lazyObsoleteAttributeData;
             }
         }

diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEFieldSymbol.cs
@@ -600,7 +600,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
         {
             get
             {
-                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref _lazyObsoleteAttributeData, _handle, (PEModuleSymbol)(this.ContainingModule), ignoreByRefLikeMarker: false);
+                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref _lazyObsoleteAttributeData, _handle, (PEModuleSymbol)(this.ContainingModule), ignoreByRefLikeMarker: false, ignoreRequiredMemberMarker: false);
                 return _lazyObsoleteAttributeData;
             }
         }

diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEMethodSymbol.cs
@@ -1456,7 +1456,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
             {
                 if (!_packedFlags.IsObsoleteAttributePopulated)
                 {
-                    var result = ObsoleteAttributeHelpers.GetObsoleteDataFromMetadata(_handle, (PEModuleSymbol)ContainingModule, ignoreByRefLikeMarker: false);
+                    var result = ObsoleteAttributeHelpers.GetObsoleteDataFromMetadata(_handle, (PEModuleSymbol)ContainingModule, ignoreByRefLikeMarker: false, ignoreRequiredMemberMarker: MethodKind == MethodKind.Constructor);
                     if (result != null)
                     {
                         result = InterlockedOperations.Initialize(ref AccessUncommonFields()._lazyObsoleteAttributeData, result, ObsoleteAttributeData.Uninitialized);

diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PENamedTypeSymbol.cs
@@ -2262,7 +2262,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
                 }
 
                 bool ignoreByRefLikeMarker = this.IsRefLikeType;
-                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref uncommon.lazyObsoleteAttributeData, _handle, ContainingPEModule, ignoreByRefLikeMarker);
+                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref uncommon.lazyObsoleteAttributeData, _handle, ContainingPEModule, ignoreByRefLikeMarker, ignoreRequiredMemberMarker: false);
                 return uncommon.lazyObsoleteAttributeData;
             }
         }

diff --git a/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Metadata/PE/PEPropertySymbol.cs
@@ -794,7 +794,7 @@ internal override ObsoleteAttributeData ObsoleteAttributeData
         {
             get
             {
-                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref _lazyObsoleteAttributeData, _handle, (PEModuleSymbol)(this.ContainingModule), ignoreByRefLikeMarker: false);
+                ObsoleteAttributeHelpers.InitializeObsoleteDataFromMetadata(ref _lazyObsoleteAttributeData, _handle, (PEModuleSymbol)(this.ContainingModule), ignoreByRefLikeMarker: false, ignoreRequiredMemberMarker: false);
                 return _lazyObsoleteAttributeData;
             }
         }

diff --git a/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/MethodSymbol.cs
@@ -1216,5 +1216,25 @@ public override int GetHashCode()
         {
             return base.GetHashCode();
         }
+
+#nullable enable
+        protected static void AddRequiredMembersMarkerAttributes(ref ArrayBuilder<SynthesizedAttributeData> attributes, MethodSymbol methodToAttribute)
+        {
+            if (methodToAttribute.ShouldCheckRequiredMembers() && methodToAttribute.ContainingType.HasAnyRequiredMembers)
+            {
+                var obsoleteData = methodToAttribute.ObsoleteAttributeData;
+                Debug.Assert(obsoleteData != ObsoleteAttributeData.Uninitialized, "getting synthesized attributes before attributes are decoded");
+
+                if (obsoleteData == null)
+                {
+                    CSharpCompilation declaringCompilation = methodToAttribute.DeclaringCompilation;
+                    AddSynthesizedAttribute(ref attributes, declaringCompilation.TrySynthesizeAttribute(WellKnownMember.System_ObsoleteAttribute__ctor,
+                        ImmutableArray.Create(
+                            new TypedConstant(declaringCompilation.GetSpecialType(SpecialType.System_String), TypedConstantKind.Primitive, PEModule.RequiredMembersMarker), // message
+                            new TypedConstant(declaringCompilation.GetSpecialType(SpecialType.System_Boolean), TypedConstantKind.Primitive, true)) // error
+                        ));
+                }
+            }
+        }
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs b/src/Compilers/CSharp/Portable/Symbols/ObsoleteAttributeHelpers.cs
@@ -26,11 +26,11 @@ internal static class ObsoleteAttributeHelpers
         /// Initialize the ObsoleteAttributeData by fetching attributes and decoding ObsoleteAttributeData. This can be 
         /// done for Metadata symbol easily whereas trying to do this for source symbols could result in cycles.
         /// </summary>
-        internal static void InitializeObsoleteDataFromMetadata(ref ObsoleteAttributeData data, EntityHandle token, PEModuleSymbol containingModule, bool ignoreByRefLikeMarker)
+        internal static void InitializeObsoleteDataFromMetadata(ref ObsoleteAttributeData data, EntityHandle token, PEModuleSymbol containingModule, bool ignoreByRefLikeMarker, bool ignoreRequiredMemberMarker)
         {
             if (ReferenceEquals(data, ObsoleteAttributeData.Uninitialized))
             {
-                ObsoleteAttributeData obsoleteAttributeData = GetObsoleteDataFromMetadata(token, containingModule, ignoreByRefLikeMarker);
+                ObsoleteAttributeData obsoleteAttributeData = GetObsoleteDataFromMetadata(token, containingModule, ignoreByRefLikeMarker, ignoreRequiredMemberMarker);
                 Interlocked.CompareExchange(ref data, obsoleteAttributeData, ObsoleteAttributeData.Uninitialized);
             }
         }
@@ -39,9 +39,9 @@ internal static void InitializeObsoleteDataFromMetadata(ref ObsoleteAttributeDat
         /// Get the ObsoleteAttributeData by fetching attributes and decoding ObsoleteAttributeData. This can be 
         /// done for Metadata symbol easily whereas trying to do this for source symbols could result in cycles.
         /// </summary>
-        internal static ObsoleteAttributeData GetObsoleteDataFromMetadata(EntityHandle token, PEModuleSymbol containingModule, bool ignoreByRefLikeMarker)
+        internal static ObsoleteAttributeData GetObsoleteDataFromMetadata(EntityHandle token, PEModuleSymbol containingModule, bool ignoreByRefLikeMarker, bool ignoreRequiredMemberMarker)
         {
-            var obsoleteAttributeData = containingModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(token, new MetadataDecoder(containingModule), ignoreByRefLikeMarker);
+            var obsoleteAttributeData = containingModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(token, new MetadataDecoder(containingModule), ignoreByRefLikeMarker, ignoreRequiredMemberMarker);
             Debug.Assert(obsoleteAttributeData == null || !obsoleteAttributeData.IsUninitialized);
             return obsoleteAttributeData;
         }

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceConstructorSymbolBase.cs
@@ -6,7 +6,9 @@
 
 using System.Collections.Immutable;
 using System.Diagnostics;
+using Microsoft.CodeAnalysis.CSharp.Emit;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
 
@@ -281,5 +283,11 @@ internal sealed override (CSharpAttributeData?, BoundAttribute?) EarlyDecodeWell
 
             return base.EarlyDecodeWellKnownAttribute(ref arguments);
         }
+
+        internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<SynthesizedAttributeData> attributes)
+        {
+            base.AddSynthesizedAttributes(moduleBuilder, ref attributes);
+            AddRequiredMembersMarkerAttributes(ref attributes, this);
+        }
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs b/src/Compilers/CSharp/Portable/Symbols/Synthesized/SynthesizedInstanceConstructor.cs
@@ -7,6 +7,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
+using Microsoft.CodeAnalysis.CSharp.Emit;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 
@@ -317,5 +318,11 @@ internal virtual void GenerateMethodBodyStatements(SyntheticBoundNodeFactory fac
         }
 
         protected override bool HasSetsRequiredMembersImpl => false;
+
+        internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, ref ArrayBuilder<SynthesizedAttributeData> attributes)
+        {
+            base.AddSynthesizedAttributes(moduleBuilder, ref attributes);
+            AddRequiredMembersMarkerAttributes(ref attributes, this);
+        }
     }
 }
diff --git a/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_IsByRefLike.cs b/src/Compilers/CSharp/Test/Emit2/Attributes/AttributeTests_IsByRefLike.cs
@@ -1002,7 +1002,7 @@ private static void AssertReferencedIsByRefLike(TypeSymbol type, bool hasObsolet
             Assert.Empty(peType.GetAttributes());
 
             var peModule = (PEModuleSymbol)peType.ContainingModule;
-            var obsoleteAttribute = peModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(peType.Handle, new MetadataDecoder(peModule), ignoreByRefLikeMarker: false);
+            var obsoleteAttribute = peModule.Module.TryGetDeprecatedOrExperimentalOrObsoleteAttribute(peType.Handle, new MetadataDecoder(peModule), ignoreByRefLikeMarker: false, ignoreRequiredMemberMarker: false);
 
             if (hasObsolete)
             {

diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs
@@ -48,6 +48,8 @@ private static Action<ModuleSymbol> ValidateRequiredMembersInModule(string[] mem
                 AssertEx.AssertEqualToleratingWhitespaceDifferences(expectedAttributeLayout, actualAttributes);
             }
 
+            var requiredTypes = new HashSet<NamedTypeSymbol>();
+
             foreach (var memberPath in memberPaths)
             {
                 var member = module.GlobalNamespace.GetMember(memberPath);
@@ -62,11 +64,47 @@ private static Action<ModuleSymbol> ValidateRequiredMembersInModule(string[] mem
                 {
                     AssertEx.Any(member.GetAttributes(), attr => attr.AttributeClass.ToTestDisplayString() == "System.Runtime.CompilerServices.RequiredMemberAttribute");
                 }
-                Assert.True(((NamedTypeSymbol)member.ContainingSymbol).HasDeclaredRequiredMembers);
+
+                requiredTypes.Add((NamedTypeSymbol)member.ContainingType);
+            }
+
+            foreach (var type in requiredTypes)
+            {
+                AssertTypeRequiredMembersInvariants(module, type);
             }
         };
     }
 
+    private static Action<ModuleSymbol> GetTypeRequiredMembersInvariantsValidator(string expectedType)
+    {
+        return module =>
+        {
+            var type = module.GlobalNamespace.GetTypeMember(expectedType);
+            Assert.NotNull(type);
+            AssertTypeRequiredMembersInvariants(module, type);
+        };
+    }
+
+    private static void AssertTypeRequiredMembersInvariants(ModuleSymbol module, NamedTypeSymbol type)
+    {
+        Assert.True(type.HasAnyRequiredMembers);
+
+        foreach (var ctor in type.GetMembers().Where(m => m is MethodSymbol { MethodKind: MethodKind.Constructor }))
+        {
+            var ctorAttributes = ctor.GetAttributes();
+            if (ctorAttributes.Any(attr => attr.AttributeClass.ToTestDisplayString() == "System.Diagnostics.CodeAnalysis.SetsRequiredMembersAttribute"))
+            {
+                Assert.DoesNotContain(ctorAttributes, attr => attr.AttributeClass.ToTestDisplayString() == "System.ObsoleteAttribute");
+            }
+            else if (module is not SourceModuleSymbol)
+            {
+                Assert.Contains(ctorAttributes, attr =>
+                    attr.AttributeConstructor.ToTestDisplayString() == "System.ObsoleteAttribute..ctor(System.String message, System.Boolean error)"
+                    && attr.ConstructorArguments.ToArray() is [{ ValueInternal: PEModule.RequiredMembersMarker }, { ValueInternal: true }]);
+            }
+        }
+    }
+
     [Fact]
     public void InvalidModifierLocations()
     {
@@ -1685,18 +1723,18 @@ public Derived() {}
         var code = @"_ = new Derived();";
 
         var comp = CreateCompilationWithRequiredMembers(new[] { @base, derived, code });
-
-        comp.VerifyDiagnostics();
+        var validator = GetTypeRequiredMembersInvariantsValidator("Derived");
+        CompileAndVerify(comp, sourceSymbolValidator: validator, symbolValidator: validator).VerifyDiagnostics();
 
         var baseComp = CreateCompilationWithRequiredMembers(@base);
         baseComp.VerifyEmitDiagnostics();
         var baseRef = useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference();
 
         var derivedComp = CreateCompilation(derived, new[] { baseRef });
-        derivedComp.VerifyEmitDiagnostics();
+        CompileAndVerify(derivedComp, sourceSymbolValidator: validator, symbolValidator: validator).VerifyDiagnostics();
 
         comp = CreateCompilation(code, new[] { baseRef, useMetadataReference ? derivedComp.ToMetadataReference() : derivedComp.EmitToImageReference() });
-        comp.VerifyDiagnostics();
+        CompileAndVerify(comp).VerifyDiagnostics();
     }
 
     [Theory]
@@ -1774,13 +1812,45 @@ public class Derived : Base
 ";
 
         var comp = CreateCompilationWithRequiredMembers(new[] { @base, code });
-        CompileAndVerify(comp).VerifyDiagnostics();
+        var validator = GetTypeRequiredMembersInvariantsValidator("Derived");
+        CompileAndVerify(comp, sourceSymbolValidator: validator, symbolValidator: validator).VerifyDiagnostics();
 
         var baseComp = CreateCompilationWithRequiredMembers(@base);
         baseComp.VerifyEmitDiagnostics();
 
         comp = CreateCompilation(code, new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() });
-        CompileAndVerify(comp).VerifyDiagnostics();
+        CompileAndVerify(comp, sourceSymbolValidator: validator, symbolValidator: validator).VerifyDiagnostics();
+    }
+
+    [Theory]
+    [CombinatorialData]
+    public void EnforcedRequiredMembers_Inheritance_NoMembersOnDerivedType(bool useMetadataReference)
+    {
+        var @base = @"
+public class Base
+{
+    public required int Prop1 { get; set; }
+    public required int Field1;
+}
+";
+
+        var code = @"
+_ = new Derived() { Prop1 = 1, Field1 = 1 };
+
+public class Derived : Base
+{
+}
+";
+
+        var comp = CreateCompilationWithRequiredMembers(new[] { @base, code });
+        var validator = GetTypeRequiredMembersInvariantsValidator("Derived");
+        CompileAndVerify(comp, sourceSymbolValidator: validator, symbolValidator: validator).VerifyDiagnostics();
+
+        var baseComp = CreateCompilationWithRequiredMembers(@base);
+        baseComp.VerifyEmitDiagnostics();
+
+        comp = CreateCompilation(code, new[] { useMetadataReference ? baseComp.ToMetadataReference() : baseComp.EmitToImageReference() });
+        CompileAndVerify(comp, sourceSymbolValidator: validator, symbolValidator: validator).VerifyDiagnostics();
     }
 
     [Fact]
@@ -4168,4 +4238,93 @@ struct C
         var comp = CreateCompilationWithRequiredMembers(code);
         CompileAndVerify(comp).VerifyDiagnostics();
     }
+
+    [Fact]
+    public void HasExistingObsoleteAttribute_RequiredMembersOnSelf()
+    {
+        var code = """
+            using System;
+            class C
+            {
+                public required int Prop { get; set; }
+
+                [Obsolete("Reason 1", false)]
+                public C() { }
+
+
+                [Obsolete("Reason 2", true)]
+                public C(int unused) { }
+            }
+            """;
+
+        var comp = CreateCompilationWithRequiredMembers(code);
+        CompileAndVerify(comp, symbolValidator: validate, sourceSymbolValidator: validate).VerifyDiagnostics();
+
+        static void validate(ModuleSymbol m)
+        {
+            var c = m.GlobalNamespace.GetTypeMember("C");
+            var ctors = c.GetMembers(".ctor").Cast<MethodSymbol>().ToArray();
+
+            Assert.Equal(2, ctors.Length);
+
+            assertAttributeData(ctors[0], "Reason 1", expectedError: false);
+            assertAttributeData(ctors[1], "Reason 2", expectedError: true);
+
+            static void assertAttributeData(MethodSymbol ctor, string expectedReason, bool expectedError)
+            {
+                var attrData = ctor.GetAttributes().Single();
+                AssertEx.Equal("System.ObsoleteAttribute", attrData.AttributeClass.ToTestDisplayString());
+                var attrArgs = attrData.ConstructorArguments.ToArray();
+                Assert.Equal(2, attrArgs.Length);
+                AssertEx.Equal(expectedReason, (string)attrArgs[0].ValueInternal!);
+                Assert.Equal(expectedError, (bool)attrArgs[1].ValueInternal!);
+            }
+        }
+    }
+
+    [Fact]
+    public void HasExistingObsoleteAttribute_RequiredMembersOnBase()
+    {
+        var code = """
+            using System;
+            class Base
+            {
+                public required int Prop { get; set; }
+            }
+
+            class Derived : Base
+            {
+                [Obsolete("Reason 1", false)]
+                public Derived() { }
+
+
+                [Obsolete("Reason 2", true)]
+                public Derived(int unused) { }
+            }
+            """;
+
+        var comp = CreateCompilationWithRequiredMembers(code);
+        CompileAndVerify(comp, symbolValidator: validate, sourceSymbolValidator: validate).VerifyDiagnostics();
+
+        static void validate(ModuleSymbol m)
+        {
+            var c = m.GlobalNamespace.GetTypeMember("Derived");
+            var ctors = c.GetMembers(".ctor").Cast<MethodSymbol>().ToArray();
+
+            Assert.Equal(2, ctors.Length);
+
+            assertAttributeData(ctors[0], "Reason 1", expectedError: false);
+            assertAttributeData(ctors[1], "Reason 2", expectedError: true);
+
+            static void assertAttributeData(MethodSymbol ctor, string expectedReason, bool expectedError)
+            {
+                var attrData = ctor.GetAttributes().Single();
+                AssertEx.Equal("System.ObsoleteAttribute", attrData.AttributeClass.ToTestDisplayString());
+                var attrArgs = attrData.ConstructorArguments.ToArray();
+                Assert.Equal(2, attrArgs.Length);
+                AssertEx.Equal(expectedReason, (string)attrArgs[0].ValueInternal!);
+                Assert.Equal(expectedError, (bool)attrArgs[1].ValueInternal!);
+            }
+        }
+    }
 }
diff --git a/src/Compilers/Core/Portable/MetadataReader/PEModule.cs b/src/Compilers/Core/Portable/MetadataReader/PEModule.cs
@@ -1107,11 +1107,13 @@ internal bool HasIsByRefLikeAttribute(EntityHandle token)
         }
 
         internal const string ByRefLikeMarker = "Types with embedded references are not supported in this version of your compiler.";
+        internal const string RequiredMembersMarker = "Constructors of types with required members are not supported in this version of your compiler.";
 
         internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute(
             EntityHandle token,
             IAttributeNamedArgumentDecoder decoder,
-            bool ignoreByRefLikeMarker)
+            bool ignoreByRefLikeMarker,
+            bool ignoreRequiredMemberMarker)
         {
             AttributeInfo info;
 
@@ -1127,9 +1129,10 @@ internal ObsoleteAttributeData TryGetDeprecatedOrExperimentalOrObsoleteAttribute
                 ObsoleteAttributeData obsoleteData = TryExtractObsoleteDataFromAttribute(info, decoder);
                 switch (obsoleteData?.Message)
                 {
-                    // PROTOTYPE(req): Ignore required obsolete marker
                     case ByRefLikeMarker when ignoreByRefLikeMarker:
                         return null;
+                    case RequiredMembersMarker when ignoreRequiredMemberMarker:
+                        return null;
                 }
                 return obsoleteData;
             }