From 79c3416fa72af616069572795832aa15112f0251 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 11 Feb 2022 20:36:19 -0500 Subject: [PATCH 01/12] Avoid Attribute.GetCustomAttributes() returning null for open generic type. Fix #64335 --- .../src/System/Attribute.CoreCLR.cs | 13 ++-- .../Reflection/RuntimeCustomAttributeData.cs | 40 +++++++----- .../System.Runtime/tests/System/Attributes.cs | 65 +++++++++++++++++++ 3 files changed, 97 insertions(+), 21 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs index f33849d2a949e..757b04bf8e5f0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs @@ -438,6 +438,9 @@ private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elem { return (Attribute[])Array.CreateInstance(elementType, elementCount); } + + private static Attribute[] ToAttributeArray(object[] getCustomAttributesResult) => + getCustomAttributesResult as Attribute[] ?? Array.Empty(); #endregion #endregion @@ -459,7 +462,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, Type attribu { MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, attributeType, inherit), MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, attributeType, inherit), - _ => (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!, + _ => ToAttributeArray(element.GetCustomAttributes(attributeType, inherit)) }; } @@ -474,7 +477,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, bool inherit { MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, typeof(Attribute), inherit), MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, typeof(Attribute), inherit), - _ => (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!, + _ => ToAttributeArray(element.GetCustomAttributes(typeof(Attribute), inherit)) }; } @@ -536,12 +539,11 @@ public static Attribute[] GetCustomAttributes(ParameterInfo element!!, Type attr if (element.Member == null) throw new ArgumentException(SR.Argument_InvalidParameterInfo, nameof(element)); - MemberInfo member = element.Member; if (member.MemberType == MemberTypes.Method && inherit) return InternalParamGetCustomAttributes(element, attributeType, inherit); - return (element.GetCustomAttributes(attributeType, inherit) as Attribute[])!; + return ToAttributeArray(element.GetCustomAttributes(attributeType, inherit)); } public static Attribute[] GetCustomAttributes(ParameterInfo element!!, bool inherit) @@ -549,12 +551,11 @@ public static Attribute[] GetCustomAttributes(ParameterInfo element!!, bool inhe if (element.Member == null) throw new ArgumentException(SR.Argument_InvalidParameterInfo, nameof(element)); - MemberInfo member = element.Member; if (member.MemberType == MemberTypes.Method && inherit) return InternalParamGetCustomAttributes(element, null, inherit); - return (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!; + return ToAttributeArray(element.GetCustomAttributes(typeof(Attribute), inherit)); } public static bool IsDefined(ParameterInfo element, Type attributeType) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index 9eecc089e72c4..c6542ffe0fa16 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -902,7 +902,7 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp Debug.Assert(caType is not null); if (type.GetElementType() is not null) - return (caType.IsValueType) ? Array.Empty() : CreateAttributeArrayHelper(caType, 0); + return CreateAttributeArrayHelper(caType, 0); if (type.IsGenericType && !type.IsGenericTypeDefinition) type = (type.GetGenericTypeDefinition() as RuntimeType)!; @@ -922,8 +922,6 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp RuntimeType.ListBuilder result = default; bool mustBeInheritable = false; - bool useObjectArray = (caType.IsValueType || caType.ContainsGenericParameters); - RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType; for (int i = 0; i < pcas.Count; i++) result.Add(pcas[i]); @@ -935,7 +933,7 @@ internal static object[] GetCustomAttributes(RuntimeType type, RuntimeType caTyp type = (type.BaseType as RuntimeType)!; } - object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count); + object[] typedResult = CreateAttributeArrayHelper(caType, result.Count); for (int i = 0; i < result.Count; i++) { typedResult[i] = result[i]; @@ -966,8 +964,6 @@ internal static object[] GetCustomAttributes(RuntimeMethodInfo method, RuntimeTy RuntimeType.ListBuilder result = default; bool mustBeInheritable = false; - bool useObjectArray = (caType.IsValueType || caType.ContainsGenericParameters); - RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType; for (int i = 0; i < pcas.Count; i++) result.Add(pcas[i]); @@ -979,7 +975,7 @@ internal static object[] GetCustomAttributes(RuntimeMethodInfo method, RuntimeTy method = method.GetParentDefinition()!; } - object[] typedResult = CreateAttributeArrayHelper(arrayType, result.Count); + object[] typedResult = CreateAttributeArrayHelper(caType, result.Count); for (int i = 0; i < result.Count; i++) { typedResult[i] = result[i]; @@ -1126,16 +1122,13 @@ private static bool IsCustomAttributeDefined( } private static object[] GetCustomAttributes( - RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType? attributeFilterType) + RuntimeModule decoratedModule, int decoratedMetadataToken, int pcaCount, RuntimeType attributeFilterType) { RuntimeType.ListBuilder attributes = default; AddCustomAttributes(ref attributes, decoratedModule, decoratedMetadataToken, attributeFilterType, false, default); - bool useObjectArray = attributeFilterType is null || attributeFilterType.IsValueType || attributeFilterType.ContainsGenericParameters; - RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : attributeFilterType!; - - object[] result = CreateAttributeArrayHelper(arrayType, attributes.Count + pcaCount); + object[] result = CreateAttributeArrayHelper(attributeFilterType, attributes.Count + pcaCount); for (int i = 0; i < attributes.Count; i++) { result[i] = attributes[i]; @@ -1480,15 +1473,32 @@ private static void GetPropertyOrFieldData( blobStart = (IntPtr)pBlobStart; } - private static object[] CreateAttributeArrayHelper(RuntimeType elementType, int elementCount) + private static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount) { + if (caType.IsValueType) + { + return CreateArray(); + } + + if (caType.ContainsGenericParameters) + { + if (caType.IsSubclassOf(typeof(Attribute))) + { + return CreateArray(); + } + + return CreateArray(); + } + // If we have 0 elements, don't allocate a new array if (elementCount == 0) { - return elementType.GetEmptyArray(); + return caType.GetEmptyArray(); } - return (object[])Array.CreateInstance(elementType, elementCount); + return (object[])Array.CreateInstance(caType, elementCount); + + T[] CreateArray() => elementCount == 0 ? Array.Empty() : new T[elementCount]; } #endregion } diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index e219d30a2bef4..9323d9ce1a0ea 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -533,6 +533,33 @@ public static void NegTest8() AssertExtensions.Throws(null, () => (ArgumentUsageAttribute)Attribute.GetCustomAttribute(paramInfos[0], attributeType, false)); } + + // reproduces https://github.com/dotnet/runtime/issues/64335 + [Fact] + public static void GetCustomAttributesDoesNotReturnNullForOpenGenericType() + { + Attribute[] openGenericAttributes = Attribute.GetCustomAttributes(typeof(HasGenericAttribute), typeof(GenericAttribute<>)); + Assert.Empty(openGenericAttributes); + Assert.Equal(typeof(Attribute[]), openGenericAttributes.GetType()); + + Attribute[] closedGenericAttributes = Attribute.GetCustomAttributes(typeof(HasGenericAttribute), typeof(GenericAttribute)); + Assert.Equal(1, closedGenericAttributes.Length); + Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); + } + + [Fact] + public static void GetCustomAttributesReturnsAttributeArrayForMisbehavingCustomMemberInfo() + { + FieldInfo fieldWithNullAttributes = new CustomFieldInfo(null!); + Attribute[] fieldWithNullAttributesAttributes = Attribute.GetCustomAttributes(fieldWithNullAttributes); + Assert.NotNull(fieldWithNullAttributesAttributes); + Assert.Empty(fieldWithNullAttributesAttributes); + + FieldInfo fieldWithStringAttributes = new CustomFieldInfo(new[] { "a", "b" }); + Attribute[] fieldWithStringAttributesAttributes = Attribute.GetCustomAttributes(fieldWithStringAttributes); + Assert.NotNull(fieldWithStringAttributesAttributes); + Assert.Empty(fieldWithStringAttributesAttributes); + } } [AttributeUsage(AttributeTargets.All)] public class TestAttribute : Attribute @@ -824,4 +851,42 @@ public class NameableAttribute : Attribute, INameable [Nameable] public class ExampleWithAttribute { } + + public class GenericAttribute : Attribute + { + } + + [GenericAttribute] + public class HasGenericAttribute + { + } + + public class CustomFieldInfo : FieldInfo + { + private readonly object[] _customAttributes; + + public CustomFieldInfo(object[] customAttributes) + { + this._customAttributes = customAttributes; + } + + public override FieldAttributes Attributes => default; + public override RuntimeFieldHandle FieldHandle => default; + public override Type FieldType => typeof(object); + public override Type DeclaringType => null; + public override string Name => "custom"; + public override Type ReflectedType => null; + + public override object[] GetCustomAttributes(bool inherit) => this._customAttributes; + + public override object[] GetCustomAttributes(Type attributeType, bool inherit) => this._customAttributes; + + public override object GetValue(object obj) => null; + + public override bool IsDefined(Type attributeType, bool inherit) => false; + + public override void SetValue(object obj, object value, BindingFlags invokeAttr, Binder binder, System.Globalization.CultureInfo culture) + { + } + } } From f0f1e6ce4e5d8b4c5d2ff887a7622391c2078950 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 12 Feb 2022 15:56:25 -0500 Subject: [PATCH 02/12] Fix GenericAttributeTests to reflect #64335 Previously, this test asserted that GetCustomAttributes on an open generic type would return null. Now we return empty instead. --- .../reflection/GenericAttribute/GenericAttributeTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs index e7661d2d5aea8..912f34acd5977 100644 --- a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs +++ b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs @@ -152,8 +152,8 @@ static int Main(string[] args) AssertAny(b10, a => (a as MultiAttribute)?.Value == typeof(Class)); AssertAny(b10, a => (a as MultiAttribute)?.Value == typeof(Class.Derive)); - Assert(CustomAttributeExtensions.GetCustomAttributes(programTypeInfo, typeof(MultiAttribute<>), false) == null); - Assert(CustomAttributeExtensions.GetCustomAttributes(programTypeInfo, typeof(MultiAttribute<>), true) == null); + Assert(!CustomAttributeExtensions.GetCustomAttributes(programTypeInfo, typeof(MultiAttribute<>), false).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(programTypeInfo, typeof(MultiAttribute<>), true).GetEnumerator().MoveNext()); Assert(!((ICustomAttributeProvider)programTypeInfo).GetCustomAttributes(typeof(MultiAttribute<>), true).GetEnumerator().MoveNext()); // Test coverage for CustomAttributeData api surface From fc0f4019694e653c9247afbc246ac148cc2c3de0 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 12 Feb 2022 17:03:49 -0500 Subject: [PATCH 03/12] Skip new attribute tests on Mono. --- src/libraries/System.Runtime/tests/System/Attributes.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index 9323d9ce1a0ea..42f58a02977b5 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -536,6 +536,7 @@ public static void NegTest8() // reproduces https://github.com/dotnet/runtime/issues/64335 [Fact] + [SkipOnMono("Mono throws on open generic type passed to GetCustomAttributes")] public static void GetCustomAttributesDoesNotReturnNullForOpenGenericType() { Attribute[] openGenericAttributes = Attribute.GetCustomAttributes(typeof(HasGenericAttribute), typeof(GenericAttribute<>)); @@ -548,6 +549,7 @@ public static void GetCustomAttributesDoesNotReturnNullForOpenGenericType() } [Fact] + [SkipOnMono("Mono throws NullReferenceException internally if ICustomAttributeProvider.GetCustomAttributes returns null")] public static void GetCustomAttributesReturnsAttributeArrayForMisbehavingCustomMemberInfo() { FieldInfo fieldWithNullAttributes = new CustomFieldInfo(null!); From 224c985c63df2a543330ad092842c911c544efc5 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Wed, 16 Feb 2022 07:08:47 -0500 Subject: [PATCH 04/12] Address feedback from https://github.com/dotnet/runtime/pull/65237 --- .../src/System/Attribute.CoreCLR.cs | 11 +-- .../src/System/Reflection/Attribute.CoreRT.cs | 16 +--- .../System.Runtime/tests/System/Attributes.cs | 81 +++++++++++++++---- 3 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs index 757b04bf8e5f0..12f0899b31e6c 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs @@ -438,9 +438,6 @@ private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elem { return (Attribute[])Array.CreateInstance(elementType, elementCount); } - - private static Attribute[] ToAttributeArray(object[] getCustomAttributesResult) => - getCustomAttributesResult as Attribute[] ?? Array.Empty(); #endregion #endregion @@ -462,7 +459,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, Type attribu { MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, attributeType, inherit), MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, attributeType, inherit), - _ => ToAttributeArray(element.GetCustomAttributes(attributeType, inherit)) + _ => (Attribute[])element.GetCustomAttributes(attributeType, inherit) }; } @@ -477,7 +474,7 @@ public static Attribute[] GetCustomAttributes(MemberInfo element!!, bool inherit { MemberTypes.Property => InternalGetCustomAttributes((PropertyInfo)element, typeof(Attribute), inherit), MemberTypes.Event => InternalGetCustomAttributes((EventInfo)element, typeof(Attribute), inherit), - _ => ToAttributeArray(element.GetCustomAttributes(typeof(Attribute), inherit)) + _ => (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit) }; } @@ -543,7 +540,7 @@ public static Attribute[] GetCustomAttributes(ParameterInfo element!!, Type attr if (member.MemberType == MemberTypes.Method && inherit) return InternalParamGetCustomAttributes(element, attributeType, inherit); - return ToAttributeArray(element.GetCustomAttributes(attributeType, inherit)); + return (Attribute[])element.GetCustomAttributes(attributeType, inherit); } public static Attribute[] GetCustomAttributes(ParameterInfo element!!, bool inherit) @@ -555,7 +552,7 @@ public static Attribute[] GetCustomAttributes(ParameterInfo element!!, bool inhe if (member.MemberType == MemberTypes.Method && inherit) return InternalParamGetCustomAttributes(element, null, inherit); - return ToAttributeArray(element.GetCustomAttributes(typeof(Attribute), inherit)); + return (Attribute[])element.GetCustomAttributes(typeof(Attribute), inherit); } public static bool IsDefined(ParameterInfo element, Type attributeType) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Attribute.CoreRT.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Attribute.CoreRT.cs index d7f9bb693ea03..f7604612b5ad5 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Attribute.CoreRT.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/Reflection/Attribute.CoreRT.cs @@ -141,19 +141,9 @@ private static Attribute[] Instantiate(IEnumerable cads, Ty attributes.Add(instantiatedAttribute); } int count = attributes.Count; - Attribute[] result; - try - { - result = (Attribute[])Array.CreateInstance(actualElementType, count); - } - catch (NotSupportedException) when (actualElementType.ContainsGenericParameters) - { - // This is here for desktop compatibility (using try-catch as control flow to avoid slowing down the mainline case.) - // GetCustomAttributes() normally returns an array of the exact attribute type requested except when - // the requested type is an open type. Its ICustomAttributeProvider counterpart would return an Object[] array but that's - // not possible with this api's return type so it returns null instead. - return null; - } + Attribute[] result = actualElementType.ContainsGenericParameters + ? new Attribute[count] + : (Attribute[])Array.CreateInstance(actualElementType, count); attributes.CopyTo(result, 0); return result; } diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index 42f58a02977b5..e94d23aac97af 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -20,6 +20,8 @@ using System.Runtime.InteropServices; using Xunit; +[assembly: System.Tests.GenericAttribute] +[module: System.Tests.GenericAttribute] [module:Debuggable(true,false)] namespace System.Tests { @@ -231,7 +233,8 @@ public static void customAttributeCount() // [TestAttributes.FooAttribute()] // [TestAttributes.ComplicatedAttribute((Int32)1, Stuff = 2)] // [System.Diagnostics.DebuggableAttribute((Boolean)True, (Boolean)False)] - Assert.Equal(4, customAttributes.Count); + // [System.Tests.GenericAttribute] + Assert.Equal(5, customAttributes.Count); } [Fact] @@ -534,33 +537,66 @@ public static void NegTest8() } - // reproduces https://github.com/dotnet/runtime/issues/64335 [Fact] - [SkipOnMono("Mono throws on open generic type passed to GetCustomAttributes")] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] public static void GetCustomAttributesDoesNotReturnNullForOpenGenericType() { - Attribute[] openGenericAttributes = Attribute.GetCustomAttributes(typeof(HasGenericAttribute), typeof(GenericAttribute<>)); - Assert.Empty(openGenericAttributes); - Assert.Equal(typeof(Attribute[]), openGenericAttributes.GetType()); + Assembly assembly = Assembly.GetExecutingAssembly(); + Check(t => Attribute.GetCustomAttributes(assembly, t)); - Attribute[] closedGenericAttributes = Attribute.GetCustomAttributes(typeof(HasGenericAttribute), typeof(GenericAttribute)); - Assert.Equal(1, closedGenericAttributes.Length); - Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); + Module module = typeof(HasGenericAttribute).Module; + Check(t => Attribute.GetCustomAttributes(module, t)); + + Check(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute))); + + FieldInfo field = typeof(HasGenericAttribute).GetField(nameof(HasGenericAttribute.Field), BindingFlags.NonPublic | BindingFlags.Instance); + Check(t => Attribute.GetCustomAttributes(field, t)); + + MethodInfo method = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)); + Check(t => Attribute.GetCustomAttributes(method, t)); + + ParameterInfo parameter = method.GetParameters()[0]; + Check(t => Attribute.GetCustomAttributes(parameter, t)); + + PropertyInfo property = typeof(HasGenericAttribute).GetProperty(nameof(HasGenericAttribute.Property)); + Check>(t => Attribute.GetCustomAttributes(property, t)); + + EventInfo @event = typeof(HasGenericAttribute).GetEvent(nameof(HasGenericAttribute.Event)); + Check(t => Attribute.GetCustomAttributes(@event, t)); + + void Check(Func getCustomAttributes) + { + Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>)); + Assert.Empty(openGenericAttributes); + + Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute)); + Assert.Equal(1, closedGenericAttributes.Length); + Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); + } } [Fact] - [SkipOnMono("Mono throws NullReferenceException internally if ICustomAttributeProvider.GetCustomAttributes returns null")] public static void GetCustomAttributesReturnsAttributeArrayForMisbehavingCustomMemberInfo() { FieldInfo fieldWithNullAttributes = new CustomFieldInfo(null!); - Attribute[] fieldWithNullAttributesAttributes = Attribute.GetCustomAttributes(fieldWithNullAttributes); - Assert.NotNull(fieldWithNullAttributesAttributes); - Assert.Empty(fieldWithNullAttributesAttributes); + if (PlatformDetection.IsMonoRuntime) + { + Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithNullAttributes)); + } + else + { + Attribute[] fieldWithNullAttributesAttributes = Attribute.GetCustomAttributes(fieldWithNullAttributes); + Assert.Null(fieldWithNullAttributesAttributes); + } + + FieldInfo fieldWithEmptyObjectAttributes = new CustomFieldInfo(Array.Empty()); + Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithEmptyObjectAttributes)); - FieldInfo fieldWithStringAttributes = new CustomFieldInfo(new[] { "a", "b" }); - Attribute[] fieldWithStringAttributesAttributes = Attribute.GetCustomAttributes(fieldWithStringAttributes); - Assert.NotNull(fieldWithStringAttributesAttributes); - Assert.Empty(fieldWithStringAttributesAttributes); + FieldInfo fieldWithStringAttributes = new CustomFieldInfo(new[] { "a" }); + Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithStringAttributes)); + + FieldInfo fieldWithAttributeInObjectArray = new CustomFieldInfo(new object[] { new TestAttribute(0) }); + Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithAttributeInObjectArray)); } } [AttributeUsage(AttributeTargets.All)] @@ -861,6 +897,17 @@ public class GenericAttribute : Attribute [GenericAttribute] public class HasGenericAttribute { + [GenericAttribute] + internal bool Field; + + [GenericAttribute] + public void Method([GenericAttribute] int parameter) { this.Field = true; } + + [GenericAttribute>] + public int Property { get; set; } + + [GenericAttribute] + public Action Event; } public class CustomFieldInfo : FieldInfo From a1717a9f6a81346f6ad941b844d997d43011fa9c Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Wed, 16 Feb 2022 08:31:10 -0500 Subject: [PATCH 05/12] Fixes for https://github.com/dotnet/runtime/pull/65237 Enable open generic attribute type requests for parameters, break out larger tests into separate cases. --- .../src/System/Attribute.CoreCLR.cs | 6 +- .../System.Runtime/tests/System/Attributes.cs | 124 +++++++++++++----- 2 files changed, 95 insertions(+), 35 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs index 12f0899b31e6c..c84e918d84303 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Attribute.CoreCLR.cs @@ -434,10 +434,8 @@ private static AttributeUsageAttribute InternalGetAttributeUsage(Type type) SR.Format(SR.Format_AttributeUsage, type)); } - private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount) - { - return (Attribute[])Array.CreateInstance(elementType, elementCount); - } + private static Attribute[] CreateAttributeArrayHelper(Type elementType, int elementCount) => + elementType.ContainsGenericParameters ? new Attribute[elementCount] : (Attribute[])Array.CreateInstance(elementType, elementCount); #endregion #endregion diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index e94d23aac97af..d211d07865454 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -539,64 +539,119 @@ public static void NegTest8() [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesDoesNotReturnNullForOpenGenericType() + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForAssembly() { Assembly assembly = Assembly.GetExecutingAssembly(); - Check(t => Attribute.GetCustomAttributes(assembly, t)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(assembly, t)); + } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForModule() + { Module module = typeof(HasGenericAttribute).Module; - Check(t => Attribute.GetCustomAttributes(module, t)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(module, t)); + } - Check(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute))); + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForType() + { + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute), t)); + } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForField() + { FieldInfo field = typeof(HasGenericAttribute).GetField(nameof(HasGenericAttribute.Field), BindingFlags.NonPublic | BindingFlags.Instance); - Check(t => Attribute.GetCustomAttributes(field, t)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(field, t)); + } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForConstructor() + { + ConstructorInfo method = typeof(HasGenericAttribute).GetConstructor(Type.EmptyTypes); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(method, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForMethod() + { MethodInfo method = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)); - Check(t => Attribute.GetCustomAttributes(method, t)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(method, t)); + } - ParameterInfo parameter = method.GetParameters()[0]; - Check(t => Attribute.GetCustomAttributes(parameter, t)); + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForParameter() + { + ParameterInfo parameter = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)).GetParameters()[0]; + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(parameter, t)); + } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForProperty() + { PropertyInfo property = typeof(HasGenericAttribute).GetProperty(nameof(HasGenericAttribute.Property)); - Check>(t => Attribute.GetCustomAttributes(property, t)); + GenericAttributesTestHelper>(t => Attribute.GetCustomAttributes(property, t)); + } + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForEvent() + { EventInfo @event = typeof(HasGenericAttribute).GetEvent(nameof(HasGenericAttribute.Event)); - Check(t => Attribute.GetCustomAttributes(@event, t)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(@event, t)); + } - void Check(Func getCustomAttributes) - { - Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>)); - Assert.Empty(openGenericAttributes); + private static void GenericAttributesTestHelper(Func getCustomAttributes) + { + Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>)); + Assert.Empty(openGenericAttributes); - Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute)); - Assert.Equal(1, closedGenericAttributes.Length); - Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); - } + Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute)); + Assert.Equal(1, closedGenericAttributes.Length); + Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); } [Fact] - public static void GetCustomAttributesReturnsAttributeArrayForMisbehavingCustomMemberInfo() + public static void GetCustomAttributesBehaviorMemberInfoReturnsNull() { - FieldInfo fieldWithNullAttributes = new CustomFieldInfo(null!); + FieldInfo field = new CustomFieldInfo(null!); if (PlatformDetection.IsMonoRuntime) { - Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithNullAttributes)); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); } else { - Attribute[] fieldWithNullAttributesAttributes = Attribute.GetCustomAttributes(fieldWithNullAttributes); - Assert.Null(fieldWithNullAttributesAttributes); + Attribute[] attributes = Attribute.GetCustomAttributes(field); + Assert.Null(attributes); } + } - FieldInfo fieldWithEmptyObjectAttributes = new CustomFieldInfo(Array.Empty()); - Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithEmptyObjectAttributes)); + [Fact] + public static void GetCustomAttributesBehaviorMemberInfoReturnsStringArray() + { + FieldInfo field = new CustomFieldInfo(new[] { "a" }); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); + } - FieldInfo fieldWithStringAttributes = new CustomFieldInfo(new[] { "a" }); - Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithStringAttributes)); + [Fact] + public static void GetCustomAttributesBehaviorMemberInfoReturnsAttributeInObjectArray() + { + FieldInfo field = new CustomFieldInfo(new object[] { new TestAttribute(0) }); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); + } - FieldInfo fieldWithAttributeInObjectArray = new CustomFieldInfo(new object[] { new TestAttribute(0) }); - Assert.Throws(() => Attribute.GetCustomAttributes(fieldWithAttributeInObjectArray)); + [Fact] + public static void GetCustomAttributesBehaviorMemberInfoReturnsStringAndAttributeInObjectArray() + { + FieldInfo field = new CustomFieldInfo(new object[] { "a", new TestAttribute(0) }); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); } } [AttributeUsage(AttributeTargets.All)] @@ -900,14 +955,21 @@ public class HasGenericAttribute [GenericAttribute] internal bool Field; + [GenericAttribute] + public HasGenericAttribute() { } + [GenericAttribute] - public void Method([GenericAttribute] int parameter) { this.Field = true; } + public void Method([GenericAttribute] int parameter) + { + this.Field = true; + this.Event += () => { }; + } [GenericAttribute>] public int Property { get; set; } [GenericAttribute] - public Action Event; + public event Action Event; } public class CustomFieldInfo : FieldInfo From 3631f5adcc75894602435defc1eae64d1953b41a Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 18 Feb 2022 15:42:13 -0500 Subject: [PATCH 06/12] Additional follow-up from https://github.com/dotnet/runtime/pull/65237 --- .../Reflection/RuntimeCustomAttributeData.cs | 64 +++++++++++-------- .../System/Reflection/RuntimeParameterInfo.cs | 6 +- 2 files changed, 39 insertions(+), 31 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs index c6542ffe0fa16..1e77dabfc5975 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeCustomAttributeData.cs @@ -1435,6 +1435,42 @@ internal static AttributeUsageAttribute GetAttributeUsage(RuntimeType decoratedA return attributeUsageAttribute ?? AttributeUsageAttribute.Default; } + + internal static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount) + { + bool useAttributeArray = false; + bool useObjectArray = false; + + if (caType == typeof(Attribute)) + { + useAttributeArray = true; + } + else if (caType.IsValueType) + { + useObjectArray = true; + } + else if (caType.ContainsGenericParameters) + { + if (caType.IsSubclassOf(typeof(Attribute))) + { + useAttributeArray = true; + } + else + { + useObjectArray = true; + } + } + + if (useAttributeArray) + { + return elementCount == 0 ? Array.Empty() : new Attribute[elementCount]; + } + if (useObjectArray) + { + return elementCount == 0 ? Array.Empty() : new object[elementCount]; + } + return elementCount == 0 ? caType.GetEmptyArray() : (object[])Array.CreateInstance(caType, elementCount); + } #endregion #region Private Static FCalls @@ -1472,34 +1508,6 @@ private static void GetPropertyOrFieldData( module, &pBlobStart, (byte*)blobEnd, out name, out isProperty, out type, out value); blobStart = (IntPtr)pBlobStart; } - - private static object[] CreateAttributeArrayHelper(RuntimeType caType, int elementCount) - { - if (caType.IsValueType) - { - return CreateArray(); - } - - if (caType.ContainsGenericParameters) - { - if (caType.IsSubclassOf(typeof(Attribute))) - { - return CreateArray(); - } - - return CreateArray(); - } - - // If we have 0 elements, don't allocate a new array - if (elementCount == 0) - { - return caType.GetEmptyArray(); - } - - return (object[])Array.CreateInstance(caType, elementCount); - - T[] CreateArray() => elementCount == 0 ? Array.Empty() : new T[elementCount]; - } #endregion } diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs index 33777c745dbfc..f889133b531f9 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeParameterInfo.cs @@ -509,12 +509,12 @@ public override object[] GetCustomAttributes(bool inherit) public override object[] GetCustomAttributes(Type attributeType!!, bool inherit) { - if (MdToken.IsNullToken(m_tkParamDef)) - return Array.Empty(); - if (attributeType.UnderlyingSystemType is not RuntimeType attributeRuntimeType) throw new ArgumentException(SR.Arg_MustBeType, nameof(attributeType)); + if (MdToken.IsNullToken(m_tkParamDef)) + return CustomAttribute.CreateAttributeArrayHelper(attributeRuntimeType, 0); + return CustomAttribute.GetCustomAttributes(this, attributeRuntimeType); } From e62566d2ea9ffdf2679591c9c1086a349a0f6638 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 19 Feb 2022 16:11:43 -0500 Subject: [PATCH 07/12] Further test cleanup for https://github.com/dotnet/runtime/pull/65237 --- .../tests/ParameterInfoTests.cs | 8 + .../System.Runtime/tests/System/Attributes.cs | 229 ++++++++---------- 2 files changed, 115 insertions(+), 122 deletions(-) diff --git a/src/libraries/System.Reflection/tests/ParameterInfoTests.cs b/src/libraries/System.Reflection/tests/ParameterInfoTests.cs index f6285546a8713..26d2d65867f69 100644 --- a/src/libraries/System.Reflection/tests/ParameterInfoTests.cs +++ b/src/libraries/System.Reflection/tests/ParameterInfoTests.cs @@ -245,6 +245,14 @@ public void CustomAttributesInheritanceTest(int paramIndex, bool exists, int exp Assert.Equal(expectedMyAttributeValue, exists ? myAttribute.Value : expectedMyAttributeValue); } + [Fact] + public static void GetCustomAttributesOnParameterWithNullMetadataTokenReturnsArrayOfCorrectType() + { + var parameterWithNullMetadataToken = typeof(int[]).GetProperty(nameof(Array.Length)).GetMethod.ReturnParameter; + Assert.Equal(typeof(Attribute[]), Attribute.GetCustomAttributes(parameterWithNullMetadataToken).GetType()); + Assert.Equal(typeof(MyAttribute[]), Attribute.GetCustomAttributes(parameterWithNullMetadataToken, typeof(MyAttribute)).GetType()); + } + [Fact] public void VerifyGetCustomAttributesData() { diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index d211d07865454..71eadc8234590 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -22,7 +22,7 @@ [assembly: System.Tests.GenericAttribute] [module: System.Tests.GenericAttribute] -[module:Debuggable(true,false)] +[module: Debuggable(true, false)] namespace System.Tests { public class AttributeIsDefinedTests @@ -220,6 +220,108 @@ public static void GetCustomAttributes_Interface() { Assert.True(typeof(ExampleWithAttribute).GetCustomAttributes(typeof(INameable), inherit: false)[0] is NameableAttribute); } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForAssembly() + { + Assembly assembly = Assembly.GetExecutingAssembly(); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(assembly, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForModule() + { + Module module = typeof(HasGenericAttribute).Module; + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(module, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForType() + { + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute), t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForField() + { + FieldInfo field = typeof(HasGenericAttribute).GetField(nameof(HasGenericAttribute.Field), BindingFlags.NonPublic | BindingFlags.Instance); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(field, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForConstructor() + { + ConstructorInfo method = typeof(HasGenericAttribute).GetConstructor(Type.EmptyTypes); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(method, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForMethod() + { + MethodInfo method = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(method, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForParameter() + { + ParameterInfo parameter = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)).GetParameters()[0]; + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(parameter, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForProperty() + { + PropertyInfo property = typeof(HasGenericAttribute).GetProperty(nameof(HasGenericAttribute.Property)); + GenericAttributesTestHelper>(t => Attribute.GetCustomAttributes(property, t)); + } + + [Fact] + [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] + public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForEvent() + { + EventInfo @event = typeof(HasGenericAttribute).GetEvent(nameof(HasGenericAttribute.Event)); + GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(@event, t)); + } + + private static void GenericAttributesTestHelper(Func getCustomAttributes) + { + Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>)); + Assert.Empty(openGenericAttributes); + + Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute)); + Assert.Equal(1, closedGenericAttributes.Length); + Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); + } + + [Fact] + public static void GetCustomAttributesBehaviorMemberInfoReturnsStringArray() + { + FieldInfo field = new CustomFieldInfo(new[] { "a" }); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); + } + + [Fact] + public static void GetCustomAttributesBehaviorMemberInfoReturnsAttributeInObjectArray() + { + FieldInfo field = new CustomFieldInfo(new object[] { new TestAttribute(0) }); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); + } + + [Fact] + public static void GetCustomAttributesBehaviorMemberInfoReturnsStringAndAttributeInObjectArray() + { + FieldInfo field = new CustomFieldInfo(new object[] { "a", new TestAttribute(0) }); + Assert.Throws(() => Attribute.GetCustomAttributes(field)); + } } public static class GetCustomAttribute @@ -228,7 +330,7 @@ public static class GetCustomAttribute [Fact] public static void customAttributeCount() { - List customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList(); + List customAttributes = typeof(GetCustomAttribute).Module.CustomAttributes.ToList(); // [System.Security.UnverifiableCodeAttribute()] // [TestAttributes.FooAttribute()] // [TestAttributes.ComplicatedAttribute((Int32)1, Stuff = 2)] @@ -536,123 +638,6 @@ public static void NegTest8() AssertExtensions.Throws(null, () => (ArgumentUsageAttribute)Attribute.GetCustomAttribute(paramInfos[0], attributeType, false)); } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForAssembly() - { - Assembly assembly = Assembly.GetExecutingAssembly(); - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(assembly, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForModule() - { - Module module = typeof(HasGenericAttribute).Module; - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(module, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForType() - { - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(typeof(HasGenericAttribute), t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForField() - { - FieldInfo field = typeof(HasGenericAttribute).GetField(nameof(HasGenericAttribute.Field), BindingFlags.NonPublic | BindingFlags.Instance); - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(field, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForConstructor() - { - ConstructorInfo method = typeof(HasGenericAttribute).GetConstructor(Type.EmptyTypes); - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(method, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForMethod() - { - MethodInfo method = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)); - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(method, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForParameter() - { - ParameterInfo parameter = typeof(HasGenericAttribute).GetMethod(nameof(HasGenericAttribute.Method)).GetParameters()[0]; - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(parameter, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForProperty() - { - PropertyInfo property = typeof(HasGenericAttribute).GetProperty(nameof(HasGenericAttribute.Property)); - GenericAttributesTestHelper>(t => Attribute.GetCustomAttributes(property, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForEvent() - { - EventInfo @event = typeof(HasGenericAttribute).GetEvent(nameof(HasGenericAttribute.Event)); - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(@event, t)); - } - - private static void GenericAttributesTestHelper(Func getCustomAttributes) - { - Attribute[] openGenericAttributes = getCustomAttributes(typeof(GenericAttribute<>)); - Assert.Empty(openGenericAttributes); - - Attribute[] closedGenericAttributes = getCustomAttributes(typeof(GenericAttribute)); - Assert.Equal(1, closedGenericAttributes.Length); - Assert.Equal(typeof(GenericAttribute[]), closedGenericAttributes.GetType()); - } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsNull() - { - FieldInfo field = new CustomFieldInfo(null!); - if (PlatformDetection.IsMonoRuntime) - { - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } - else - { - Attribute[] attributes = Attribute.GetCustomAttributes(field); - Assert.Null(attributes); - } - } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsStringArray() - { - FieldInfo field = new CustomFieldInfo(new[] { "a" }); - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsAttributeInObjectArray() - { - FieldInfo field = new CustomFieldInfo(new object[] { new TestAttribute(0) }); - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsStringAndAttributeInObjectArray() - { - FieldInfo field = new CustomFieldInfo(new object[] { "a", new TestAttribute(0) }); - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } } [AttributeUsage(AttributeTargets.All)] public class TestAttribute : Attribute @@ -780,7 +765,7 @@ public override object TypeId } public class BaseClass { - public virtual void TestMethod([ArgumentUsage("for test")]string[] strArray, params string[] strList) + public virtual void TestMethod([ArgumentUsage("for test")] string[] strArray, params string[] strList) { } } @@ -936,7 +921,7 @@ public interface INameable string Name { get; } } - [AttributeUsage (AttributeTargets.All, AllowMultiple = true)] + [AttributeUsage(AttributeTargets.All, AllowMultiple = true)] public class NameableAttribute : Attribute, INameable { string INameable.Name => "Nameable"; @@ -969,7 +954,7 @@ public void Method([GenericAttribute] int parameter) public int Property { get; set; } [GenericAttribute] - public event Action Event; + public event Action Event; } public class CustomFieldInfo : FieldInfo From e0aef33f482b61da01fdc5b9a1ab398014862fe7 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Thu, 24 Feb 2022 07:08:44 -0500 Subject: [PATCH 08/12] Attempt to fix Mono test crashes by moving generic attribute testing for assembly and module into CoreCLR-only tests. --- .../System.Runtime/tests/System/Attributes.cs | 21 +------------------ .../GenericAttributeMetadata.cs | 2 ++ .../GenericAttribute/GenericAttributeTests.cs | 12 ++++++++--- 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index 71eadc8234590..65db042cb59f3 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -20,8 +20,6 @@ using System.Runtime.InteropServices; using Xunit; -[assembly: System.Tests.GenericAttribute] -[module: System.Tests.GenericAttribute] [module: Debuggable(true, false)] namespace System.Tests { @@ -221,22 +219,6 @@ public static void GetCustomAttributes_Interface() Assert.True(typeof(ExampleWithAttribute).GetCustomAttributes(typeof(INameable), inherit: false)[0] is NameableAttribute); } - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForAssembly() - { - Assembly assembly = Assembly.GetExecutingAssembly(); - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(assembly, t)); - } - - [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] - public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForModule() - { - Module module = typeof(HasGenericAttribute).Module; - GenericAttributesTestHelper(t => Attribute.GetCustomAttributes(module, t)); - } - [Fact] [ActiveIssue("https://github.com/dotnet/runtime/issues/56887)", TestRuntimes.Mono)] public static void GetCustomAttributesWorksWithOpenAndClosedGenericTypesForType() @@ -335,8 +317,7 @@ public static void customAttributeCount() // [TestAttributes.FooAttribute()] // [TestAttributes.ComplicatedAttribute((Int32)1, Stuff = 2)] // [System.Diagnostics.DebuggableAttribute((Boolean)True, (Boolean)False)] - // [System.Tests.GenericAttribute] - Assert.Equal(5, customAttributes.Count); + Assert.Equal(4, customAttributes.Count); } [Fact] diff --git a/src/tests/reflection/GenericAttribute/GenericAttributeMetadata.cs b/src/tests/reflection/GenericAttribute/GenericAttributeMetadata.cs index d0ef6353f4a8f..3bea1e621945c 100644 --- a/src/tests/reflection/GenericAttribute/GenericAttributeMetadata.cs +++ b/src/tests/reflection/GenericAttribute/GenericAttributeMetadata.cs @@ -17,6 +17,8 @@ [assembly: MultiAttribute()] [assembly: MultiAttribute(true)] +[module: SingleAttribute()] + [AttributeUsage(AttributeTargets.Assembly | AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false)] public class SingleAttribute : Attribute { diff --git a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs index 912f34acd5977..cbd95c6a0e120 100644 --- a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs +++ b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs @@ -8,7 +8,6 @@ class Program { static int Main(string[] args) { -/* Re-enable once the fix to https://github.com/dotnet/msbuild/issues/6734 propagates to this repo. Assembly assembly = typeof(Class).GetTypeInfo().Assembly; Assert(CustomAttributeExtensions.GetCustomAttribute>(assembly) != null); Assert(((ICustomAttributeProvider)assembly).GetCustomAttributes(typeof(SingleAttribute), true) != null); @@ -18,8 +17,8 @@ static int Main(string[] args) Assert(((ICustomAttributeProvider)assembly).IsDefined(typeof(SingleAttribute), true)); Assert(CustomAttributeExtensions.IsDefined(assembly, typeof(SingleAttribute))); Assert(((ICustomAttributeProvider)assembly).IsDefined(typeof(SingleAttribute), true)); - -*/ + Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>), false).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>), true).GetEnumerator().MoveNext()); TypeInfo programTypeInfo = typeof(Class).GetTypeInfo(); Assert(CustomAttributeExtensions.GetCustomAttribute>(programTypeInfo) != null); @@ -156,6 +155,13 @@ static int Main(string[] args) Assert(!CustomAttributeExtensions.GetCustomAttributes(programTypeInfo, typeof(MultiAttribute<>), true).GetEnumerator().MoveNext()); Assert(!((ICustomAttributeProvider)programTypeInfo).GetCustomAttributes(typeof(MultiAttribute<>), true).GetEnumerator().MoveNext()); + Module module = programTypeInfo.Module; + AssertAny(CustomAttributeExtensions.GetCustomAttributes(module), a => a is SingleAttribute); + Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute), false).GetEnumerator().MoveNext()); + Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute), false).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>), false).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>), true).GetEnumerator().MoveNext()); + // Test coverage for CustomAttributeData api surface var a1_data = CustomAttributeData.GetCustomAttributes(programTypeInfo); AssertAny(a1_data, a => a.AttributeType == typeof(SingleAttribute)); From a3ecfe181e4e5ff372e4db36614038819620cb16 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Thu, 24 Feb 2022 19:43:20 -0500 Subject: [PATCH 09/12] Fix GenericAttributeTests.cs compilation --- .../GenericAttribute/GenericAttributeTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs index cbd95c6a0e120..9db0a764e0d21 100644 --- a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs +++ b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs @@ -17,8 +17,8 @@ static int Main(string[] args) Assert(((ICustomAttributeProvider)assembly).IsDefined(typeof(SingleAttribute), true)); Assert(CustomAttributeExtensions.IsDefined(assembly, typeof(SingleAttribute))); Assert(((ICustomAttributeProvider)assembly).IsDefined(typeof(SingleAttribute), true)); - Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>), false).GetEnumerator().MoveNext()); - Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>), true).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); TypeInfo programTypeInfo = typeof(Class).GetTypeInfo(); Assert(CustomAttributeExtensions.GetCustomAttribute>(programTypeInfo) != null); @@ -157,10 +157,10 @@ static int Main(string[] args) Module module = programTypeInfo.Module; AssertAny(CustomAttributeExtensions.GetCustomAttributes(module), a => a is SingleAttribute); - Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute), false).GetEnumerator().MoveNext()); - Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute), false).GetEnumerator().MoveNext()); - Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>), false).GetEnumerator().MoveNext()); - Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>), true).GetEnumerator().MoveNext()); + Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute)).GetEnumerator().MoveNext()); + Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute)).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); + Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); // Test coverage for CustomAttributeData api surface var a1_data = CustomAttributeData.GetCustomAttributes(programTypeInfo); From 5179891dfd54541392f4277d3faaa43ff03af816 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 25 Feb 2022 16:33:57 -0500 Subject: [PATCH 10/12] Comment out generic attribute assembly tests --- src/tests/reflection/GenericAttribute/GenericAttributeTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs index 9db0a764e0d21..01d81cc7ac1d0 100644 --- a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs +++ b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs @@ -8,6 +8,7 @@ class Program { static int Main(string[] args) { +/* Re-enable once the fix to https://github.com/dotnet/msbuild/issues/6734 propagates to this repo. Assembly assembly = typeof(Class).GetTypeInfo().Assembly; Assert(CustomAttributeExtensions.GetCustomAttribute>(assembly) != null); Assert(((ICustomAttributeProvider)assembly).GetCustomAttributes(typeof(SingleAttribute), true) != null); @@ -19,6 +20,7 @@ static int Main(string[] args) Assert(((ICustomAttributeProvider)assembly).IsDefined(typeof(SingleAttribute), true)); Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); +*/ TypeInfo programTypeInfo = typeof(Class).GetTypeInfo(); Assert(CustomAttributeExtensions.GetCustomAttribute>(programTypeInfo) != null); From 86160e1a7175efb328b9b9c874b2990880bedddf Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 26 Feb 2022 17:04:49 -0500 Subject: [PATCH 11/12] Trim more tests that fail on Mono --- .../System.Runtime/tests/System/Attributes.cs | 50 ------------------- .../GenericAttribute/GenericAttributeTests.cs | 14 +++--- 2 files changed, 7 insertions(+), 57 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System/Attributes.cs b/src/libraries/System.Runtime/tests/System/Attributes.cs index 65db042cb59f3..3dda3f56b3566 100644 --- a/src/libraries/System.Runtime/tests/System/Attributes.cs +++ b/src/libraries/System.Runtime/tests/System/Attributes.cs @@ -283,27 +283,6 @@ private static void GenericAttributesTestHelper(Func[]), closedGenericAttributes.GetType()); } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsStringArray() - { - FieldInfo field = new CustomFieldInfo(new[] { "a" }); - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsAttributeInObjectArray() - { - FieldInfo field = new CustomFieldInfo(new object[] { new TestAttribute(0) }); - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } - - [Fact] - public static void GetCustomAttributesBehaviorMemberInfoReturnsStringAndAttributeInObjectArray() - { - FieldInfo field = new CustomFieldInfo(new object[] { "a", new TestAttribute(0) }); - Assert.Throws(() => Attribute.GetCustomAttributes(field)); - } } public static class GetCustomAttribute @@ -937,33 +916,4 @@ public void Method([GenericAttribute] int parameter) [GenericAttribute] public event Action Event; } - - public class CustomFieldInfo : FieldInfo - { - private readonly object[] _customAttributes; - - public CustomFieldInfo(object[] customAttributes) - { - this._customAttributes = customAttributes; - } - - public override FieldAttributes Attributes => default; - public override RuntimeFieldHandle FieldHandle => default; - public override Type FieldType => typeof(object); - public override Type DeclaringType => null; - public override string Name => "custom"; - public override Type ReflectedType => null; - - public override object[] GetCustomAttributes(bool inherit) => this._customAttributes; - - public override object[] GetCustomAttributes(Type attributeType, bool inherit) => this._customAttributes; - - public override object GetValue(object obj) => null; - - public override bool IsDefined(Type attributeType, bool inherit) => false; - - public override void SetValue(object obj, object value, BindingFlags invokeAttr, Binder binder, System.Globalization.CultureInfo culture) - { - } - } } diff --git a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs index 01d81cc7ac1d0..b1dcd6bbbe397 100644 --- a/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs +++ b/src/tests/reflection/GenericAttribute/GenericAttributeTests.cs @@ -22,6 +22,13 @@ static int Main(string[] args) Assert(!CustomAttributeExtensions.GetCustomAttributes(assembly, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); */ + // Module module = programTypeInfo.Module; + // AssertAny(CustomAttributeExtensions.GetCustomAttributes(module), a => a is SingleAttribute); + // Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute)).GetEnumerator().MoveNext()); + // Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute)).GetEnumerator().MoveNext()); + // Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); + // Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); + TypeInfo programTypeInfo = typeof(Class).GetTypeInfo(); Assert(CustomAttributeExtensions.GetCustomAttribute>(programTypeInfo) != null); Assert(((ICustomAttributeProvider)programTypeInfo).GetCustomAttributes(typeof(SingleAttribute), true) != null); @@ -157,13 +164,6 @@ static int Main(string[] args) Assert(!CustomAttributeExtensions.GetCustomAttributes(programTypeInfo, typeof(MultiAttribute<>), true).GetEnumerator().MoveNext()); Assert(!((ICustomAttributeProvider)programTypeInfo).GetCustomAttributes(typeof(MultiAttribute<>), true).GetEnumerator().MoveNext()); - Module module = programTypeInfo.Module; - AssertAny(CustomAttributeExtensions.GetCustomAttributes(module), a => a is SingleAttribute); - Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute)).GetEnumerator().MoveNext()); - Assert(CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute)).GetEnumerator().MoveNext()); - Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); - Assert(!CustomAttributeExtensions.GetCustomAttributes(module, typeof(SingleAttribute<>)).GetEnumerator().MoveNext()); - // Test coverage for CustomAttributeData api surface var a1_data = CustomAttributeData.GetCustomAttributes(programTypeInfo); AssertAny(a1_data, a => a.AttributeType == typeof(SingleAttribute)); From 0bfa71cc6331f9fc4be45a531447d4c0ec97134a Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Tue, 1 Mar 2022 07:28:56 -0500 Subject: [PATCH 12/12] retrigger checks