Unify reflection helpers

src/Adapter/MSTestAdapter.PlatformServices/Discovery/AssemblyEnumerator.cs

@@ -7,6 +7,7 @@
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Microsoft.VisualStudio.TestTools.UnitTesting.Internal;
 
@@ -21,29 +22,32 @@ internal class AssemblyEnumerator : MarshalByRefObject
     /// <summary>
     /// Helper for reflection API's.
     /// </summary>
-    private static readonly ReflectHelper ReflectHelper = ReflectHelper.Instance;
+    private readonly ReflectionOperations _reflectionOperations;
 
     /// <summary>
     /// Type cache.
     /// </summary>
-    private readonly TypeCache _typeCache = new(ReflectHelper);
+    private readonly TypeCache _typeCache;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AssemblyEnumerator"/> class.
     /// </summary>
     public AssemblyEnumerator()
     {
+        _reflectionOperations = (ReflectionOperations)PlatformServiceProvider.Instance.ReflectionOperations;
+        _typeCache = new TypeCache(_reflectionOperations);
     }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="AssemblyEnumerator"/> class.
     /// </summary>
     /// <param name="settings">The settings for the session.</param>
     /// <remarks>Use this constructor when creating this object in a new app domain so the settings for this app domain are set.</remarks>
-    public AssemblyEnumerator(MSTestSettings settings) =>
+    public AssemblyEnumerator(MSTestSettings settings)
+        : this()
         // Populate the settings into the domain(Desktop workflow) performing discovery.
         // This would just be resetting the settings to itself in non desktop workflows.
-        MSTestSettings.PopulateSettings(settings);
+        => MSTestSettings.PopulateSettings(settings);
 
     /// <summary>
     /// Returns object to be used for controlling lifetime, null means infinite lifetime.
@@ -66,21 +70,26 @@ internal AssemblyEnumerationResult EnumerateAssembly(string assemblyFileName)
     {
         List<string> warnings = [];
         DebugEx.Assert(!StringEx.IsNullOrWhiteSpace(assemblyFileName), "Invalid assembly file name.");
-        var tests = new List<UnitTestElement>();
+        List<UnitTestElement> tests = [];
 
         Assembly assembly = PlatformServiceProvider.Instance.FileOperations.LoadAssembly(assemblyFileName);
 
         Type[] types = GetTypes(assembly);
-        bool discoverInternals = ReflectHelper.GetDiscoverInternalsAttribute(assembly) != null;
-
-        TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy = ReflectHelper.GetTestDataSourceOptions(assembly)?.UnfoldingStrategy switch
+        bool discoverInternals = _reflectionOperations.GetCustomAttributes(assembly, typeof(DiscoverInternalsAttribute))
+            .OfType<DiscoverInternalsAttribute>()
+            .Any();
+
+        TestDataSourceOptionsAttribute? assemblyUnfoldingStrategyAttribute = _reflectionOperations.GetCustomAttributes(assembly, typeof(TestDataSourceOptionsAttribute))
+            .OfType<TestDataSourceOptionsAttribute>()
+            .FirstOrDefault();
+        TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy = assemblyUnfoldingStrategyAttribute?.UnfoldingStrategy switch
         {
             // When strategy is auto we want to unfold
             TestDataSourceUnfoldingStrategy.Auto => TestDataSourceUnfoldingStrategy.Unfold,
             // When strategy is set, let's use it
             { } value => value,
             // When the attribute is not set, let's look at the legacy attribute
-            null => ReflectHelper.GetTestDataSourceDiscoveryOption(assembly) switch
+            null => _reflectionOperations.GetCustomAttributes(assembly, typeof(TestDataSourceDiscoveryAttribute)).OfType<TestDataSourceDiscoveryAttribute>().FirstOrDefault()?.DiscoveryOption switch
             {
                 TestDataSourceDiscoveryOption.DuringExecution => TestDataSourceUnfoldingStrategy.Fold,
                 _ => TestDataSourceUnfoldingStrategy.Unfold,
@@ -142,10 +151,10 @@ internal static Type[] GetTypes(Assembly assembly)
     /// <returns>a TypeEnumerator instance.</returns>
     internal virtual TypeEnumerator GetTypeEnumerator(Type type, string assemblyFileName, bool discoverInternals)
     {
-        var typeValidator = new TypeValidator(ReflectHelper, discoverInternals);
-        var testMethodValidator = new TestMethodValidator(ReflectHelper, discoverInternals);
+        var typeValidator = new TypeValidator(_reflectionOperations, discoverInternals);
+        var testMethodValidator = new TestMethodValidator(_reflectionOperations, discoverInternals);
 
-        return new TypeEnumerator(type, assemblyFileName, ReflectHelper, typeValidator, testMethodValidator);
+        return new TypeEnumerator(type, assemblyFileName, _reflectionOperations, typeValidator, testMethodValidator);
     }
 
     private List<UnitTestElement> DiscoverTestsInType(
@@ -156,7 +165,7 @@ private List<UnitTestElement> DiscoverTestsInType(
         TestDataSourceUnfoldingStrategy dataSourcesUnfoldingStrategy)
     {
         string? typeFullName = null;
-        var tests = new List<UnitTestElement>();
+        List<UnitTestElement> tests = [];
 
         try
         {
@@ -221,7 +230,7 @@ private static bool TryUnfoldITestDataSources(UnitTestElement test, DiscoveryTes
         // We don't have a special method to filter attributes that are not derived from Attribute, so we take all
         // attributes and filter them. We don't have to care if there is one, because this method is only entered when
         // there is at least one (we determine this in TypeEnumerator.GetTestFromMethod.
-        IEnumerable<ITestDataSource> testDataSources = ReflectHelper.Instance.GetAttributes<Attribute>(testMethodInfo.MethodInfo).OfType<ITestDataSource>();
+        IEnumerable<ITestDataSource> testDataSources = ((ReflectionOperations)PlatformServiceProvider.Instance.ReflectionOperations).GetAttributes<Attribute>(testMethodInfo.MethodInfo).OfType<ITestDataSource>();
 
         // We need to use a temporary list to avoid adding tests to the main list if we fail to expand any data source.
         List<UnitTestElement> tempListOfTests = [];

src/Adapter/MSTestAdapter.PlatformServices/Discovery/TestMethodValidator.cs

@@ -2,7 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Extensions;
-using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
@@ -13,18 +13,18 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 [SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class TestMethodValidator
 {
-    private readonly ReflectHelper _reflectHelper;
+    private readonly IReflectionOperations _reflectionOperation;
     private readonly bool _discoverInternals;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="TestMethodValidator"/> class.
     /// </summary>
-    /// <param name="reflectHelper">An instance to reflection helper for type information.</param>
+    /// <param name="reflectionOperation">An instance to reflection helper for type information.</param>
     /// <param name="discoverInternals">True to discover methods which are declared internal in addition to methods
     /// which are declared public.</param>
-    internal TestMethodValidator(ReflectHelper reflectHelper, bool discoverInternals)
+    internal TestMethodValidator(IReflectionOperations reflectionOperation, bool discoverInternals)
     {
-        _reflectHelper = reflectHelper;
+        _reflectionOperation = reflectionOperation;
         _discoverInternals = discoverInternals;
     }
 
@@ -44,7 +44,7 @@ internal virtual bool IsValidTestMethod(MethodInfo testMethodInfo, Type type, IC
         // but the difference is quite small, and we don't expect a huge amount of non-test methods in the assembly.
         //
         // Also skip all methods coming from object, because they cannot be tests.
-        if (testMethodInfo.DeclaringType == typeof(object) || !_reflectHelper.IsAttributeDefined<TestMethodAttribute>(testMethodInfo))
+        if (testMethodInfo.DeclaringType == typeof(object) || !_reflectionOperation.IsAttributeDefined<TestMethodAttribute>(testMethodInfo))
         {
             return false;
         }
@@ -55,7 +55,7 @@ internal virtual bool IsValidTestMethod(MethodInfo testMethodInfo, Type type, IC
         // Todo: Decide whether parameter count matters.
         bool isValidTestMethod = isAccessible &&
                                  testMethodInfo is { IsAbstract: false, IsStatic: false } &&
-                                 testMethodInfo.IsValidReturnType(_reflectHelper);
+                                 testMethodInfo.IsValidReturnType(_reflectionOperation);
 
         if (!isValidTestMethod)
         {

src/Adapter/MSTestAdapter.PlatformServices/Discovery/TypeEnumerator.cs

@@ -1,9 +1,9 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Helpers;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
@@ -18,21 +18,21 @@ internal class TypeEnumerator
     private readonly string _assemblyFilePath;
     private readonly TypeValidator _typeValidator;
     private readonly TestMethodValidator _testMethodValidator;
-    private readonly ReflectHelper _reflectHelper;
+    private readonly IReflectionOperations _reflectionOperation;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="TypeEnumerator"/> class.
     /// </summary>
     /// <param name="type"> The reflected type. </param>
     /// <param name="assemblyFilePath"> The name of the assembly being reflected. </param>
-    /// <param name="reflectHelper"> An instance to reflection helper for type information. </param>
+    /// <param name="reflectionOperation"> An instance to reflection helper for type information. </param>
     /// <param name="typeValidator"> The validator for test classes. </param>
     /// <param name="testMethodValidator"> The validator for test methods. </param>
-    internal TypeEnumerator(Type type, string assemblyFilePath, ReflectHelper reflectHelper, TypeValidator typeValidator, TestMethodValidator testMethodValidator)
+    internal TypeEnumerator(Type type, string assemblyFilePath, IReflectionOperations reflectionOperation, TypeValidator typeValidator, TestMethodValidator testMethodValidator)
     {
         _type = type;
         _assemblyFilePath = assemblyFilePath;
-        _reflectHelper = reflectHelper;
+        _reflectionOperation = reflectionOperation;
         _typeValidator = typeValidator;
         _testMethodValidator = testMethodValidator;
     }
@@ -69,7 +69,7 @@ internal List<UnitTestElement> GetTests(List<string> warnings)
         // if we rely on analyzers to identify all invalid methods on build, we can change this to fit the current settings.
         foreach (MethodInfo method in PlatformServiceProvider.Instance.ReflectionOperations.GetRuntimeMethods(_type))
         {
-            bool isMethodDeclaredInTestTypeAssembly = _reflectHelper.IsMethodDeclaredInSameAssemblyAsType(method, _type);
+            bool isMethodDeclaredInTestTypeAssembly = method.DeclaringType!.Assembly.Equals(_type.Assembly); // TODO: Investigate if we rely on NRE;
             bool enableMethodsFromOtherAssemblies = MSTestSettings.CurrentSettings.EnableBaseClassTestMethodsFromOtherAssemblies;
 
             if (!isMethodDeclaredInTestTypeAssembly && !enableMethodsFromOtherAssemblies)
@@ -136,19 +136,20 @@ internal UnitTestElement GetTestFromMethod(MethodInfo method, ICollection<string
 
         var testElement = new UnitTestElement(testMethod)
         {
-            TestCategory = _reflectHelper.GetTestCategories(method, _type),
-            DoNotParallelize = _reflectHelper.IsDoNotParallelizeSet(method, _type),
-            Priority = _reflectHelper.GetPriority(method),
+            TestCategory = _reflectionOperation.GetTestCategories(method, _type),
+            DoNotParallelize = _reflectionOperation.IsAttributeDefined<DoNotParallelizeAttribute>(method)
+                || _reflectionOperation.IsAttributeDefined<DoNotParallelizeAttribute>(_type),
+            Priority = _reflectionOperation.GetFirstAttributeOrDefault<PriorityAttribute>(method)?.Priority,
 #if !WINDOWS_UWP && !WIN_UI
             DeploymentItems = PlatformServiceProvider.Instance.TestDeployment.GetDeploymentItems(method, _type, warnings),
 #endif
-            Traits = [.. _reflectHelper.GetTestPropertiesAsTraits(method)],
+            Traits = [.. _reflectionOperation.GetTestPropertiesAsTraits(method)],
         };
 
-        Attribute[] attributes = _reflectHelper.GetCustomAttributesCached(method);
+        Attribute[] attributes = _reflectionOperation.GetCustomAttributesCached(method);
         TestMethodAttribute? testMethodAttribute = null;
 
-        // Backward looping for backcompat. This used to be calls to _reflectHelper.GetFirstAttributeOrDefault
+        // Backward looping for backcompat. This used to be calls to _reflectionOperation.GetFirstAttributeOrDefault
         // So, to make sure the first attribute always wins, we loop from end to start.
         for (int i = attributes.Length - 1; i >= 0; i--)
         {

src/Adapter/MSTestAdapter.PlatformServices/Discovery/TypeValidator.cs

@@ -1,7 +1,7 @@
 // Copyright (c) Microsoft Corporation. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
-using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Interface;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
@@ -15,27 +15,27 @@ internal class TypeValidator
     // Setting this to a string representation instead of a typeof(TestContext).FullName
     // since the later would require a load of the Test Framework extension assembly at this point.
     private const string TestContextFullName = "Microsoft.VisualStudio.TestTools.UnitTesting.TestContext";
-    private readonly ReflectHelper _reflectHelper;
+    private readonly IReflectionOperations _reflectionOperation;
     private readonly bool _discoverInternals;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="TypeValidator"/> class.
     /// </summary>
-    /// <param name="reflectHelper">An instance to reflection helper for type information.</param>
-    internal TypeValidator(ReflectHelper reflectHelper)
-        : this(reflectHelper, false)
+    /// <param name="reflectionOperation">An instance to reflection helper for type information.</param>
+    internal TypeValidator(IReflectionOperations reflectionOperation)
+        : this(reflectionOperation, false)
     {
     }
 
     /// <summary>
     /// Initializes a new instance of the <see cref="TypeValidator"/> class.
     /// </summary>
-    /// <param name="reflectHelper">An instance to reflection helper for type information.</param>
+    /// <param name="reflectionOperation">An instance to reflection helper for type information.</param>
     /// <param name="discoverInternals">True to discover test classes which are declared internal in
     /// addition to test classes which are declared public.</param>
-    internal TypeValidator(ReflectHelper reflectHelper, bool discoverInternals)
+    internal TypeValidator(IReflectionOperations reflectionOperation, bool discoverInternals)
     {
-        _reflectHelper = reflectHelper;
+        _reflectionOperation = reflectionOperation;
         _discoverInternals = discoverInternals;
     }
 
@@ -52,7 +52,7 @@ internal virtual bool IsValidTestClass(Type type, List<string> warnings)
         // gives us a better performance.
         // It would be possible to use non-caching reflection here if we knew that we are only doing discovery that won't be followed by run,
         // but the difference is quite small, and we don't expect a huge amount of non-test classes in the assembly.
-        if (!type.IsClass || !_reflectHelper.IsAttributeDefined<TestClassAttribute>(type))
+        if (!type.IsClass || !_reflectionOperation.IsAttributeDefined<TestClassAttribute>(type))
         {
             return false;
         }

src/Adapter/MSTestAdapter.PlatformServices/Execution/TestAssemblySettingsProvider.cs

@@ -3,8 +3,8 @@
 
 using System.Security;
 
-using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
+using Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
@@ -34,7 +34,10 @@ internal TestAssemblySettings GetSettings(string source)
         // Load the source.
         Assembly testAssembly = PlatformServiceProvider.Instance.FileOperations.LoadAssembly(source);
 
-        ParallelizeAttribute? parallelizeAttribute = ReflectHelper.GetParallelizeAttribute(testAssembly);
+        var reflectionOperations = (ReflectionOperations)PlatformServiceProvider.Instance.ReflectionOperations;
+        ParallelizeAttribute? parallelizeAttribute = reflectionOperations.GetCustomAttributes(testAssembly, typeof(ParallelizeAttribute))
+            .OfType<ParallelizeAttribute>()
+            .FirstOrDefault();
 
         if (parallelizeAttribute != null)
         {
@@ -47,7 +50,7 @@ internal TestAssemblySettings GetSettings(string source)
             }
         }
 
-        testAssemblySettings.CanParallelizeAssembly = !ReflectHelper.IsDoNotParallelizeSet(testAssembly);
+        testAssemblySettings.CanParallelizeAssembly = reflectionOperations.GetCustomAttributes(testAssembly, typeof(DoNotParallelizeAttribute)).Length == 0;
 
         return testAssemblySettings;
     }