Remove DAM annotations from enum converter (dotnet#100347)

* Remove DAM annotations from enum converter * expand the debug assert for enum types * FB * FB2
matouskozak · Apr 30, 2024 · 96433fb · 96433fb
1 parent a4d2139
commit 96433fb
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 16 deletions.
diff --git a/src/libraries/System.ComponentModel.TypeConverter/ref/System.ComponentModel.TypeConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/ref/System.ComponentModel.TypeConverter.cs
@@ -428,9 +428,8 @@ public DoubleConverter() { }
     }
     public partial class EnumConverter : System.ComponentModel.TypeConverter
     {
-        public EnumConverter([System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicFields | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)] System.Type type) { }
+        public EnumConverter(System.Type type) { }
         protected virtual System.Collections.IComparer Comparer { get { throw null; } }
-        [System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute(System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicFields | System.Diagnostics.CodeAnalysis.DynamicallyAccessedMemberTypes.PublicParameterlessConstructor)]
         protected System.Type EnumType { get { throw null; } }
         protected System.ComponentModel.TypeConverter.StandardValuesCollection? Values { get { throw null; } set { } }
         public override bool CanConvertFrom(System.ComponentModel.ITypeDescriptorContext? context, System.Type sourceType) { throw null; }

diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx b/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx
@@ -76,6 +76,9 @@
   <data name="EnumConverterInvalidValue" xml:space="preserve">
     <value>The value '{0}' is not a valid value for the enum '{1}'.</value>
   </data>
+  <data name="EnumInvalidValue" xml:space="preserve">
+    <value>Type provided must be an Enum.</value>
+  </data>
   <data name="ErrorInvalidEventHandler" xml:space="preserve">
     <value>Invalid event handler for the {0} event.</value>
   </data>

diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs
@@ -4,6 +4,7 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.ComponentModel.Design.Serialization;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
@@ -20,12 +21,16 @@ public class EnumConverter : TypeConverter
         /// Initializes a new instance of the <see cref='System.ComponentModel.EnumConverter'/> class for the given
         /// type.
         /// </summary>
-        public EnumConverter([DynamicallyAccessedMembers(TypeDescriptor.ReflectTypesDynamicallyAccessedMembers)] Type type)
+        public EnumConverter(Type type)
         {
+            if (!type.IsEnum && !type.Equals(typeof(Enum)))
+            {
+                throw new ArgumentException(SR.EnumInvalidValue);
+            }
+
             EnumType = type;
         }
 
-        [DynamicallyAccessedMembers(TypeDescriptor.ReflectTypesDynamicallyAccessedMembers)]
         protected Type EnumType { get; }
 
         protected StandardValuesCollection? Values { get; set; }
@@ -156,7 +161,10 @@ private static long GetEnumValue(bool isUnderlyingTypeUInt64, object enumVal, Cu
                 }
                 else
                 {
-                    FieldInfo? info = EnumType.GetField(enumName);
+                    [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim Enums")]
+                    FieldInfo? GetEnumField(string name) => EnumType.GetField(name);
+
+                    FieldInfo? info = GetEnumField(enumName);
                     if (info != null)
                     {
                         return new InstanceDescriptor(info, null);
@@ -227,9 +235,27 @@ public override StandardValuesCollection GetStandardValues(ITypeDescriptorContex
                 // We need to get the enum values in this rather round-about way so we can filter
                 // out fields marked Browsable(false). Note that if multiple fields have the same value,
                 // the behavior is undefined, since what we return are just enum values, not names.
-                Type reflectType = TypeDescriptor.GetReflectionType(EnumType) ?? EnumType;
+                // Given that EnumType is constrained to be an enum, we suppress calls for reflection with Enum.
+
+                [UnconditionalSuppressMessage("Trimming", "IL2067:", Justification = "Trimmer does not trim Enums")]
+                [return: DynamicallyAccessedMembers(TypeDescriptor.ReflectTypesDynamicallyAccessedMembers)]
+                static Type GetTypeDescriptorReflectionType(Type enumType) => TypeDescriptor.GetReflectionType(enumType);
+
+                Type _reflectType = GetTypeDescriptorReflectionType(EnumType);
+                FieldInfo[]? fields;
+
+                if (_reflectType == null)
+                {
+                    [UnconditionalSuppressMessage("Trimming", "IL2070:", Justification = "Trimmer does not trim Enums")]
+                    static FieldInfo[]? GetPublicStaticEnumFields(Type type) => type.GetFields(BindingFlags.Public | BindingFlags.Static);
+
+                    fields = GetPublicStaticEnumFields(EnumType);
+                }
+                else
+                {
+                    fields = _reflectType.GetFields(BindingFlags.Public | BindingFlags.Static);
+                }
 
-                FieldInfo[]? fields = reflectType.GetFields(BindingFlags.Public | BindingFlags.Static);
                 ArrayList? objValues = null;
 
                 if (fields != null && fields.Length > 0)

diff --git a/....ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/....ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs
@@ -193,7 +193,7 @@ private static Dictionary<object, IntrinsicTypeConverterData> IntrinsicTypeConve
                     //
                     [typeof(Array)] = new IntrinsicTypeConverterData((type) => new ArrayConverter()),
                     [typeof(ICollection)] = new IntrinsicTypeConverterData((type) => new CollectionConverter()),
-                    [typeof(Enum)] = new IntrinsicTypeConverterData((type) => CreateEnumConverter(type), cacheConverterInstance: false),
+                    [typeof(Enum)] = new IntrinsicTypeConverterData((type) => new EnumConverter(type), cacheConverterInstance: false),
                     [s_intrinsicNullableKey] = new IntrinsicTypeConverterData((type) => CreateNullableConverter(type), cacheConverterInstance: false),
                     [s_intrinsicReferenceKey] = new IntrinsicTypeConverterData((type) => new ReferenceConverter(type), cacheConverterInstance: false),
                 });
@@ -204,14 +204,6 @@ private static Dictionary<object, IntrinsicTypeConverterData> IntrinsicTypeConve
             Justification = "IntrinsicTypeConverters is marked with RequiresUnreferencedCode. It is the only place that should call this.")]
         private static NullableConverter CreateNullableConverter(Type type) => new NullableConverter(type);
 
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:UnrecognizedReflectionPattern",
-            Justification = "Trimmer does not trim enums")]
-        private static EnumConverter CreateEnumConverter(Type type)
-        {
-            Debug.Assert(type.IsEnum || type == typeof(Enum));
-            return new EnumConverter(type);
-        }
-
         private static Hashtable PropertyCache => LazyInitializer.EnsureInitialized(ref s_propertyCache, () => new Hashtable());
 
         private static Hashtable EventCache => LazyInitializer.EnsureInitialized(ref s_eventCache, () => new Hashtable());

diff --git a/src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml b/src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml
@@ -85,6 +85,12 @@
     <Left>net8.0/netstandard.dll</Left>
     <Right>net9.0/netstandard.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0014</DiagnosticId>
+    <Target>M:System.ComponentModel.EnumConverter.#ctor(System.Type)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
+    <Left>net8.0/netstandard.dll</Left>
+    <Right>net9.0/netstandard.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0014</DiagnosticId>
     <Target>P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
@@ -109,6 +115,12 @@
     <Left>net8.0/netstandard.dll</Left>
     <Right>net9.0/netstandard.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0014</DiagnosticId>
+    <Target>P:System.ComponentModel.EnumConverter.EnumType:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
+    <Left>net8.0/netstandard.dll</Left>
+    <Right>net9.0/netstandard.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0014</DiagnosticId>
     <Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
@@ -301,6 +313,12 @@
     <Left>net8.0/System.ComponentModel.TypeConverter.dll</Left>
     <Right>net9.0/System.ComponentModel.TypeConverter.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0014</DiagnosticId>
+    <Target>M:System.ComponentModel.EnumConverter.#ctor(System.Type)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
+    <Left>net8.0/System.ComponentModel.TypeConverter.dll</Left>
+    <Right>net9.0/System.ComponentModel.TypeConverter.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0014</DiagnosticId>
     <Target>P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
@@ -325,6 +343,12 @@
     <Left>net8.0/System.ComponentModel.TypeConverter.dll</Left>
     <Right>net9.0/System.ComponentModel.TypeConverter.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0014</DiagnosticId>
+    <Target>P:System.ComponentModel.EnumConverter.EnumType:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
+    <Left>net8.0/System.ComponentModel.TypeConverter.dll</Left>
+    <Right>net9.0/System.ComponentModel.TypeConverter.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0014</DiagnosticId>
     <Target>M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
@@ -409,6 +433,12 @@
     <Left>net8.0/System.dll</Left>
     <Right>net9.0/System.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0014</DiagnosticId>
+    <Target>M:System.ComponentModel.EnumConverter.#ctor(System.Type)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
+    <Left>net8.0/System.dll</Left>
+    <Right>net9.0/System.dll</Right>
+  </Suppression>
   <Suppression>
     <DiagnosticId>CP0014</DiagnosticId>
     <Target>P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
@@ -433,4 +463,10 @@
     <Left>net8.0/System.dll</Left>
     <Right>net9.0/System.dll</Right>
   </Suppression>
+  <Suppression>
+    <DiagnosticId>CP0014</DiagnosticId>
+    <Target>P:System.ComponentModel.EnumConverter.EnumType:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute]</Target>
+    <Left>net8.0/System.dll</Left>
+    <Right>net9.0/System.dll</Right>
+  </Suppression>
 </Suppressions>