Update FxCop analyzers (#17157)

common/Stress/Azure.Test.Stress/StressMetrics.cs

@@ -225,10 +225,10 @@ private static string FormatTable(IEnumerable<(string Key, object Value)> data)
             foreach (var kvp in data)
             {
                 sb.Append(kvp.Key);
-                sb.Append(":");
+                sb.Append(':');
                 for (var i = kvp.Key.Length + 1; i < longestKeyLength + padding + 1; i++)
                 {
-                    sb.Append(" ");
+                    sb.Append(' ');
                 }
                 sb.Append(kvp.Value);
                 sb.AppendLine();

eng/Directory.Build.Data.props

@@ -25,6 +25,10 @@
       CA2231; <!-- Override Equality operators -->
       CA2225; <!-- Provide alternative to implicit operators -->
       CA1714; <!-- Flags should have plural names -->
+      CA1062; <!-- Public parameter should be checked for null -->
+      CA1031; <!-- Don't catch generic exceptions -->
+      CA2000; <!-- Call dispose on IDisposable objects -->
+      CA2012; <!-- ValueTask should only be awaited once - conflicts with EnsureCompleted check -->
     </NoWarn>
 
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>

eng/Packages.Data.props

@@ -9,7 +9,7 @@
   <ItemGroup>
     <PackageReference Update="ApprovalTests" Version="3.0.22" />
     <PackageReference Update="ApprovalUtilities" Version="3.0.22" />
-    <PackageReference Update="AutoRest.CSharp.V3" Version="1.0.0-alpha.20201113.1" />
+    <PackageReference Update="AutoRest.CSharp.V3" Version="1.0.0-alpha.20201123.1" />
     <PackageReference Update="Azure.AI.FormRecognizer" Version="3.0.0" />
 		<PackageReference Update="Azure.AI.TextAnalytics" Version="5.0.0" />
     <PackageReference Update="Azure.Data.AppConfiguration" Version="1.0.0" />
@@ -63,7 +63,7 @@
     <PackageReference Update="Microsoft.Build.Tasks.Core" Version="15.5.180" />
     <PackageReference Update="Microsoft.Build" Version="15.5.180" />
     <PackageReference Update="Microsoft.CodeAnalysis.CSharp.Workspaces" Version="2.6.1" />
-    <PackageReference Update="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.6.2" />
+    <PackageReference Update="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="3.3.1" />
     <PackageReference Update="Microsoft.CodeAnalysis" Version="2.3.0" />
     <PackageReference Update="Microsoft.DotNet.ApiCompat" Version="5.0.0-beta.20467.1" />
     <PackageReference Update="Microsoft.IdentityModel.Clients.ActiveDirectory" Version="4.5.1" />

sdk/batch/Microsoft.Azure.Batch/src/AddTasksWorkflowManager.cs

@@ -42,7 +42,7 @@ internal class AddTasksWorkflowManager
         /// <param name="fileStagingArtifacts">File staging artifacts associated with this operation.  If the customer does not set this, it is unviewable by them.</param>
         /// <param name="bhMgr">The behavior manager.</param>
         internal AddTasksWorkflowManager(
-            JobOperations jobOperations, 
+            JobOperations jobOperations,
             string jobId,
             BatchClientParallelOptions parallelOptions,
             ConcurrentBag<ConcurrentDictionary<Type, IFileStagingArtifact>> fileStagingArtifacts,
@@ -55,7 +55,7 @@ internal AddTasksWorkflowManager(
             {
                 parallelOptions = new BatchClientParallelOptions();
             }
-            
+
             //
             // Set up the data structures associated with this workflow
             //
@@ -69,7 +69,7 @@ internal AddTasksWorkflowManager(
             this._behaviorManager = bhMgr;
             this._maxTasks = Constants.MaxTasksInSingleAddTaskCollectionRequest;
             this._hasRun = HasNotRun; //Has not run by default
-            
+
             //Read the behavior manager and populate the collection
             List<AddTaskCollectionResultHandler> behaviorList = this._behaviorManager.GetBehaviors<AddTaskCollectionResultHandler>();
             foreach (AddTaskCollectionResultHandler handler in behaviorList)
@@ -96,7 +96,7 @@ public async System.Threading.Tasks.Task AddTasksAsync(IEnumerable<CloudTask> ta
             {
                 throw new RunOnceException(string.Format(CultureInfo.InvariantCulture, BatchErrorMessages.CanOnlyBeRunOnceFailure, this.GetType().Name));
             }
-            
+
             //Determine what time to timeout at
             if (timeout != null)
             {
@@ -131,12 +131,12 @@ public async System.Threading.Tasks.Task AddTasksAsync(IEnumerable<CloudTask> ta
                 {
                     throw new ArgumentNullException(nameof(tasksToAdd), BatchErrorMessages.CollectionMustNotContainNull);
                 }
-                
+
                 if (cloudTask.BindingState == BindingState.Bound)
                 {
                     throw UtilitiesInternal.OperationForbiddenOnBoundObjects;
                 }
-                
+
                 this._remainingTasksToAdd.Enqueue(new TrackedCloudTask(this._jobId, this._jobOperations, cloudTask));
             }
 
@@ -149,12 +149,12 @@ public async System.Threading.Tasks.Task AddTasksAsync(IEnumerable<CloudTask> ta
                 //1. We have no free request slots
                 //2. We have no more tasks to add and there are ongoing pending operations which could result in task add retries (causing us to get more tasks to add)
                 bool noFreeSlots = this._pendingAsyncOperations.Count >= this._parallelOptions.MaxDegreeOfParallelism;
-                bool noTasksToAddButSomePendingLegsRemain = this._remainingTasksToAdd.Count == 0 && this._pendingAsyncOperations.Count > 0;
+                bool noTasksToAddButSomePendingLegsRemain = this._remainingTasksToAdd.IsEmpty && this._pendingAsyncOperations.Count > 0;
                 if (noFreeSlots || noTasksToAddButSomePendingLegsRemain)
                 {
                     await this.ProcessPendingOperationResults().ConfigureAwait(continueOnCapturedContext: false);
                 }
-                
+
                 //If we get here, we are starting a single new leg.  Another iteration of this loop will get to any tasks which do not fit in this leg.
 
                 //Add any tasks (up to max count in 1 request) which are remaining since we have an open parallel slot
@@ -193,7 +193,7 @@ private void CheckForCancellationOrTimeoutAndThrow()
         {
             //We always throw when cancelation is requested
             this._parallelOptions.CancellationToken.ThrowIfCancellationRequested();
-         
+
 
             DateTime currentTime = DateTime.UtcNow;
 
@@ -209,7 +209,7 @@ private void CheckForCancellationOrTimeoutAndThrow()
         /// <returns>True if the workflow has successfully completed, false if it has not.</returns>
         private bool IsWorkflowDone()
         {
-            return !(this._remainingTasksToAdd.Count > 0 || this._pendingAsyncOperations.Count > 0);
+            return !(!this._remainingTasksToAdd.IsEmpty || this._pendingAsyncOperations.Count > 0);
         }
 
         /// <summary>
@@ -254,7 +254,7 @@ private async Task StageFilesAndAddTasks(
 
             // wait for file staging async task
             await fileStagingTask.ConfigureAwait(continueOnCapturedContext: false);
-            
+
             // now update each non-finalized Task with its new ResourceFiles
             foreach (TrackedCloudTask taskToAdd in tasksToAdd.Values)
             {
@@ -279,7 +279,7 @@ private async Task StageFilesAndAddTasks(
                             }
                         }
                     }
-                    
+
                     //Mark the file staging collection as read only just incase there's another reference to it
                     ConcurrentChangeTrackedList<IFileStagingProvider> filesToStageListImpl =
                         taskToAdd.Task.FilesToStage as ConcurrentChangeTrackedList<IFileStagingProvider>;
@@ -353,13 +353,13 @@ private void ProcessProtocolAddTaskResults(
             {
                 string taskId = protoAddTaskResult.TaskId;
                 TrackedCloudTask trackedTask = taskMap[taskId];
-                
+
                 AddTaskResult omResult = new AddTaskResult(trackedTask.Task, trackedTask.RetryCount, protoAddTaskResult);
-                
+
                 //We know that there must be at least one AddTaskResultHandler so the below ForEach will always be called
                 //at least once.
                 AddTaskResultStatus status = AddTaskResultStatus.Success; //The default is success to avoid infinite retry
-                
+
                 //Call the customer defined result handler
                 foreach (var resultHandlerFunction in this._addTaskResultHandlerCollection)
                 {
@@ -385,7 +385,7 @@ private void ProcessProtocolAddTaskResults(
         private async Task ProcessPendingOperationResults()
         {
             //Wait for any task to complete
-            Task completedTask = await Task.WhenAny(this._pendingAsyncOperations).ConfigureAwait(continueOnCapturedContext: false); 
+            Task completedTask = await Task.WhenAny(this._pendingAsyncOperations).ConfigureAwait(continueOnCapturedContext: false);
 
             //Check for a task failure -- if there is one, we a-wait for all remaining tasks to complete (this will throw an exception since at least one of them failed).
             if (completedTask.IsFaulted)
@@ -395,7 +395,7 @@ private async Task ProcessPendingOperationResults()
             else
             {
                 await completedTask.ConfigureAwait(continueOnCapturedContext: false); //This await should finish immediately and will not fail since the task has not faulted
-                
+
                 //Remove the task which completed
                 this._pendingAsyncOperations.Remove(completedTask);
             }
@@ -421,7 +421,7 @@ private static async Task WaitForTasksAndThrowParallelOperationsExceptionAsync(L
 #endregion
 
 #region Private classes
-        
+
         /// <summary>
         /// Internal task wrapper which tracks a tasks retry count and holds a reference to the protocol object and CloudTask.
         /// </summary>
@@ -444,7 +444,7 @@ internal TrackedCloudTask(
                 this.JobOperations = jobOperations;  // matt-c: do we really need one of these in every instance?  Even when they were wiMgrs couldnt something outside keep a reference?
                 this.RetryCount = 0;
             }
-            
+
             public void IncrementRetryCount()
             {
                 this.RetryCount++;

sdk/batch/Microsoft.Azure.Batch/src/BatchClient.cs

@@ -12,9 +12,9 @@ namespace Microsoft.Azure.Batch
     /// <summary>
     /// The dispose pattern sets all references to null.
     /// Put all references into this box.
-    /// 
+    ///
     /// ONLY ACCESS VIA GetStateThrowIfNotOpen() method!
-    /// 
+    ///
     /// </summary>
     internal class BatchClientDisposableStateBox
     {
@@ -56,7 +56,7 @@ public BatchClientDisposableStateBox(BatchClient parentBatchClient)
     public class BatchClient : IDisposable
     {
         private BatchClientDisposableStateBox _disposableStateBox;  // null state box signals that the instance is closed
-        private bool _disposed = false;  // used for dispose pattern
+        private bool _disposed;  // used for dispose pattern
         private readonly object _closeLocker = new object();
 
 #region // constructors
@@ -103,7 +103,7 @@ internal BatchClient(IProtocolLayer protocolLayer)
         }
 
 #endregion  Constructors
-        
+
 #region IInheritedBehaviors
 
         /// <summary>
@@ -275,13 +275,13 @@ internal BatchClientDisposableStateBox GetStateThrowIfNotOpen()
         /// <summary>
         /// Holds the protocol layer to be used for this client instance.
         /// This enables "mock"ing the protocol layer for testing.
-        /// 
+        ///
         /// Since 100% of all calls indirect through this property, it
         /// provides a single place to immediately stop all (new) call attempts
         /// when the underlying BatchClient is closed.
         /// </summary>
-        internal IProtocolLayer ProtocolLayer 
-        { 
+        internal IProtocolLayer ProtocolLayer
+        {
             get
             {
                 IProtocolLayer localProto = GetStateThrowIfNotOpen().ProtocolLayer;

sdk/batch/Microsoft.Azure.Batch/src/ConcurrentChangeTrackedList.cs

@@ -12,9 +12,9 @@ internal class ConcurrentChangeTrackedList<T> : IList<T>, IPropertyMetadata, IRe
     {
         protected readonly IList<T> _list;
         protected readonly object _listLock = new object();
-        
-        protected bool _hasBeenModified = false;
-        
+
+        protected bool _hasBeenModified;
+
         public ConcurrentChangeTrackedList()
         {
             this._list = new List<T>();
@@ -24,7 +24,7 @@ public ConcurrentChangeTrackedList()
         public ConcurrentChangeTrackedList(IEnumerable<T> other, bool isReadOnly = false)
         {
             this._list = new List<T>();
-            
+
             foreach (T item in other)
             {
                 this._list.Add(item);
@@ -73,11 +73,11 @@ IEnumerator IEnumerable.GetEnumerator()
         #endregion
 
         #region IList
-        
+
         public void Add(T item)
         {
             this.ThrowOnReadOnly();
-            
+
             lock (this._listLock)
             {
                 this._list.Add(item);
@@ -125,8 +125,8 @@ public bool Remove(T item)
             }
         }
 
-        public int Count 
-        { 
+        public int Count
+        {
             get
             {
                 lock (this._listLock)
@@ -170,16 +170,16 @@ public void RemoveAt(int index)
 
         public T this[int index]
         {
-            get 
-            { 
+            get
+            {
                 lock (this._listLock)
                 {
                     T result = this._list[index];
                     return result;
                 }
             }
 
-            set 
+            set
             {
                 this.ThrowOnReadOnly();
 
@@ -208,7 +208,7 @@ public void AddRange(IEnumerable<T> items)
 
         public IReadOnlyList<T> AsReadOnly()
         {
-            //As per http://msdn.microsoft.com/en-us/library/ms132474%28v=vs.110%29.aspx this is a wrapper around the collection, which will disallow modification but 
+            //As per http://msdn.microsoft.com/en-us/library/ms132474%28v=vs.110%29.aspx this is a wrapper around the collection, which will disallow modification but
             //will reflect any changes made to the underlying list
             return new ReadOnlyCollection<T>(this._list);
         }

sdk/batch/Microsoft.Azure.Batch/src/FileStaging/FileStagingUtils.cs

@@ -55,7 +55,7 @@ internal static async Task StageFilesAsync(List<IFileStagingProvider> filesToSta
                     throw new ArgumentNullException(nameof(allFileStagingArtifacts));
                 }
 
-                if (allFileStagingArtifacts.Count > 0)
+                if (!allFileStagingArtifacts.IsEmpty)
                 {
                     throw new ArgumentOutOfRangeException(nameof(allFileStagingArtifacts));
                 }

sdk/batch/Microsoft.Azure.Batch/src/ODATAMonitor.cs

@@ -35,7 +35,7 @@ public TimeSpan DelayBetweenDataFetch
                 {
                     value = _lowerBoundDelayBetweenRefresh;
                 }
-                
+
                 _delayBetweenDataFetch = value;
             }
         }
@@ -52,7 +52,7 @@ public TimeSpan DelayBetweenDataFetch
     /// <typeparam name="T"></typeparam>
     /// <returns></returns>
     internal delegate IPagedEnumerable<T> ListDelegate<T>();
-    
+
     /// <summary>
     /// A class that leverages OData predicates to poll the states of instances.
     /// </summary>
@@ -179,7 +179,7 @@ internal RefreshViaODATAFilterList(
             // queue that holds the instances that have been refreshed and failed the condition...
             // to be refreshed again on the next pass
             public Queue<MonitorLastCall<T>> NextPassQueue = new Queue<MonitorLastCall<T>>();
-            
+
             public CancellationToken CancellationToken { get; private set; }
 
             /// <summary>
@@ -206,7 +206,7 @@ internal RefreshViaODATAFilterList(
             private readonly StringBuilder _odataFilterSB = new StringBuilder();
 
             private const string IdPrefix = "id eq '";
-            private const string IdPostfix = "'";
+            private const char IdPostfix = '\'';
             private const string OrOperator = " or ";
 
             internal async Task ProcessOneInstance(MonitorLastCall<T> nextInstance, Func<T, string> getName)
@@ -322,7 +322,7 @@ internal async Task CallListAndProcessResults()
                     return;
                 }
 
-                // update the detail level 
+                // update the detail level
                 _odataDetailLevel.FilterClause = predicate;
 
                 // get the enumerable to refresh the instances

sdk/batch/Microsoft.Azure.Batch/src/PagedEnumeratorBase.cs

@@ -132,7 +132,7 @@ public void Dispose()
     internal class SkipTokenHandler
     {
         private string _skipToken;
-        private bool _hasBeenCalled = false;
+        private bool _hasBeenCalled;
 
         public bool AtLeastOneCallMade { get { return _hasBeenCalled; } set { _hasBeenCalled = value; } }