Code Health | Enforcing Ordinal for StringComparison (#2068)

dotnet · Jun 27, 2023 · 3b45ee7 · 3b45ee7
1 parent 8fad4a4
commit 3b45ee7
Show file tree

Hide file tree

Showing 30 changed files with 113 additions and 89 deletions.
diff --git a/src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/.editorconfig b/src/Microsoft.Data.SqlClient/add-ons/AzureKeyVaultProvider/.editorconfig
@@ -0,0 +1,9 @@
+# editorconfig.org
+
+# top-most EditorConfig file
+root = false
+
+[*.cs]
+
+# CA1310: Specify StringComparison for correctness
+dotnet_diagnostic.CA1310.severity = error
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/.editorconfig b/src/Microsoft.Data.SqlClient/netcore/src/.editorconfig
@@ -13,3 +13,6 @@ csharp_prefer_simple_using_statement = false
 
 # SYSLIB0039: Type or member is obsolete
 dotnet_diagnostic.SYSLIB0039.severity = suggestion
+
+# CA1310: Specify StringComparison for correctness
+dotnet_diagnostic.CA1310.severity = error
diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/SNI/SNIProxy.cs
@@ -675,7 +675,7 @@ private void ReportSNIError(SNIProviders provider)
         private bool InferNamedPipesInformation()
         {
             // If we have a datasource beginning with a pipe or we have already determined that the protocol is Named Pipe
-            if (_dataSourceAfterTrimmingProtocol.StartsWith(PipeBeginning) || _connectionProtocol == Protocol.NP)
+            if (_dataSourceAfterTrimmingProtocol.StartsWith(PipeBeginning, StringComparison.Ordinal) || _connectionProtocol == Protocol.NP)
             {
                 // If the data source is "np:servername"
                 if (!_dataSourceAfterTrimmingProtocol.Contains(PipeBeginning))
@@ -714,7 +714,7 @@ private bool InferNamedPipesInformation()
                         return false;
                     }
 
-                    if (tokensByBackSlash[4].StartsWith(NamedPipeInstanceNameHeader))
+                    if (tokensByBackSlash[4].StartsWith(NamedPipeInstanceNameHeader, StringComparison.Ordinal))
                     {
                         InstanceName = tokensByBackSlash[4].Substring(NamedPipeInstanceNameHeader.Length);
                     }

diff --git a/....SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCngProvider.Windows.cs b/....SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCngProvider.Windows.cs
@@ -369,7 +369,7 @@ private RSACng CreateRSACngProvider(string keyPath, bool isSystemOp)
         /// <param name="keyIdentifier">Key identifier inside the CNG provider</param>
         private void GetCngProviderAndKeyId(string keyPath, bool isSystemOp, out string cngProvider, out string keyIdentifier)
         {
-            int indexOfSlash = keyPath.IndexOf(@"/");
+            int indexOfSlash = keyPath.IndexOf(@"/", StringComparison.Ordinal);
             if (indexOfSlash == -1)
             {
                 throw SQL.InvalidCngPath(keyPath, isSystemOp);

diff --git a/....SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCspProvider.Windows.cs b/....SqlClient/netcore/src/Microsoft/Data/SqlClient/SqlColumnEncryptionCspProvider.Windows.cs
@@ -395,7 +395,7 @@ private RSACryptoServiceProvider CreateRSACryptoProvider(string keyPath, bool is
         /// <param name="keyIdentifier">output containing the key name</param>
         private void GetCspProviderAndKeyName(string keyPath, bool isSystemOp, out string cspProviderName, out string keyIdentifier)
         {
-            int indexOfSlash = keyPath.IndexOf(@"/");
+            int indexOfSlash = keyPath.IndexOf(@"/", StringComparison.Ordinal);
             if (indexOfSlash == -1)
             {
                 throw SQL.InvalidCspPath(keyPath, isSystemOp);

diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlClient/TdsParser.cs
@@ -434,7 +434,7 @@ internal void ProcessPendingAck(TdsParserStateObject stateObj)
 
             FQDNforDNSCache = serverInfo.ResolvedServerName;
 
-            int commaPos = FQDNforDNSCache.IndexOf(",");
+            int commaPos = FQDNforDNSCache.IndexOf(",", StringComparison.Ordinal);
             if (commaPos != -1)
             {
                 FQDNforDNSCache = FQDNforDNSCache.Substring(0, commaPos);

diff --git a/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlTypes/SqlFileStream.Windows.cs b/src/Microsoft.Data.SqlClient/netcore/src/Microsoft/Data/SqlTypes/SqlFileStream.Windows.cs
@@ -453,7 +453,7 @@ static private string GetFullPathInternal(string path)
             }
 
             // make sure path is not DOS device path
-            if (!path.StartsWith(@"\\") && !System.IO.PathInternal.IsDevice(path.AsSpan()))
+            if (!path.StartsWith(@"\\", StringComparison.Ordinal) && !System.IO.PathInternal.IsDevice(path.AsSpan()))
             {
                 throw ADP.Argument(StringsHelper.GetString(Strings.SqlFileStream_InvalidPath), "path");
             }

diff --git a/src/Microsoft.Data.SqlClient/netfx/src/.editorconfig b/src/Microsoft.Data.SqlClient/netfx/src/.editorconfig
@@ -7,3 +7,6 @@ root = false
 
 # IDE0090: Use 'new(...)'
 csharp_style_implicit_object_creation_when_type_is_apparent = false
+
+# CA1310: Specify StringComparison for correctness
+dotnet_diagnostic.CA1310.severity = error
diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/Common/DbConnectionStringCommon.cs
@@ -373,7 +373,7 @@ private static bool IsValidAuthenticationMethodEnum()
             {
                 for (int i = 0; i < l; i++)
                 {
-                    if (s_supportedAuthenticationModes[i].CompareTo(names[i]) != 0)
+                    if (string.Compare(s_supportedAuthenticationModes[i], names[i], StringComparison.Ordinal) != 0)
                     {
                         listValid = false;
                     }

diff --git a/...soft.Data.SqlClient/src/Microsoft/Data/SqlClient/ActiveDirectoryAuthenticationProvider.cs b/...soft.Data.SqlClient/src/Microsoft/Data/SqlClient/ActiveDirectoryAuthenticationProvider.cs
@@ -122,7 +122,7 @@ public override async Task<SqlAuthenticationToken> AcquireTokenAsync(SqlAuthenti
             // Use Connection timeout value to cancel token acquire request after certain period of time.
             cts.CancelAfter(parameters.ConnectionTimeout * 1000); // Convert to milliseconds
 
-            string scope = parameters.Resource.EndsWith(s_defaultScopeSuffix) ? parameters.Resource : parameters.Resource + s_defaultScopeSuffix;
+            string scope = parameters.Resource.EndsWith(s_defaultScopeSuffix, StringComparison.Ordinal) ? parameters.Resource : parameters.Resource + s_defaultScopeSuffix;
             string[] scopes = new string[] { scope };
             TokenRequestContext tokenRequestContext = new(scopes);
 

diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlConnectionString.cs
@@ -983,7 +983,7 @@ private static string GetComputerNameDnsFullyQualified()
             {
                 var domainName = "." + IPGlobalProperties.GetIPGlobalProperties().DomainName;
                 var hostName = Dns.GetHostName();
-                if (domainName != "." && !hostName.EndsWith(domainName))
+                if (domainName != "." && !hostName.EndsWith(domainName, StringComparison.Ordinal))
                     hostName += domainName;
                 return hostName;
             }

diff --git a/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs b/src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/SqlSecurityUtility.cs
@@ -406,7 +406,7 @@ internal static void VerifyColumnMasterKeySignature(string keyStoreName, string
         private static bool ShouldUseInstanceLevelProviderFlow(string keyStoreName, SqlConnection connection, SqlCommand command)
         {
             return InstanceLevelProvidersAreRegistered(connection, command) &&
-                !keyStoreName.StartsWith(ADP.ColumnEncryptionSystemProviderNamePrefix);
+                !keyStoreName.StartsWith(ADP.ColumnEncryptionSystemProviderNamePrefix, StringComparison.Ordinal);
         }
 
         private static bool InstanceLevelProvidersAreRegistered(SqlConnection connection, SqlCommand command) =>

diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/DataCommon/AssemblyResourceManager.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/DataCommon/AssemblyResourceManager.cs
@@ -47,7 +47,7 @@ public override bool TryGetMember(GetMemberBinder binder, out object result)
         public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, out object result)
         {
             var resourceName = binder.Name;
-            if (resourceName.StartsWith("Get_"))
+            if (resourceName.StartsWith("Get_", StringComparison.Ordinal))
                 resourceName = resourceName.Remove(0, 4);
 
             return TryGetResourceValue(resourceName, args, out result);

diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlCommandBuilderTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlCommandBuilderTest.cs
@@ -38,8 +38,8 @@ public void CatalogLocation_Value_Invalid()
                 Assert.Equal(typeof(ArgumentException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("'CatalogLocation'") != -1);
-                Assert.True(ex.Message.IndexOf("'Start'") != -1);
+                Assert.True(ex.Message.IndexOf("'CatalogLocation'", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("'Start'", StringComparison.Ordinal) != -1);
                 Assert.Null(ex.ParamName);
             }
             Assert.Equal(CatalogLocation.Start, cb.CatalogLocation);
@@ -55,8 +55,8 @@ public void CatalogLocation_Value_Invalid()
                 Assert.Equal(typeof(ArgumentException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("'CatalogLocation'") != -1);
-                Assert.True(ex.Message.IndexOf("'Start'") != -1);
+                Assert.True(ex.Message.IndexOf("'CatalogLocation'", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("'Start'", StringComparison.Ordinal) != -1);
                 Assert.Null(ex.ParamName);
             }
             Assert.Equal(CatalogLocation.Start, cb.CatalogLocation);
@@ -92,8 +92,8 @@ public void CatalogSeparator_Value_Invalid(string separator)
                 Assert.Equal(typeof(ArgumentException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("'CatalogSeparator'") != -1);
-                Assert.True(ex.Message.IndexOf("'.'") != -1);
+                Assert.True(ex.Message.IndexOf("'CatalogSeparator'", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("'.'", StringComparison.Ordinal) != -1);
                 Assert.Null(ex.ParamName);
             }
         }
@@ -122,8 +122,8 @@ public void ConflictOption_Value_Invalid()
                 Assert.Equal(typeof(ArgumentOutOfRangeException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("ConflictOption") != -1);
-                Assert.True(ex.Message.IndexOf("666") != -1);
+                Assert.True(ex.Message.IndexOf("ConflictOption", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("666", StringComparison.Ordinal) != -1);
                 Assert.Equal("ConflictOption", ex.ParamName);
             }
             Assert.Equal(ConflictOption.CompareRowVersion, cb.ConflictOption);
@@ -190,8 +190,8 @@ public void QuoteIdentifier_PrefixSuffix_NoMatch()
                 Assert.Equal(typeof(ArgumentException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("QuotePrefix") != -1);
-                Assert.True(ex.Message.IndexOf("QuoteSuffix") != -1);
+                Assert.True(ex.Message.IndexOf("QuotePrefix", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("QuoteSuffix", StringComparison.Ordinal) != -1);
                 Assert.Null(ex.ParamName);
             }
 
@@ -208,8 +208,8 @@ public void QuoteIdentifier_PrefixSuffix_NoMatch()
                 Assert.Equal(typeof(ArgumentException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("QuotePrefix") != -1);
-                Assert.True(ex.Message.IndexOf("QuoteSuffix") != -1);
+                Assert.True(ex.Message.IndexOf("QuotePrefix", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("QuoteSuffix", StringComparison.Ordinal) != -1);
                 Assert.Null(ex.ParamName);
             }
         }
@@ -340,8 +340,8 @@ public void SchemaSeparator_Value_Invalid(string separator)
                 Assert.Equal(typeof(ArgumentException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("'SchemaSeparator'") != -1);
-                Assert.True(ex.Message.IndexOf("'.'") != -1);
+                Assert.True(ex.Message.IndexOf("'SchemaSeparator'", StringComparison.Ordinal) != -1);
+                Assert.True(ex.Message.IndexOf("'.'", StringComparison.Ordinal) != -1);
                 Assert.Null(ex.ParamName);
             }
         }

diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlCommandTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlCommandTest.cs
@@ -304,7 +304,7 @@ public void CommandType_Value_Invalid()
             // The CommandType enumeration value, 666, is invalid
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
-            Assert.True(ex.Message.IndexOf("666") != -1);
+            Assert.True(ex.Message.IndexOf("666", StringComparison.Ordinal) != -1);
             Assert.Equal("CommandType", ex.ParamName);
         }
 
@@ -334,7 +334,7 @@ public void ExecuteNonQuery_Connection_Closed()
             // closed.
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
-            Assert.True(ex.Message.IndexOf("ExecuteNonQuery") != -1);
+            Assert.True(ex.Message.IndexOf("ExecuteNonQuery", StringComparison.Ordinal) != -1);
         }
 
         [Fact]
@@ -364,7 +364,7 @@ public void ExecuteReader_Connection_Closed()
             // closed.
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
-            Assert.True(ex.Message.IndexOf("ExecuteReader") != -1);
+            Assert.True(ex.Message.IndexOf("ExecuteReader", StringComparison.Ordinal) != -1);
         }
 
         [Fact]
@@ -395,7 +395,7 @@ public void ExecuteScalar_Connection_Closed()
             // closed.
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
-            Assert.True(ex.Message.IndexOf("ExecuteScalar") != -1);
+            Assert.True(ex.Message.IndexOf("ExecuteScalar", StringComparison.Ordinal) != -1);
         }
 
         [Fact] // bug #412584
@@ -449,7 +449,7 @@ public void Prepare_Connection_Closed()
             // is Closed
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
-            Assert.True(ex.Message.IndexOf("Prepare") != -1);
+            Assert.True(ex.Message.IndexOf("Prepare", StringComparison.Ordinal) != -1);
 
             // Text, parameters cleared
             cmd = new SqlCommand("select count(*) from whatever", cn);

diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlConnectionTest.cs
@@ -34,7 +34,7 @@ public void Constructor1()
             Assert.Null(cn.Site);
             Assert.Equal(ConnectionState.Closed, cn.State);
             Assert.False(cn.StatisticsEnabled);
-            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, true) == 0);
+            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, StringComparison.OrdinalIgnoreCase) == 0);
         }
 
         [Fact]
@@ -54,7 +54,7 @@ public void Constructor2()
             Assert.Null(cn.Site);
             Assert.Equal(ConnectionState.Closed, cn.State);
             Assert.False(cn.StatisticsEnabled);
-            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, true) == 0);
+            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, StringComparison.OrdinalIgnoreCase) == 0);
 
             cn = new SqlConnection((string)null);
             Assert.Equal(string.Empty, cn.ConnectionString);
@@ -68,7 +68,7 @@ public void Constructor2()
             Assert.Null(cn.Site);
             Assert.Equal(ConnectionState.Closed, cn.State);
             Assert.False(cn.StatisticsEnabled);
-            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, true) == 0);
+            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, StringComparison.OrdinalIgnoreCase) == 0);
         }
 
         [Fact]
@@ -201,7 +201,7 @@ public void ChangePassword_NewPassword_Empty()
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
             Assert.NotNull(ex.ParamName);
-            Assert.True(ex.ParamName.IndexOf("'newPassword'") != -1);
+            Assert.True(ex.ParamName.IndexOf("'newPassword'", StringComparison.Ordinal) != -1);
         }
 
         [Fact]
@@ -212,8 +212,8 @@ public void ChangePassword_NewPassword_ExceedMaxLength()
             // its limit of '128'
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
-            Assert.True(ex.Message.IndexOf("'newPassword'") != -1);
-            Assert.True(ex.Message.IndexOf("128") != -1);
+            Assert.True(ex.Message.IndexOf("'newPassword'", StringComparison.Ordinal) != -1);
+            Assert.True(ex.Message.IndexOf("128", StringComparison.Ordinal) != -1);
             Assert.Null(ex.ParamName);
         }
 
@@ -224,7 +224,7 @@ public void ChangePassword_NewPassword_Null()
             Assert.Null(ex.InnerException);
             Assert.NotNull(ex.Message);
             Assert.NotNull(ex.ParamName);
-            Assert.True(ex.ParamName.IndexOf("'newPassword'") != -1);
+            Assert.True(ex.ParamName.IndexOf("'newPassword'", StringComparison.Ordinal) != -1);
         }
 
         [Fact]
@@ -305,7 +305,7 @@ public void Dispose()
             Assert.Equal(string.Empty, cn.Database);
             Assert.Equal(string.Empty, cn.DataSource);
             Assert.Equal(8000, cn.PacketSize);
-            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, true) == 0);
+            Assert.True(string.Compare(Environment.MachineName, cn.WorkstationId, StringComparison.OrdinalIgnoreCase) == 0);
             Assert.Equal(ConnectionState.Closed, cn.State);
             cn.Dispose();
 

diff --git a/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs b/src/Microsoft.Data.SqlClient/tests/FunctionalTests/SqlParameterTest.cs
@@ -1155,7 +1155,7 @@ public void SqlDbTypeTest_Value_Invalid()
                 Assert.Equal(typeof(ArgumentOutOfRangeException), ex.GetType());
                 Assert.Null(ex.InnerException);
                 Assert.NotNull(ex.Message);
-                Assert.True(ex.Message.IndexOf("666") != -1);
+                Assert.True(ex.Message.IndexOf("666", StringComparison.Ordinal) != -1);
                 Assert.Equal("SqlDbType", ex.ParamName);
             }
         }

diff --git a/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/CspProviderExt.cs b/src/Microsoft.Data.SqlClient/tests/ManualTests/AlwaysEncrypted/CspProviderExt.cs
@@ -46,7 +46,7 @@ public void TestKeysFromCertificatesCreatedWithMultipleCryptoProviders(string co
                 using (Microsoft.Win32.RegistryKey providerKey = defaultCryptoProvidersRegistryKey.OpenSubKey(subKeyName))
                 {
                     // Get Provider Name and its type
-                    providerName = providerKey.Name.Substring(providerKey.Name.LastIndexOf(@"\") + 1);
+                    providerName = providerKey.Name.Substring(providerKey.Name.LastIndexOf(@"\", StringComparison.Ordinal) + 1);
                     providerType = providerKey.GetValue(@"Type").ToString();
 
                     // Create a certificate from that provider