chore: seal internal classes when possible (#4235)

.editorconfig

@@ -214,6 +214,7 @@ dotnet_diagnostic.CA1827.severity = warning             # CA1827: Do not use Cou
 dotnet_diagnostic.CA1836.severity = warning             # CA1836: Prefer IsEmpty over Count
 dotnet_diagnostic.CA1840.severity = warning             # CA1840: Use 'Environment.CurrentManagedThreadId'
 dotnet_diagnostic.CA1852.severity = warning             # CA1852: Seal internal types
+dotnet_code_quality.CA1852.ignore_internalsvisibleto = true
 dotnet_diagnostic.CA1854.severity = warning             # CA1854: Prefer the 'IDictionary.TryGetValue(TKey, out TValue)' method
 # Disabled as it's making the code complex to deal with when multi targeting
 dotnet_diagnostic.CA1863.severity = none                # CA1863: Use 'CompositeFormat'

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumerator.cs

@@ -20,6 +20,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 /// <summary>
 /// Enumerates through all types in the assembly in search of valid test methods.
 /// </summary>
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class AssemblyEnumerator : MarshalByRefObject
 {
     /// <summary>

src/Adapter/MSTest.TestAdapter/Discovery/AssemblyEnumeratorWrapper.cs

@@ -13,7 +13,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 /// <summary>
 /// Enumerates through an assembly to get a set of test methods.
 /// </summary>
-internal class AssemblyEnumeratorWrapper
+internal sealed class AssemblyEnumeratorWrapper
 {
     /// <summary>
     /// Assembly name for UTF.

src/Adapter/MSTest.TestAdapter/Discovery/TestMethodValidator.cs

@@ -1,6 +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 System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
 
@@ -13,6 +14,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 /// <summary>
 /// Determines if a method is a valid test method.
 /// </summary>
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class TestMethodValidator
 {
     private readonly ReflectHelper _reflectHelper;

src/Adapter/MSTest.TestAdapter/Discovery/TypeEnumerator.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.ObjectModel;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
 
@@ -17,6 +18,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 /// <summary>
 /// Enumerates through the type looking for Valid Test Methods to execute.
 /// </summary>
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class TypeEnumerator
 {
     private readonly Type _type;

src/Adapter/MSTest.TestAdapter/Discovery/TypeValidator.cs

@@ -1,6 +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 System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
 
@@ -12,6 +13,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
 /// <summary>
 /// Determines whether a type is a valid test class for this adapter.
 /// </summary>
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class TypeValidator
 {
     // Setting this to a string representation instead of a typeof(TestContext).FullName

src/Adapter/MSTest.TestAdapter/Discovery/UnitTestDiscoverer.cs

@@ -1,6 +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 System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 
 using Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Discovery;
@@ -12,6 +13,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
 
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class UnitTestDiscoverer
 {
     private readonly AssemblyEnumeratorWrapper _assemblyEnumeratorWrapper;

src/Adapter/MSTest.TestAdapter/Execution/RunCleanupResult.cs

@@ -7,7 +7,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// Result of the run cleanup operation.
 /// </summary>
 [Serializable]
-internal class RunCleanupResult
+internal sealed class RunCleanupResult
 {
     /// <summary>
     /// Gets or sets the standard out of the cleanup methods.

src/Adapter/MSTest.TestAdapter/Execution/TestAssemblySettingsProvider.cs

@@ -10,7 +10,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 
-internal class TestAssemblySettingsProvider : MarshalByRefObject
+internal sealed class TestAssemblySettingsProvider : MarshalByRefObject
 {
     /// <summary>
     /// Returns object to be used for controlling lifetime, null means infinite lifetime.

src/Adapter/MSTest.TestAdapter/Execution/TestCaseDiscoverySink.cs

@@ -11,7 +11,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// <summary>
 /// The test case discovery sink.
 /// </summary>
-internal class TestCaseDiscoverySink : ITestCaseDiscoverySink
+internal sealed class TestCaseDiscoverySink : ITestCaseDiscoverySink
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="TestCaseDiscoverySink"/> class.

src/Adapter/MSTest.TestAdapter/Execution/TestMethodRunner.cs

@@ -22,7 +22,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// <summary>
 /// This class is responsible to running tests and converting framework TestResults to adapter TestResults.
 /// </summary>
-internal class TestMethodRunner
+internal sealed class TestMethodRunner
 {
     /// <summary>
     /// Test context which needs to be passed to the various methods of the test.

src/Adapter/MSTest.TestAdapter/Execution/TypeCache.cs

@@ -21,7 +21,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// <summary>
 /// Defines type cache which reflects upon a type and cache its test artifacts.
 /// </summary>
-internal class TypeCache : MarshalByRefObject
+internal sealed class TypeCache : MarshalByRefObject
 {
     /// <summary>
     /// Test context property name.

src/Adapter/MSTest.TestAdapter/Execution/UnitTestRunner.cs

@@ -21,7 +21,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Execution;
 /// <summary>
 /// The runner that runs a single unit test. Also manages the assembly and class cleanup methods at the end of the run.
 /// </summary>
-internal class UnitTestRunner : MarshalByRefObject
+internal sealed class UnitTestRunner : MarshalByRefObject
 {
     private readonly ConcurrentDictionary<string, TestMethodInfo> _fixtureTests = new();
     private readonly TypeCache _typeCache;

src/Adapter/MSTest.TestAdapter/Helpers/EnvironmentWrapper.cs

@@ -3,7 +3,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 
-internal class EnvironmentWrapper : IEnvironment
+internal sealed class EnvironmentWrapper : IEnvironment
 {
     public string MachineName => Environment.MachineName;
 }

src/Adapter/MSTest.TestAdapter/Helpers/ReflectHelper.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System.Collections.Concurrent;
+using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
 using System.Security;
@@ -13,6 +14,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for mocking")]
 internal class ReflectHelper : MarshalByRefObject
 {
 #pragma warning disable RS0030 // Do not use banned APIs

src/Adapter/MSTest.TestAdapter/Helpers/RunSettingsUtilities.cs

@@ -11,7 +11,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.Helpers;
 
-internal class RunSettingsUtilities
+internal static class RunSettingsUtilities
 {
     /// <summary>
     /// Gets the settings to be used while creating XmlReader for runsettings.

src/Adapter/MSTest.TestAdapter/ObjectModel/AdapterSettingsException.cs

@@ -3,7 +3,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 
-internal class AdapterSettingsException : Exception
+internal sealed class AdapterSettingsException : Exception
 {
     internal AdapterSettingsException(string? message)
         : base(message)

src/Adapter/MSTest.TestAdapter/ObjectModel/StackTraceInformation.cs

@@ -6,7 +6,7 @@
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 
 [Serializable]
-internal class StackTraceInformation
+internal sealed class StackTraceInformation
 {
     public StackTraceInformation(string stackTrace)
         : this(stackTrace, null, 0, 0)

src/Adapter/MSTest.TestAdapter/ObjectModel/TestAssemblySettings.cs

@@ -6,7 +6,7 @@
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 
 [Serializable]
-internal class TestAssemblySettings
+internal sealed class TestAssemblySettings
 {
     public TestAssemblySettings() => Workers = -1;
 

src/Adapter/MSTest.TestAdapter/ObjectModel/TestFailedException.cs

@@ -9,7 +9,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 /// Internal class to indicate Test Execution failure.
 /// </summary>
 [Serializable]
-internal class TestFailedException : Exception
+internal sealed class TestFailedException : Exception
 {
     public TestFailedException(UnitTestOutcome outcome, string errorMessage)
         : this(outcome, errorMessage, null, null)

src/Adapter/MSTest.TestAdapter/ObjectModel/TestMethodOptions.cs

@@ -10,4 +10,4 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 /// <summary>
 ///  A facade service for options passed to a test method.
 /// </summary>
-internal record TestMethodOptions(TimeoutInfo TimeoutInfo, ExpectedExceptionBaseAttribute? ExpectedException, ITestContext? TestContext, bool CaptureDebugTraces, TestMethodAttribute? Executor);
+internal sealed record TestMethodOptions(TimeoutInfo TimeoutInfo, ExpectedExceptionBaseAttribute? ExpectedException, ITestContext? TestContext, bool CaptureDebugTraces, TestMethodAttribute? Executor);

src/Adapter/MSTest.TestAdapter/ObjectModel/TypeInspectionException.cs

@@ -7,7 +7,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 /// Internal class to indicate type inspection failure.
 /// </summary>
 [Serializable]
-internal class TypeInspectionException : Exception
+internal sealed class TypeInspectionException : Exception
 {
     public TypeInspectionException()
         : base()

src/Adapter/MSTest.TestAdapter/ObjectModel/UnitTestElement.cs

@@ -16,7 +16,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter.ObjectModel;
 /// </summary>
 [Serializable]
 [DebuggerDisplay("{GetDisplayName()} ({TestMethod.ManagedTypeName})")]
-internal class UnitTestElement
+internal sealed class UnitTestElement
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="UnitTestElement"/> class.

src/Adapter/MSTest.TestAdapter/PlatformServiceProvider.cs

@@ -15,7 +15,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
 /// <summary>
 /// The main service provider class that exposes all the platform services available.
 /// </summary>
-internal class PlatformServiceProvider : IPlatformServiceProvider
+internal sealed class PlatformServiceProvider : IPlatformServiceProvider
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="PlatformServiceProvider"/> class - a singleton.

src/Adapter/MSTest.TestAdapter/SourceGeneratedDynamicDataOperations.cs

@@ -3,6 +3,6 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
 
-internal class SourceGeneratedDynamicDataOperations : DynamicDataOperations
+internal sealed class SourceGeneratedDynamicDataOperations : DynamicDataOperations
 {
 }

src/Adapter/MSTest.TestAdapter/TestMethodFilter.cs

@@ -10,7 +10,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTest.TestAdapter;
 
-internal class TestMethodFilter
+internal sealed class TestMethodFilter
 {
     /// <summary>
     /// Supported properties for filtering.

src/Adapter/MSTestAdapter.PlatformServices/Constants.cs

@@ -5,7 +5,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 
-internal class Constants
+internal static class Constants
 {
 #if NETFRAMEWORK
     /// <summary>

src/Adapter/MSTestAdapter.PlatformServices/Data/TestDataConnectionFactory.cs

@@ -64,12 +64,12 @@ public virtual TestDataConnection Create(string invariantProviderName, string co
 
     #region TestDataConnectionFactories
 
-    private class XmlTestDataConnectionFactory : TestDataConnectionFactory
+    private sealed class XmlTestDataConnectionFactory : TestDataConnectionFactory
     {
         public override TestDataConnection Create(string invariantProviderName, string connectionString, List<string> dataFolders) => new XmlDataConnection(connectionString, dataFolders);
     }
 
-    private class CsvTestDataConnectionFactory : TestDataConnectionFactory
+    private sealed class CsvTestDataConnectionFactory : TestDataConnectionFactory
     {
         public override TestDataConnection Create(string invariantProviderName, string connectionString, List<string> dataFolders) => new CsvDataConnection(connectionString, dataFolders);
     }

src/Adapter/MSTestAdapter.PlatformServices/Deployment/AssemblyLoadWorker.cs

@@ -21,7 +21,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Dep
 /// Utility function for Assembly related info
 /// The caller is supposed to create AppDomain and create instance of given class in there.
 /// </summary>
-internal class AssemblyLoadWorker : MarshalByRefObject
+internal sealed class AssemblyLoadWorker : MarshalByRefObject
 {
     private readonly IAssemblyUtility _assemblyUtility;
 

src/Adapter/MSTestAdapter.PlatformServices/Deployment/DeploymentItem.cs

@@ -39,7 +39,7 @@ internal enum DeploymentItemOriginType
 /// The deployment item for a test class or a test method.
 /// </summary>
 [Serializable]
-internal class DeploymentItem
+internal sealed class DeploymentItem
 {
     /// <summary>
     /// Initializes a new instance of the <see cref="DeploymentItem"/> class.

src/Adapter/MSTestAdapter.PlatformServices/Services/ReflectionOperations2.cs

@@ -7,7 +7,7 @@
 
 namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 
-internal class ReflectionOperations2 : ReflectionOperations, IReflectionOperations2
+internal sealed class ReflectionOperations2 : ReflectionOperations, IReflectionOperations2
 {
     public ReflectionOperations2()
     {

src/Adapter/MSTestAdapter.PlatformServices/Services/ThreadSafeStringWriter.cs

@@ -177,7 +177,7 @@ private ThreadSafeStringBuilder GetOrAddStringBuilder()
     /// <summary>
     /// This StringBuilder puts locks around all the methods to avoid conflicts when writing or reading from multiple threads.
     /// </summary>
-    private class ThreadSafeStringBuilder
+    private sealed class ThreadSafeStringBuilder
     {
         private readonly StringBuilder _stringBuilder = new();
         private readonly Lock _instanceLockObject = new();

src/Adapter/MSTestAdapter.PlatformServices/Utilities/AppDomainUtilities.cs

@@ -257,7 +257,7 @@ internal static Version GetTargetFrameworkVersionFromVersionString(string versio
         appDomainCultureHelper?.SetUICulture(uiCulture);
     }
 
-    private class AppDomainCultureHelper : MarshalByRefObject
+    private sealed class AppDomainCultureHelper : MarshalByRefObject
     {
 #pragma warning disable CA1822 // Mark members as static - Should not be static for our need
         public void SetUICulture(CultureInfo uiCulture) => CultureInfo.DefaultThreadCurrentUICulture = uiCulture;

src/Adapter/MSTestAdapter.PlatformServices/Utilities/AppDomainWrapper.cs

@@ -10,11 +10,13 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices;
 /// <summary>
 /// Abstraction over the AppDomain APIs.
 /// </summary>
-internal class AppDomainWrapper : IAppDomain
+internal sealed class AppDomainWrapper : IAppDomain
 {
-    public AppDomain CreateDomain(string friendlyName, Evidence securityInfo, AppDomainSetup info) => AppDomain.CreateDomain(friendlyName, securityInfo, info);
+    public AppDomain CreateDomain(string friendlyName, Evidence securityInfo, AppDomainSetup info)
+        => AppDomain.CreateDomain(friendlyName, securityInfo, info);
 
-    public void Unload(AppDomain appDomain) => AppDomain.Unload(appDomain);
+    public void Unload(AppDomain appDomain)
+        => AppDomain.Unload(appDomain);
 }
 
 #endif

src/Adapter/MSTestAdapter.PlatformServices/Utilities/AssemblyUtility.cs

@@ -3,9 +3,9 @@
 
 #if !WINDOWS_UWP
 
+using System.Diagnostics.CodeAnalysis;
 #if NETFRAMEWORK
 using System.Diagnostics;
-using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.Reflection;
 
@@ -19,6 +19,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Uti
 /// <summary>
 /// Utility for assembly specific functionality.
 /// </summary>
+[SuppressMessage("Performance", "CA1852: Seal internal types", Justification = "Overrides required for testability")]
 internal class AssemblyUtility
 #if NETFRAMEWORK
     : IAssemblyUtility

src/Adapter/MSTestAdapter.PlatformServices/Utilities/DeploymentItemUtility.cs

@@ -17,7 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.Uti
 /// <summary>
 /// The deployment utility.
 /// </summary>
-internal class DeploymentItemUtility
+internal sealed class DeploymentItemUtility
 {
     // REVIEW: it would be better if this was a ReflectionHelper, because helper is able to cache. But we don't have reflection helper here, because this is platform services dll.
     private readonly ReflectionUtility _reflectionUtility;