Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update FxCop analyzers #17157

Merged
merged 5 commits into from
Nov 24, 2020
Merged

Update FxCop analyzers #17157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bda1e99
Update FxCop analyzers
pakrym Nov 23, 2020
ec15637
a few more fixes
pakrym Nov 23, 2020
1eec675
Merge remote-tracking branch 'azure/master' into pakrym/Update-FxCop-…
pakrym Nov 23, 2020
8c542e1
EnableBatchedOperations = true
pakrym Nov 23, 2020
bd4beac
fix storage tests
pakrym Nov 23, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions common/Stress/Azure.Test.Stress/StressMetrics.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
4 changes: 4 additions & 0 deletions eng/Directory.Build.Data.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 -->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Way too many false-positives

CA1031; <!-- Don't catch generic exceptions -->
CA2000; <!-- Call dispose on IDisposable objects -->
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Way too many false-positives

CA2012; <!-- ValueTask should only be awaited once - conflicts with EnsureCompleted check -->
</NoWarn>

<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
Expand Down
4 changes: 2 additions & 2 deletions eng/Packages.Data.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" />
Expand Down Expand Up @@ -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" />
Expand Down
38 changes: 19 additions & 19 deletions sdk/batch/Microsoft.Azure.Batch/src/AddTasksWorkflowManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -55,7 +55,7 @@ internal AddTasksWorkflowManager(
{
parallelOptions = new BatchClientParallelOptions();
}

//
// Set up the data structures associated with this workflow
//
Expand All @@ -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)
Expand All @@ -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)
{
Expand Down Expand Up @@ -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));
}

Expand All @@ -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
Expand Down Expand Up @@ -193,7 +193,7 @@ private void CheckForCancellationOrTimeoutAndThrow()
{
//We always throw when cancelation is requested
this._parallelOptions.CancellationToken.ThrowIfCancellationRequested();


DateTime currentTime = DateTime.UtcNow;

Expand All @@ -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>
Expand Down Expand Up @@ -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)
{
Expand All @@ -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>;
Expand Down Expand Up @@ -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)
{
Expand All @@ -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)
Expand All @@ -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);
}
Expand All @@ -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>
Expand All @@ -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++;
Expand Down
14 changes: 7 additions & 7 deletions sdk/batch/Microsoft.Azure.Batch/src/BatchClient.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -103,7 +103,7 @@ internal BatchClient(IProtocolLayer protocolLayer)
}

#endregion Constructors

#region IInheritedBehaviors

/// <summary>
Expand Down Expand Up @@ -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;
Expand Down
24 changes: 12 additions & 12 deletions sdk/batch/Microsoft.Azure.Batch/src/ConcurrentChangeTrackedList.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
Expand All @@ -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);
Expand Down Expand Up @@ -73,11 +73,11 @@ IEnumerator IEnumerable.GetEnumerator()
#endregion

#region IList

public void Add(T item)
{
this.ThrowOnReadOnly();

lock (this._listLock)
{
this._list.Add(item);
Expand Down Expand Up @@ -125,8 +125,8 @@ public bool Remove(T item)
}
}

public int Count
{
public int Count
{
get
{
lock (this._listLock)
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion sdk/batch/Microsoft.Azure.Batch/src/FileStaging/FileStagingUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
10 changes: 5 additions & 5 deletions sdk/batch/Microsoft.Azure.Batch/src/ODATAMonitor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public TimeSpan DelayBetweenDataFetch
{
value = _lowerBoundDelayBetweenRefresh;
}

_delayBetweenDataFetch = value;
}
}
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion sdk/batch/Microsoft.Azure.Batch/src/PagedEnumeratorBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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; } }

Expand Down
Loading