From 4e75ec728802a9e5c7aa840d944d5e1b95332731 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 19 Jul 2022 17:54:16 +0200 Subject: [PATCH] CA1001: Types that own disposable fields should be disposable (#3860) Co-authored-by: Marco Rossignoli --- .editorconfig | 3 +++ TestPlatform.sln | 3 +++ .../DataCollectionAttachmentManager.cs | 15 ++++++++++++++- .../ExtensionFramework/TestPluginCache.cs | 1 + .../SocketClient.cs | 2 ++ .../SocketCommunicationManager.cs | 2 ++ .../SocketServer.cs | 2 ++ .../DataCollectorAttachmentProcessorWrapper.cs | 2 +- src/vstest.console/Internal/ProgressIndicator.cs | 8 +++++++- test/.editorconfig | 3 +++ 10 files changed, 38 insertions(+), 3 deletions(-) diff --git a/.editorconfig b/.editorconfig index 0b172c41ab..6912591fa1 100644 --- a/.editorconfig +++ b/.editorconfig @@ -218,6 +218,9 @@ dotnet_diagnostic.RS0041.severity = none # not default, decreased severity becau # CA1824: Mark assemblies with NeutralResourcesLanguageAttribute dotnet_diagnostic.CA1824.severity = warning # not default, increased severity to ensure it is applied +# CA1001: Types that own disposable fields should be disposable +dotnet_diagnostic.CA1001.severity = warning # not default, increased severity to ensure it is applied + # CA1304: Specify CultureInfo dotnet_diagnostic.CA1304.severity = warning # not default, increased severity to ensure it is applied diff --git a/TestPlatform.sln b/TestPlatform.sln index 32f9c47ff0..8b2a046a97 100644 --- a/TestPlatform.sln +++ b/TestPlatform.sln @@ -8,6 +8,9 @@ EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.TestPlatform.CoreUtilities", "src\Microsoft.TestPlatform.CoreUtilities\Microsoft.TestPlatform.CoreUtilities.csproj", "{50C00046-0DA3-4B5C-9F6F-7BE1145E156A}" EndProject Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{B27FAFDF-2DBA-4AB0-BA85-FD5F21D359D6}" + ProjectSection(SolutionItems) = preProject + test\.editorconfig = test\.editorconfig + EndProjectSection EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.TestPlatform.CoreUtilities.UnitTests", "test\Microsoft.TestPlatform.CoreUtilities.UnitTests\Microsoft.TestPlatform.CoreUtilities.UnitTests.csproj", "{01409D95-A5F1-4EBE-94B1-909D5D2D0DC3}" EndProject diff --git a/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionAttachmentManager.cs b/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionAttachmentManager.cs index c7d177c3d9..99e5e982a5 100644 --- a/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionAttachmentManager.cs +++ b/src/Microsoft.TestPlatform.Common/DataCollection/DataCollectionAttachmentManager.cs @@ -34,7 +34,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.DataCollector; /// we don't know how the user will implement the datacollector and they could send file out of events(wrong usage, no more expected sequential access AddAttachment->GetAttachments), /// so we prefer protect every collection. This not means that outcome will be "always correct"(file attached in a correct way) but at least we avoid exceptions. /// -internal class DataCollectionAttachmentManager : IDataCollectionAttachmentManager +internal class DataCollectionAttachmentManager : IDataCollectionAttachmentManager, IDisposable { private readonly object _attachmentTaskLock = new(); @@ -356,4 +356,17 @@ private void LogError(string errorMessage, Uri collectorUri, string collectorFri _messageSink?.SendMessage(args); } + public void Dispose() + { + Dispose(disposing: true); + GC.SuppressFinalize(this); + } + + protected virtual void Dispose(bool disposing) + { + if (disposing) + { + _cancellationTokenSource.Dispose(); + } + } } diff --git a/src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginCache.cs b/src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginCache.cs index c3c87db9c5..337bc7c60e 100644 --- a/src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginCache.cs +++ b/src/Microsoft.TestPlatform.Common/ExtensionFramework/TestPluginCache.cs @@ -25,6 +25,7 @@ namespace Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework; /// The test plugin cache. /// /// Making this a singleton to offer better unit testing. +[SuppressMessage("Design", "CA1001:Types that own disposable fields should be disposable", Justification = "Would cause a breaking change if users are inheriting this class and implement IDisposable")] public class TestPluginCache { private readonly Dictionary _resolvedAssemblies = new(); diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketClient.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketClient.cs index 31447a4c81..d162c5fe43 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketClient.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketClient.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Net.Sockets; using System.Threading; @@ -16,6 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities; /// /// Communication client implementation over sockets. /// +[SuppressMessage("Design", "CA1001:Types that own disposable fields should be disposable", Justification = "Would cause a breaking change if users are inheriting this class and implement IDisposable")] public class SocketClient : ICommunicationEndPoint { private readonly CancellationTokenSource _cancellation; diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketCommunicationManager.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketCommunicationManager.cs index 1d9b118a18..ebd6aba8ba 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketCommunicationManager.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketCommunicationManager.cs @@ -3,6 +3,7 @@ using System; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Net; using System.Net.Sockets; @@ -19,6 +20,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities; /// /// Facilitates communication using sockets /// +[SuppressMessage("Design", "CA1001:Types that own disposable fields should be disposable", Justification = "Would cause a breaking change if users are inheriting this class and implement IDisposable")] public class SocketCommunicationManager : ICommunicationManager { /// diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketServer.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketServer.cs index 5189850544..3ab072694a 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/SocketServer.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/SocketServer.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See LICENSE file in the project root for full license information. using System; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Net.Sockets; using System.Threading; @@ -16,6 +17,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommunicationUtilities; /// /// Communication server implementation over sockets. /// +[SuppressMessage("Design", "CA1001:Types that own disposable fields should be disposable", Justification = "Would cause a breaking change if users are inheriting this class and implement IDisposable")] public class SocketServer : ICommunicationEndPoint { private readonly CancellationTokenSource _cancellation; diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/AttachmentsProcessing/DataCollectorAttachmentProcessorWrapper.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/AttachmentsProcessing/DataCollectorAttachmentProcessorWrapper.cs index 773026148f..83fcc1e992 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/AttachmentsProcessing/DataCollectorAttachmentProcessorWrapper.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/AttachmentsProcessing/DataCollectorAttachmentProcessorWrapper.cs @@ -27,7 +27,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.TestRunAttachments /// It tries to load the extension and it receives calls from the DataCollectorAttachmentProcessorAppDomain that /// acts as a proxy for the main AppDomain(the runner one). /// -internal class DataCollectorAttachmentProcessorRemoteWrapper : MarshalByRefObject +internal sealed class DataCollectorAttachmentProcessorRemoteWrapper : MarshalByRefObject, IDisposable { private readonly AnonymousPipeServerStream _pipeServerStream = new(PipeDirection.Out, HandleInheritability.None); private readonly object _pipeClientLock = new(); diff --git a/src/vstest.console/Internal/ProgressIndicator.cs b/src/vstest.console/Internal/ProgressIndicator.cs index a11f82c259..0821ca4b69 100644 --- a/src/vstest.console/Internal/ProgressIndicator.cs +++ b/src/vstest.console/Internal/ProgressIndicator.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; using System.Globalization; using Microsoft.VisualStudio.TestPlatform.Utilities; @@ -12,7 +13,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Internal; /// /// Indicates the test run progress /// -internal class ProgressIndicator : IProgressIndicator +internal sealed class ProgressIndicator : IProgressIndicator, IDisposable { private readonly object _syncObject = new(); private int _dotCounter; @@ -115,4 +116,9 @@ private void Timer_Elapsed(object sender, System.Timers.ElapsedEventArgs e) } } } + + public void Dispose() + { + _timer?.Dispose(); + } } diff --git a/test/.editorconfig b/test/.editorconfig index 2be7bb24d6..db532a1f2f 100644 --- a/test/.editorconfig +++ b/test/.editorconfig @@ -15,6 +15,9 @@ dotnet_diagnostic.IDE0060.severity = warning # CA1018: Mark attributes with AttributeUsageAttribute dotnet_diagnostic.CA1018.severity = warning +# CA1001: Types that own disposable fields should be disposable +dotnet_diagnostic.CA1001.severity = silent # Disabled on tests as it does not matter + #### C# Coding Conventions #### #### .NET Formatting Rules ####