Added FxCop to Microsoft.BotBuilder and addressed errors.

BotBuilder-DotNet.ruleset

@@ -6,11 +6,16 @@
     <Rule Id="UseAsyncSuffix" Action="Error" />
     <Rule Id="UseConfigureAwait" Action="Error" />
   </Rules>
-
-  <Rules AnalyzerId="Microsoft.CodeAnalysis.CSharp.Features" RuleNamespace="Microsoft.CodeAnalysis.CSharp.Features">
-    <Rule Id="IDE0003" Action="None" />
+  <Rules AnalyzerId="Microsoft.CodeQuality.Analyzers" RuleNamespace="Microsoft.CodeQuality.Analyzers">
+    <Rule Id="CA1054" Action="None" /> <!-- UriParametersShouldNotBeStrings -->
+    <Rule Id="CA1062" Action="None" /> <!-- ValidateArgumentsOfPublicMethods -->
+  </Rules>
+  <Rules AnalyzerId="Microsoft.NetCore.Analyzers" RuleNamespace="Microsoft.NetCore.Analyzers">
+    <Rule Id="CA1303" Action="None" /> <!-- DoNotPassLiteralsAsLocalizedParameters -->
+  </Rules>
+  <Rules AnalyzerId="Microsoft.NetCore.CSharp.Analyzers" RuleNamespace="Microsoft.NetCore.CSharp.Analyzers">
+    <Rule Id="CA1309" Action="Error" /> <!-- Use ordinal StringComparison -->
   </Rules>
-
   <Rules AnalyzerId="StyleCop.Analyzers" RuleNamespace="StyleCop.Analyzers">
     <Rule Id="SA0001" Action="None" />
     <Rule Id="SA0002" Action="Error" />
@@ -186,15 +191,6 @@
     <Rule Id="SA1648" Action="Error" />
     <Rule Id="SA1649" Action="Error" />
     <Rule Id="SA1651" Action="Error" />
+	<Rule Id="SX1309" Action="Error" /> <!-- FieldNamesShouldBeginWithUnderscores -->
   </Rules>
-
-  <Rules AnalyzerId="Microsoft.CodeQuality.Analyzers" RuleNamespace="Microsoft.CodeQuality.Analyzers">
-	<Rule Id="CA1054" Action="None" /> <!-- UriParametersShouldNotBeStrings -->	
-	<Rule Id="CA1062" Action="None" /> <!-- ValidateArgumentsOfPublicMethods -->	
-  </Rules>
-
-  <Rules AnalyzerId="Microsoft.NetCore.Analyzers" RuleNamespace="Microsoft.NetCore.Analyzers">
-	<Rule Id="CA1303" Action="None" /> <!-- DoNotPassLiteralsAsLocalizedParameters -->
-  </Rules>  
-
 </RuleSet>

Directory.Build.props

@@ -59,7 +59,7 @@
       Suppress a warning about upcoming deprecation of PackageLicenseUrl. When embedding licenses are supported,
       replace PackageLicenseUrl with PackageLicenseExpression.
     -->
-    <NoWarn>NU5125</NoWarn>
+    <NoWarn>$(NoWarn);NU5125</NoWarn>
     <PublishRepositoryUrl>true</PublishRepositoryUrl>
     <IncludeSymbols>true</IncludeSymbols>
     <SymbolPackageFormat>snupkg</SymbolPackageFormat>

FunctionalTests/Directory.Build.props

@@ -1,8 +1,9 @@
 <Project>
   <!-- Contains common properties that apply to projects under the FunctionalTests folder -->
   <PropertyGroup>
-    <!-- For tests, we don't generate documentation. Supress related rules. -->
-    <NoWarn>$(NoWarn);SA0001;CS1573,CS1591,CS1712</NoWarn>
+    <!-- SA0001;CS1573,CS1591,CS1712: For tests, we don't generate documentation. Supress related rules. -->
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);SA0001;CS1573;CS1591;CS1712;SX1309</NoWarn>
   </PropertyGroup>
 
   <!-- This ensures that Directory.Build.props in parent folders are merged with this one -->

libraries/Adapters/Directory.Build.props

@@ -1,7 +1,11 @@
 <Project>
   <!-- Contains common properties that apply to projects under the Adapters folder -->
   <ItemGroup>
-    <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.6">
+    <PackageReference Include="AsyncUsageAnalyzers" Version="1.0.0-alpha003">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.0.0">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>

libraries/Adapters/Microsoft.Bot.Builder.Adapters.Twilio/TwilioAdapter.cs

@@ -81,7 +81,7 @@ public override async Task<ResourceResponse[]> SendActivitiesAsync(ITurnContext
                 {
                     var messageOptions = TwilioHelper.ActivityToTwilio(activity, _twilioClient.Options.TwilioNumber);
 
-                    var res = await _twilioClient.SendMessage(messageOptions, cancellationToken).ConfigureAwait(false);
+                    var res = await _twilioClient.SendMessageAsync(messageOptions, cancellationToken).ConfigureAwait(false);
 
                     var response = new ResourceResponse() { Id = res, };
 

libraries/Adapters/Microsoft.Bot.Builder.Adapters.Twilio/TwilioClientWrapper.cs

@@ -60,7 +60,7 @@ public TwilioClientWrapper(TwilioAdapterOptions options)
         /// <param name="messageOptions">An object containing the parameters for the message to send.</param>
         /// <param name="cancellationToken">A cancellation token for the task.</param>
         /// <returns>The SID of the Twilio message sent.</returns>
-        public virtual async Task<string> SendMessage(CreateMessageOptions messageOptions, CancellationToken cancellationToken)
+        public virtual async Task<string> SendMessageAsync(CreateMessageOptions messageOptions, CancellationToken cancellationToken)
         {
             var messageResource = await MessageResource.CreateAsync(messageOptions).ConfigureAwait(false);
             return messageResource.Sid;

libraries/AdaptiveExpressions/AdaptiveExpressions.csproj

@@ -18,7 +18,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <PropertyGroup>

libraries/Microsoft.Bot.Builder.AI.LUIS/Microsoft.Bot.Builder.AI.Luis.csproj

@@ -25,7 +25,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.AI.QnA/Microsoft.Bot.Builder.AI.QnA.csproj

@@ -24,7 +24,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.Azure/Microsoft.Bot.Builder.Azure.csproj

@@ -23,7 +23,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <PropertyGroup>

libraries/Microsoft.Bot.Builder.Dialogs.Adaptive.Testing/Microsoft.Bot.Builder.Dialogs.Adaptive.Testing.csproj

@@ -29,7 +29,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.Dialogs.Adaptive/Microsoft.Bot.Builder.Dialogs.Adaptive.csproj

@@ -26,7 +26,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/Microsoft.Bot.Builder.Dialogs.Debugging.csproj

@@ -26,7 +26,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.Dialogs.Declarative/Microsoft.Bot.Builder.Dialogs.Declarative.csproj

@@ -25,7 +25,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.Dialogs/Microsoft.Bot.Builder.Dialogs.csproj

@@ -27,7 +27,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.LanguageGeneration/Microsoft.Bot.Builder.LanguageGeneration.csproj

@@ -25,7 +25,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder.TemplateManager/Microsoft.Bot.Builder.TemplateManager.csproj

@@ -23,7 +23,8 @@
 
   <PropertyGroup>
     <!-- These documentation warnings excludes should be removed as part of https://github.com/microsoft/botbuilder-dotnet/issues/4052 -->
-    <NoWarn>$(NoWarn);CS1591</NoWarn>
+    <!-- SX1309: FieldNamesShouldBeginWithUnderscores should be fixed as part of https://github.com/microsoft/botframework-sdk/issues/5933 -->
+    <NoWarn>$(NoWarn);CS1591;SX1309</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

libraries/Microsoft.Bot.Builder/ActivityFactory.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.Linq;
 using System.Reflection;
 using Microsoft.Bot.Schema;
@@ -14,11 +15,15 @@ namespace Microsoft.Bot.Builder
     /// The ActivityFactory
     /// to generate text and then uses simple markdown semantics like chatdown to create Activity.
     /// </summary>
+#pragma warning disable CA1052 // Static holder types should be Static or NotInheritable (we can't change this without breaking binary compat)
     public class ActivityFactory
+#pragma warning restore CA1052 // Static holder types should be Static or NotInheritable
     {
+#pragma warning disable CA1308 // Normalize strings to uppercase (LG is heavily invested in lowercase, ignoring this rule in this class)
         private const string LGType = "lgType";
-        private static readonly string ErrorPrefix = "[ERROR]";
-        private static readonly string WarningPrefix = "[WARNING]";
+        private const string ErrorPrefix = "[ERROR]";
+        private const string WarningPrefix = "[WARNING]";
+        private const string AdaptiveCardType = "application/vnd.microsoft.card.adaptive";
 
         private static readonly IList<string> AllActivityTypes = GetAllPublicConstantValues<string>(typeof(ActivityTypes));
         private static readonly IList<string> AllActivityProperties = GetAllProperties(typeof(Activity));
@@ -36,8 +41,6 @@ public class ActivityFactory
             { nameof(ReceiptCard).ToLowerInvariant(), ReceiptCard.ContentType },
         };
 
-        private static readonly string AdaptiveCardType = "application/vnd.microsoft.card.adaptive";
-
         /// <summary>
         /// Generate the activity.
         /// Support Both string LG result and structured LG result.
@@ -47,7 +50,7 @@ public class ActivityFactory
         public static Activity FromObject(object lgResult)
         {
             var diagnostics = CheckLGResult(lgResult);
-            var errors = diagnostics.Where(u => u.StartsWith(ErrorPrefix));
+            var errors = diagnostics.Where(u => u.StartsWith(ErrorPrefix, StringComparison.Ordinal));
 
             if (errors.Any())
             {
@@ -64,7 +67,9 @@ public static Activity FromObject(object lgResult)
                 var lgJsonResult = JObject.FromObject(lgResult);
                 return BuildActivityFromLGStructuredResult(lgJsonResult);
             }
+#pragma warning disable CA1031 // Do not catch general exception types (we should narrow down the exception being caught but for now we just attempt to build the activity from the text property)
             catch
+#pragma warning restore CA1031 // Do not catch general exception types
             {
                 return BuildActivityFromText(lgResult?.ToString()?.Trim());
             }
@@ -84,7 +89,7 @@ public static IList<string> CheckLGResult(object lgResult)
                     return new List<string> { BuildDiagnostic("LG output is empty", false) };
                 }
 
-                if (!lgStringResult.StartsWith("{") || !lgStringResult.EndsWith("}"))
+                if (!lgStringResult.StartsWith("{", StringComparison.Ordinal) || !lgStringResult.EndsWith("}", StringComparison.Ordinal))
                 {
                     return new List<string> { BuildDiagnostic("LG output is not a json object, and will fallback to string format.", false) };
                 }
@@ -94,7 +99,9 @@ public static IList<string> CheckLGResult(object lgResult)
                 {
                     lgStructuredResult = JObject.Parse(lgStringResult);
                 }
+#pragma warning disable CA1031 // Do not catch general exception types (we should narrow down the exception being caught but for now we just show an error message)
                 catch
+#pragma warning restore CA1031 // Do not catch general exception types
                 {
                     return new List<string> { BuildDiagnostic("LG output is not a json object, and will fallback to string format.", false) };
                 }
@@ -108,7 +115,9 @@ public static IList<string> CheckLGResult(object lgResult)
                 {
                     lgStructuredResult = JObject.FromObject(lgResult);
                 }
+#pragma warning disable CA1031 // Do not catch general exception types (we should narrow down the exception being caught but for now we just show an error message)
                 catch
+#pragma warning restore CA1031 // Do not catch general exception types
                 {
                     return new List<string> { BuildDiagnostic("LG output is not a json object, and will fallback to string format.", false) };
                 }
@@ -414,7 +423,8 @@ private static bool IsValidBooleanValue(string boolValue, out bool boolResult)
                 boolResult = true;
                 return true;
             }
-            else if (boolValue.ToLowerInvariant() == "false")
+
+            if (boolValue.ToLowerInvariant() == "false")
             {
                 boolResult = false;
                 return true;
@@ -757,5 +767,6 @@ private static IList<string> GetAllProperties(Type type)
         {
             return type.GetProperties().Select(u => u.Name.ToLowerInvariant()).ToList();
         }
+#pragma warning restore CA1308 // Normalize strings to uppercase
     }
 }

libraries/Microsoft.Bot.Builder/ActivityHandler.cs

@@ -563,20 +563,40 @@ protected virtual Task OnUnrecognizedActivityTypeAsync(ITurnContext turnContext,
             return Task.CompletedTask;
         }
 
+#pragma warning disable CA1064 // Exceptions should be public (we can't change this without breaking binary compat, we may consider making this type public in the future)
         protected class InvokeResponseException : Exception
+#pragma warning restore CA1064 // Exceptions should be public
         {
-            private HttpStatusCode _statusCode;
-            private object _body;
+            private readonly HttpStatusCode _statusCode;
+            private readonly object _body;
 
             public InvokeResponseException(HttpStatusCode statusCode, object body = null)
             {
                 _statusCode = statusCode;
                 _body = body;
             }
 
+            public InvokeResponseException()
+            {
+            }
+
+            public InvokeResponseException(string message) 
+                : base(message)
+            {
+            }
+
+            public InvokeResponseException(string message, Exception innerException) 
+                : base(message, innerException)
+            {
+            }
+
             public InvokeResponse CreateInvokeResponse()
             {
-                return new InvokeResponse { Status = (int)_statusCode, Body = _body };
+                return new InvokeResponse
+                {
+                    Status = (int)_statusCode,
+                    Body = _body
+                };
             }
         }
     }