From 93e941f921ffb67749ca728356c4bcdb865034d9 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Thu, 7 Nov 2024 06:40:24 +0100 Subject: [PATCH 01/20] Trim warnings progress --- .../Microsoft.TestPlatform.ObjectModel.csproj | 1 + .../Navigation/DiaSession.cs | 9 +++++++++ .../Navigation/FullSymbolReader.cs | 8 ++++++++ .../Navigation/ISymbolReader.cs | 7 +++++++ .../Navigation/PortableSymbolReader.cs | 9 +++++++++ .../TestProperty/CustomKeyValueConverter.cs | 18 ++++++++++++++++- .../CustomStringArrayConverter.cs | 20 +++++++++++++++++-- 7 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj index 9278611cbd..7913f386e5 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj +++ b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj @@ -6,6 +6,7 @@ net7.0;$(NetFrameworkMinimum);$(NetCoreAppMinimum);netstandard2.0; $(NoWarn);SYSLIB0051 + true diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs index 1130173962..ff3332f6ce 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs @@ -35,6 +35,9 @@ public class DiaSession : INavigationSession /// /// The binary path. /// +#if NET7_0_OR_GREATER + [RequiresUnreferencedCode("Uses CacheSymbols which is not trimmer friendly (specifically, the PortableSymbolReader impl)")] +#endif public DiaSession(string binaryPath) : this(binaryPath, null) { @@ -49,11 +52,17 @@ public DiaSession(string binaryPath) /// /// search path. /// +#if NET7_0_OR_GREATER + [RequiresUnreferencedCode("Uses CacheSymbols which is not trimmer friendly (specifically, the PortableSymbolReader impl)")] +#endif public DiaSession(string binaryPath, string? searchPath) : this(binaryPath, searchPath, GetSymbolReader(binaryPath)) { } +#if NET7_0_OR_GREATER + [RequiresUnreferencedCode("Uses CacheSymbols which is not trimmer friendly (specifically, the PortableSymbolReader impl)")] +#endif internal DiaSession(string binaryPath, string? searchPath, ISymbolReader symbolReader) { _symbolReader = symbolReader; diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs index 856fbd2127..d46791e1d4 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs @@ -3,6 +3,9 @@ using System; using System.Collections.Generic; +#if NET7_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif using System.Globalization; using System.IO; using System.Runtime.InteropServices; @@ -58,6 +61,11 @@ public void Dispose() /// /// search path. /// +#if NET7_0_OR_GREATER + // This is actually okay and doesn't need the attribute, but it must be added because it's there in the interface. + // And it's in the interface because PortableSymbolReader implementation of CacheSymbols needs it. + [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] +#endif public void CacheSymbols(string binaryPath, string? searchPath) { try diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs index 286a5baa3b..79cd643d88 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs @@ -2,6 +2,9 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +#if NET7_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Navigation; @@ -19,6 +22,10 @@ internal interface ISymbolReader : IDisposable /// /// search path. /// +#if NET7_0_OR_GREATER + // NOTE: Not all implementations are trimmer unfriendly, but if one implementation is, we are required to add the attribute in the interface. + [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] +#endif void CacheSymbols(string binaryPath, string? searchPath); /// diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs index 983e7fa3cd..7b8e0ac2b1 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs @@ -3,6 +3,9 @@ using System; using System.Collections.Generic; +#if NET7_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif using System.IO; using System.Reflection; using System.Reflection.PortableExecutable; @@ -33,6 +36,9 @@ internal class PortableSymbolReader : ISymbolReader /// /// The search path. /// +#if NET7_0_OR_GREATER + [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] +#endif public void CacheSymbols(string binaryPath, string? searchPath) { PopulateCacheForTypeAndMethodSymbols(binaryPath); @@ -84,6 +90,9 @@ public void Dispose() /// /// The binary path. /// +#if NET7_0_OR_GREATER + [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] +#endif private void PopulateCacheForTypeAndMethodSymbols(string binaryPath) { try diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs index 75f4742a45..f4ce197c68 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs @@ -8,16 +8,28 @@ using System.IO; using System.Linq; using System.Text; +#if NET7_0_OR_GREATER +using System.Text.Json.Serialization; +#endif +#if !NET7_0_OR_GREATER using System.Runtime.Serialization.Json; +#endif namespace Microsoft.VisualStudio.TestPlatform.ObjectModel; /// /// Converts a json representation of to an object. /// -internal class CustomKeyValueConverter : TypeConverter +internal partial class CustomKeyValueConverter : TypeConverter { +#if NET7_0_OR_GREATER + [JsonSerializable(typeof(TraitObject[]))] + private partial class TraitObjectSerializerContext : JsonSerializerContext + { + } +#else private readonly DataContractJsonSerializer _serializer = new(typeof(TraitObject[])); +#endif /// public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) @@ -44,7 +56,11 @@ public override bool CanConvertFrom(ITypeDescriptorContext? context, Type source using var stream = new MemoryStream(Encoding.Unicode.GetBytes(data)); // Converting Json data to array of KeyValuePairs with duplicate keys. +#if NET7_0_OR_GREATER + var listOfTraitObjects = System.Text.Json.JsonSerializer.Deserialize(stream, TraitObjectSerializerContext.Default.TraitObjectArray); +#else var listOfTraitObjects = _serializer.ReadObject(stream) as TraitObject[]; +#endif return listOfTraitObjects?.Select(trait => new KeyValuePair(trait.Key, trait.Value)).ToArray() ?? []; } diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs index 423d46efd4..939fabdef6 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs @@ -5,14 +5,26 @@ using System.ComponentModel; using System.Globalization; using System.IO; -using System.Runtime.Serialization.Json; using System.Text; +#if NET7_0_OR_GREATER +using System.Text.Json.Serialization; +#endif +#if !NET7_0_OR_GREATER +using System.Runtime.Serialization.Json; +#endif namespace Microsoft.VisualStudio.TestPlatform.ObjectModel; -internal class CustomStringArrayConverter : TypeConverter +internal partial class CustomStringArrayConverter : TypeConverter { +#if NET7_0_OR_GREATER + [JsonSerializable(typeof(string[]))] + private partial class StringArraySerializerContext : JsonSerializerContext + { + } +#else private readonly DataContractJsonSerializer _serializer = new(typeof(string[])); +#endif /// public override bool CanConvertFrom(ITypeDescriptorContext? context, Type sourceType) @@ -36,7 +48,11 @@ public override bool CanConvertFrom(ITypeDescriptorContext? context, Type source if (value is string data) { using var stream = new MemoryStream(Encoding.Unicode.GetBytes(data)); +#if NET7_0_OR_GREATER + var strings = System.Text.Json.JsonSerializer.Deserialize(stream, StringArraySerializerContext.Default.StringArray); +#else var strings = _serializer.ReadObject(stream) as string[]; +#endif return strings; } From 992d873f310e5a470110bf2e45c0a8be644eb424 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Thu, 7 Nov 2024 07:07:28 +0100 Subject: [PATCH 02/20] Progress with TestProperty --- .../TestProperty/TestProperty.cs | 68 +++++++++++++++++-- 1 file changed, 62 insertions(+), 6 deletions(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs index 911901b439..3d51673e90 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs @@ -21,6 +21,9 @@ public class TestProperty : IEquatable private static bool DisableFastJson { get; set; } = FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_FASTER_JSON_SERIALIZATION); +#if NET7_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif private Type _valueType; //public static Stopwatch @@ -35,7 +38,18 @@ private TestProperty() // Default constructor for Serialization. } - private TestProperty(string id, string label, string category, string description, Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes) +#if NET7_0_OR_GREATER + [UnconditionalSuppressMessage( + "Trimming", + "IL2072:Target parameter argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.", + Justification = "Only problematic part is ValueType = valueType.FullName, but as we annotated valueType parameter, it should be good.")] +#endif + private TestProperty(string id, string label, string category, string description, +#if NET7_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + Type valueType, + ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes) { ValidateArg.NotNullOrEmpty(id, nameof(id)); ValidateArg.NotNull(label, nameof(label)); @@ -122,6 +136,9 @@ private TestProperty(string id, string label, string category, string descriptio /// Gets or sets a string representation of the type for value. /// [DataMember] +#if NET7_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif public string ValueType { get; set; } #region IEquatable @@ -157,6 +174,9 @@ public override string ToString() /// /// Only works for the valueType that is in the currently executing assembly or in Mscorlib.dll. The default valueType is of string valueType. /// The valueType of the test property +#if NET7_0_OR_GREATER + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif public Type GetValueType() { _valueType ??= GetType(ValueType); @@ -164,7 +184,15 @@ public Type GetValueType() return _valueType; } - private Type GetType(string typeName) +#if NET7_0_OR_GREATER + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + private Type GetType( +#if NET7_0_OR_GREATER + // TODO: Confirm if this is the right thing to do to avoid Type.GetType warning + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + string typeName) { ValidateArg.NotNull(typeName, nameof(typeName)); @@ -190,7 +218,7 @@ private Type GetType(string typeName) } // Try 2.0 version as discovery returns version of 4.0 for all cases - type ??= Type.GetType(typeName.Replace("Version=4.0.0.0", "Version=2.0.0.0")); + type ??= GetTypeByReplacingVersion(typeName); // For UAP the type namespace for System.Uri,System.TimeSpan and System.DateTimeOffset differs from the desktop version. if (type == null && typeName.StartsWith("System.Uri")) @@ -252,6 +280,19 @@ private Type GetType(string typeName) return type; } +#if NET7_0_OR_GREATER + [UnconditionalSuppressMessage( + "Trimming", + "IL2057:Unrecognized value passed to the parameter of method. It's not possible to guarantee the availability of the target type.", + Justification = "Only part incompatible with trimming is the Type.GetType call that does version replacement. It probably wouldn't cause trouble and is safe to suppress")] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + private static Type? GetTypeByReplacingVersion(string typeName) + { + // This line is extracted into a separate method to ensure the UnconditionalSuppressMessage applies to it exactly (i.e, avoid unintentionally hiding more warnings) + return Type.GetType(typeName.Replace("Version=4.0.0.0", "Version=2.0.0.0")); + } + private static readonly Dictionary>> Properties = new(); #if FullCLR @@ -284,7 +325,12 @@ public static void ClearRegisteredProperties() return result; } - public static TestProperty Register(string id, string label, Type valueType, Type owner) + public static TestProperty Register(string id, string label, +#if NET7_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + Type valueType, + Type owner) { ValidateArg.NotNullOrEmpty(id, nameof(id)); ValidateArg.NotNull(label, nameof(label)); @@ -294,7 +340,12 @@ public static TestProperty Register(string id, string label, Type valueType, Typ return Register(id, label, string.Empty, string.Empty, valueType, null, TestPropertyAttributes.None, owner); } - public static TestProperty Register(string id, string label, Type valueType, TestPropertyAttributes attributes, Type owner) + public static TestProperty Register(string id, string label, +#if NET7_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + Type valueType, + TestPropertyAttributes attributes, Type owner) { ValidateArg.NotNullOrEmpty(id, nameof(id)); ValidateArg.NotNull(label, nameof(label)); @@ -304,7 +355,12 @@ public static TestProperty Register(string id, string label, Type valueType, Tes return Register(id, label, string.Empty, string.Empty, valueType, null, attributes, owner); } - public static TestProperty Register(string id, string label, string category, string description, Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes, Type owner) + public static TestProperty Register(string id, string label, string category, string description, +#if NET7_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + Type valueType, + ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes, Type owner) { ValidateArg.NotNullOrEmpty(id, nameof(id)); ValidateArg.NotNull(label, nameof(label)); From b11ffe7349988550d338841263536db8c802f8c5 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Thu, 7 Nov 2024 07:38:09 +0100 Subject: [PATCH 03/20] Suppress trim warning for AssemblyHelper.GetCustomAttributes --- .../Utilities/AssemblyHelper.cs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs b/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs index d704625aed..752ff09af1 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs @@ -3,6 +3,9 @@ using System; using System.Collections.Generic; +#if NET7_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif #if NETFRAMEWORK using System.IO; #endif @@ -322,6 +325,12 @@ internal static void SetNETFrameworkCompatiblityMode(AppDomainSetup setup, IRunC } #endif +#if NET7_0_OR_GREATER + [UnconditionalSuppressMessage( + "Trimming", + "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", + Justification = "We don't care if the given attribute type is removed. If it's removed, then it's unused and we are going to return empty either way.")] +#endif public static IEnumerable GetCustomAttributes(this Assembly assembly, string fullyQualifiedName) { ValidateArg.NotNull(assembly, nameof(assembly)); From ce092a42d845569e164afe5025610e7a21c50239 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Thu, 7 Nov 2024 17:36:33 +0100 Subject: [PATCH 04/20] Work on TestObject trimmability --- .../TestObject.cs | 200 ++++++++++++++++++ 1 file changed, 200 insertions(+) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs index 634fe33e5f..b6bd938b1e 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs @@ -23,6 +23,46 @@ public abstract class TestObject private static readonly CustomKeyValueConverter KeyValueConverter = new(); private static readonly CustomStringArrayConverter StringArrayConverter = new(); + // https://github.com/dotnet/runtime/blob/36bcc2c96045c6793dcfe3151c51a0597537917a/src/libraries/System.ComponentModel.TypeConverter/src/System/ComponentModel/ReflectTypeDescriptionProvider.cs#L154-L180 + // TODO: Validate if some types are not needed and remove them. + private static readonly BooleanConverter BooleanConverter = new(); + private static readonly ByteConverter ByteConverter = new(); + private static readonly SByteConverter SByteConverter = new(); + private static readonly CharConverter CharConverter = new(); + private static readonly DoubleConverter DoubleConverter = new(); + private static readonly StringConverter StringConverter = new(); + private static readonly Int32Converter IntConverter = new(); +#if NET7_0_OR_GREATER + private static readonly Int128Converter Int128Converter = new(); +#endif + private static readonly Int16Converter Int16Converter = new(); + private static readonly Int64Converter Int64Converter = new(); + private static readonly SingleConverter SingleConverter = new(); +#if NET7_0_OR_GREATER + private static readonly HalfConverter HalfConverter = new(); + private static readonly UInt128Converter UInt128Converter = new(); +#endif + private static readonly UInt16Converter UInt16Converter = new(); + private static readonly UInt32Converter UInt32Converter = new(); + private static readonly UInt64Converter UInt64Converter = new(); + private static readonly TypeConverter TypeConverter = new(); + private static readonly CultureInfoConverter CultureInfoConverter = new(); +#if NET7_0_OR_GREATER + private static readonly DateOnlyConverter DateOnlyConverter = new(); +#endif + private static readonly DateTimeConverter DateTimeConverter = new(); + private static readonly DateTimeOffsetConverter DateTimeOffsetConverter = new(); + private static readonly DecimalConverter DecimalConverter = new(); +#if NET7_0_OR_GREATER + private static readonly TimeOnlyConverter TimeOnlyConverter = new(); +#endif + private static readonly TimeSpanConverter TimeSpanConverter = new(); + private static readonly GuidConverter GuidConverter = new(); + private static readonly UriTypeConverter UriTypeConverter = new(); +#if NET7_0_OR_GREATER + private static readonly VersionConverter VersionConverter = new(); +#endif + /// /// The store for all the properties registered. /// @@ -239,6 +279,12 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? /// Convert passed in value from TestProperty's specified value type. /// /// Converted object +#if NET7_0_OR_GREATER + [UnconditionalSuppressMessage( + "Trimming", + "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", + Justification = "Awaiting more evidence that we need to take action. If this caused issues, we should still be able to special case some specific types instead of relying on TypeDescriptor")] +#endif private static object? ConvertPropertyFrom(TestProperty property, CultureInfo culture, object? value) { ValidateArg.NotNull(property, nameof(property)); @@ -265,6 +311,157 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? return StringArrayConverter.ConvertFrom(null, culture, (string?)value); } + // These are already handled by TypeDescriptor.GetConverter, but it's not trimmer safe and + // we want to make sure they are guaranteed to work. + if (valueType == typeof(bool)) + { + return BooleanConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(byte)) + { + return ByteConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(sbyte)) + { + return SByteConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(char)) + { + return CharConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(double)) + { + return DoubleConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(string)) + { + return StringConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(int)) + { + return IntConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(Int128)) + { + return Int128Converter.ConvertFrom(null, culture, value ?? string.Empty); + } +#endif + + if (valueType == typeof(short)) + { + return Int16Converter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(long)) + { + return Int64Converter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(float)) + { + return SingleConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(Half)) + { + return HalfConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(UInt128)) + { + return UInt128Converter.ConvertFrom(null, culture, value ?? string.Empty); + } +#endif + + if (valueType == typeof(ushort)) + { + return UInt16Converter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(uint)) + { + return UInt32Converter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(ulong)) + { + return UInt64Converter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(Type)) + { + return TypeConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(CultureInfo)) + { + return CultureInfoConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(DateOnly)) + { + return DateOnlyConverter.ConvertFrom(null, culture, value ?? string.Empty); + } +#endif + + if (valueType == typeof(DateTime)) + { + return DateTimeConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(DateTimeOffset)) + { + return DateTimeOffsetConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(decimal)) + { + return DecimalConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(TimeOnly)) + { + return TimeOnlyConverter.ConvertFrom(null, culture, value ?? string.Empty); + } +#endif + + if (valueType == typeof(TimeSpan)) + { + return TimeSpanConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(Guid)) + { + return GuidConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + + if (valueType == typeof(Uri)) + { + return UriTypeConverter.ConvertFrom(null, culture, value ?? string.Empty); + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(Version)) + { + return VersionConverter.ConvertFrom(null, culture, value ?? string.Empty); + } +#endif + + // TODO: Consider detecting that we are in source gen mode (or in NativeAOT mode) and avoid calling TypeDescriptor altogether? + // Each of the approaches has pros and cons. + // Ignoring this trimmer unfriendly code when in NativeAOT will help catch issues earlier, and have more deterministic behavior. + // Keeping this trimmer unfriendly code even in NativeAOT will allow us to still have the possibility to work in case we fall in this path. TPDebug.Assert(valueType is not null, "valueType is null"); TypeConverter converter = TypeDescriptor.GetConverter(valueType); if (converter == null) @@ -292,6 +489,9 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? /// /// Converted object [return: NotNullIfNotNull("value")] +#if NET7_0_OR_GREATER + [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "")] +#endif private static T? ConvertPropertyTo(TestProperty property, CultureInfo culture, object? value) { ValidateArg.NotNull(property, nameof(property)); From 60eca319e26171053939eefff3d37292dd448ebe Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Fri, 8 Nov 2024 06:13:07 +0100 Subject: [PATCH 05/20] Polly fills --- shared/DynamicallyAccessedMemberTypes.cs | 102 ++++++++++++++++++ shared/DynamicallyAccessedMembersAttribute.cs | 58 ++++++++++ shared/RequiresUnreferencedCodeAttribute.cs | 47 ++++++++ .../UnconditionalSuppressMessageAttribute.cs | 92 ++++++++++++++++ ...icrosoft.TestPlatform.CoreUtilities.csproj | 4 + .../Navigation/DiaSession.cs | 6 -- .../Navigation/FullSymbolReader.cs | 4 - .../Navigation/ISymbolReader.cs | 4 - .../Navigation/PortableSymbolReader.cs | 6 -- .../TestObject.cs | 4 - .../TestProperty/CustomKeyValueConverter.cs | 2 +- .../CustomStringArrayConverter.cs | 2 +- .../TestProperty/TestProperty.cs | 24 +---- .../Utilities/AssemblyHelper.cs | 4 - 14 files changed, 306 insertions(+), 53 deletions(-) create mode 100644 shared/DynamicallyAccessedMemberTypes.cs create mode 100644 shared/DynamicallyAccessedMembersAttribute.cs create mode 100644 shared/RequiresUnreferencedCodeAttribute.cs create mode 100644 shared/UnconditionalSuppressMessageAttribute.cs diff --git a/shared/DynamicallyAccessedMemberTypes.cs b/shared/DynamicallyAccessedMemberTypes.cs new file mode 100644 index 0000000000..c62a9ce9cd --- /dev/null +++ b/shared/DynamicallyAccessedMemberTypes.cs @@ -0,0 +1,102 @@ +#if !NET5_0_OR_GREATER +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + /// + /// Specifies the types of members that are dynamically accessed. + /// + /// This enumeration has a attribute that allows a + /// bitwise combination of its member values. + /// + [Flags] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + enum DynamicallyAccessedMemberTypes + { + /// + /// Specifies no members. + /// + None = 0, + + /// + /// Specifies the default, parameterless public constructor. + /// + PublicParameterlessConstructor = 0x0001, + + /// + /// Specifies all public constructors. + /// + PublicConstructors = 0x0002 | PublicParameterlessConstructor, + + /// + /// Specifies all non-public constructors. + /// + NonPublicConstructors = 0x0004, + + /// + /// Specifies all public methods. + /// + PublicMethods = 0x0008, + + /// + /// Specifies all non-public methods. + /// + NonPublicMethods = 0x0010, + + /// + /// Specifies all public fields. + /// + PublicFields = 0x0020, + + /// + /// Specifies all non-public fields. + /// + NonPublicFields = 0x0040, + + /// + /// Specifies all public nested types. + /// + PublicNestedTypes = 0x0080, + + /// + /// Specifies all non-public nested types. + /// + NonPublicNestedTypes = 0x0100, + + /// + /// Specifies all public properties. + /// + PublicProperties = 0x0200, + + /// + /// Specifies all non-public properties. + /// + NonPublicProperties = 0x0400, + + /// + /// Specifies all public events. + /// + PublicEvents = 0x0800, + + /// + /// Specifies all non-public events. + /// + NonPublicEvents = 0x1000, + + /// + /// Specifies all interfaces implemented by the type. + /// + Interfaces = 0x2000, + + /// + /// Specifies all members. + /// + All = ~None + } +} +#endif diff --git a/shared/DynamicallyAccessedMembersAttribute.cs b/shared/DynamicallyAccessedMembersAttribute.cs new file mode 100644 index 0000000000..02190c2868 --- /dev/null +++ b/shared/DynamicallyAccessedMembersAttribute.cs @@ -0,0 +1,58 @@ +#if !NET5_0_OR_GREATER +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + /// + /// Indicates that certain members on a specified are accessed dynamically, + /// for example through . + /// + /// + /// This allows tools to understand which members are being accessed during the execution + /// of a program. + /// + /// This attribute is valid on members whose type is or . + /// + /// When this attribute is applied to a location of type , the assumption is + /// that the string represents a fully qualified type name. + /// + /// When this attribute is applied to a class, interface, or struct, the members specified + /// can be accessed dynamically on instances returned from calling + /// on instances of that class, interface, or struct. + /// + /// If the attribute is applied to a method it's treated as a special case and it implies + /// the attribute should be applied to the "this" parameter of the method. As such the attribute + /// should only be used on instance methods of types assignable to System.Type (or string, but no methods + /// will use it there). + /// + [AttributeUsage( + AttributeTargets.Field | AttributeTargets.ReturnValue | AttributeTargets.GenericParameter | + AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.Method | + AttributeTargets.Class | AttributeTargets.Interface | AttributeTargets.Struct, + Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class DynamicallyAccessedMembersAttribute : Attribute + { + /// + /// Initializes a new instance of the class + /// with the specified member types. + /// + /// The types of members dynamically accessed. + public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes memberTypes) + { + MemberTypes = memberTypes; + } + + /// + /// Gets the which specifies the type + /// of members dynamically accessed. + /// + public DynamicallyAccessedMemberTypes MemberTypes { get; } + } +} +#endif \ No newline at end of file diff --git a/shared/RequiresUnreferencedCodeAttribute.cs b/shared/RequiresUnreferencedCodeAttribute.cs new file mode 100644 index 0000000000..df417571e3 --- /dev/null +++ b/shared/RequiresUnreferencedCodeAttribute.cs @@ -0,0 +1,47 @@ +#if !NET5_0_OR_GREATER +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + /// + /// Indicates that the specified method requires dynamic access to code that is not referenced + /// statically, for example through . + /// + /// + /// This allows tools to understand which methods are unsafe to call when removing unreferenced + /// code from an application. + /// + [AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class RequiresUnreferencedCodeAttribute : Attribute + { + /// + /// Initializes a new instance of the class + /// with the specified message. + /// + /// + /// A message that contains information about the usage of unreferenced code. + /// + public RequiresUnreferencedCodeAttribute(string message) + { + Message = message; + } + + /// + /// Gets a message that contains information about the usage of unreferenced code. + /// + public string Message { get; } + + /// + /// Gets or sets an optional URL that contains more information about the method, + /// why it requires unreferenced code, and what options a consumer has to deal with it. + /// + public string? Url { get; set; } + } +} +#endif diff --git a/shared/UnconditionalSuppressMessageAttribute.cs b/shared/UnconditionalSuppressMessageAttribute.cs new file mode 100644 index 0000000000..c67160df5f --- /dev/null +++ b/shared/UnconditionalSuppressMessageAttribute.cs @@ -0,0 +1,92 @@ +#if !NET5_0_OR_GREATER +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace System.Diagnostics.CodeAnalysis +{ + /// + /// Suppresses reporting of a specific rule violation, allowing multiple suppressions on a + /// single code artifact. + /// + /// + /// is different than + /// in that it doesn't have a + /// . So it is always preserved in the compiled assembly. + /// + [AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = true)] +#if SYSTEM_PRIVATE_CORELIB + public +#else + internal +#endif + sealed class UnconditionalSuppressMessageAttribute : Attribute + { + /// + /// Initializes a new instance of the + /// class, specifying the category of the tool and the identifier for an analysis rule. + /// + /// The category for the attribute. + /// The identifier of the analysis rule the attribute applies to. + public UnconditionalSuppressMessageAttribute(string category, string checkId) + { + Category = category; + CheckId = checkId; + } + + /// + /// Gets the category identifying the classification of the attribute. + /// + /// + /// The property describes the tool or tool analysis category + /// for which a message suppression attribute applies. + /// + public string Category { get; } + + /// + /// Gets the identifier of the analysis tool rule to be suppressed. + /// + /// + /// Concatenated together, the and + /// properties form a unique check identifier. + /// + public string CheckId { get; } + + /// + /// Gets or sets the scope of the code that is relevant for the attribute. + /// + /// + /// The Scope property is an optional argument that specifies the metadata scope for which + /// the attribute is relevant. + /// + public string? Scope { get; set; } + + /// + /// Gets or sets a fully qualified path that represents the target of the attribute. + /// + /// + /// The property is an optional argument identifying the analysis target + /// of the attribute. An example value is "System.IO.Stream.ctor():System.Void". + /// Because it is fully qualified, it can be long, particularly for targets such as parameters. + /// The analysis tool user interface should be capable of automatically formatting the parameter. + /// + public string? Target { get; set; } + + /// + /// Gets or sets an optional argument expanding on exclusion criteria. + /// + /// + /// The property is an optional argument that specifies additional + /// exclusion where the literal metadata target is not sufficiently precise. For example, + /// the cannot be applied within a method, + /// and it may be desirable to suppress a violation against a statement in the method that will + /// give a rule violation, but not against all statements in the method. + /// + public string? MessageId { get; set; } + + /// + /// Gets or sets the justification for suppressing the code analysis message. + /// + public string? Justification { get; set; } + } +} +#endif \ No newline at end of file diff --git a/src/Microsoft.TestPlatform.CoreUtilities/Microsoft.TestPlatform.CoreUtilities.csproj b/src/Microsoft.TestPlatform.CoreUtilities/Microsoft.TestPlatform.CoreUtilities.csproj index ea9329bd81..99721f319a 100644 --- a/src/Microsoft.TestPlatform.CoreUtilities/Microsoft.TestPlatform.CoreUtilities.csproj +++ b/src/Microsoft.TestPlatform.CoreUtilities/Microsoft.TestPlatform.CoreUtilities.csproj @@ -62,7 +62,11 @@ + + + + diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs index ff3332f6ce..73f591e97d 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/DiaSession.cs @@ -35,9 +35,7 @@ public class DiaSession : INavigationSession /// /// The binary path. /// -#if NET7_0_OR_GREATER [RequiresUnreferencedCode("Uses CacheSymbols which is not trimmer friendly (specifically, the PortableSymbolReader impl)")] -#endif public DiaSession(string binaryPath) : this(binaryPath, null) { @@ -52,17 +50,13 @@ public DiaSession(string binaryPath) /// /// search path. /// -#if NET7_0_OR_GREATER [RequiresUnreferencedCode("Uses CacheSymbols which is not trimmer friendly (specifically, the PortableSymbolReader impl)")] -#endif public DiaSession(string binaryPath, string? searchPath) : this(binaryPath, searchPath, GetSymbolReader(binaryPath)) { } -#if NET7_0_OR_GREATER [RequiresUnreferencedCode("Uses CacheSymbols which is not trimmer friendly (specifically, the PortableSymbolReader impl)")] -#endif internal DiaSession(string binaryPath, string? searchPath, ISymbolReader symbolReader) { _symbolReader = symbolReader; diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs index d46791e1d4..a3e61cb61a 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/FullSymbolReader.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -#if NET7_0_OR_GREATER using System.Diagnostics.CodeAnalysis; -#endif using System.Globalization; using System.IO; using System.Runtime.InteropServices; @@ -61,11 +59,9 @@ public void Dispose() /// /// search path. /// -#if NET7_0_OR_GREATER // This is actually okay and doesn't need the attribute, but it must be added because it's there in the interface. // And it's in the interface because PortableSymbolReader implementation of CacheSymbols needs it. [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] -#endif public void CacheSymbols(string binaryPath, string? searchPath) { try diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs index 79cd643d88..7a3697821c 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/ISymbolReader.cs @@ -2,9 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; -#if NET7_0_OR_GREATER using System.Diagnostics.CodeAnalysis; -#endif namespace Microsoft.VisualStudio.TestPlatform.ObjectModel.Navigation; @@ -22,10 +20,8 @@ internal interface ISymbolReader : IDisposable /// /// search path. /// -#if NET7_0_OR_GREATER // NOTE: Not all implementations are trimmer unfriendly, but if one implementation is, we are required to add the attribute in the interface. [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] -#endif void CacheSymbols(string binaryPath, string? searchPath); /// diff --git a/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs b/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs index 7b8e0ac2b1..fc5007bc3c 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Navigation/PortableSymbolReader.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -#if NET7_0_OR_GREATER using System.Diagnostics.CodeAnalysis; -#endif using System.IO; using System.Reflection; using System.Reflection.PortableExecutable; @@ -36,9 +34,7 @@ internal class PortableSymbolReader : ISymbolReader /// /// The search path. /// -#if NET7_0_OR_GREATER [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] -#endif public void CacheSymbols(string binaryPath, string? searchPath) { PopulateCacheForTypeAndMethodSymbols(binaryPath); @@ -90,9 +86,7 @@ public void Dispose() /// /// The binary path. /// -#if NET7_0_OR_GREATER [RequiresUnreferencedCode("Uses Assembly.Load which is not trimmer friendly")] -#endif private void PopulateCacheForTypeAndMethodSymbols(string binaryPath) { try diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs index b6bd938b1e..779ed299a8 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs @@ -279,12 +279,10 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? /// Convert passed in value from TestProperty's specified value type. /// /// Converted object -#if NET7_0_OR_GREATER [UnconditionalSuppressMessage( "Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "Awaiting more evidence that we need to take action. If this caused issues, we should still be able to special case some specific types instead of relying on TypeDescriptor")] -#endif private static object? ConvertPropertyFrom(TestProperty property, CultureInfo culture, object? value) { ValidateArg.NotNull(property, nameof(property)); @@ -489,9 +487,7 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? /// /// Converted object [return: NotNullIfNotNull("value")] -#if NET7_0_OR_GREATER [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "")] -#endif private static T? ConvertPropertyTo(TestProperty property, CultureInfo culture, object? value) { ValidateArg.NotNull(property, nameof(property)); diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs index f4ce197c68..5623915ef8 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomKeyValueConverter.cs @@ -22,7 +22,7 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel; /// internal partial class CustomKeyValueConverter : TypeConverter { -#if NET7_0_OR_GREATER +#if NET7_0_OR_GREATER // TODO: We actually don't yet produce net7.0 in the NuGet package. [JsonSerializable(typeof(TraitObject[]))] private partial class TraitObjectSerializerContext : JsonSerializerContext { diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs index 939fabdef6..06f15676d4 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/CustomStringArrayConverter.cs @@ -17,7 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.ObjectModel; internal partial class CustomStringArrayConverter : TypeConverter { -#if NET7_0_OR_GREATER +#if NET7_0_OR_GREATER // TODO: We actually don't yet produce net7.0 in the NuGet package. [JsonSerializable(typeof(string[]))] private partial class StringArraySerializerContext : JsonSerializerContext { diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs index 3d51673e90..8c0b4d00df 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs @@ -21,9 +21,7 @@ public class TestProperty : IEquatable private static bool DisableFastJson { get; set; } = FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_FASTER_JSON_SERIALIZATION); -#if NET7_0_OR_GREATER [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif private Type _valueType; //public static Stopwatch @@ -38,16 +36,12 @@ private TestProperty() // Default constructor for Serialization. } -#if NET7_0_OR_GREATER [UnconditionalSuppressMessage( "Trimming", "IL2072:Target parameter argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.", Justification = "Only problematic part is ValueType = valueType.FullName, but as we annotated valueType parameter, it should be good.")] -#endif private TestProperty(string id, string label, string category, string description, -#if NET7_0_OR_GREATER - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes) { @@ -136,9 +130,7 @@ private TestProperty(string id, string label, string category, string descriptio /// Gets or sets a string representation of the type for value. /// [DataMember] -#if NET7_0_OR_GREATER [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif public string ValueType { get; set; } #region IEquatable @@ -174,9 +166,7 @@ public override string ToString() /// /// Only works for the valueType that is in the currently executing assembly or in Mscorlib.dll. The default valueType is of string valueType. /// The valueType of the test property -#if NET7_0_OR_GREATER [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif public Type GetValueType() { _valueType ??= GetType(ValueType); @@ -184,14 +174,10 @@ public Type GetValueType() return _valueType; } -#if NET7_0_OR_GREATER [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif private Type GetType( -#if NET7_0_OR_GREATER // TODO: Confirm if this is the right thing to do to avoid Type.GetType warning [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif string typeName) { ValidateArg.NotNull(typeName, nameof(typeName)); @@ -280,13 +266,11 @@ private Type GetType( return type; } -#if NET7_0_OR_GREATER [UnconditionalSuppressMessage( "Trimming", "IL2057:Unrecognized value passed to the parameter of method. It's not possible to guarantee the availability of the target type.", Justification = "Only part incompatible with trimming is the Type.GetType call that does version replacement. It probably wouldn't cause trouble and is safe to suppress")] [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif private static Type? GetTypeByReplacingVersion(string typeName) { // This line is extracted into a separate method to ensure the UnconditionalSuppressMessage applies to it exactly (i.e, avoid unintentionally hiding more warnings) @@ -326,9 +310,7 @@ public static void ClearRegisteredProperties() } public static TestProperty Register(string id, string label, -#if NET7_0_OR_GREATER [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif Type valueType, Type owner) { @@ -341,9 +323,7 @@ public static TestProperty Register(string id, string label, } public static TestProperty Register(string id, string label, -#if NET7_0_OR_GREATER [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif Type valueType, TestPropertyAttributes attributes, Type owner) { @@ -356,9 +336,7 @@ public static TestProperty Register(string id, string label, } public static TestProperty Register(string id, string label, string category, string description, -#if NET7_0_OR_GREATER [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] -#endif Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes, Type owner) { diff --git a/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs b/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs index 752ff09af1..9fececf355 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/Utilities/AssemblyHelper.cs @@ -3,9 +3,7 @@ using System; using System.Collections.Generic; -#if NET7_0_OR_GREATER using System.Diagnostics.CodeAnalysis; -#endif #if NETFRAMEWORK using System.IO; #endif @@ -325,12 +323,10 @@ internal static void SetNETFrameworkCompatiblityMode(AppDomainSetup setup, IRunC } #endif -#if NET7_0_OR_GREATER [UnconditionalSuppressMessage( "Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "We don't care if the given attribute type is removed. If it's removed, then it's unused and we are going to return empty either way.")] -#endif public static IEnumerable GetCustomAttributes(this Assembly assembly, string fullyQualifiedName) { ValidateArg.NotNull(assembly, nameof(assembly)); From 05d5146c5fbff75d814122f325045aadb207958b Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Fri, 8 Nov 2024 06:16:39 +0100 Subject: [PATCH 06/20] File header comment --- shared/DynamicallyAccessedMemberTypes.cs | 3 +++ shared/DynamicallyAccessedMembersAttribute.cs | 7 +++++-- shared/RequiresUnreferencedCodeAttribute.cs | 5 ++++- shared/UnconditionalSuppressMessageAttribute.cs | 5 ++++- 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/shared/DynamicallyAccessedMemberTypes.cs b/shared/DynamicallyAccessedMemberTypes.cs index c62a9ce9cd..3976c8aa98 100644 --- a/shared/DynamicallyAccessedMemberTypes.cs +++ b/shared/DynamicallyAccessedMemberTypes.cs @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + #if !NET5_0_OR_GREATER // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. diff --git a/shared/DynamicallyAccessedMembersAttribute.cs b/shared/DynamicallyAccessedMembersAttribute.cs index 02190c2868..9f1c114186 100644 --- a/shared/DynamicallyAccessedMembersAttribute.cs +++ b/shared/DynamicallyAccessedMembersAttribute.cs @@ -1,4 +1,7 @@ -#if !NET5_0_OR_GREATER +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if !NET5_0_OR_GREATER // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. @@ -55,4 +58,4 @@ public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes member public DynamicallyAccessedMemberTypes MemberTypes { get; } } } -#endif \ No newline at end of file +#endif diff --git a/shared/RequiresUnreferencedCodeAttribute.cs b/shared/RequiresUnreferencedCodeAttribute.cs index df417571e3..493d6dbf84 100644 --- a/shared/RequiresUnreferencedCodeAttribute.cs +++ b/shared/RequiresUnreferencedCodeAttribute.cs @@ -1,4 +1,7 @@ -#if !NET5_0_OR_GREATER +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if !NET5_0_OR_GREATER // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. diff --git a/shared/UnconditionalSuppressMessageAttribute.cs b/shared/UnconditionalSuppressMessageAttribute.cs index c67160df5f..81e9e65501 100644 --- a/shared/UnconditionalSuppressMessageAttribute.cs +++ b/shared/UnconditionalSuppressMessageAttribute.cs @@ -1,4 +1,7 @@ -#if !NET5_0_OR_GREATER +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +#if !NET5_0_OR_GREATER // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. From 0f05beaea721de0cb4d131fb7362d8c73d1396a2 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Fri, 8 Nov 2024 06:51:33 +0100 Subject: [PATCH 07/20] Satisfy file-scoped namespaces analyzer --- shared/DynamicallyAccessedMemberTypes.cs | 185 +++++++++--------- shared/DynamicallyAccessedMembersAttribute.cs | 93 +++++---- shared/RequiresUnreferencedCodeAttribute.cs | 69 ++++--- .../UnconditionalSuppressMessageAttribute.cs | 153 +++++++-------- 4 files changed, 248 insertions(+), 252 deletions(-) diff --git a/shared/DynamicallyAccessedMemberTypes.cs b/shared/DynamicallyAccessedMemberTypes.cs index 3976c8aa98..6231ac35c9 100644 --- a/shared/DynamicallyAccessedMemberTypes.cs +++ b/shared/DynamicallyAccessedMemberTypes.cs @@ -5,101 +5,100 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -namespace System.Diagnostics.CodeAnalysis -{ - /// - /// Specifies the types of members that are dynamically accessed. - /// - /// This enumeration has a attribute that allows a - /// bitwise combination of its member values. - /// - [Flags] +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Specifies the types of members that are dynamically accessed. +/// +/// This enumeration has a attribute that allows a +/// bitwise combination of its member values. +/// +[Flags] #if SYSTEM_PRIVATE_CORELIB - public +public #else - internal +internal #endif - enum DynamicallyAccessedMemberTypes - { - /// - /// Specifies no members. - /// - None = 0, - - /// - /// Specifies the default, parameterless public constructor. - /// - PublicParameterlessConstructor = 0x0001, - - /// - /// Specifies all public constructors. - /// - PublicConstructors = 0x0002 | PublicParameterlessConstructor, - - /// - /// Specifies all non-public constructors. - /// - NonPublicConstructors = 0x0004, - - /// - /// Specifies all public methods. - /// - PublicMethods = 0x0008, - - /// - /// Specifies all non-public methods. - /// - NonPublicMethods = 0x0010, - - /// - /// Specifies all public fields. - /// - PublicFields = 0x0020, - - /// - /// Specifies all non-public fields. - /// - NonPublicFields = 0x0040, - - /// - /// Specifies all public nested types. - /// - PublicNestedTypes = 0x0080, - - /// - /// Specifies all non-public nested types. - /// - NonPublicNestedTypes = 0x0100, - - /// - /// Specifies all public properties. - /// - PublicProperties = 0x0200, - - /// - /// Specifies all non-public properties. - /// - NonPublicProperties = 0x0400, - - /// - /// Specifies all public events. - /// - PublicEvents = 0x0800, - - /// - /// Specifies all non-public events. - /// - NonPublicEvents = 0x1000, - - /// - /// Specifies all interfaces implemented by the type. - /// - Interfaces = 0x2000, - - /// - /// Specifies all members. - /// - All = ~None - } +enum DynamicallyAccessedMemberTypes +{ + /// + /// Specifies no members. + /// + None = 0, + + /// + /// Specifies the default, parameterless public constructor. + /// + PublicParameterlessConstructor = 0x0001, + + /// + /// Specifies all public constructors. + /// + PublicConstructors = 0x0002 | PublicParameterlessConstructor, + + /// + /// Specifies all non-public constructors. + /// + NonPublicConstructors = 0x0004, + + /// + /// Specifies all public methods. + /// + PublicMethods = 0x0008, + + /// + /// Specifies all non-public methods. + /// + NonPublicMethods = 0x0010, + + /// + /// Specifies all public fields. + /// + PublicFields = 0x0020, + + /// + /// Specifies all non-public fields. + /// + NonPublicFields = 0x0040, + + /// + /// Specifies all public nested types. + /// + PublicNestedTypes = 0x0080, + + /// + /// Specifies all non-public nested types. + /// + NonPublicNestedTypes = 0x0100, + + /// + /// Specifies all public properties. + /// + PublicProperties = 0x0200, + + /// + /// Specifies all non-public properties. + /// + NonPublicProperties = 0x0400, + + /// + /// Specifies all public events. + /// + PublicEvents = 0x0800, + + /// + /// Specifies all non-public events. + /// + NonPublicEvents = 0x1000, + + /// + /// Specifies all interfaces implemented by the type. + /// + Interfaces = 0x2000, + + /// + /// Specifies all members. + /// + All = ~None } #endif diff --git a/shared/DynamicallyAccessedMembersAttribute.cs b/shared/DynamicallyAccessedMembersAttribute.cs index 9f1c114186..5273878d33 100644 --- a/shared/DynamicallyAccessedMembersAttribute.cs +++ b/shared/DynamicallyAccessedMembersAttribute.cs @@ -5,57 +5,56 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -namespace System.Diagnostics.CodeAnalysis -{ - /// - /// Indicates that certain members on a specified are accessed dynamically, - /// for example through . - /// - /// - /// This allows tools to understand which members are being accessed during the execution - /// of a program. - /// - /// This attribute is valid on members whose type is or . - /// - /// When this attribute is applied to a location of type , the assumption is - /// that the string represents a fully qualified type name. - /// - /// When this attribute is applied to a class, interface, or struct, the members specified - /// can be accessed dynamically on instances returned from calling - /// on instances of that class, interface, or struct. - /// - /// If the attribute is applied to a method it's treated as a special case and it implies - /// the attribute should be applied to the "this" parameter of the method. As such the attribute - /// should only be used on instance methods of types assignable to System.Type (or string, but no methods - /// will use it there). - /// - [AttributeUsage( - AttributeTargets.Field | AttributeTargets.ReturnValue | AttributeTargets.GenericParameter | - AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.Method | - AttributeTargets.Class | AttributeTargets.Interface | AttributeTargets.Struct, - Inherited = false)] +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Indicates that certain members on a specified are accessed dynamically, +/// for example through . +/// +/// +/// This allows tools to understand which members are being accessed during the execution +/// of a program. +/// +/// This attribute is valid on members whose type is or . +/// +/// When this attribute is applied to a location of type , the assumption is +/// that the string represents a fully qualified type name. +/// +/// When this attribute is applied to a class, interface, or struct, the members specified +/// can be accessed dynamically on instances returned from calling +/// on instances of that class, interface, or struct. +/// +/// If the attribute is applied to a method it's treated as a special case and it implies +/// the attribute should be applied to the "this" parameter of the method. As such the attribute +/// should only be used on instance methods of types assignable to System.Type (or string, but no methods +/// will use it there). +/// +[AttributeUsage( + AttributeTargets.Field | AttributeTargets.ReturnValue | AttributeTargets.GenericParameter | + AttributeTargets.Parameter | AttributeTargets.Property | AttributeTargets.Method | + AttributeTargets.Class | AttributeTargets.Interface | AttributeTargets.Struct, + Inherited = false)] #if SYSTEM_PRIVATE_CORELIB - public +public #else - internal +internal #endif - sealed class DynamicallyAccessedMembersAttribute : Attribute +sealed class DynamicallyAccessedMembersAttribute : Attribute +{ + /// + /// Initializes a new instance of the class + /// with the specified member types. + /// + /// The types of members dynamically accessed. + public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes memberTypes) { - /// - /// Initializes a new instance of the class - /// with the specified member types. - /// - /// The types of members dynamically accessed. - public DynamicallyAccessedMembersAttribute(DynamicallyAccessedMemberTypes memberTypes) - { - MemberTypes = memberTypes; - } - - /// - /// Gets the which specifies the type - /// of members dynamically accessed. - /// - public DynamicallyAccessedMemberTypes MemberTypes { get; } + MemberTypes = memberTypes; } + + /// + /// Gets the which specifies the type + /// of members dynamically accessed. + /// + public DynamicallyAccessedMemberTypes MemberTypes { get; } } #endif diff --git a/shared/RequiresUnreferencedCodeAttribute.cs b/shared/RequiresUnreferencedCodeAttribute.cs index 493d6dbf84..b28ac82beb 100644 --- a/shared/RequiresUnreferencedCodeAttribute.cs +++ b/shared/RequiresUnreferencedCodeAttribute.cs @@ -5,46 +5,45 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -namespace System.Diagnostics.CodeAnalysis -{ - /// - /// Indicates that the specified method requires dynamic access to code that is not referenced - /// statically, for example through . - /// - /// - /// This allows tools to understand which methods are unsafe to call when removing unreferenced - /// code from an application. - /// - [AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Indicates that the specified method requires dynamic access to code that is not referenced +/// statically, for example through . +/// +/// +/// This allows tools to understand which methods are unsafe to call when removing unreferenced +/// code from an application. +/// +[AttributeUsage(AttributeTargets.Method | AttributeTargets.Constructor | AttributeTargets.Class, Inherited = false)] #if SYSTEM_PRIVATE_CORELIB - public +public #else - internal +internal #endif - sealed class RequiresUnreferencedCodeAttribute : Attribute + sealed class RequiresUnreferencedCodeAttribute : Attribute +{ + /// + /// Initializes a new instance of the class + /// with the specified message. + /// + /// + /// A message that contains information about the usage of unreferenced code. + /// + public RequiresUnreferencedCodeAttribute(string message) { - /// - /// Initializes a new instance of the class - /// with the specified message. - /// - /// - /// A message that contains information about the usage of unreferenced code. - /// - public RequiresUnreferencedCodeAttribute(string message) - { - Message = message; - } + Message = message; + } - /// - /// Gets a message that contains information about the usage of unreferenced code. - /// - public string Message { get; } + /// + /// Gets a message that contains information about the usage of unreferenced code. + /// + public string Message { get; } - /// - /// Gets or sets an optional URL that contains more information about the method, - /// why it requires unreferenced code, and what options a consumer has to deal with it. - /// - public string? Url { get; set; } - } + /// + /// Gets or sets an optional URL that contains more information about the method, + /// why it requires unreferenced code, and what options a consumer has to deal with it. + /// + public string? Url { get; set; } } #endif diff --git a/shared/UnconditionalSuppressMessageAttribute.cs b/shared/UnconditionalSuppressMessageAttribute.cs index 81e9e65501..5c6e8d9f99 100644 --- a/shared/UnconditionalSuppressMessageAttribute.cs +++ b/shared/UnconditionalSuppressMessageAttribute.cs @@ -5,91 +5,90 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -namespace System.Diagnostics.CodeAnalysis -{ - /// - /// Suppresses reporting of a specific rule violation, allowing multiple suppressions on a - /// single code artifact. - /// - /// - /// is different than - /// in that it doesn't have a - /// . So it is always preserved in the compiled assembly. - /// - [AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = true)] +namespace System.Diagnostics.CodeAnalysis; + +/// +/// Suppresses reporting of a specific rule violation, allowing multiple suppressions on a +/// single code artifact. +/// +/// +/// is different than +/// in that it doesn't have a +/// . So it is always preserved in the compiled assembly. +/// +[AttributeUsage(AttributeTargets.All, Inherited = false, AllowMultiple = true)] #if SYSTEM_PRIVATE_CORELIB - public +public #else - internal +internal #endif - sealed class UnconditionalSuppressMessageAttribute : Attribute +sealed class UnconditionalSuppressMessageAttribute : Attribute +{ + /// + /// Initializes a new instance of the + /// class, specifying the category of the tool and the identifier for an analysis rule. + /// + /// The category for the attribute. + /// The identifier of the analysis rule the attribute applies to. + public UnconditionalSuppressMessageAttribute(string category, string checkId) { - /// - /// Initializes a new instance of the - /// class, specifying the category of the tool and the identifier for an analysis rule. - /// - /// The category for the attribute. - /// The identifier of the analysis rule the attribute applies to. - public UnconditionalSuppressMessageAttribute(string category, string checkId) - { - Category = category; - CheckId = checkId; - } + Category = category; + CheckId = checkId; + } - /// - /// Gets the category identifying the classification of the attribute. - /// - /// - /// The property describes the tool or tool analysis category - /// for which a message suppression attribute applies. - /// - public string Category { get; } + /// + /// Gets the category identifying the classification of the attribute. + /// + /// + /// The property describes the tool or tool analysis category + /// for which a message suppression attribute applies. + /// + public string Category { get; } - /// - /// Gets the identifier of the analysis tool rule to be suppressed. - /// - /// - /// Concatenated together, the and - /// properties form a unique check identifier. - /// - public string CheckId { get; } + /// + /// Gets the identifier of the analysis tool rule to be suppressed. + /// + /// + /// Concatenated together, the and + /// properties form a unique check identifier. + /// + public string CheckId { get; } - /// - /// Gets or sets the scope of the code that is relevant for the attribute. - /// - /// - /// The Scope property is an optional argument that specifies the metadata scope for which - /// the attribute is relevant. - /// - public string? Scope { get; set; } + /// + /// Gets or sets the scope of the code that is relevant for the attribute. + /// + /// + /// The Scope property is an optional argument that specifies the metadata scope for which + /// the attribute is relevant. + /// + public string? Scope { get; set; } - /// - /// Gets or sets a fully qualified path that represents the target of the attribute. - /// - /// - /// The property is an optional argument identifying the analysis target - /// of the attribute. An example value is "System.IO.Stream.ctor():System.Void". - /// Because it is fully qualified, it can be long, particularly for targets such as parameters. - /// The analysis tool user interface should be capable of automatically formatting the parameter. - /// - public string? Target { get; set; } + /// + /// Gets or sets a fully qualified path that represents the target of the attribute. + /// + /// + /// The property is an optional argument identifying the analysis target + /// of the attribute. An example value is "System.IO.Stream.ctor():System.Void". + /// Because it is fully qualified, it can be long, particularly for targets such as parameters. + /// The analysis tool user interface should be capable of automatically formatting the parameter. + /// + public string? Target { get; set; } - /// - /// Gets or sets an optional argument expanding on exclusion criteria. - /// - /// - /// The property is an optional argument that specifies additional - /// exclusion where the literal metadata target is not sufficiently precise. For example, - /// the cannot be applied within a method, - /// and it may be desirable to suppress a violation against a statement in the method that will - /// give a rule violation, but not against all statements in the method. - /// - public string? MessageId { get; set; } + /// + /// Gets or sets an optional argument expanding on exclusion criteria. + /// + /// + /// The property is an optional argument that specifies additional + /// exclusion where the literal metadata target is not sufficiently precise. For example, + /// the cannot be applied within a method, + /// and it may be desirable to suppress a violation against a statement in the method that will + /// give a rule violation, but not against all statements in the method. + /// + public string? MessageId { get; set; } - /// - /// Gets or sets the justification for suppressing the code analysis message. - /// - public string? Justification { get; set; } - } + /// + /// Gets or sets the justification for suppressing the code analysis message. + /// + public string? Justification { get; set; } } -#endif \ No newline at end of file +#endif From db08e272705b4e0d91c0339069495e3865a339f6 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Fri, 8 Nov 2024 06:51:45 +0100 Subject: [PATCH 08/20] Mark as trim-compatible --- .../Microsoft.TestPlatform.ObjectModel.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj index 7913f386e5..974d75f3cd 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj +++ b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj @@ -6,7 +6,7 @@ net7.0;$(NetFrameworkMinimum);$(NetCoreAppMinimum);netstandard2.0; $(NoWarn);SYSLIB0051 - true + true From 6ce630f96526999102e4739e1132de1ddf7e20f9 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Fri, 8 Nov 2024 08:27:57 +0100 Subject: [PATCH 09/20] Publish ObjectModel for net7.0 --- .../Microsoft.TestPlatform.ObjectModel.csproj | 2 +- .../Microsoft.TestPlatform.ObjectModel.nuspec | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj index 974d75f3cd..155ea70f88 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj +++ b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj @@ -6,7 +6,7 @@ net7.0;$(NetFrameworkMinimum);$(NetCoreAppMinimum);netstandard2.0; $(NoWarn);SYSLIB0051 - true + true diff --git a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.nuspec b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.nuspec index 105050657a..f997242552 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.nuspec +++ b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.nuspec @@ -15,6 +15,10 @@ + + + + @@ -78,6 +82,17 @@ + + + + + + + + + + + From 799e84829a79784fa0d115801ea4c1e714c05d16 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 08:24:15 +0100 Subject: [PATCH 10/20] Use IsAotCompatible --- .../Microsoft.TestPlatform.ObjectModel.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj index 155ea70f88..cb61d95d83 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj +++ b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj @@ -6,7 +6,7 @@ net7.0;$(NetFrameworkMinimum);$(NetCoreAppMinimum);netstandard2.0; $(NoWarn);SYSLIB0051 - true + true From 4b83b25c32f514ef3fba02955ae01170e27a60f0 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 08:24:26 +0100 Subject: [PATCH 11/20] Update ObjectModel package verification --- eng/verify-nupkgs.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eng/verify-nupkgs.ps1 b/eng/verify-nupkgs.ps1 index e6d54183c1..4135deccdc 100644 --- a/eng/verify-nupkgs.ps1 +++ b/eng/verify-nupkgs.ps1 @@ -23,7 +23,7 @@ function Verify-Nuget-Packages { "Microsoft.TestPlatform.Build" = 20; "Microsoft.TestPlatform.CLI" = 471; "Microsoft.TestPlatform.Extensions.TrxLogger" = 34; - "Microsoft.TestPlatform.ObjectModel" = 92; + "Microsoft.TestPlatform.ObjectModel" = 121; "Microsoft.TestPlatform.AdapterUtilities" = 75; "Microsoft.TestPlatform.Portable" = 592; "Microsoft.TestPlatform.TestHost" = 62; From 8d3fdf3172a2111ee5a6442b57dd810256d7c884 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 08:40:40 +0100 Subject: [PATCH 12/20] Reduce DynamicallyAccessedMembers scope --- .../TestProperty/TestProperty.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs index 8c0b4d00df..cffa11425f 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs @@ -21,7 +21,7 @@ public class TestProperty : IEquatable private static bool DisableFastJson { get; set; } = FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_FASTER_JSON_SERIALIZATION); - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private Type _valueType; //public static Stopwatch @@ -41,7 +41,7 @@ private TestProperty() "IL2072:Target parameter argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.", Justification = "Only problematic part is ValueType = valueType.FullName, but as we annotated valueType parameter, it should be good.")] private TestProperty(string id, string label, string category, string description, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes) { @@ -130,7 +130,7 @@ private TestProperty(string id, string label, string category, string descriptio /// Gets or sets a string representation of the type for value. /// [DataMember] - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] public string ValueType { get; set; } #region IEquatable @@ -166,7 +166,7 @@ public override string ToString() /// /// Only works for the valueType that is in the currently executing assembly or in Mscorlib.dll. The default valueType is of string valueType. /// The valueType of the test property - [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] public Type GetValueType() { _valueType ??= GetType(ValueType); @@ -174,10 +174,10 @@ public Type GetValueType() return _valueType; } - [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private Type GetType( // TODO: Confirm if this is the right thing to do to avoid Type.GetType warning - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] string typeName) { ValidateArg.NotNull(typeName, nameof(typeName)); @@ -270,7 +270,7 @@ private Type GetType( "Trimming", "IL2057:Unrecognized value passed to the parameter of method. It's not possible to guarantee the availability of the target type.", Justification = "Only part incompatible with trimming is the Type.GetType call that does version replacement. It probably wouldn't cause trouble and is safe to suppress")] - [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private static Type? GetTypeByReplacingVersion(string typeName) { // This line is extracted into a separate method to ensure the UnconditionalSuppressMessage applies to it exactly (i.e, avoid unintentionally hiding more warnings) @@ -310,7 +310,7 @@ public static void ClearRegisteredProperties() } public static TestProperty Register(string id, string label, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, Type owner) { @@ -323,7 +323,7 @@ public static TestProperty Register(string id, string label, } public static TestProperty Register(string id, string label, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, TestPropertyAttributes attributes, Type owner) { @@ -336,7 +336,7 @@ public static TestProperty Register(string id, string label, } public static TestProperty Register(string id, string label, string category, string description, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes, Type owner) { From a05452d8a6f92d3c2fac2a858286c31f2ab45d9a Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 09:13:19 +0100 Subject: [PATCH 13/20] Avoid TypeDescriptor altogether for now --- .../TestObject.cs | 243 +++++++++--------- 1 file changed, 119 insertions(+), 124 deletions(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs index 779ed299a8..0d70089950 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs @@ -309,167 +309,154 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? return StringArrayConverter.ConvertFrom(null, culture, (string?)value); } - // These are already handled by TypeDescriptor.GetConverter, but it's not trimmer safe and - // we want to make sure they are guaranteed to work. - if (valueType == typeof(bool)) + try { - return BooleanConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + // These are already handled by TypeDescriptor.GetConverter, but it's not trimmer safe and + // we want to make sure they are guaranteed to work. + if (valueType == typeof(bool)) + { + return BooleanConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(byte)) - { - return ByteConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(byte)) + { + return ByteConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(sbyte)) - { - return SByteConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(sbyte)) + { + return SByteConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(char)) - { - return CharConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(char)) + { + return CharConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(double)) - { - return DoubleConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(double)) + { + return DoubleConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(string)) - { - return StringConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(string)) + { + return StringConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(int)) - { - return IntConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(int)) + { + return IntConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #if NET7_0_OR_GREATER - if (valueType == typeof(Int128)) - { - return Int128Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(Int128)) + { + return Int128Converter.ConvertFrom(null, culture, value ?? string.Empty); + } #endif - if (valueType == typeof(short)) - { - return Int16Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(short)) + { + return Int16Converter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(long)) - { - return Int64Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(long)) + { + return Int64Converter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(float)) - { - return SingleConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(float)) + { + return SingleConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #if NET7_0_OR_GREATER - if (valueType == typeof(Half)) - { - return HalfConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(Half)) + { + return HalfConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(UInt128)) - { - return UInt128Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(UInt128)) + { + return UInt128Converter.ConvertFrom(null, culture, value ?? string.Empty); + } #endif - if (valueType == typeof(ushort)) - { - return UInt16Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(ushort)) + { + return UInt16Converter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(uint)) - { - return UInt32Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(uint)) + { + return UInt32Converter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(ulong)) - { - return UInt64Converter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(ulong)) + { + return UInt64Converter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(Type)) - { - return TypeConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(Type)) + { + return TypeConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(CultureInfo)) - { - return CultureInfoConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(CultureInfo)) + { + return CultureInfoConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #if NET7_0_OR_GREATER - if (valueType == typeof(DateOnly)) - { - return DateOnlyConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(DateOnly)) + { + return DateOnlyConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #endif - if (valueType == typeof(DateTime)) - { - return DateTimeConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(DateTime)) + { + return DateTimeConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(DateTimeOffset)) - { - return DateTimeOffsetConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(DateTimeOffset)) + { + return DateTimeOffsetConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(decimal)) - { - return DecimalConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(decimal)) + { + return DecimalConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #if NET7_0_OR_GREATER - if (valueType == typeof(TimeOnly)) - { - return TimeOnlyConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(TimeOnly)) + { + return TimeOnlyConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #endif - if (valueType == typeof(TimeSpan)) - { - return TimeSpanConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(TimeSpan)) + { + return TimeSpanConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(Guid)) - { - return GuidConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(Guid)) + { + return GuidConverter.ConvertFrom(null, culture, value ?? string.Empty); + } - if (valueType == typeof(Uri)) - { - return UriTypeConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(Uri)) + { + return UriTypeConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #if NET7_0_OR_GREATER - if (valueType == typeof(Version)) - { - return VersionConverter.ConvertFrom(null, culture, value ?? string.Empty); - } + if (valueType == typeof(Version)) + { + return VersionConverter.ConvertFrom(null, culture, value ?? string.Empty); + } #endif - - // TODO: Consider detecting that we are in source gen mode (or in NativeAOT mode) and avoid calling TypeDescriptor altogether? - // Each of the approaches has pros and cons. - // Ignoring this trimmer unfriendly code when in NativeAOT will help catch issues earlier, and have more deterministic behavior. - // Keeping this trimmer unfriendly code even in NativeAOT will allow us to still have the possibility to work in case we fall in this path. - TPDebug.Assert(valueType is not null, "valueType is null"); - TypeConverter converter = TypeDescriptor.GetConverter(valueType); - if (converter == null) - { - throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.Resources.ConverterNotSupported, valueType.Name)); - } - - try - { - return converter.ConvertFrom(null, culture, value!); } catch (FormatException) { @@ -480,6 +467,14 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? // some type converters throw strange exceptions (e.g.: System.Exception by Int32Converter) throw new FormatException(e.Message, e); } + + // TODO: Consider detecting that we are in source gen mode (or in NativeAOT mode) and avoid calling TypeDescriptor altogether? + // Each of the approaches has pros and cons. + // Ignoring this trimmer unfriendly code when in NativeAOT will help catch issues earlier, and have more deterministic behavior. + // Keeping this trimmer unfriendly code even in NativeAOT will allow us to still have the possibility to work in case we fall in this path. + TPDebug.Assert(valueType is not null, "valueType is null"); + + throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.Resources.ConverterNotSupported, valueType.Name)); } /// From 4e60901e5f607ea4d11a43da340fed212442cbc1 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 09:34:48 +0100 Subject: [PATCH 14/20] Comment out unused code for now --- .../Serialization/DefaultTestPlatformContractResolver.cs | 2 ++ .../Serialization/TestObjectConverter.cs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/DefaultTestPlatformContractResolver.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/DefaultTestPlatformContractResolver.cs index 354ad4d793..4e0919c3a1 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/DefaultTestPlatformContractResolver.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/DefaultTestPlatformContractResolver.cs @@ -44,6 +44,7 @@ protected override JsonContract CreateContract(Type objectType) } } +#if false /// TODO: This is not used now, but I was experimenting with this quite a bit for performance, leaving it here in case I was wrong /// and the serializer settings actually have signigicant impact on the speed. /// @@ -81,3 +82,4 @@ protected override JsonContract CreateContract(Type objectType) return contract; } } +#endif diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs index fdc615a411..4f04a38f2f 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs @@ -93,6 +93,7 @@ public override void WriteJson(JsonWriter writer, object? value, JsonSerializer } } +#if false /// TODO: This is not used now, but I was experimenting with this quite a bit for performance, leaving it here in case I was wrong /// and the serializer settings actually have signigicant impact on the speed. /// @@ -195,3 +196,4 @@ private class TestPropertyTemplate public string? ValueType { get; set; } } } +#endif From 11af49b02b982803beecb1950298f3305321b186 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 09:35:01 +0100 Subject: [PATCH 15/20] Avoid TypeDescriptor --- .../TestObject.cs | 156 +++++++++++++++++- 1 file changed, 148 insertions(+), 8 deletions(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs index 0d70089950..2805d4476e 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs @@ -503,16 +503,156 @@ protected virtual void ProtectedSetPropertyValue(TestProperty property, object? var valueType = property.GetValueType(); - TypeConverter converter = TypeDescriptor.GetConverter(valueType); - - if (converter == null) - { - throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.Resources.ConverterNotSupported, valueType.Name)); - } - try { - return (T?)converter.ConvertTo(null, culture, value, typeof(T))!; + // These are already handled by TypeDescriptor.GetConverter, but it's not trimmer safe and + // we want to make sure they are guaranteed to work. + if (valueType == typeof(bool)) + { + return (T?)BooleanConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(byte)) + { + return (T?)ByteConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(sbyte)) + { + return (T?)SByteConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(char)) + { + return (T?)CharConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(double)) + { + return (T?)DoubleConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(string)) + { + return (T?)StringConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(int)) + { + return (T?)IntConverter.ConvertTo(null, culture, value, typeof(T))!; + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(Int128)) + { + return (T?)Int128Converter.ConvertTo(null, culture, value, typeof(T))!; + } +#endif + + if (valueType == typeof(short)) + { + return (T?)Int16Converter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(long)) + { + return (T?)Int64Converter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(float)) + { + return (T?)SingleConverter.ConvertTo(null, culture, value, typeof(T))!; + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(Half)) + { + return (T?)HalfConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(UInt128)) + { + return (T?)UInt128Converter.ConvertTo(null, culture, value, typeof(T))!; + } +#endif + + if (valueType == typeof(ushort)) + { + return (T?)UInt16Converter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(uint)) + { + return (T?)UInt32Converter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(ulong)) + { + return (T?)UInt64Converter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(Type)) + { + return (T?)TypeConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(CultureInfo)) + { + return (T?)CultureInfoConverter.ConvertTo(null, culture, value, typeof(T))!; + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(DateOnly)) + { + return (T?)DateOnlyConverter.ConvertTo(null, culture, value, typeof(T))!; + } +#endif + + if (valueType == typeof(DateTime)) + { + return (T?)DateTimeConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(DateTimeOffset)) + { + return (T?)DateTimeOffsetConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(decimal)) + { + return (T?)DecimalConverter.ConvertTo(null, culture, value, typeof(T))!; + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(TimeOnly)) + { + return (T?)TimeOnlyConverter.ConvertTo(null, culture, value, typeof(T))!; + } +#endif + + if (valueType == typeof(TimeSpan)) + { + return (T?)TimeSpanConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(Guid)) + { + return (T?)GuidConverter.ConvertTo(null, culture, value, typeof(T))!; + } + + if (valueType == typeof(Uri)) + { + return (T?)UriTypeConverter.ConvertTo(null, culture, value, typeof(T))!; + } + +#if NET7_0_OR_GREATER + if (valueType == typeof(Version)) + { + return (T?)VersionConverter.ConvertTo(null, culture, value, typeof(T))!; + } +#endif + + throw new NotSupportedException(string.Format(CultureInfo.CurrentCulture, Resources.Resources.ConverterNotSupported, valueType.Name)); } catch (FormatException) { From d12a2792ae322878f069e70ebde34fde5ad22506 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 09:36:56 +0100 Subject: [PATCH 16/20] Remove unused using --- .../Serialization/TestObjectConverter.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs index 4f04a38f2f..b01a1fce19 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/Serialization/TestObjectConverter.cs @@ -3,7 +3,6 @@ using System; using System.Collections.Generic; -using System.Reflection; using Microsoft.VisualStudio.TestPlatform.ObjectModel; From 8d9108e80a23d87ce3f62438cd32bcc997a7c52a Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 21:55:58 +0100 Subject: [PATCH 17/20] Hardcode supported types --- .../TestProperty/TestProperty.cs | 44 +++++-------------- 1 file changed, 12 insertions(+), 32 deletions(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs index cffa11425f..8b55e43d5c 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestProperty/TestProperty.cs @@ -21,7 +21,6 @@ public class TestProperty : IEquatable private static bool DisableFastJson { get; set; } = FeatureFlag.Instance.IsSet(FeatureFlag.VSTEST_DISABLE_FASTER_JSON_SERIALIZATION); - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] private Type _valueType; //public static Stopwatch @@ -36,12 +35,7 @@ private TestProperty() // Default constructor for Serialization. } - [UnconditionalSuppressMessage( - "Trimming", - "IL2072:Target parameter argument does not satisfy 'DynamicallyAccessedMembersAttribute' in call to target method. The return value of the source method does not have matching annotations.", - Justification = "Only problematic part is ValueType = valueType.FullName, but as we annotated valueType parameter, it should be good.")] private TestProperty(string id, string label, string category, string description, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes) { @@ -130,7 +124,6 @@ private TestProperty(string id, string label, string category, string descriptio /// Gets or sets a string representation of the type for value. /// [DataMember] - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] public string ValueType { get; set; } #region IEquatable @@ -166,7 +159,6 @@ public override string ToString() /// /// Only works for the valueType that is in the currently executing assembly or in Mscorlib.dll. The default valueType is of string valueType. /// The valueType of the test property - [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] public Type GetValueType() { _valueType ??= GetType(ValueType); @@ -174,11 +166,7 @@ public Type GetValueType() return _valueType; } - [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] - private Type GetType( - // TODO: Confirm if this is the right thing to do to avoid Type.GetType warning - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] - string typeName) + private Type GetType(string typeName) { ValidateArg.NotNull(typeName, nameof(typeName)); @@ -191,8 +179,17 @@ private Type GetType( try { - // This only works for the type is in the currently executing assembly or in Mscorlib.dll. - type = Type.GetType(typeName); + type = typeName switch + { + "System.String" => typeof(string), + "System.Int32" => typeof(int), + "System.String[]" => typeof(string[]), + "System.Guid" => typeof(Guid), + "System.Uri" => typeof(Uri), + "System.Boolean" => typeof(bool), + "System.Collections.Generic.KeyValuePair`2[[System.String],[System.String]][]" => typeof(KeyValuePair[]), + _ => throw new ArgumentException($"The type name '{typeName}' is unexpected."), + }; if (!DisableFastJson) { @@ -203,9 +200,6 @@ private Type GetType( } } - // Try 2.0 version as discovery returns version of 4.0 for all cases - type ??= GetTypeByReplacingVersion(typeName); - // For UAP the type namespace for System.Uri,System.TimeSpan and System.DateTimeOffset differs from the desktop version. if (type == null && typeName.StartsWith("System.Uri")) { @@ -266,17 +260,6 @@ private Type GetType( return type; } - [UnconditionalSuppressMessage( - "Trimming", - "IL2057:Unrecognized value passed to the parameter of method. It's not possible to guarantee the availability of the target type.", - Justification = "Only part incompatible with trimming is the Type.GetType call that does version replacement. It probably wouldn't cause trouble and is safe to suppress")] - [return: DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] - private static Type? GetTypeByReplacingVersion(string typeName) - { - // This line is extracted into a separate method to ensure the UnconditionalSuppressMessage applies to it exactly (i.e, avoid unintentionally hiding more warnings) - return Type.GetType(typeName.Replace("Version=4.0.0.0", "Version=2.0.0.0")); - } - private static readonly Dictionary>> Properties = new(); #if FullCLR @@ -310,7 +293,6 @@ public static void ClearRegisteredProperties() } public static TestProperty Register(string id, string label, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, Type owner) { @@ -323,7 +305,6 @@ public static TestProperty Register(string id, string label, } public static TestProperty Register(string id, string label, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, TestPropertyAttributes attributes, Type owner) { @@ -336,7 +317,6 @@ public static TestProperty Register(string id, string label, } public static TestProperty Register(string id, string label, string category, string description, - [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.PublicConstructors | DynamicallyAccessedMemberTypes.NonPublicConstructors)] Type valueType, ValidateValueCallback? validateValueCallback, TestPropertyAttributes attributes, Type owner) { From e1794a2c98e5f90e8166597901e85a9a426904ea Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 22:06:37 +0100 Subject: [PATCH 18/20] More fix --- .../TestObject.cs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs index 2805d4476e..0f9bd17ca0 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs +++ b/src/Microsoft.TestPlatform.ObjectModel/TestObject.cs @@ -149,7 +149,22 @@ public virtual IEnumerable Properties if (valueType != null && valueType.IsValueType) { - defaultValue = Activator.CreateInstance(valueType); + if (valueType == typeof(int)) + { + defaultValue = 0; + } + else if (valueType == typeof(Guid)) + { + defaultValue = new Guid(); + } + else if (valueType == typeof(bool)) + { + defaultValue = false; + } + else + { + throw new ArgumentException($"The type '{valueType}' is unexpected."); + } } return ProtectedGetPropertyValue(property, defaultValue); From 518a4ccc153330e3a050c1b048a764726a8066b7 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 22:33:00 +0100 Subject: [PATCH 19/20] Use IsTargetFrameworkCompatible --- .../Microsoft.TestPlatform.ObjectModel.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj index cb61d95d83..8bacaf710b 100644 --- a/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj +++ b/src/Microsoft.TestPlatform.ObjectModel/Microsoft.TestPlatform.ObjectModel.csproj @@ -6,7 +6,7 @@ net7.0;$(NetFrameworkMinimum);$(NetCoreAppMinimum);netstandard2.0; $(NoWarn);SYSLIB0051 - true + true From 5971eb1914de713fc610d1d6f58b0981840e018c Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Mon, 11 Nov 2024 22:33:08 +0100 Subject: [PATCH 20/20] AdapterUtilities progress --- .../ManagedNameHelper.Reflection.cs | 13 ++++++++++++- .../Microsoft.TestPlatform.AdapterUtilities.csproj | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.AdapterUtilities/ManagedNameUtilities/ManagedNameHelper.Reflection.cs b/src/Microsoft.TestPlatform.AdapterUtilities/ManagedNameUtilities/ManagedNameHelper.Reflection.cs index d8eefb9288..1350accba7 100644 --- a/src/Microsoft.TestPlatform.AdapterUtilities/ManagedNameUtilities/ManagedNameHelper.Reflection.cs +++ b/src/Microsoft.TestPlatform.AdapterUtilities/ManagedNameUtilities/ManagedNameHelper.Reflection.cs @@ -4,6 +4,9 @@ using Microsoft.TestPlatform.AdapterUtilities.Helpers; using System; +#if NET5_0_OR_GREATER +using System.Diagnostics.CodeAnalysis; +#endif using System.Globalization; using System.Linq; using System.Reflection; @@ -230,6 +233,9 @@ private static void GetManagedNameAndHierarchy(MethodBase method, bool useClosed /// More information about and can be found in /// the RFC. /// +#if NET5_0_OR_GREATER + [RequiresUnreferencedCode("Types might be removed by trimming. If the type name is a string literal, consider using Type.GetType instead.")] +#endif public static MethodBase GetMethod(Assembly assembly, string managedTypeName, string managedMethodName) { var parsedManagedTypeName = ReflectionHelpers.ParseEscapedString(managedTypeName); @@ -258,7 +264,12 @@ public static MethodBase GetMethod(Assembly assembly, string managedTypeName, st return method; } - private static MethodInfo? FindMethod(Type type, string methodName, int methodArity, string[]? parameterTypes) + private static MethodInfo? FindMethod( +#if NET5_0_OR_GREATER + [DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] +#endif + Type type, + string methodName, int methodArity, string[]? parameterTypes) { bool Filter(MemberInfo mbr, object? param) { diff --git a/src/Microsoft.TestPlatform.AdapterUtilities/Microsoft.TestPlatform.AdapterUtilities.csproj b/src/Microsoft.TestPlatform.AdapterUtilities/Microsoft.TestPlatform.AdapterUtilities.csproj index 53049b4413..aae232afe6 100644 --- a/src/Microsoft.TestPlatform.AdapterUtilities/Microsoft.TestPlatform.AdapterUtilities.csproj +++ b/src/Microsoft.TestPlatform.AdapterUtilities/Microsoft.TestPlatform.AdapterUtilities.csproj @@ -4,6 +4,7 @@ netstandard2.0;$(NetFrameworkMinimum);net6.0;net8.0;$(NetCurrent) Microsoft.TestPlatform.AdapterUtilities Microsoft.TestPlatform.AdapterUtilities + true