From 6f2896d2ef0350207abca056905fe8d7c1555ea3 Mon Sep 17 00:00:00 2001 From: Lakshan Fernando Date: Mon, 25 Mar 2024 13:56:57 -0700 Subject: [PATCH 1/4] Remove DAM annotations from enum converter --- .../System.ComponentModel.TypeConverter.cs | 3 +- .../System/ComponentModel/EnumConverter.cs | 8 +++-- .../ReflectTypeDescriptionProvider.cs | 2 -- ...iCompatBaseline.NetCoreAppLatestStable.xml | 36 +++++++++++++++++++ 4 files changed, 43 insertions(+), 6 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 index 5b3f3fdef12ff..59c3284a7fabf 100644 --- 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/System/ComponentModel/EnumConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs index 5dcc263e0d0c0..76d1d519bb952 100644 --- 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,12 @@ public class EnumConverter : TypeConverter /// Initializes a new instance of the class for the given /// type. /// - public EnumConverter([DynamicallyAccessedMembers(TypeDescriptor.ReflectTypesDynamicallyAccessedMembers)] Type type) + public EnumConverter(Type type) { + Debug.Assert(type.IsEnum, "type should be an Enum type"); EnumType = type; } - [DynamicallyAccessedMembers(TypeDescriptor.ReflectTypesDynamicallyAccessedMembers)] protected Type EnumType { get; } protected StandardValuesCollection? Values { get; set; } @@ -115,6 +116,7 @@ private static long GetEnumValue(bool isUnderlyingTypeUInt64, object enumVal, Cu /// /// Converts the given value object to the specified destination type. /// + [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim enums")] public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType) { ArgumentNullException.ThrowIfNull(destinationType); @@ -220,6 +222,8 @@ private static long GetEnumValue(bool isUnderlyingTypeUInt64, object enumVal, Cu /// Gets a collection of standard values for the data type this validator is /// designed for. /// + [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim enums")] + [UnconditionalSuppressMessage("Trimming", "IL2072:", Justification = "Trimmer does not trim enums")] public override StandardValuesCollection GetStandardValues(ITypeDescriptorContext? context) { if (Values == null) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs index 5dacbaa8a2526..b8ed399783c60 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs @@ -204,8 +204,6 @@ private static Dictionary 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)); diff --git a/src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml b/src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml index b908b6cbab071..09cfe417e3812 100644 --- a/src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml +++ b/src/libraries/apicompat/ApiCompatBaseline.NetCoreAppLatestStable.xml @@ -85,6 +85,12 @@ net8.0/netstandard.dll net9.0/netstandard.dll + + CP0014 + M:System.ComponentModel.EnumConverter.#ctor(System.Type)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] + net8.0/netstandard.dll + net9.0/netstandard.dll + CP0014 P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] @@ -109,6 +115,12 @@ net8.0/netstandard.dll net9.0/netstandard.dll + + CP0014 + P:System.ComponentModel.EnumConverter.EnumType:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] + net8.0/netstandard.dll + net9.0/netstandard.dll + CP0014 M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] @@ -301,6 +313,12 @@ net8.0/System.ComponentModel.TypeConverter.dll net9.0/System.ComponentModel.TypeConverter.dll + + CP0014 + M:System.ComponentModel.EnumConverter.#ctor(System.Type)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] + net8.0/System.ComponentModel.TypeConverter.dll + net9.0/System.ComponentModel.TypeConverter.dll + CP0014 P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] @@ -325,6 +343,12 @@ net8.0/System.ComponentModel.TypeConverter.dll net9.0/System.ComponentModel.TypeConverter.dll + + CP0014 + P:System.ComponentModel.EnumConverter.EnumType:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] + net8.0/System.ComponentModel.TypeConverter.dll + net9.0/System.ComponentModel.TypeConverter.dll + CP0014 M:System.ComponentModel.DesignerAttribute.#ctor(System.String,System.String)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] @@ -409,6 +433,12 @@ net8.0/System.dll net9.0/System.dll + + CP0014 + M:System.ComponentModel.EnumConverter.#ctor(System.Type)$0:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] + net8.0/System.dll + net9.0/System.dll + CP0014 P:System.ComponentModel.DesignerAttribute.DesignerBaseTypeName:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] @@ -433,4 +463,10 @@ net8.0/System.dll net9.0/System.dll + + CP0014 + P:System.ComponentModel.EnumConverter.EnumType:[T:System.Diagnostics.CodeAnalysis.DynamicallyAccessedMembersAttribute] + net8.0/System.dll + net9.0/System.dll + \ No newline at end of file From 1179d0382c34bbef263a63d52b6c78264c923cd9 Mon Sep 17 00:00:00 2001 From: Lakshan Fernando Date: Thu, 28 Mar 2024 10:27:25 -0700 Subject: [PATCH 2/4] expand the debug assert for enum types --- .../src/System/ComponentModel/EnumConverter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs index 76d1d519bb952..b280cf891b7d5 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs @@ -23,7 +23,7 @@ public class EnumConverter : TypeConverter /// public EnumConverter(Type type) { - Debug.Assert(type.IsEnum, "type should be an Enum type"); + Debug.Assert(type.IsEnum || type.Equals(typeof(Enum)), "type should be an Enum type"); EnumType = type; } From 398fea50b9836d17f679165aa1f6d118e8249d2f Mon Sep 17 00:00:00 2001 From: Lakshan Fernando Date: Mon, 1 Apr 2024 09:10:14 -0700 Subject: [PATCH 3/4] FB --- .../src/Resources/Strings.resx | 3 +++ .../System/ComponentModel/EnumConverter.cs | 23 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx b/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx index 6930398b39814..d4d4ea85dcee8 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx +++ b/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx @@ -76,6 +76,9 @@ The value '{0}' is not a valid value for the enum '{1}'. + + Type '{0}' should be an Enum type. + Invalid event handler for the {0} event. diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs index b280cf891b7d5..ec44c789640c3 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs @@ -24,6 +24,11 @@ public class EnumConverter : TypeConverter public EnumConverter(Type type) { Debug.Assert(type.IsEnum || type.Equals(typeof(Enum)), "type should be an Enum type"); + if (!type.IsEnum && !type.Equals(typeof(Enum))) + { + throw new ArgumentException(SR.Format(SR.EnumInvalidValue, nameof(type))); + } + EnumType = type; } @@ -116,7 +121,6 @@ private static long GetEnumValue(bool isUnderlyingTypeUInt64, object enumVal, Cu /// /// Converts the given value object to the specified destination type. /// - [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim enums")] public override object? ConvertTo(ITypeDescriptorContext? context, CultureInfo? culture, object? value, Type destinationType) { ArgumentNullException.ThrowIfNull(destinationType); @@ -158,7 +162,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); @@ -222,8 +229,6 @@ private static long GetEnumValue(bool isUnderlyingTypeUInt64, object enumVal, Cu /// Gets a collection of standard values for the data type this validator is /// designed for. /// - [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim enums")] - [UnconditionalSuppressMessage("Trimming", "IL2072:", Justification = "Trimmer does not trim enums")] public override StandardValuesCollection GetStandardValues(ITypeDescriptorContext? context) { if (Values == null) @@ -231,9 +236,15 @@ 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; + [UnconditionalSuppressMessage("Trimming", "IL2067:", Justification = "Trimmer does not trim enums")] + static Type GetTypeDescriptorReflectionType(Type enumType) => TypeDescriptor.GetReflectionType(enumType); + + Type reflectType = GetTypeDescriptorReflectionType(EnumType) ?? EnumType; + + [UnconditionalSuppressMessage("Trimming", "IL2070:", Justification = "Trimmer does not trim enums")] + static FieldInfo[]? GetPublicStaticEnumFields(Type type) => type.GetFields(BindingFlags.Public | BindingFlags.Static); - FieldInfo[]? fields = reflectType.GetFields(BindingFlags.Public | BindingFlags.Static); + FieldInfo[]? fields = GetPublicStaticEnumFields(reflectType); ArrayList? objValues = null; if (fields != null && fields.Length > 0) From 03afd4bb41752763ec39415579fed7d451443431 Mon Sep 17 00:00:00 2001 From: Lakshan Fernando Date: Wed, 3 Apr 2024 07:17:46 -0700 Subject: [PATCH 4/4] FB2 --- .../src/Resources/Strings.resx | 2 +- .../System/ComponentModel/EnumConverter.cs | 27 +++++++++++++------ .../ReflectTypeDescriptionProvider.cs | 8 +----- 3 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx b/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx index d4d4ea85dcee8..ea75a80145686 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx +++ b/src/libraries/System.ComponentModel.TypeConverter/src/Resources/Strings.resx @@ -77,7 +77,7 @@ The value '{0}' is not a valid value for the enum '{1}'. - Type '{0}' should be an Enum type. + Type provided must be an Enum. Invalid event handler for the {0} event. diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs index ec44c789640c3..c9d6d5a13a384 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/EnumConverter.cs @@ -23,10 +23,9 @@ public class EnumConverter : TypeConverter /// public EnumConverter(Type type) { - Debug.Assert(type.IsEnum || type.Equals(typeof(Enum)), "type should be an Enum type"); if (!type.IsEnum && !type.Equals(typeof(Enum))) { - throw new ArgumentException(SR.Format(SR.EnumInvalidValue, nameof(type))); + throw new ArgumentException(SR.EnumInvalidValue); } EnumType = type; @@ -162,7 +161,7 @@ private static long GetEnumValue(bool isUnderlyingTypeUInt64, object enumVal, Cu } else { - [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim enums")] + [UnconditionalSuppressMessage("Trimming", "IL2075:", Justification = "Trimmer does not trim Enums")] FieldInfo? GetEnumField(string name) => EnumType.GetField(name); FieldInfo? info = GetEnumField(enumName); @@ -236,15 +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. - [UnconditionalSuppressMessage("Trimming", "IL2067:", Justification = "Trimmer does not trim enums")] + // 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) ?? 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); - [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 = GetPublicStaticEnumFields(reflectType); ArrayList? objValues = null; if (fields != null && fields.Length > 0) diff --git a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs index b8ed399783c60..5fc0d064f8109 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs @@ -193,7 +193,7 @@ private static Dictionary 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,12 +204,6 @@ private static Dictionary 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); - 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());