Merge | SqlConnection cosmetic changes (#3275)

* Reorganise fields

* Reformat attribute syntax

* Reorder properties

* Cleanup syntax and comments

* Adjust usings, if/else chain in constructor

* Replace ADP.IsEmpty, use nameof, variable rename

* Remove optional default parameters

* Align implementation property attributes with ref

* Switch Attestation properties to => syntax

* Remove unnecessary assignments

* Follow-up of merge

Author: edwardneal

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs

@@ -1620,7 +1620,7 @@ private object ConvertValue(object value, _SqlMetaData metadata, bool isNull, re
                         // in byte[] form.
                         if (!(value is byte[]))
                         {
-                            value = _connection.GetBytes(value);
+                            value = _connection.GetBytes(value, out _, out _);
                             typeChanged = true;
                         }
                         break;

src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlConnection.cs

@@ -99,7 +99,7 @@ private static readonly Dictionary<string, SqlColumnEncryptionKeyStoreProvider>
 
         /// <summary>
         /// Global custom provider list should be provided by the user. We shallow copy the user supplied dictionary into a ReadOnlyDictionary.
-        /// Global custom provider list can only supplied once per application.
+        /// Global custom provider list can only be supplied once per application.
         /// </summary>
         private static IReadOnlyDictionary<string, SqlColumnEncryptionKeyStoreProvider> s_globalCustomColumnEncryptionKeyStoreProviders;
 
@@ -164,7 +164,7 @@ public SqlConnection(string connectionString) : this()
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/ctorConnectionStringCredential/*' />
         public SqlConnection(string connectionString, SqlCredential credential) : this()
         {
-            ConnectionString = connectionString;
+            ConnectionString = connectionString; // setting connection string first so that ConnectionOption is available
             if (credential != null)
             {
                 // The following checks are necessary as setting Credential property will call CheckAndThrowOnInvalidCombinationOfConnectionStringAndSqlCredential
@@ -370,7 +370,6 @@ private static void ValidateCustomProviders(IDictionary<string, SqlColumnEncrypt
             foreach (string key in customProviders.Keys)
             {
                 // Validate the provider name
-                //
                 // Check for null or empty
                 if (string.IsNullOrWhiteSpace(key))
                 {
@@ -394,18 +393,17 @@ private static void ValidateCustomProviders(IDictionary<string, SqlColumnEncrypt
         /// <summary>
         /// Get enclave attestation url to be used with enclave based Always Encrypted
         /// </summary>
-        internal string EnclaveAttestationUrl => ((SqlConnectionString)ConnectionOptions).EnclaveAttestationUrl;
+        internal string EnclaveAttestationUrl
+        {
+            get => ((SqlConnectionString)ConnectionOptions).EnclaveAttestationUrl;
+        }
 
         /// <summary>
         /// Get attestation protocol
         /// </summary>
         internal SqlConnectionAttestationProtocol AttestationProtocol
         {
-            get
-            {
-                SqlConnectionString opt = (SqlConnectionString)ConnectionOptions;
-                return opt.AttestationProtocol;
-            }
+            get => ((SqlConnectionString)ConnectionOptions).AttestationProtocol;
         }
 
         /// <summary>
@@ -658,6 +656,7 @@ public override string ConnectionString
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/ConnectionTimeout/*' />
         [ResDescription(StringsHelper.ResourceNames.SqlConnection_ConnectionTimeout)]
         [ResCategory(StringsHelper.ResourceNames.SqlConnection_DataSource)]
+        [DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
         public override int ConnectionTimeout
         {
             get
@@ -688,7 +687,6 @@ public string AccessToken
         {
             get
             {
-                string result = _accessToken;
                 // When a connection is connecting or is ever opened, make AccessToken available only if "Persist Security Info" is set to true
                 // otherwise, return null
                 SqlConnectionString connectionOptions = (SqlConnectionString)UserConnectionOptions;
@@ -740,6 +738,7 @@ public Func<SqlAuthenticationParameters, CancellationToken, Task<SqlAuthenticati
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/Database/*' />
         [ResDescription(StringsHelper.ResourceNames.SqlConnection_Database)]
         [ResCategory(StringsHelper.ResourceNames.SqlConnection_DataSource)]
+        [DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
         public override string Database
         {
             // if the connection is open, we need to ask the inner connection what it's
@@ -766,6 +765,7 @@ public override string Database
         ///
         /// To indicate the IsSupported flag sent by the server for DNS Caching. This property is for internal testing only.
         ///
+        [DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
         internal string SQLDNSCachingSupportedState
         {
             get
@@ -789,6 +789,7 @@ internal string SQLDNSCachingSupportedState
         ///
         /// To indicate the IsSupported flag sent by the server for DNS Caching before redirection. This property is for internal testing only.
         ///
+        [DesignerSerializationVisibility(DesignerSerializationVisibility.Hidden)]
         internal string SQLDNSCachingSupportedStateBeforeRedirect
         {
             get
@@ -1079,7 +1080,7 @@ private void CheckAndThrowOnInvalidCombinationOfConnectionOptionAndAccessToken(S
                 throw ADP.InvalidMixedUsageOfCredentialAndAccessToken();
             }
 
-            if(_accessTokenCallback != null)
+            if (_accessTokenCallback != null)
             {
                 throw ADP.InvalidMixedUsageOfAccessTokenAndTokenCallback();
             }
@@ -1101,7 +1102,7 @@ private void CheckAndThrowOnInvalidCombinationOfConnectionOptionAndAccessTokenCa
                 throw ADP.InvalidMixedUsageOfAccessTokenCallbackAndAuthentication();
             }
 
-            if(_accessToken != null)
+            if (_accessToken != null)
             {
                 throw ADP.InvalidMixedUsageOfAccessTokenAndTokenCallback();
             }
@@ -1113,8 +1114,6 @@ protected override DbProviderFactory DbProviderFactory
             get => SqlClientFactory.Instance;
         }
 
-        // SqlCredential: Pair User Id and password in SecureString which are to be used for SQL authentication
-
         //
         // PUBLIC EVENTS
         //
@@ -1155,11 +1154,11 @@ protected override void OnStateChange(StateChangeEventArgs stateChange)
         new public SqlTransaction BeginTransaction()
         {
             // this is just a delegate. The actual method tracks executiontime
-            return BeginTransaction(System.Data.IsolationLevel.Unspecified, null);
+            return BeginTransaction(IsolationLevel.Unspecified, null);
         }
 
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/BeginTransactionIso/*' />
-        new public SqlTransaction BeginTransaction(System.Data.IsolationLevel iso)
+        new public SqlTransaction BeginTransaction(IsolationLevel iso)
         {
             // this is just a delegate. The actual method tracks executiontime
             return BeginTransaction(iso, null);
@@ -1172,18 +1171,18 @@ public SqlTransaction BeginTransaction(string transactionName)
             // BEGIN...COMMIT or BEGIN...ROLLBACK statements.  Transaction names
             // are ignored for nested BEGIN's.  The only way to rollback a nested
             // transaction is to have a save point from a SAVE TRANSACTION call.
-            return BeginTransaction(System.Data.IsolationLevel.Unspecified, transactionName);
+            return BeginTransaction(IsolationLevel.Unspecified, transactionName);
         }
 
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/BeginDbTransaction/*' />
         [SuppressMessage("Microsoft.Reliability", "CA2004:RemoveCallsToGCKeepAlive")]
-        override protected DbTransaction BeginDbTransaction(System.Data.IsolationLevel isolationLevel)
+        override protected DbTransaction BeginDbTransaction(IsolationLevel isolationLevel)
         {
             using (TryEventScope.Create("SqlConnection.BeginDbTransaction | API | Object Id {0}, Isolation Level {1}", ObjectID, (int)isolationLevel))
             {
                 DbTransaction transaction = BeginTransaction(isolationLevel);
 
-                //   InnerConnection doesn't maintain a ref on the outer connection (this) and
+                // VSTFDEVDIV# 560355 - InnerConnection doesn't maintain a ref on the outer connection (this) and
                 //   subsequently leaves open the possibility that the outer connection could be GC'ed before the SqlTransaction
                 //   is fully hooked up (leaving a DbTransaction with a null connection property). Ensure that this is reachable
                 //   until the completion of BeginTransaction with KeepAlive
@@ -1194,7 +1193,7 @@ override protected DbTransaction BeginDbTransaction(System.Data.IsolationLevel i
         }
 
         /// <include file='../../../../../../../doc/snippets/Microsoft.Data.SqlClient/SqlConnection.xml' path='docs/members[@name="SqlConnection"]/BeginTransactionIsoTransactionName/*' />
-        public SqlTransaction BeginTransaction(System.Data.IsolationLevel iso, string transactionName)
+        public SqlTransaction BeginTransaction(IsolationLevel iso, string transactionName)
         {
             WaitForPendingReconnection();
             SqlStatistics statistics = null;
@@ -1287,7 +1286,6 @@ public static void ClearPool(SqlConnection connection)
             }
         }
 
-
         private void CloseInnerConnection()
         {
             // CloseConnection() now handles the lock
@@ -1625,6 +1623,7 @@ internal Task ValidateAndReconnect(Action beforeDisconnect, int timeout)
                                             _originalConnectionId = ClientConnectionId;
                                             SqlClientEventSource.Log.TryTraceEvent("SqlConnection.ValidateAndReconnect | Info | Connection Client Connection Id {0} is invalid, reconnecting", _originalConnectionId);
                                             _recoverySessionData = cData;
+
                                             if (beforeDisconnect != null)
                                             {
                                                 beforeDisconnect();
@@ -1877,6 +1876,7 @@ internal void Retry(Task<DbConnectionInternal> retryTask)
             {
                 SqlClientEventSource.Log.TryTraceEvent("SqlConnection.Retry | Info | Object Id {0}", _parent?.ObjectID);
                 _registration.Dispose();
+
                 try
                 {
                     SqlStatistics statistics = null;
@@ -2078,7 +2078,6 @@ internal bool HasLocalTransactionFromAPI
             }
         }
 
-
         internal bool Is2008OrNewer
         {
             get
@@ -2100,7 +2099,6 @@ internal TdsParser Parser
             }
         }
 
-
         //
         // INTERNAL METHODS
         //
@@ -2152,7 +2150,6 @@ internal void OnError(SqlException exception, bool breakConnection, Action<Actio
         {
             Debug.Assert(exception != null && exception.Errors.Count != 0, "SqlConnection: OnError called with null or empty exception!");
 
-
             if (breakConnection && (ConnectionState.Open == State))
             {
                 if (wrapCloseInAction != null)
@@ -2483,6 +2480,7 @@ private Assembly ResolveTypeAssembly(AssemblyName asmRef, bool throwOnError)
                 }
                 asmRef.Version = TypeSystemAssemblyVersion;
             }
+
             try
             {
                 return Assembly.Load(asmRef);
@@ -2546,12 +2544,6 @@ internal object GetUdtValue(object value, SqlMetaDataPriv metaData, bool returnD
             }
         }
 
-        internal byte[] GetBytes(object o)
-        {
-            Format format = Format.Native;
-            return GetBytes(o, out format, out int maxSize);
-        }
-
         internal byte[] GetBytes(object o, out Format format, out int maxSize)
         {
             SqlUdtInfo attr = GetInfoFromType(o.GetType());
@@ -2575,7 +2567,7 @@ internal byte[] GetBytes(object o, out Format format, out int maxSize)
 
         private SqlUdtInfo GetInfoFromType(Type t)
         {
-            Debug.Assert(t != null, "Type object cant be NULL");
+            Debug.Assert(t != null, "Type object can't be NULL");
             Type orig = t;
             do
             {
@@ -2584,10 +2576,8 @@ private SqlUdtInfo GetInfoFromType(Type t)
                 {
                     return attr;
                 }
-                else
-                {
-                    t = t.BaseType;
-                }
+
+                t = t.BaseType;
             }
             while (t != null);
 

src/Microsoft.Data.SqlClient/netfx/src/Microsoft/Data/SqlClient/SqlBulkCopy.cs

@@ -1634,7 +1634,7 @@ private object ConvertValue(object value, _SqlMetaData metadata, bool isNull, re
                         // in byte[] form.
                         if (!(value is byte[]))
                         {
-                            value = _connection.GetBytes(value);
+                            value = _connection.GetBytes(value, out _, out _);
                             typeChanged = true;
                         }
                         break;