Enable dispose analyzers CA2000 and CA2213 and fix all issues (#3670)

Author: marcschier

.editorconfig

@@ -384,6 +384,17 @@ dotnet_diagnostic.CA1511.severity                                             =
 dotnet_diagnostic.CA1512.severity                                             = none
 dotnet_diagnostic.CA1513.severity                                             = none
 
+# CA2213: Disposable fields should be disposed
+dotnet_diagnostic.CA2213.severity                                             = error
+# CA2000: Dispose objects before losing scope
+dotnet_diagnostic.CA2000.severity                                             = error
+# CA1001: Types that own disposable fields should be disposable
+dotnet_diagnostic.CA1001.severity                                             = warning
+# CA1849: Call async methods when in an async method
+dotnet_diagnostic.CA1849.severity                                             = suggestion
+# CA2225: Operator overloads have named alternates
+dotnet_diagnostic.CA2225.severity                                             = suggestion
+
 #
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/language-rules
 #
@@ -409,6 +420,8 @@ dotnet_diagnostic.IDE1005.severity                                            =
 dotnet_diagnostic.IDE0046.severity                                            = silent
 # IDE0130: Namespace does not match folder structure
 dotnet_diagnostic.IDE0130.severity                                            = none
+# IDE0032: Use auto property
+dotnet_diagnostic.IDE0032.severity                                            = suggestion
 
 #
 # Introduced in .net 10 as part of all analysis configuration
@@ -433,23 +446,8 @@ dotnet_diagnostic.CA1716.severity                                             =
 # CA1724: The type conflicts in whole or in part with a namespace name
 dotnet_diagnostic.CA1724.severity                                             = none
 
-# TODO Fix as P1:
-
-# CA2213: Disposable fields should be disposed
-dotnet_diagnostic.CA2213.severity = none
-# CA2000: Dispose objects before losing scope
-dotnet_diagnostic.CA2000.severity = none
-# CA1001: Types that own disposable fields should be disposable
-dotnet_diagnostic.CA1001.severity = none
-# IDE0032: Use auto property
-dotnet_diagnostic.IDE0032.severity = none
-# CA1849: Call async methods when in an async method
-dotnet_diagnostic.CA1849.severity = none
-
 # TODO: Fix as P2:
 
-# CA2225: Operator overloads have named alternates
-dotnet_diagnostic.CA2225.severity = none
 # CA1848: Use the LoggerMessage delegates
 dotnet_diagnostic.CA1848.severity = none
 # CA1873: Avoid potentially expensive logging

.github/workflows/buildandtest.yml

@@ -123,3 +123,10 @@ jobs:
         name: aot-results-${{ matrix.os }}
         path: '**/TestResults/**'
       if: ${{ always() }}
+
+    - name: Upload HTML Test Report
+      uses: actions/upload-artifact@v7
+      with:
+        name: TestReport
+        path: '**/*-report.html'
+      if: ${{ always() }}

Applications/ConsoleReferenceClient/ClientSamples.cs

@@ -62,7 +62,7 @@ public interface IUAClient
     /// <summary>
     /// Sample Session calls based on the reference server node model.
     /// </summary>
-    public partial class ClientSamples
+    public class ClientSamples
     {
         private const int kMaxSearchDepth = 128;
 

…/ConsoleReferenceClient/RunConnectAll.cs …/ConsoleReferenceClient/ConnectTester.csApplications/ConsoleReferenceClient/RunConnectAll.cs renamed to Applications/ConsoleReferenceClient/ConnectTester.cs Applications/ConsoleReferenceClient/RunConnectAll.cs renamed to Applications/ConsoleReferenceClient/ConnectTester.cs

@@ -28,6 +28,7 @@
  * ======================================================================*/
 
 using System;
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
@@ -42,9 +43,33 @@
 
 namespace Quickstarts
 {
-    public partial class ClientSamples
+    /// <summary>
+    /// Wraps connect testing functionality
+    /// </summary>
+    public sealed class ConnectTester : IDisposable
     {
-        public async Task<bool> RunAsync(ManualResetEvent quitEvent, CancellationToken ct)
+        public ConnectTester(
+            ITelemetryContext telemetry,
+            ManualResetEvent quitEvent = null)
+        {
+            m_quitEvent = quitEvent;
+            m_telemetry = telemetry;
+            m_logger = telemetry.CreateLogger<ConnectTester>();
+        }
+
+        /// <inheritdoc/>
+        public void Dispose()
+        {
+            m_reconnectHandler?.Dispose();
+        }
+
+        /// <summary>
+        /// Run the tests until cancelled or quit
+        /// </summary>
+        /// <param name="ct"></param>
+        /// <returns></returns>
+        /// <exception cref="InvalidOperationException"></exception>
+        public async Task RunAsync(CancellationToken ct)
         {
             try
             {
@@ -54,7 +79,6 @@ public async Task<bool> RunAsync(ManualResetEvent quitEvent, CancellationToken c
                     kReconnectPeriodExponentialBackoff);
 
                 m_logger.LogInformation("OPC UA Security Test Client");
-                CryptoTrace.Enabled = false;
 
                 // The application name and config file names
                 const string applicationName = "ConsoleReferenceClient";
@@ -95,8 +119,6 @@ public async Task<bool> RunAsync(ManualResetEvent quitEvent, CancellationToken c
                     kServerUrl,
                     ct).ConfigureAwait(false);
 
-                //endpoints = endpoints.Where(x => x.SecurityPolicyUri == TargetPolicy).ToList();
-
                 var endpointConfiguration = EndpointConfiguration.Create(m_configuration);
                 var sessionFactory = new DefaultSessionFactory(m_telemetry);
                 var userNameidentity = new UserIdentity(kUserName, new UTF8Encoding(false).GetBytes(kPassword));
@@ -204,54 +226,17 @@ public async Task<bool> RunAsync(ManualResetEvent quitEvent, CancellationToken c
                             ii.SecurityMode);
 
                         m_logger.LogWarning("{Line}", new string('=', 80));
-                        //break;
                     }
-
-                    //break;
                 }
 
                 Console.WriteLine("Ctrl-C to stop.");
-                quitEvent.WaitOne();
+                m_quitEvent.WaitOne();
             }
             catch (Exception e)
             {
                 m_logger.LogError("Exception: {Message}", e.Message);
                 m_logger.LogTrace("StackTrace: {StackTrace}", e.StackTrace);
             }
-
-            return true;
-        }
-
-        private async Task<UserIdentity> LoadUserCertificateAsync(
-            string thumbprint,
-            string password,
-            CancellationToken ct)
-        {
-            CertificateTrustList store = m_configuration.SecurityConfiguration.TrustedUserCertificates;
-#if NET8_0_OR_GREATER
-            // get user certificate with matching thumbprint
-            X509Certificate2Collection certificates =
-                await store.GetCertificatesAsync(m_telemetry, ct).ConfigureAwait(false);
-            X509Certificate2 hit = certificates
-                .Find(X509FindType.FindByThumbprint, thumbprint, false)
-                .FirstOrDefault();
-
-            // create Certificate Identifier
-            var cid = new CertificateIdentifier(hit)
-            {
-                StorePath = store.StorePath,
-                StoreType = store.StoreType
-            };
-
-            return await UserIdentity.CreateAsync(
-                cid,
-                new CertificatePasswordProvider(new UTF8Encoding(false).GetBytes(password)),
-                m_telemetry,
-                ct).ConfigureAwait(false);
-#else
-            await Task.Delay(1, ct).ConfigureAwait(false);
-            throw new NotSupportedException("User certificate identity is only supported on .NET 8 or greater.");
-#endif
         }
 
         internal async Task<SessionWrapper> RunTestAsync(
@@ -293,7 +278,8 @@ internal async Task<SessionWrapper> RunTestAsync(
             wrapper.Session.KeepAliveInterval = 10000;
             wrapper.Session.KeepAlive += Session_KeepAlive;
 
-            ArrayOf<ReferenceDescription> nodes = await BrowseFullAddressSpaceAsync(
+            var samples = new ClientSamples(m_telemetry, null, m_quitEvent);
+            ArrayOf<ReferenceDescription> nodes = await samples.BrowseFullAddressSpaceAsync(
                 wrapper,
                 ObjectIds.ObjectsFolder,
                 null,
@@ -302,6 +288,38 @@ internal async Task<SessionWrapper> RunTestAsync(
             return wrapper;
         }
 
+        private async Task<UserIdentity> LoadUserCertificateAsync(
+            string thumbprint,
+            string password,
+            CancellationToken ct)
+        {
+            CertificateTrustList store = m_configuration.SecurityConfiguration.TrustedUserCertificates;
+#if NET8_0_OR_GREATER
+            // get user certificate with matching thumbprint
+            X509Certificate2Collection certificates =
+                await store.GetCertificatesAsync(m_telemetry, ct).ConfigureAwait(false);
+            X509Certificate2 hit = certificates
+                .Find(X509FindType.FindByThumbprint, thumbprint, false)
+                .FirstOrDefault();
+
+            // create Certificate Identifier
+            var cid = new CertificateIdentifier(hit)
+            {
+                StorePath = store.StorePath,
+                StoreType = store.StoreType
+            };
+
+            return await UserIdentity.CreateAsync(
+                cid,
+                new CertificatePasswordProvider(new UTF8Encoding(false).GetBytes(password)),
+                m_telemetry,
+                ct).ConfigureAwait(false);
+#else
+            await Task.Delay(1, ct).ConfigureAwait(false);
+            throw new NotSupportedException("User certificate identity is only supported on .NET 8 or greater.");
+#endif
+        }
+
         private static async ValueTask<ArrayOf<EndpointDescription>> GetEndpointsAsync(
             ApplicationConfiguration application,
             string discoveryUrl,
@@ -425,7 +443,7 @@ private void Client_ReconnectComplete(object sender, EventArgs e)
                         );
                         ISession session = m_wrapper.Session;
                         m_wrapper = new SessionWrapper { Session = m_reconnectHandler.Session };
-                        Utils.SilentDispose(session);
+                        session?.Dispose();
                     }
                     else
                     {
@@ -469,6 +487,9 @@ internal sealed class SessionWrapper : IUAClient
         private SessionReconnectHandler m_reconnectHandler;
         private ApplicationConfiguration m_configuration;
         private SessionWrapper m_wrapper;
+        private ILogger m_logger;
+        private ITelemetryContext m_telemetry;
+        private readonly ManualResetEvent m_quitEvent;
 
         private const string kServerUrl = "opc.tcp://localhost:62541";
         private const string kUserName = "sysadmin";

Applications/ConsoleReferenceClient/Program.cs

@@ -55,10 +55,6 @@ public static class Program
         /// Main entry point.
         /// </summary>
         /// <exception cref="ErrorExitException"></exception>
-        [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2026",
-            Justification = "NodeSet2 export uses XmlSerializer with known OPC UA types.")]
-        [UnconditionalSuppressMessage("AOT", "IL3050",
-            Justification = "NodeSet2 export uses XmlSerializer with known OPC UA types.")]
         public static Task<int> Main(string[] args)
         {
             Console.WriteLine("OPC UA Console Reference Client");
@@ -293,7 +289,7 @@ public static Task<int> Main(string[] args)
                         logConsole,
                         fileLog,
                         appLog,
-                        LogLevel.Warning);
+                        LogLevel.Information);
 
                     // delete old certificate
                     if (renewCertificate)
@@ -331,16 +327,11 @@ await application.DeleteApplicationInstanceCertificateAsync(ct: cancellationToke
                     // handle connect all endpoints test.
                     if (testallEndpoints)
                     {
-                        var tester = new ClientSamples(
+                        var tester = new ConnectTester(
                             telemetry,
-                            null,
-                            quitEvent,
-                            verbose);
-
-                        if (await tester.RunAsync(quitEvent, ct).ConfigureAwait(false))
-                        {
-                            return;
-                        }
+                            quitEvent);
+                        await tester.RunAsync(ct).ConfigureAwait(false);
+                        return;
                     }
 
                     var userIdentity = new UserIdentity();
@@ -742,7 +733,7 @@ await uaClient
                 }
                 finally
                 {
-                    Utils.SilentDispose(reverseConnectManager);
+                    reverseConnectManager?.Dispose();
                 }
             });
 

Applications/ConsoleReferenceClient/UAClient.cs

@@ -93,7 +93,8 @@ protected virtual void Dispose(bool disposing)
             }
             if (disposing)
             {
-                Utils.SilentDispose(Session);
+                m_reconnectHandler?.Dispose();
+                Session?.Dispose();
                 m_configuration.CertificateValidator.CertificateValidation -= CertificateValidation;
             }
             m_disposed = true;
@@ -437,7 +438,7 @@ private void Client_ReconnectComplete(object sender, EventArgs e)
                         );
                         ISession session = Session;
                         Session = m_reconnectHandler.Session;
-                        Utils.SilentDispose(session);
+                        session?.Dispose();
                     }
                     else
                     {

Applications/ConsoleReferenceServer/ConsoleUtils.cs

@@ -63,8 +63,16 @@ public ConsoleTelemetry(Action<ILoggingBuilder> configure = null)
                 {
                     builder.SetMinimumLevel(LogLevel.Information);
                     m_configure?.Invoke(builder);
-                })
-                .AddSerilog(Log.Logger);
+                });
+            try
+            {
+                LoggerFactory = LoggerFactory.AddSerilog(Log.Logger);
+            }
+            catch
+            {
+                LoggerFactory.Dispose();
+                throw;
+            }
 
             ActivitySource = new ActivitySource("Quickstarts", "1.0.0");
 
@@ -186,15 +194,22 @@ public void ConfigureLogging(
             // create the serilog logger
             Serilog.Core.Logger serilogger = loggerConfiguration.CreateLogger();
 
-            // Dispose the old LoggerFactory and create a new one with the updated configuration
             ILoggerFactory oldLoggerFactory = LoggerFactory;
             LoggerFactory = Microsoft.Extensions.Logging.LoggerFactory
                 .Create(builder =>
                 {
                     builder.SetMinimumLevel(consoleLogLevel);
                     m_configure?.Invoke(builder);
-                })
-                .AddSerilog(serilogger);
+                });
+            try
+            {
+                LoggerFactory = LoggerFactory.AddSerilog(serilogger);
+            }
+            catch
+            {
+                LoggerFactory.Dispose();
+                throw;
+            }
             m_logger = LoggerFactory.CreateLogger("Main");
 
             oldLoggerFactory.Dispose();

Applications/ConsoleReferenceServer/Program.cs

@@ -179,7 +179,7 @@ await server
                         logConsole,
                         fileLog,
                         appLog,
-                        LogLevel.Warning);
+                        LogLevel.Information);
 
                     // check or renew the certificate
                     Console.WriteLine("Check the certificate.");