Add code analysis (#463)

JamesNK · web-flow · commit 487259cb9c8d · 2019-08-21T06:34:51.000+12:00
diff --git a/Directory.Build.props b/Directory.Build.props
@@ -13,6 +13,7 @@
 
     <LangVersion>8.0</LangVersion>
     <Nullable>enable</Nullable>
+    <CodeAnalysisRuleset>$(MSBuildThisFileDirectory)Grpc.DotNet.ruleset</CodeAnalysisRuleset>
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(IsGrpcPublishedPackage)' == 'true' ">
@@ -32,6 +33,7 @@
   </PropertyGroup>
 
   <ItemGroup Condition="'$(IsGrpcPublishedPackage)' == 'true'">
+    <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="$(MicrosoftCodeAnalysisFxCopAnalyzersPackageVersion)" PrivateAssets="All" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="$(MicrosoftSourceLinkGitHubPackageVersion)" PrivateAssets="All" />
   </ItemGroup>
 
diff --git a/Grpc.DotNet.ruleset b/Grpc.DotNet.ruleset
diff --git a/build/dependencies.props b/build/dependencies.props
@@ -5,9 +5,10 @@
     <ConfigureAwaitCheckerAnalyzerPackageVersion>3.0.0</ConfigureAwaitCheckerAnalyzerPackageVersion>
     <GoogleProtobufPackageVersion>3.9.1</GoogleProtobufPackageVersion>
     <GrpcPackageVersion>2.23.0-pre1</GrpcPackageVersion>
-    <MicrosoftBuildPackageVersion>16.0.461</MicrosoftBuildPackageVersion>
-    <MicrosoftBuildLocatorPackageVersion>1.2.2</MicrosoftBuildLocatorPackageVersion>
     <MicrosoftAspNetCorePackageVersion>3.0.0-preview8.19405.7</MicrosoftAspNetCorePackageVersion>
+    <MicrosoftBuildLocatorPackageVersion>1.2.2</MicrosoftBuildLocatorPackageVersion>
+    <MicrosoftBuildPackageVersion>16.0.461</MicrosoftBuildPackageVersion>
+    <MicrosoftCodeAnalysisFxCopAnalyzersPackageVersion>2.9.4</MicrosoftCodeAnalysisFxCopAnalyzersPackageVersion>
     <MicrosoftExtensionsPackageVersion>2.1.1</MicrosoftExtensionsPackageVersion>
     <MicrosoftExtensions30PackageVersion>3.0.0-preview8.19405.4</MicrosoftExtensions30PackageVersion>
     <MicrosoftNETTestSdkPackageVersion>16.2.0</MicrosoftNETTestSdkPackageVersion>
diff --git a/src/Grpc.AspNetCore.Server.ClientFactory/Grpc.AspNetCore.Server.ClientFactory.csproj b/src/Grpc.AspNetCore.Server.ClientFactory/Grpc.AspNetCore.Server.ClientFactory.csproj
@@ -6,6 +6,10 @@
 
     <IsGrpcPublishedPackage>true</IsGrpcPublishedPackage>
     <TargetFramework>netcoreapp3.0</TargetFramework>
+
+    <!-- Disable analysis for ConfigureAwait(false) -->
+    <WarningsNotAsErrors>$(WarningsNotAsErrors);CA2007</WarningsNotAsErrors>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/Grpc.AspNetCore.Server.Reflection/Grpc.AspNetCore.Server.Reflection.csproj b/src/Grpc.AspNetCore.Server.Reflection/Grpc.AspNetCore.Server.Reflection.csproj
@@ -6,6 +6,10 @@
 
     <IsGrpcPublishedPackage>true</IsGrpcPublishedPackage>
     <TargetFramework>netcoreapp3.0</TargetFramework>
+
+    <!-- Disable analysis for ConfigureAwait(false) -->
+    <WarningsNotAsErrors>$(WarningsNotAsErrors);CA2007</WarningsNotAsErrors>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/Grpc.AspNetCore.Server/Grpc.AspNetCore.Server.csproj b/src/Grpc.AspNetCore.Server/Grpc.AspNetCore.Server.csproj
@@ -6,6 +6,10 @@
 
     <IsGrpcPublishedPackage>true</IsGrpcPublishedPackage>
     <TargetFramework>netcoreapp3.0</TargetFramework>
+    
+    <!-- Disable analysis for ConfigureAwait(false) -->
+    <WarningsNotAsErrors>$(WarningsNotAsErrors);CA2007</WarningsNotAsErrors>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/src/Grpc.AspNetCore.Server/Internal/X509CertificateHelpers.cs b/src/Grpc.AspNetCore.Server/Internal/X509CertificateHelpers.cs
@@ -18,6 +18,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Security.Cryptography.X509Certificates;
 
 namespace Grpc.AspNetCore.Server.Internal
@@ -35,7 +36,7 @@ public static string[] GetDnsFromExtensions(X509Certificate2 cert)
                     string asnString = ext.Format(false);
                     if (string.IsNullOrWhiteSpace(asnString))
                     {
-                        return new string[0];
+                        return Array.Empty<string>();
                     }
 
                     // SubjectAlternativeNames might contain something other than a dNSName, 
@@ -50,7 +51,7 @@ public static string[] GetDnsFromExtensions(X509Certificate2 cert)
                     for (var i = 0; i < rawDnsEntries.Length; i++)
                     {
                         string[] keyval = rawDnsEntries[i].Split(X509SubjectAlternativeNameConstants.Delimiter);
-                        if (string.Equals(keyval[0], X509SubjectAlternativeNameConstants.Identifier))
+                        if (string.Equals(keyval[0], X509SubjectAlternativeNameConstants.Identifier, StringComparison.Ordinal))
                         {
                             dnsEntries.Add(keyval[1]);
                         }
@@ -59,7 +60,7 @@ public static string[] GetDnsFromExtensions(X509Certificate2 cert)
                     return dnsEntries.ToArray();
                 }
             }
-            return new string[0];
+            return Array.Empty<string>();
         }
 
         // We don't have a strongly typed extension to parse Subject Alt Names, so we have to do a workaround 
@@ -106,6 +107,7 @@ private static void EnsureInitialized()
                 if (!s_successfullyInitialized)
                 {
                     throw new FormatException(string.Format(
+                        CultureInfo.InvariantCulture,
                         "There was an error detecting the identifier, delimiter, and separator for X509CertificateClaims on this platform.{0}" +
                         "Detected values were: Identifier: '{1}'; Delimiter:'{2}'; Separator:'{3}'",
                         Environment.NewLine,
@@ -138,7 +140,7 @@ static X509SubjectAlternativeNameConstants()
                     // Linux:   x509ExtensionFormattedString is: "DNS:not-real-subject-name, DNS:example.com"
                     // Parse: <identifier><delimter><value><separator(s)>
 
-                    int delimiterIndex = x509ExtensionFormattedString.IndexOf(subjectName1) - 1;
+                    int delimiterIndex = x509ExtensionFormattedString.IndexOf(subjectName1, StringComparison.Ordinal) - 1;
                     s_delimiter = x509ExtensionFormattedString[delimiterIndex];
 
                     // Make an assumption that all characters from the the start of string to the delimiter 
diff --git a/src/Grpc.Net.Client/Grpc.Net.Client.csproj b/src/Grpc.Net.Client/Grpc.Net.Client.csproj
@@ -9,8 +9,6 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <!-- Roslyn analyzer that enforces ConfigureAwait(false) -->
-    <PackageReference Include="ConfigureAwaitChecker.Analyzer" Version="$(ConfigureAwaitCheckerAnalyzerPackageVersion)" PrivateAssets="All" />
     <PackageReference Include="Grpc.Core.Api" Version="$(GrpcPackageVersion)" />
     <PackageReference Include="Microsoft.Extensions.Logging.Abstractions" Version="$(MicrosoftExtensionsPackageVersion)" />
     <PackageReference Include="System.Diagnostics.DiagnosticSource" Version="$(SystemDiagnosticsDiagnosticSourcePackageVersion)" />
diff --git a/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs b/src/Grpc.Net.Client/Internal/GrpcProtocolHelpers.cs
@@ -18,6 +18,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Net.Http;
 using System.Net.Http.Headers;
@@ -139,7 +140,7 @@ static long RoundUp(long x, long divisor)
 
         private static string FormatTimeout(long value, char ext)
         {
-            return value.ToString() + ext;
+            return value.ToString(CultureInfo.InvariantCulture) + ext;
         }
 
         private static string EncodeTimeoutSeconds(long sec)
diff --git a/src/Grpc.Net.ClientFactory/Grpc.Net.ClientFactory.csproj b/src/Grpc.Net.ClientFactory/Grpc.Net.ClientFactory.csproj
@@ -12,8 +12,6 @@
     <!-- PrivateAssets set to None to ensure the build targets/props are propagated to parent project -->
     <ProjectReference Include="..\Grpc.Net.Client\Grpc.Net.Client.csproj" PrivateAssets="None" />
 
-    <!-- Roslyn analyzer that enforces ConfigureAwait(false) -->
-    <PackageReference Include="ConfigureAwaitChecker.Analyzer" Version="$(ConfigureAwaitCheckerAnalyzerPackageVersion)" PrivateAssets="All" />
     <PackageReference Include="Grpc.Core.Api" Version="$(GrpcPackageVersion)" />
     <PackageReference Include="Microsoft.Extensions.Http" Version="$(MicrosoftExtensionsPackageVersion)" />
   </ItemGroup>
diff --git a/src/Grpc.Net.ClientFactory/Internal/TypeNameHelper.cs b/src/Grpc.Net.ClientFactory/Internal/TypeNameHelper.cs
@@ -23,7 +23,7 @@
 namespace Grpc.Net.ClientFactory.Internal
 {
     // Copied from https://github.com/aspnet/Extensions/blob/aa18b93adac411b6b98e4088284a1ffb7d83249f/src/Shared/src/TypeNameHelper/TypeNameHelper.cs
-    internal class TypeNameHelper
+    internal static class TypeNameHelper
     {
         private const char DefaultNestedTypeDelimiter = '+';
 
@@ -143,7 +143,7 @@ private static void ProcessGenericType(StringBuilder builder, Type type, Type[]
                 }
             }
 
-            var genericPartIndex = type.Name.IndexOf('`');
+            var genericPartIndex = type.Name.IndexOf('`', StringComparison.Ordinal);
             if (genericPartIndex <= 0)
             {
                 builder.Append(type.Name);
diff --git a/src/Grpc.Net.Common/Grpc.Net.Common.csproj b/src/Grpc.Net.Common/Grpc.Net.Common.csproj
@@ -9,8 +9,6 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <!-- Roslyn analyzer that enforces ConfigureAwait(false) -->
-    <PackageReference Include="ConfigureAwaitChecker.Analyzer" Version="$(ConfigureAwaitCheckerAnalyzerPackageVersion)" PrivateAssets="All" />
     <PackageReference Include="Grpc.Core.Api" Version="$(GrpcPackageVersion)" />
   </ItemGroup>
 
diff --git a/src/dotnet-grpc/Commands/CommandBase.cs b/src/dotnet-grpc/Commands/CommandBase.cs
@@ -105,7 +105,7 @@ public async Task EnsureNugetPackagesAsync(Services services)
                 if (dependency.ApplicableServices.Split(';').Any(s => string.Equals(s, services.ToString(), StringComparison.OrdinalIgnoreCase)))
                 {
                     // Check if the dependency is applicable to this SDK type
-                    if (dependency.ApplicableToWeb == null || string.Equals(dependency.ApplicableToWeb, IsUsingWebSdk.ToString(), StringComparison.OrdinalIgnoreCase))
+                    if (dependency.ApplicableToWeb == null || string.Equals(dependency.ApplicableToWeb, IsUsingWebSdk.ToString(CultureInfo.InvariantCulture), StringComparison.OrdinalIgnoreCase))
                     {
                         // Use the version specified from the remote file before falling back the packaged versions
                         var packageVersion = packageVersions?.GetValueOrDefault(dependency.Name) ?? dependency.Version;
@@ -326,7 +326,7 @@ internal string[] GlobReferences(string[] references)
                     expandedReferences.AddRange(
                         // The reference is relative to the project directory but GetFiles returns the full path.
                         // Remove the project directory portion of the path so relative references are maintained.
-                        resolvedFiles.Select(r => r.Replace(Project.DirectoryPath + Path.DirectorySeparatorChar, string.Empty)));
+                        resolvedFiles.Select(r => r.Replace(Project.DirectoryPath + Path.DirectorySeparatorChar, string.Empty, StringComparison.Ordinal)));
                 }
             }
 
@@ -343,7 +343,7 @@ public async Task DownloadFileAsync(string url, string destination, bool dryRun
             // The user must not specify a directory
             if (Path.EndsInDirectorySeparator(destination))
             {
-                throw new CLIToolException(string.Format(CoreStrings.ErrorOutputMustBeFilePath, destination));
+                throw new CLIToolException(string.Format(CultureInfo.InvariantCulture, CoreStrings.ErrorOutputMustBeFilePath, destination));
             }
 
             var resolveDestination = Path.IsPathRooted(destination) ? destination : Path.Combine(Project.DirectoryPath, destination);
@@ -354,7 +354,7 @@ public async Task DownloadFileAsync(string url, string destination, bool dryRun
                 // The user must not specify an existing directory
                 if (Directory.Exists(resolveDestination))
                 {
-                    throw new CLIToolException(string.Format(CoreStrings.ErrorOutputMustBeFilePath, destination));
+                    throw new CLIToolException(string.Format(CultureInfo.InvariantCulture, CoreStrings.ErrorOutputMustBeFilePath, destination));
                 }
 
                 // The destination file doesn't exist so content is modified.
diff --git a/src/dotnet-grpc/dotnet-grpc.csproj b/src/dotnet-grpc/dotnet-grpc.csproj
@@ -15,6 +15,10 @@
 
     <!-- Temporary workaround for MSBuild issue TODO (johluo): https://github.com/microsoft/msbuild/issues/4474 -->
     <GenerateResourceUsePreserializedResources>false</GenerateResourceUsePreserializedResources>
+
+    <!-- Disable analysis for ConfigureAwait(false) -->
+    <WarningsNotAsErrors>$(WarningsNotAsErrors);CA2007</WarningsNotAsErrors>
+    <NoWarn>$(NoWarn);CA2007</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@`
`18`	`18`
`19`	`19`	`using System;`
`20`	`20`	`using System.Collections.Generic;`
	`21`	`+using System.Globalization;`
`21`	`22`	`using System.Security.Cryptography.X509Certificates;`
`22`	`23`
`23`	`24`	`namespace Grpc.AspNetCore.Server.Internal`
`@@ -35,7 +36,7 @@ public static string[] GetDnsFromExtensions(X509Certificate2 cert)`
`35`	`36`	`string asnString = ext.Format(false);`
`36`	`37`	`if (string.IsNullOrWhiteSpace(asnString))`
`37`	`38`	`{`
`38`		`- return new string[0];`
	`39`	`+ return Array.Empty<string>();`
`39`	`40`	`}`
`40`	`41`
`41`	`42`	`// SubjectAlternativeNames might contain something other than a dNSName,`
`@@ -50,7 +51,7 @@ public static string[] GetDnsFromExtensions(X509Certificate2 cert)`
`50`	`51`	`for (var i = 0; i < rawDnsEntries.Length; i++)`
`51`	`52`	`{`
`52`	`53`	`string[] keyval = rawDnsEntries[i].Split(X509SubjectAlternativeNameConstants.Delimiter);`
`53`		`- if (string.Equals(keyval[0], X509SubjectAlternativeNameConstants.Identifier))`
	`54`	`+ if (string.Equals(keyval[0], X509SubjectAlternativeNameConstants.Identifier, StringComparison.Ordinal))`
`54`	`55`	`{`
`55`	`56`	`dnsEntries.Add(keyval[1]);`
`56`	`57`	`}`
`@@ -59,7 +60,7 @@ public static string[] GetDnsFromExtensions(X509Certificate2 cert)`
`59`	`60`	`return dnsEntries.ToArray();`
`60`	`61`	`}`
`61`	`62`	`}`
`62`		`- return new string[0];`
	`63`	`+ return Array.Empty<string>();`
`63`	`64`	`}`
`64`	`65`
`65`	`66`	`// We don't have a strongly typed extension to parse Subject Alt Names, so we have to do a workaround`
`@@ -106,6 +107,7 @@ private static void EnsureInitialized()`
`106`	`107`	`if (!s_successfullyInitialized)`
`107`	`108`	`{`
`108`	`109`	`throw new FormatException(string.Format(`
	`110`	`+ CultureInfo.InvariantCulture,`
`109`	`111`	`"There was an error detecting the identifier, delimiter, and separator for X509CertificateClaims on this platform.{0}" +`
`110`	`112`	`"Detected values were: Identifier: '{1}'; Delimiter:'{2}'; Separator:'{3}'",`
`111`	`113`	`Environment.NewLine,`
`@@ -138,7 +140,7 @@ static X509SubjectAlternativeNameConstants()`
`138`	`140`	`// Linux: x509ExtensionFormattedString is: "DNS:not-real-subject-name, DNS:example.com"`
`139`	`141`	`// Parse: <identifier><delimter><value><separator(s)>`
`140`	`142`
`141`		`- int delimiterIndex = x509ExtensionFormattedString.IndexOf(subjectName1) - 1;`
	`143`	`+ int delimiterIndex = x509ExtensionFormattedString.IndexOf(subjectName1, StringComparison.Ordinal) - 1;`
`142`	`144`	`s_delimiter = x509ExtensionFormattedString[delimiterIndex];`
`143`	`145`
`144`	`146`	`// Make an assumption that all characters from the the start of string to the delimiter`