Cleanup and muting of analyzer warnings (#1357)

* Fixed warnings in tests.

* Fixed warnings in main project.

* Fix public key.

* Further warning cleanup.

* Muted remaining warnings.

* Final cleanup.

* Fix warning.

* Fix test classes.

* Fix more internals.

* Revert "internal" in test projects

* Revert more internals.

* Revert all non-analyzer changes.

---------

Co-authored-by: Rob Hague <[email protected]>

Author: jscarle

Renci.SshNet.sln

@@ -29,7 +29,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{D21A4D03-0AC2-4613-BB6D-74D2D16A72CC}"
 	ProjectSection(SolutionItems) = preProject
-		test\.editorconfig = test\.editorconfig
 		test\Directory.Build.props = test\Directory.Build.props
 	EndProjectSection
 EndProject

src/Renci.SshNet/.editorconfig

@@ -108,10 +108,18 @@ MA0053.public_class_should_be_sealed = false
 # TODO: Remove exclusion when issues are fixed
 dotnet_diagnostic.MA0055.severity = none
 
+# MA0089: Optimize string method usage
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0089.md
+dotnet_diagnostic.MA0089.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
 
+# MA0026: Fix TODO comment
+# https://github.com/meziantou/Meziantou.Analyzer/blob/main/docs/Rules/MA0026.md
+dotnet_diagnostic.MA0026.severity = none
+
 #### .NET Compiler Platform analysers rules ####
 
 # CA1030: Use events where appropriate
@@ -152,10 +160,30 @@ dotnet_diagnostic.CA3075.severity = none
 # https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0004
 dotnet_diagnostic.IDE0004.severity = none
 
+# IDE0047: Remove unnecessary parentheses
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047-ide0048
+dotnet_diagnostic.IDE0047.severity = none
+
 # IDE0048: Add parentheses for clarity
-# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0047-ide0048
 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
+
+# IDE0046: Use conditional expression for return
+# https://learn.microsoft.com/en-ca/dotnet/fundamentals/code-analysis/style-rules/ide0046
+dotnet_diagnostic.IDE0046.severity = none
+
+# IDE0032: Use auto-implemented property
+# https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0032
+dotnet_diagnostic.IDE0032.severity = none
+
+# CA5350: Do Not Use Weak Cryptographic Algorithms
+# https://learn.microsoft.com/en-ca/dotnet/fundamentals/code-analysis/quality-rules/ca5350
+dotnet_diagnostic.CA5350.severity = none
+
+# CA5351: Do Not Use Broken Cryptographic Algorithms
+# https://learn.microsoft.com/en-ca/dotnet/fundamentals/code-analysis/quality-rules/ca5351
+dotnet_diagnostic.CA5351.severity = none

src/Renci.SshNet/Abstractions/CryptoAbstraction.cs

@@ -39,16 +39,12 @@ public static System.Security.Cryptography.RandomNumberGenerator CreateRandomNum
 
         public static System.Security.Cryptography.MD5 CreateMD5()
         {
-#pragma warning disable CA5351 // Do not use broken cryptographic algorithms
             return System.Security.Cryptography.MD5.Create();
-#pragma warning restore CA5351 // Do not use broken cryptographic algorithms
         }
 
         public static System.Security.Cryptography.SHA1 CreateSHA1()
         {
-#pragma warning disable CA5350 // Do not use weak cryptographic algorithms
             return System.Security.Cryptography.SHA1.Create();
-#pragma warning restore CA5350 // Do not use weak cryptographic algorithms
         }
 
         public static System.Security.Cryptography.SHA256 CreateSHA256()
@@ -68,30 +64,22 @@ public static System.Security.Cryptography.SHA512 CreateSHA512()
 
         public static System.Security.Cryptography.HMACMD5 CreateHMACMD5(byte[] key)
         {
-#pragma warning disable CA5351 // Do not use broken cryptographic algorithms
             return new System.Security.Cryptography.HMACMD5(key);
-#pragma warning restore CA5351 // Do not use broken cryptographic algorithms
         }
 
         public static HMACMD5 CreateHMACMD5(byte[] key, int hashSize)
         {
-#pragma warning disable CA5351 // Do not use broken cryptographic algorithms
             return new HMACMD5(key, hashSize);
-#pragma warning restore CA5351 // Do not use broken cryptographic algorithms
         }
 
         public static System.Security.Cryptography.HMACSHA1 CreateHMACSHA1(byte[] key)
         {
-#pragma warning disable CA5350 // Do not use weak cryptographic algorithms
             return new System.Security.Cryptography.HMACSHA1(key);
-#pragma warning restore CA5350 // Do not use weak cryptographic algorithms
         }
 
         public static HMACSHA1 CreateHMACSHA1(byte[] key, int hashSize)
         {
-#pragma warning disable CA5350 // Do not use weak cryptographic algorithms
             return new HMACSHA1(key, hashSize);
-#pragma warning restore CA5350 // Do not use weak cryptographic algorithms
         }
 
         public static System.Security.Cryptography.HMACSHA256 CreateHMACSHA256(byte[] key)

src/Renci.SshNet/Abstractions/SocketExtensions.cs

@@ -50,9 +50,7 @@ public void SetCancelled()
                 SetCompleted();
             }
 
-#pragma warning disable S1144 // Unused private types or members should be removed
             public AwaitableSocketAsyncEventArgs GetAwaiter()
-#pragma warning restore S1144 // Unused private types or members should be removed
             {
                 return this;
             }
@@ -68,9 +66,7 @@ void INotifyCompletion.OnCompleted(Action continuation)
                 }
             }
 
-#pragma warning disable S1144 // Unused private types or members should be removed
             public void GetResult()
-#pragma warning restore S1144 // Unused private types or members should be removed
             {
                 if (_isCancelled)
                 {

src/Renci.SshNet/Common/BigInteger.cs

@@ -1,5 +1,4 @@
 #pragma warning disable SA1028 // Code should not contain trailing whitespace
-#pragma warning disable SA1515 // Single-line comment should be preceded by blank line
 //
 // System.Numerics.BigInteger
 //
@@ -46,7 +45,6 @@
 *
 *
 * ***************************************************************************/
-#pragma warning restore SA1515 // Single-line comment should be preceded by blank line
 #pragma warning restore SA1028 // Code should not contain trailing whitespace
 
 using System;
@@ -4578,9 +4576,7 @@ public readonly byte[] ToByteArray()
 
                     if (needExtra)
                     {
-#pragma warning disable S1854 // Unused assignments should be removed
                         res[j++] = 0xFF;
-#pragma warning restore S1854 // Unused assignments should be removed
                     }
                 }
                 else
@@ -4590,9 +4586,7 @@ public readonly byte[] ToByteArray()
                     res[j++] = (byte)(word >> 8);
                     res[j++] = (byte)(word >> 16);
                     res[j++] = (byte)(word >> 24);
-#pragma warning disable S1854 // Unused assignments should be removed
                     res[j++] = 0xFF;
-#pragma warning restore S1854 // Unused assignments should be removed
                 }
             }
 

src/Renci.SshNet/Compression/ZlibStream.cs

@@ -54,5 +54,4 @@ public void Write(byte[] buffer, int offset, int count)
     }
 }
 
-#pragma warning restore SA1005 // Single line comments should begin with single space
 #pragma warning restore S125 // Sections of code should not be commented out

src/Renci.SshNet/ForwardedPort.cs

@@ -71,9 +71,7 @@ public virtual void Start()
         /// <summary>
         /// Stops port forwarding.
         /// </summary>
-#pragma warning disable CA1716 // Identifiers should not match keywords
         public virtual void Stop()
-#pragma warning restore CA1716 // Identifiers should not match keywords
         {
             if (IsStarted)
             {

src/Renci.SshNet/ISftpClient.cs

@@ -592,9 +592,7 @@ public interface ISftpClient : IBaseClient
         /// <exception cref="SftpPathNotFoundException"><paramref name="path"/> was not found on the remote host.</exception>
         /// <exception cref="ArgumentNullException"><paramref name="path" /> is <see langword="null"/>.</exception>
         /// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
-#pragma warning disable CA1716 // Identifiers should not match keywords
         ISftpFile Get(string path);
-#pragma warning restore CA1716 // Identifiers should not match keywords
 
         /// <summary>
         /// Gets the <see cref="SftpFileAttributes"/> of the file on the path.

src/Renci.SshNet/KeyboardInteractiveAuthenticationMethod.cs

@@ -15,7 +15,7 @@ namespace Renci.SshNet
     /// </summary>
     public class KeyboardInteractiveAuthenticationMethod : AuthenticationMethod, IDisposable
     {
-        private readonly RequestMessage _requestMessage;
+        private readonly RequestMessageKeyboardInteractive _requestMessage;
         private AuthenticationResult _authenticationResult = AuthenticationResult.Failure;
         private Session _session;
         private EventWaitHandle _authenticationCompleted = new AutoResetEvent(initialState: false);

src/Renci.SshNet/Messages/Authentication/FailureMessage.cs

@@ -68,9 +68,7 @@ protected override void LoadData()
         /// </summary>
         protected override void SaveData()
         {
-#pragma warning disable MA0025 // Implement the functionality instead of throwing NotImplementedException
             throw new NotImplementedException();
-#pragma warning restore MA0025 // Implement the functionality instead of throwing NotImplementedException
         }
 
         internal override void Process(Session session)
@@ -81,9 +79,7 @@ internal override void Process(Session session)
         /// <inheritdoc/>
         public override string ToString()
         {
-#pragma warning disable MA0089 // Optimize string method usage
             return $"SSH_MSG_USERAUTH_FAILURE {string.Join(",", AllowedAuthentications)} ({nameof(PartialSuccess)}:{PartialSuccess})";
-#pragma warning restore MA0089 // Optimize string method usage
         }
     }
 }

src/Renci.SshNet/PasswordAuthenticationMethod.cs

@@ -15,7 +15,7 @@ namespace Renci.SshNet
     /// </summary>
     public class PasswordAuthenticationMethod : AuthenticationMethod, IDisposable
     {
-        private readonly RequestMessage _requestMessage;
+        private readonly RequestMessagePassword _requestMessage;
         private readonly byte[] _password;
         private AuthenticationResult _authenticationResult = AuthenticationResult.Failure;
         private Session _session;

src/Renci.SshNet/PrivateKeyAuthenticationMethod.cs

@@ -17,9 +17,7 @@ public class PrivateKeyAuthenticationMethod : AuthenticationMethod, IDisposable
     {
         private AuthenticationResult _authenticationResult = AuthenticationResult.Failure;
         private EventWaitHandle _authenticationCompleted = new ManualResetEvent(initialState: false);
-#pragma warning disable S1450 // Private fields only used as local variables in methods should become local variables
         private bool _isSignatureRequired;
-#pragma warning restore S1450 // Private fields only used as local variables in methods should become local variables
         private bool _isDisposed;
 
         /// <summary>

src/Renci.SshNet/Security/Cryptography/EcdsaDigitalSignature.cs

@@ -1,8 +1,5 @@
 using System;
 using System.Globalization;
-#if NETFRAMEWORK
-using System.Security.Cryptography;
-#endif // NETFRAMEWORK
 
 using Renci.SshNet.Common;
 
@@ -45,7 +42,7 @@ public override bool Verify(byte[] input, byte[] signature)
             var sig_size = _key.KeyLength == 521 ? 132 : _key.KeyLength / 4;
             var ssh_data = new SshDataSignature(signature, sig_size);
 #if NETFRAMEWORK
-            var ecdsa = (ECDsaCng)_key.Ecdsa;
+            var ecdsa = _key.Ecdsa;
             ecdsa.HashAlgorithm = _key.HashAlgorithm;
             return ecdsa.VerifyData(input, ssh_data.Signature);
 #else
@@ -63,7 +60,7 @@ public override bool Verify(byte[] input, byte[] signature)
         public override byte[] Sign(byte[] input)
         {
 #if NETFRAMEWORK
-            var ecdsa = (ECDsaCng)_key.Ecdsa;
+            var ecdsa = _key.Ecdsa;
             ecdsa.HashAlgorithm = _key.HashAlgorithm;
             var signed = ecdsa.SignData(input);
 #else

src/Renci.SshNet/Security/Cryptography/EcdsaKey.cs

@@ -221,10 +221,17 @@ public override BigInteger[] Public
         /// </summary>
         public byte[] PrivateKey { get; private set; }
 
+#if NETFRAMEWORK
+        /// <summary>
+        /// Gets the <see cref="ECDsa"/> object.
+        /// </summary>
+        public ECDsaCng Ecdsa { get; private set; }
+#else
         /// <summary>
         /// Gets the <see cref="ECDsa"/> object.
         /// </summary>
         public ECDsa Ecdsa { get; private set; }
+#endif
 
         /// <summary>
         /// Initializes a new instance of the <see cref="EcdsaKey"/> class.
@@ -371,7 +378,7 @@ private void Import(string curve_oid, byte[] publickey, byte[] privatekey)
                 PrivateKey = privatekey;
             }
 
-            var headerSize = Marshal.SizeOf(typeof(BCRYPT_ECCKEY_BLOB));
+            var headerSize = Marshal.SizeOf<BCRYPT_ECCKEY_BLOB>();
             var blobSize = headerSize + qx.Length + qy.Length;
             if (privatekey != null)
             {

src/Renci.SshNet/Security/Cryptography/HMACMD5.cs

@@ -15,9 +15,7 @@ public class HMACMD5 : System.Security.Cryptography.HMACMD5
         /// </summary>
         /// <param name="key">The key.</param>
         public HMACMD5(byte[] key)
-#pragma warning disable CA5351 // Do Not Use Broken Cryptographic Algorithms
             : base(key)
-#pragma warning restore CA5351 // Do Not Use Broken Cryptographic Algorithms
         {
 #pragma warning disable MA0056 // Do not call overridable members in constructor
             _hashSize = base.HashSize;
@@ -31,9 +29,7 @@ public HMACMD5(byte[] key)
         /// <param name="key">The key.</param>
         /// <param name="hashSize">The size, in bits, of the computed hash code.</param>
         public HMACMD5(byte[] key, int hashSize)
-#pragma warning disable CA5351 // Do Not Use Broken Cryptographic Algorithms
             : base(key)
-#pragma warning restore CA5351 // Do Not Use Broken Cryptographic Algorithms
         {
             _hashSize = hashSize;
         }
@@ -57,9 +53,7 @@ public override int HashSize
         /// </returns>
         protected override byte[] HashFinal()
         {
-#pragma warning disable CA5351 // Do Not Use Broken Cryptographic Algorithms
             var hash = base.HashFinal();
-#pragma warning restore CA5351 // Do Not Use Broken Cryptographic Algorithms
             return hash.Take(HashSize / 8);
         }
     }

src/Renci.SshNet/Security/Cryptography/HMACSHA1.cs

@@ -16,9 +16,7 @@ public class HMACSHA1 : System.Security.Cryptography.HMACSHA1
         /// </summary>
         /// <param name="key">The key.</param>
         public HMACSHA1(byte[] key)
-#pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms
             : base(key)
-#pragma warning restore CA5350 // Do Not Use Weak Cryptographic Algorithms
         {
 #pragma warning disable MA0056 // Do not call overridable members in constructor
             _hashSize = base.HashSize;
@@ -31,9 +29,7 @@ public HMACSHA1(byte[] key)
         /// <param name="key">The key.</param>
         /// <param name="hashSize">The size, in bits, of the computed hash code.</param>
         public HMACSHA1(byte[] key, int hashSize)
-#pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms
             : base(key)
-#pragma warning restore CA5350 // Do Not Use Weak Cryptographic Algorithms
         {
             _hashSize = hashSize;
         }
@@ -57,9 +53,7 @@ public override int HashSize
         /// </returns>
         protected override byte[] HashFinal()
         {
-#pragma warning disable CA5350 // Do Not Use Weak Cryptographic Algorithms
             var hash = base.HashFinal();
-#pragma warning restore CA5350 // Do Not Use Weak Cryptographic Algorithms
             return hash.Take(HashSize / 8);
         }
     }

src/Renci.SshNet/Sftp/SftpFileStream.cs

@@ -13,9 +13,10 @@ namespace Renci.SshNet.Sftp
     /// Exposes a <see cref="Stream"/> around a remote SFTP file, supporting both synchronous and asynchronous read and write operations.
     /// </summary>
     /// <threadsafety static="true" instance="false"/>
-#pragma warning disable CA1844 // Provide memory-based overrides of async methods when subclassing 'Stream'
+#pragma warning disable IDE0079 // We intentionally want to suppress the below warning.
+    [SuppressMessage("Performance", "CA1844: Provide memory-based overrides of async methods when subclassing 'Stream'", Justification = "TODO: This should be addressed in the future.")]
+#pragma warning restore IDE0079
     public class SftpFileStream : Stream
-#pragma warning restore CA1844 // Provide memory-based overrides of async methods when subclassing 'Stream'
     {
         private readonly object _lock = new object();
         private readonly int _readBufferSize;
@@ -88,7 +89,6 @@ public override bool CanTimeout
         /// <exception cref="NotSupportedException">A class derived from Stream does not support seeking. </exception>
         /// <exception cref="ObjectDisposedException">Methods were called after the stream was closed. </exception>
         /// <exception cref="IOException">IO operation failed. </exception>
-        [SuppressMessage("Microsoft.Design", "CA1065:DoNotRaiseExceptionsInUnexpectedLocations", Justification = "Be design this is the exception that stream need to throw.")]
         public override long Length
         {
             get