Code Cleanup | Fixing Inconsistent Modifiers

.editorconfig

@@ -132,6 +132,9 @@ csharp_space_between_method_declaration_parameter_list_parentheses = false
 csharp_space_between_parentheses = false
 csharp_space_between_square_brackets = false
 
+# Ensure consistent member modifier order
+csharp_preferred_modifier_order = public,private,protected,internal,static,extern,new,virtual,abstract,sealed,override,readonly,unsafe,volatile,async:warning
+
 # Xml project files
 
 # CA1063: Implement IDisposable Correctly

src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/SqlColumnEncryptionAzureKeyVaultProvider.cs

@@ -51,9 +51,9 @@ public class SqlColumnEncryptionAzureKeyVaultProvider : SqlColumnEncryptionKeySt
         /// <summary>
         /// Algorithm version
         /// </summary>
-        private readonly static byte[] s_firstVersion = new byte[] { 0x01 };
+        private static readonly byte[] s_firstVersion = new byte[] { 0x01 };
 
-        private readonly static KeyWrapAlgorithm s_keyWrapAlgorithm = KeyWrapAlgorithm.RsaOaep;
+        private static readonly KeyWrapAlgorithm s_keyWrapAlgorithm = KeyWrapAlgorithm.RsaOaep;
 
         /// <summary>
         /// List of Trusted Endpoints

src/Microsoft.Data.SqlClient/netcore/ref/Microsoft.Data.SqlClient.Manual.cs

@@ -8,13 +8,13 @@ public sealed partial class SqlCommand : System.Data.Common.DbCommand
     {
         // SqlCommand expects IDisposable methods to be implemented via System.ComponentModel.Component, which it no longer inherits from
         /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlCommand.xml' path='docs/members[@name="SqlCommand"]/Dispose/*'/>
-        override protected void Dispose(bool disposing) { }
+        protected override void Dispose(bool disposing) { }
     }
     public sealed partial class SqlConnection : System.Data.Common.DbConnection
     {
         // SqlConnection expects IDisposable methods to be implemented via System.ComponentModel.Component, which it no longer inherits from
         /// <include file='../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/Dispose/*'/>
-        override protected void Dispose(bool disposing) { }
+        protected override void Dispose(bool disposing) { }
     }
     [System.ComponentModel.TypeConverter(typeof(SqlParameterConverter))]
     public sealed partial class SqlParameter

src/Microsoft.Data.SqlClient/netcore/src/Common/src/CoreLib/Microsoft/Win32/SafeHandles/SafeLibraryHandle.cs

@@ -4,11 +4,11 @@
 
 namespace Microsoft.Win32.SafeHandles
 {
-    sealed internal class SafeLibraryHandle : SafeHandleZeroOrMinusOneIsInvalid
+    internal sealed class SafeLibraryHandle : SafeHandleZeroOrMinusOneIsInvalid
     {
         internal SafeLibraryHandle() : base(true) { }
 
-        override protected bool ReleaseHandle()
+        protected override bool ReleaseHandle()
         {
             return Interop.Kernel32.FreeLibrary(handle);
         }

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Interop/Windows/NtDll/Interop.NtCreateFile.cs

@@ -12,7 +12,7 @@ internal partial class NtDll
         // https://msdn.microsoft.com/en-us/library/bb432380.aspx
         // https://msdn.microsoft.com/en-us/library/windows/hardware/ff566424.aspx
         [DllImport(Libraries.NtDll, CharSet = CharSet.Unicode, ExactSpelling = true)]
-        private unsafe static extern int NtCreateFile(
+        private static extern unsafe int NtCreateFile(
             out IntPtr FileHandle,
             DesiredAccess DesiredAccess,
             ref OBJECT_ATTRIBUTES ObjectAttributes,
@@ -25,7 +25,7 @@ private unsafe static extern int NtCreateFile(
             void* EaBuffer,
             uint EaLength);
 
-        internal unsafe static (int status, IntPtr handle) CreateFile(
+        internal static unsafe (int status, IntPtr handle) CreateFile(
             ReadOnlySpan<char> path,
             IntPtr rootDirectory,
             CreateDisposition createDisposition,

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Interop/Windows/NtDll/Interop.RtlNtStatusToDosError.cs

@@ -11,7 +11,7 @@ internal partial class NtDll
     {
         // https://msdn.microsoft.com/en-us/library/windows/desktop/ms680600(v=vs.85).aspx
         [DllImport(Libraries.NtDll, ExactSpelling = true)]
-        public unsafe static extern uint RtlNtStatusToDosError(
+        public static extern unsafe uint RtlNtStatusToDosError(
             int Status);
     }
 }

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Interop/Windows/kernel32/Interop.CTL_CODE.cs

@@ -13,7 +13,7 @@ internal partial class Kernel32
         /// <param name="function">Identifies the function to be performed by the driver. Values of less than 0x800 are reserved for Microsoft. Values of 0x800 and higher can be used by vendors.</param>
         /// <param name="method">Indicates how the system will pass data between the caller of DeviceIoControl (or IoBuildDeviceIoControlRequest) and the driver that handles the IRP.</param>
         /// <param name="access">Indicates the type of access that a caller must request when opening the file object that represents the device (see IRP_MJ_CREATE).</param>
-        internal unsafe static uint CTL_CODE(
+        internal static unsafe uint CTL_CODE(
             ushort deviceType,
             ushort function,
             byte method,

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbConnectionClosed.cs

@@ -10,7 +10,7 @@
 
 namespace Microsoft.Data.ProviderBase
 {
-    abstract internal partial class DbConnectionClosed : DbConnectionInternal
+    internal abstract partial class DbConnectionClosed : DbConnectionInternal
     {
         // Construct an "empty" connection
         protected DbConnectionClosed(ConnectionState state, bool hidePassword, bool allowSetConnectionString) : base(state, hidePassword, allowSetConnectionString)
@@ -39,7 +39,7 @@ internal override bool TryOpenConnection(DbConnection outerConnection, DbConnect
             => base.TryOpenConnectionInternal(outerConnection, connectionFactory, retry, userOptions);
     }
 
-    abstract internal class DbConnectionBusy : DbConnectionClosed
+    internal abstract class DbConnectionBusy : DbConnectionClosed
     {
         protected DbConnectionBusy(ConnectionState state) : base(state, true, false)
         {
@@ -49,7 +49,7 @@ internal override bool TryOpenConnection(DbConnection outerConnection, DbConnect
             => throw ADP.ConnectionAlreadyOpen(State);
     }
 
-    sealed internal class DbConnectionClosedBusy : DbConnectionBusy
+    internal sealed class DbConnectionClosedBusy : DbConnectionBusy
     {
         // Closed Connection, Currently Busy - changing connection string
         internal static readonly DbConnectionInternal SingletonInstance = new DbConnectionClosedBusy();   // singleton object
@@ -59,7 +59,7 @@ private DbConnectionClosedBusy() : base(ConnectionState.Closed)
         }
     }
 
-    sealed internal class DbConnectionOpenBusy : DbConnectionBusy
+    internal sealed class DbConnectionOpenBusy : DbConnectionBusy
     {
         // Open Connection, Currently Busy - closing connection
         internal static readonly DbConnectionInternal SingletonInstance = new DbConnectionOpenBusy();   // singleton object
@@ -69,7 +69,7 @@ private DbConnectionOpenBusy() : base(ConnectionState.Open)
         }
     }
 
-    sealed internal class DbConnectionClosedConnecting : DbConnectionBusy
+    internal sealed class DbConnectionClosedConnecting : DbConnectionBusy
     {
         // Closed Connection, Currently Connecting
 
@@ -113,7 +113,7 @@ internal override bool TryOpenConnection(DbConnection outerConnection, DbConnect
         }
     }
 
-    sealed internal class DbConnectionClosedNeverOpened : DbConnectionClosed
+    internal sealed class DbConnectionClosedNeverOpened : DbConnectionClosed
     {
         // Closed Connection, Has Never Been Opened
 
@@ -124,7 +124,7 @@ private DbConnectionClosedNeverOpened() : base(ConnectionState.Closed, false, tr
         }
     }
 
-    sealed internal class DbConnectionClosedPreviouslyOpened : DbConnectionClosed
+    internal sealed class DbConnectionClosedPreviouslyOpened : DbConnectionClosed
     {
         // Closed Connection, Has Previously Been Opened
 

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbConnectionFactory.cs

@@ -41,7 +41,7 @@ protected DbConnectionFactory()
         }
 
 
-        abstract public DbProviderFactory ProviderFactory
+        public abstract DbProviderFactory ProviderFactory
         {
             get;
         }
@@ -130,7 +130,7 @@ internal DbConnectionInternal CreatePooledConnection(DbConnectionPool pool, DbCo
             return newConnection;
         }
 
-        virtual internal DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo(DbConnectionOptions connectionOptions)
+        internal virtual DbConnectionPoolGroupProviderInfo CreateConnectionPoolGroupProviderInfo(DbConnectionOptions connectionOptions)
         {
             return null;
         }
@@ -412,7 +412,7 @@ internal void QueuePoolGroupForRelease(DbConnectionPoolGroup poolGroup)
             SqlClientEventSource.Log.ExitActiveConnectionPoolGroup();
         }
 
-        virtual protected DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection, DbConnectionOptions userOptions)
+        protected virtual DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection, DbConnectionOptions userOptions)
         {
             return CreateConnection(options, poolKey, poolGroupProviderInfo, pool, owningConnection);
         }
@@ -447,29 +447,29 @@ protected virtual DbMetaDataFactory CreateMetaDataFactory(DbConnectionInternal i
             throw ADP.NotSupported();
         }
 
-        abstract protected DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection);
+        protected abstract DbConnectionInternal CreateConnection(DbConnectionOptions options, DbConnectionPoolKey poolKey, object poolGroupProviderInfo, DbConnectionPool pool, DbConnection owningConnection);
 
-        abstract protected DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous);
+        protected abstract DbConnectionOptions CreateConnectionOptions(string connectionString, DbConnectionOptions previous);
 
-        abstract protected DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options);
+        protected abstract DbConnectionPoolGroupOptions CreateConnectionPoolGroupOptions(DbConnectionOptions options);
 
-        abstract internal DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection);
+        internal abstract DbConnectionPoolGroup GetConnectionPoolGroup(DbConnection connection);
 
-        abstract internal DbConnectionInternal GetInnerConnection(DbConnection connection);
+        internal abstract DbConnectionInternal GetInnerConnection(DbConnection connection);
 
-        abstract protected int GetObjectId(DbConnection conne);
+        protected abstract int GetObjectId(DbConnection conne);
 
-        abstract internal void PermissionDemand(DbConnection outerConnection);
+        internal abstract void PermissionDemand(DbConnection outerConnection);
 
-        abstract internal void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup);
+        internal abstract void SetConnectionPoolGroup(DbConnection outerConnection, DbConnectionPoolGroup poolGroup);
 
-        abstract internal void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to);
+        internal abstract void SetInnerConnectionEvent(DbConnection owningObject, DbConnectionInternal to);
 
-        abstract internal bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from);
+        internal abstract bool SetInnerConnectionFrom(DbConnection owningObject, DbConnectionInternal to, DbConnectionInternal from);
 
-        abstract internal void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to);
+        internal abstract void SetInnerConnectionTo(DbConnection owningObject, DbConnectionInternal to);
 
-        virtual internal void Unload()
+        internal virtual void Unload()
         {
             _pruningTimer.Dispose();
         }

src/Microsoft.Data.SqlClient/netcore/src/Common/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs

@@ -146,13 +146,13 @@ protected internal DbReferenceCollection ReferenceCollection
             }
         }
 
-        abstract public string ServerVersion
+        public abstract string ServerVersion
         {
             get;
         }
 
         // this should be abstract but until it is added to all the providers virtual will have to do
-        virtual public string ServerVersionNormalized
+        public virtual string ServerVersionNormalized
         {
             get
             {
@@ -189,39 +189,39 @@ internal void AddWeakReference(object value, int tag)
             _referenceCollection.Add(value, tag);
         }
 
-        abstract public DbTransaction BeginTransaction(System.Data.IsolationLevel il);
+        public abstract DbTransaction BeginTransaction(System.Data.IsolationLevel il);
 
-        virtual public void ChangeDatabase(string value)
+        public virtual void ChangeDatabase(string value)
         {
             throw ADP.MethodNotImplemented();
         }
 
-        virtual internal void PrepareForReplaceConnection()
+        internal virtual void PrepareForReplaceConnection()
         {
             // By default, there is no preparation required
         }
 
-        virtual protected void PrepareForCloseConnection()
+        protected virtual void PrepareForCloseConnection()
         {
             // By default, there is no preparation required
         }
 
-        virtual protected bool ObtainAdditionalLocksForClose()
+        protected virtual bool ObtainAdditionalLocksForClose()
         {
             return false; // no additional locks in default implementation
         }
 
-        virtual protected void ReleaseAdditionalLocksForClose(bool lockToken)
+        protected virtual void ReleaseAdditionalLocksForClose(bool lockToken)
         {
             // no additional locks in default implementation
         }
 
-        virtual protected DbReferenceCollection CreateReferenceCollection()
+        protected virtual DbReferenceCollection CreateReferenceCollection()
         {
             throw ADP.InternalError(ADP.InternalErrorCode.AttemptingToConstructReferenceCollectionOnStaticObject);
         }
 
-        abstract protected void Deactivate();
+        protected abstract void Deactivate();
 
         internal void DeactivateConnection()
         {

src/Microsoft.Data.SqlClient/netcore/src/Common/src/System/Net/Logging/NetEventSource.Common.cs

@@ -422,7 +422,7 @@ private static void DebugValidateArg(FormattableString arg)
             Debug.Assert(IsEnabled || arg == null, $"Should not be formatting FormattableString \"{arg}\" if tracing isn't enabled");
         }
 
-        public static new bool IsEnabled =>
+        public new static bool IsEnabled =>
             Log.IsEnabled();
         //true; // uncomment for debugging only
 

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionInternal.cs

@@ -171,7 +171,7 @@ internal int ObjectID
             }
         }
 
-        virtual protected bool UnbindOnTransactionCompletion
+        protected virtual bool UnbindOnTransactionCompletion
         {
             get
             {
@@ -180,23 +180,23 @@ virtual protected bool UnbindOnTransactionCompletion
         }
 
         // Is this a connection that must be put in stasis (or is already in stasis) pending the end of it's transaction?
-        virtual protected internal bool IsNonPoolableTransactionRoot
+        protected internal virtual bool IsNonPoolableTransactionRoot
         {
             get
             {
                 return false; // if you want to have delegated transactions that are non-poolable, you better override this...
             }
         }
 
-        virtual internal bool IsTransactionRoot
+        internal virtual bool IsTransactionRoot
         {
             get
             {
                 return false; // if you want to have delegated transactions, you better override this...
             }
         }
 
-        virtual protected bool ReadyToPrepareTransaction
+        protected virtual bool ReadyToPrepareTransaction
         {
             get
             {
@@ -206,7 +206,7 @@ virtual protected bool ReadyToPrepareTransaction
 
         internal virtual bool IsAccessTokenExpired => false;
 
-        abstract protected void Activate(Transaction transaction);
+        protected abstract void Activate(Transaction transaction);
 
         internal void ActivateConnection(Transaction transaction)
         {
@@ -331,7 +331,7 @@ internal virtual void CloseConnection(DbConnection owningObject, DbConnectionFac
             }
         }
 
-        virtual internal void DelegatedTransactionEnded()
+        internal virtual void DelegatedTransactionEnded()
         {
             // Called by System.Transactions when the delegated transaction has
             // completed.  We need to make closed connections that are in stasis
@@ -399,12 +399,12 @@ public virtual void Dispose()
             }
         }
 
-        abstract public void EnlistTransaction(Transaction transaction);
+        public abstract void EnlistTransaction(Transaction transaction);
 
         // Cleanup connection's transaction-specific structures (currently used by Delegated transaction).
         //  This is a separate method because cleanup can be triggered in multiple ways for a delegated
         //  transaction.
-        virtual protected void CleanupTransactionOnCompletion(Transaction transaction)
+        protected virtual void CleanupTransactionOnCompletion(Transaction transaction)
         {
         }
 

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/ProviderBase/DbConnectionPool.cs

@@ -15,7 +15,7 @@
 
 namespace Microsoft.Data.ProviderBase
 {
-    sealed internal partial class DbConnectionPool
+    internal sealed partial class DbConnectionPool
     {
         private enum State
         {
@@ -27,7 +27,7 @@ private enum State
         // This class is a way to stash our cloned Tx key for later disposal when it's no longer needed.
         // We can't get at the key in the dictionary without enumerating entries, so we stash an extra
         // copy as part of the value.
-        sealed private class TransactedConnectionList : List<DbConnectionInternal>
+        private sealed class TransactedConnectionList : List<DbConnectionInternal>
         {
             private Transaction _transaction;
             internal TransactedConnectionList(int initialAllocation, Transaction tx) : base(initialAllocation)
@@ -59,7 +59,7 @@ public PendingGetConnection(long dueTime, DbConnection owner, TaskCompletionSour
             public DbConnectionOptions UserOptions { get; private set; }
         }
 
-        sealed private class TransactedConnectionPool
+        private sealed class TransactedConnectionPool
         {
             Dictionary<Transaction, TransactedConnectionList> _transactedCxns;