Fix analyzer errors in Renci.SshNet and Renci.SshNet.TestTools.OpenSSH (#1229)

* Fix analyzer errors in Renci.SshNet and Renci.SshNet.TestTools.OpenSSH.
Suppress all errors in unit tests and integration tests.

* Update unit tests now that we pass 'mode' as argument name when we throw ArgumentException.

* Remove stale comment and add unit tests for SshData.ReadBytes(int length).

* Remove unnessary suppression.

* Remove Visual Studio magic.

* Removed duplicate source file.

* Clarified that suppression hides a false positive.

* Remove suppressions for S2372.

* Update ReadExtensionPair() to return concrete dictionary.

Author: drieseng

.editorconfig

@@ -77,6 +77,12 @@ dotnet_diagnostic.S1172.severity = none
 # This is a duplicate of IDE0059.
 dotnet_diagnostic.S1481.severity = none
 
+# S1854: Unused assignments should be removed
+# https://rules.sonarsource.com/csharp/RSPEC-1854
+#
+# This is a duplicate of IDE0059.
+dotnet_diagnostic.S1854.severity = none
+
 # S2259: Null pointers should not be dereferenced
 # https://rules.sonarsource.com/csharp/RSPEC-2259
 #
@@ -168,6 +174,12 @@ dotnet_diagnostic.S3928.severity = none
 # This is a duplicate of CA2002, and partial duplicate of MA0064.
 dotnet_diagnostic.S3998.severity = none
 
+# S4070: Non-flags enums should not be marked with "FlagsAttribute"
+# https://rules.sonarsource.com/csharp/RSPEC-4070
+#
+# This is a duplicate of MA0062.
+dotnet_diagnostic.S4070.severity = none
+
 # S4456: Parameter validation in yielding methods should be wrapped
 # https://rules.sonarsource.com/csharp/RSPEC-4456
 #
@@ -364,6 +376,14 @@ dotnet_diagnostic.MA0018.severity = none
 # No strong need for this, and may negatively affect performance.
 dotnet_diagnostic.MA0021.severity = none
 
+# MA0025: Implement the functionality instead of throwing NotImplementedException
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0031.md
+dotnet_diagnostic.MA0025.severity = none
+
+# MA0026: Fix TODO comment
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md
+dotnet_diagnostic.MA0026.severity = suggestion
+
 # MA0031: Optimize Enumerable.Count() usage
 # https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0031.md
 #
@@ -436,6 +456,12 @@ dotnet_diagnostic.MA0112.severity = error
 # instead of a base class or interface.
 dotnet_diagnostic.CA1002.severity = none
 
+# CA1003: Use generic event handler instances
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1003
+#
+# Similar to MA0046.
+dotnet_diagnostic.CA1003.severity = none
+
 # CA1008: Enums should have zero value
 #
 # TODO: To be discussed. Having a zero value offers a performance advantage.
@@ -460,6 +486,10 @@ dotnet_diagnostic.CA1051.severity = none
 # By default, this diagnostic is only reported for public types.
 dotnet_code_quality.CA1052.api_surface = all
 
+# CA1065: Do not raise exceptions in unexpected locations
+# https://learn.microsoft.com/en-US/dotnet/fundamentals/code-analysis/quality-rules/ca1065
+dotnet_diagnostic.CA1065.severity = none
+
 # CA1303: Do not pass literals as localized parameters
 #
 # We don't care about localization.
@@ -473,6 +503,10 @@ dotnet_diagnostic.CA1303.severity = none
 # Submitted https://github.com/dotnet/roslyn-analyzers/issues/6096 to fix CA1305.
 dotnet_diagnostic.CA1305.severity = none
 
+# CA1309: Use ordinal StringComparison
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1309
+dotnet_diagnostic.CA1309.severity = none
+
 # CA1510: Use ArgumentNullException throw helper
 #
 # This is only available in .NET 6.0 and higher. We'd need to use conditional compilation to only
@@ -577,6 +611,22 @@ dotnet_diagnostic.IDE0130.severity = none
 # var inputPath = originalDossierPathList.Find(x => x.id == updatedPath.id) ?? throw new PcsException($"Path id ({updatedPath.id}) unknown in PCS for dossier id {dossierFromTs.dossier.id}", updatedPath.id);
 dotnet_diagnostic.IDE0270.severity = none
 
+# IDE0290: Use primary constructor
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0290
+dotnet_diagnostic.IDE0290.severity = none
+
+# IDE0300: Collection initialization can be simplified
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0300
+#
+# TODO: Discuss whether we want to start using this
+dotnet_diagnostic.IDE0300.severity = none
+
+# IDE0301: Simplify collection initialization
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0301
+#
+# TODO: Discuss whether we want to start using this
+dotnet_diagnostic.IDE0301.severity = none
+
 #### .NET Compiler Platform code style rules ####
 
 ### Language rules ###

Directory.Build.props

@@ -18,7 +18,7 @@
         Code analysis properties.
     -->
     <PropertyGroup>
-        <EnableNETAnalyzers>false</EnableNETAnalyzers>
+        <EnableNETAnalyzers>true</EnableNETAnalyzers>
         <AnalysisLevel>preview-All</AnalysisLevel>
         <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
     </PropertyGroup>
@@ -34,11 +34,9 @@
         Use fixed version of analyzers.
     -->
     <ItemGroup>
-        <!--
         <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0-preview1.23165.1" PrivateAssets="all" />
         <PackageReference Include="StyleCop.Analyzers" Version="1.2.0-beta.435" PrivateAssets="all" />
-        <PackageReference Include="Meziantou.Analyzer" Version="2.0.54" PrivateAssets="all" />
-        <PackageReference Include="SonarAnalyzer.CSharp" Version="8.55.0.65544" PrivateAssets="all" />
-        -->        
+        <PackageReference Include="Meziantou.Analyzer" Version="2.0.103" PrivateAssets="all" />
+        <PackageReference Include="SonarAnalyzer.CSharp" Version="9.12.0.78982" PrivateAssets="all" />
     </ItemGroup>
 </Project>

src/Renci.SshNet/.editorconfig

@@ -1,42 +1,161 @@
 [*.cs]
 
+#### Sonar rules ####
+
+# S1264: A "while" loop should be used instead of a "for" loop
+# https://rules.sonarsource.com/csharp/RSPEC-1264
+dotnet_diagnostic.S1264.severity = none
+
+# S1450: Private fields only used as local variables in methods should become local variables
+# https://rules.sonarsource.com/csharp/RSPEC-1450
+#
+# TODO: Re-enable when the following issue is resolved:
+# https://github.com/SonarSource/sonar-dotnet/issues/8239
+dotnet_diagnostic.S1450.severity = none
+
+# S2372: Exceptions should not be thrown from property getters
+# https://rules.sonarsource.com/csharp/RSPEC-2372/
+dotnet_diagnostic.S2372.severity = none
+
+# S2583: Conditionally executed code should be reachable
+# https://rules.sonarsource.com/csharp/RSPEC-2583/
+#
+# TODO: Re-enable when the following issue is resolved:
+# https://github.com/SonarSource/sonar-dotnet/issues/8264
+dotnet_diagnostic.S2583.severity = none
+
+# S2589: Boolean expressions should not be gratuitous
+# https://rules.sonarsource.com/csharp/RSPEC-2589/
+#
+# TODO: Re-enable when the following issue is resolved:
+# https://github.com/SonarSource/sonar-dotnet/issues/8262
+dotnet_diagnostic.S2589.severity = none
+
+dotnet_diagnostic.S2372.severity = none
+
 #### SYSLIB diagnostics ####
 
 # SYSLIB1045: Use 'GeneratedRegexAttribute' to generate the regular expression implementation at compile-time
 #
 # TODO: Remove this when https://github.com/sshnet/SSH.NET/issues/1131 is implemented.
 dotnet_diagnostic.SYSLIB1045.severity = none
 
-### StyleCop Analyzers rules ###
+#### StyleCop Analyzers rules ####
+
+# SA1123: Do not place regions within elements
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1123.md
+dotnet_diagnostic.SA1123.severity = none
+
+# SA1124: Do not use regions
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1124.md
+dotnet_diagnostic.SA1124.severity = none
 
 # SA1202: Elements must be ordered by access
 # https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1202.md
 dotnet_diagnostic.SA1202.severity = none
 
+# SA1204: Static elements must appear before instance elements
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1204.md
+dotnet_diagnostic.SA1204.severity = none
+
+# SA1310: Field names must not contain underscore
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1310.md
+#dotnet_diagnostic.SA1310.severity = none
+
+# SA1312: Variable names should begin with lower-case letter
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1312.md
+dotnet_diagnostic.SA1312.severity = none
+
+# SA1636: File header copyright text should match
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1636.md
+dotnet_diagnostic.SA1636.severity = none
+
+# SA1643: Destructor summary documentation must begin with standard text
+# https://github.com/DotNetAnalyzers/StyleCopAnalyzers/blob/master/documentation/SA1643.md
+dotnet_diagnostic.SA1643.severity = none
+
 #### Meziantou.Analyzer rules ####
 
+# MA0001: StringComparison is missing
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0001.md
+dotnet_diagnostic.MA0001.severity = none
+
+# MA0011: IFormatProvider is missing
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0011.md
+#
+# TODO: Remove exclusion when issues are fixed
+dotnet_diagnostic.MA0011.severity = none
+
+# MA0015: Specify the parameter name in ArgumentException
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0015.md
+#
+# TODO: Remove exclusion when issues are fixed
+dotnet_diagnostic.MA0015.severity = none
+
+# MA0050: Validate arguments correctly in iterator methods
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0050.md
+#
+# TODO: Re-enable when https://github.com/meziantou/Meziantou.Analyzer/issues/617 is fixed
+dotnet_diagnostic.MA0050.severity = none
+
 # MA0053: Make class sealed
 # https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0053.md
 MA0053.public_class_should_be_sealed = false
 
+# MA0055: Do not use finalizer
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0055.md
+#
+# TODO: Remove exclusion when issues are fixed
+dotnet_diagnostic.MA0055.severity = none
+
+# MA0110: Use the Regex source generator
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0110.md
+dotnet_diagnostic.MA0110.severity = none
+
 #### .NET Compiler Platform analysers rules ####
 
 # CA1030: Use events where appropriate
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1030
-dotnet_diagnostic.CA10310.severity = none
+dotnet_diagnostic.CA1030.severity = none
 
 # CA1031: Do not catch general exception types
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1031
 dotnet_diagnostic.CA1031.severity = none
 
+# CA1062: Validate arguments of public methods
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1062
+#
+# TODO: Remove exclusion when issues are fixed
+dotnet_diagnostic.CA1062.severity = none
+
+# CA1307: Specify StringComparison for clarity
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1307
+dotnet_diagnostic.CA1307.severity = none
+
+# CA1716: Identifiers should not match keywords
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca1716
+dotnet_diagnostic.CA1716.severity = none
+
+# CA1822: Mark members as static
+# https://learn.microsoft.com/en-US/dotnet/fundamentals/code-analysis/quality-rules/ca1822
+dotnet_code_quality.CA1822.api_surface = private,internal
+
 # CA2213: Disposable fields should be disposed
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2213
 dotnet_diagnostic.CA2213.severity = none
 
+# CA3075: Insecure DTD Processing
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca3075
+dotnet_diagnostic.CA3075.severity = none
+
 # IDE0004: Types that own disposable fields should be disposable
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0004
 dotnet_diagnostic.IDE0004.severity = none
 
 # IDE0048: Add parentheses for clarity
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047
 dotnet_diagnostic.IDE0048.severity = none
+
+# IDE0305: Collection initialization can be simplified
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0305
+dotnet_diagnostic.IDE0305.severity = none

src/Renci.SshNet/Abstractions/DiagnosticAbstraction.cs

@@ -22,11 +22,7 @@ public static bool IsEnabled(TraceEventType traceEventType)
         public static void Log(string text)
         {
             Loggging.TraceEvent(TraceEventType.Verbose,
-#if NET6_0_OR_GREATER
                                 System.Environment.CurrentManagedThreadId,
-#else
-                                System.Threading.Thread.CurrentThread.ManagedThreadId,
-#endif // NET6_0_OR_GREATER
                                 text);
         }
     }

src/Renci.SshNet/Abstractions/DnsAbstraction.cs

@@ -25,7 +25,7 @@ internal static class DnsAbstraction
         /// <summary>
         /// Returns the Internet Protocol (IP) addresses for the specified host.
         /// </summary>
-        /// <param name="hostNameOrAddress">The host name or IP address to resolve</param>
+        /// <param name="hostNameOrAddress">The host name or IP address to resolve.</param>
         /// <returns>
         /// An array of type <see cref="IPAddress"/> that holds the IP addresses for the host that
         /// is specified by the <paramref name="hostNameOrAddress"/> parameter.
@@ -34,7 +34,7 @@ internal static class DnsAbstraction
         /// <exception cref="SocketException">An error is encountered when resolving <paramref name="hostNameOrAddress"/>.</exception>
         public static IPAddress[] GetHostAddresses(string hostNameOrAddress)
         {
-            // TODO Eliminate sync variant, and implement timeout
+            /* TODO Eliminate sync variant, and implement timeout */
 
 #if FEATURE_DNS_SYNC
             return Dns.GetHostAddresses(hostNameOrAddress);
@@ -92,7 +92,7 @@ public static IPAddress[] GetHostAddresses(string hostNameOrAddress)
         /// <summary>
         /// Returns the Internet Protocol (IP) addresses for the specified host.
         /// </summary>
-        /// <param name="hostNameOrAddress">The host name or IP address to resolve</param>
+        /// <param name="hostNameOrAddress">The host name or IP address to resolve.</param>
         /// <returns>
         /// A task with result of an array of type <see cref="IPAddress"/> that holds the IP addresses for the host that
         /// is specified by the <paramref name="hostNameOrAddress"/> parameter.

src/Renci.SshNet/Abstractions/ReflectionAbstraction.cs

@@ -7,7 +7,7 @@ namespace Renci.SshNet.Abstractions
     internal static class ReflectionAbstraction
     {
         public static IEnumerable<T> GetCustomAttributes<T>(this Type type, bool inherit)
-            where T:Attribute
+            where T : Attribute
         {
             var attributes = type.GetCustomAttributes(typeof(T), inherit);
             return attributes.Cast<T>();

src/Renci.SshNet/Abstractions/SocketAbstraction.cs

@@ -261,11 +261,6 @@ public static byte[] Read(Socket socket, int size, TimeSpan timeout)
             return buffer;
         }
 
-        public static Task<int> ReadAsync(Socket socket, byte[] buffer, int offset, int length, CancellationToken cancellationToken)
-        {
-            return socket.ReceiveAsync(buffer, offset, length, cancellationToken);
-        }
-
         /// <summary>
         /// Receives data from a bound <see cref="Socket"/> into a receive buffer.
         /// </summary>
@@ -293,7 +288,7 @@ public static int Read(Socket socket, byte[] buffer, int offset, int size, TimeS
             var totalBytesRead = 0;
             var totalBytesToRead = size;
 
-            socket.ReceiveTimeout = (int)readTimeout.TotalMilliseconds;
+            socket.ReceiveTimeout = (int) readTimeout.TotalMilliseconds;
 
             do
             {
@@ -330,6 +325,11 @@ public static int Read(Socket socket, byte[] buffer, int offset, int size, TimeS
             return totalBytesRead;
         }
 
+        public static Task<int> ReadAsync(Socket socket, byte[] buffer, int offset, int length, CancellationToken cancellationToken)
+        {
+            return socket.ReceiveAsync(buffer, offset, length, cancellationToken);
+        }
+
         public static void Send(Socket socket, byte[] data)
         {
             Send(socket, data, 0, data.Length);