Skip to content

Commit

Permalink
Turning on CodeAnalysis
Browse files Browse the repository at this point in the history
- Fixed over 300 errors.
- Identified potential issues to be fixed later.
  • Loading branch information
robertmclaws committed Jan 1, 2019
1 parent 74f12fa commit 85d0770
Show file tree
Hide file tree
Showing 68 changed files with 749 additions and 765 deletions.
46 changes: 34 additions & 12 deletions Directory.Build.props
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
<Project>
<!--<Import Project="version.props" />-->

<!-- Folder layout -->
<PropertyGroup>
<IsBenchmarkProject Condition="$(MSBuildProjectName.EndsWith('.Performance'))">true</IsBenchmarkProject>
<IsTestProject Condition="$(MSBuildProjectName.ToLower().Contains('.tests.'))">true</IsTestProject>
<IsTestAssetProject Condition="$(RepoRelativeProjectDir.Contains('testassets'))">true</IsTestAssetProject>
<IsSampleProject Condition="$(MSBuildProjectName.ToLower().Contains('.samples.'))">true</IsSampleProject>

<IncludeSource>false</IncludeSource>
<IncludeSymbols>true</IncludeSymbols>

</PropertyGroup>

<PropertyGroup>
<Product>Microsoft Restier</Product>

Expand Down Expand Up @@ -43,21 +55,20 @@

<!-- Fixes a common error in targets implementing a NoBuild mode. -->
<BuildProjectReferences Condition=" '$(NoBuild)' == 'true' ">false</BuildProjectReferences>
</PropertyGroup>

<!-- Folder layout -->
<PropertyGroup>
<IsBenchmarkProject Condition="$(MSBuildProjectName.EndsWith('.Performance'))">true</IsBenchmarkProject>
<IsTestProject Condition="$(MSBuildProjectName.Contains('.Tests.'))">true</IsTestProject>
<IsTestAssetProject Condition="$(RepoRelativeProjectDir.Contains('testassets'))">true</IsTestAssetProject>
<IsSampleProject Condition="$(MSBuildProjectName.Contains('.Samples.'))">true</IsSampleProject>

<IncludeSource>false</IncludeSource>
<IncludeSymbols>true</IncludeSymbols>

<!-- Suppress warnings about uninstantiated classes. -->
<NoWarn>$(NoWarn);CA1812</NoWarn>
</PropertyGroup>

<PropertyGroup Condition=" '$(IsBenchmarkProject)' != 'true' And '$(IsTestProject)' != 'true' And '$(IsTestAssetProject)' != 'true' And '!$(IsSampleProject)' != 'true' and $(Configuration) != 'Release' ">
<PropertyGroup Condition=" $(IsTestProject) == 'true' ">
<NoWarn>$(NoWarn);CA1001;CA1707;CA2007;CA1801</NoWarn>
</PropertyGroup>

<PropertyGroup Condition=" $(IsSampleProject) == 'true' ">
<NoWarn>$(NoWarn);CA1001;CA1707;CA1716;CA1801;CA1822</NoWarn>
</PropertyGroup>

<PropertyGroup Condition=" '$(IsBenchmarkProject)' != 'true' And '$(IsTestProject)' != 'true' And '$(IsTestAssetProject)' != 'true' And '!$(IsSampleProject)' != 'true' and $(Configuration) == 'Debug' ">
<GeneratePackageOnBuild>true</GeneratePackageOnBuild>
<GenerateDocumentationFile>true</GenerateDocumentationFile>
</PropertyGroup>
Expand All @@ -66,4 +77,15 @@
<StandardTestTfms>netcoreapp2.2;net461</StandardTestTfms>
</PropertyGroup>

<ItemGroup Condition=" $(IsTestProject) != 'true' and $(IsSampleProject) != 'true'">
<!-- RWM: The code quality on the unit tests right now is not awesome, so let's skip checking it for now. -->
<PackageReference Include="Microsoft.CodeQuality.Analyzers" Version="2.6.2" />
</ItemGroup>

<ItemGroup Condition=" $(IsTestProject) == 'true' ">
<PackageReference Include="FluentAssertions" Version="5.5.3" />
<PackageReference Include="FluentAssertions.Analyzers" Version="0.11.4" />
</ItemGroup>


</Project>
1 change: 1 addition & 0 deletions RESTier.sln
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution
ProjectSection(SolutionItems) = preProject
src\Directory.Build.props = src\Directory.Build.props
Directory.Build.props = Directory.Build.props
README.md = README.md
EndProjectSection
EndProject
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Samples", "Samples", "{DB42E0B8-C0C7-4DE4-9437-2B2A229B5F8F}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -40,21 +39,23 @@ public override async Task<ODataBatchResponseItem> SendRequestAsync(
HttpMessageInvoker invoker,
CancellationToken cancellationToken)
{
Ensure.NotNull(invoker, "invoker");
Ensure.NotNull(invoker, nameof(invoker));

RestierChangeSetProperty changeSetProperty = new RestierChangeSetProperty(this);
changeSetProperty.ChangeSet = new ChangeSet();
this.SetChangeSetProperty(changeSetProperty);
var changeSetProperty = new RestierChangeSetProperty(this)
{
ChangeSet = new ChangeSet()
};
SetChangeSetProperty(changeSetProperty);

Dictionary<string, string> contentIdToLocationMapping = new Dictionary<string, string>();
var contentIdToLocationMapping = new Dictionary<string, string>();
var responseTasks = new List<Task<Task<HttpResponseMessage>>>();

foreach (HttpRequestMessage request in Requests)
foreach (var request in Requests)
{
// Since exceptions may occure before the request is sent to RestierController,
// we must catch the exceptions here and call OnChangeSetCompleted,
// so as to avoid deadlock mentioned in Github Issue #82.
TaskCompletionSource<HttpResponseMessage> tcs = new TaskCompletionSource<HttpResponseMessage>();
var tcs = new TaskCompletionSource<HttpResponseMessage>();
var task =
SendMessageAsync(invoker, request, cancellationToken, contentIdToLocationMapping)
.ContinueWith(
Expand Down Expand Up @@ -86,14 +87,14 @@ public override async Task<ODataBatchResponseItem> SendRequestAsync(
// - the ChangeSet is submitted
// - the responses are created and
// - the controller actions have returned
await Task.WhenAll(responseTasks);
await Task.WhenAll(responseTasks).ConfigureAwait(false);

List<HttpResponseMessage> responses = new List<HttpResponseMessage>();
var responses = new List<HttpResponseMessage>();
try
{
foreach (var responseTask in responseTasks)
{
HttpResponseMessage response = responseTask.Result.Result;
var response = responseTask.Result.Result;
if (response.IsSuccessStatusCode)
{
responses.Add(response);
Expand All @@ -116,18 +117,21 @@ public override async Task<ODataBatchResponseItem> SendRequestAsync(
return new ChangeSetResponseItem(responses);
}

#pragma warning disable CA1822 // Do not declare static members on generic types
internal async Task SubmitChangeSet(HttpRequestMessage request, ChangeSet changeSet)
#pragma warning restore CA1822 // Do not declare static members on generic types

{
var requestContainer = request.GetRequestContainer();
using (var api = requestContainer.GetService<ApiBase>())
{
SubmitResult submitResults = await api.SubmitAsync(changeSet);
var submitResults = await api.SubmitAsync(changeSet).ConfigureAwait(false);
}
}

private static void DisposeResponses(IEnumerable<HttpResponseMessage> responses)
{
foreach (HttpResponseMessage response in responses)
foreach (var response in responses)
{
if (response != null)
{
Expand All @@ -138,7 +142,7 @@ private static void DisposeResponses(IEnumerable<HttpResponseMessage> responses)

private void SetChangeSetProperty(RestierChangeSetProperty changeSetProperty)
{
foreach (HttpRequestMessage request in this.Requests)
foreach (var request in Requests)
{
request.SetChangeSet(changeSetProperty);
}
Expand Down
32 changes: 12 additions & 20 deletions src/Microsoft.Restier.AspNet/Batch/RestierBatchHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,29 +35,25 @@ public RestierBatchHandler(HttpServer httpServer)
/// <param name="request">The HTTP request that contains the batch requests.</param>
/// <param name="cancellationToken">The cancellation token.</param>
/// <returns>The task object that represents this asynchronous operation.</returns>
public override async Task<IList<ODataBatchRequestItem>> ParseBatchRequestsAsync(
HttpRequestMessage request,
CancellationToken cancellationToken)
public override async Task<IList<ODataBatchRequestItem>> ParseBatchRequestsAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
Ensure.NotNull(request, "request");
Ensure.NotNull(request, nameof(request));

IServiceProvider requestContainer = request.CreateRequestContainer(ODataRouteName);
var requestContainer = request.CreateRequestContainer(ODataRouteName);
requestContainer.GetRequiredService<ODataMessageReaderSettings>().BaseUri = GetBaseUri(request);

ODataMessageReader reader
= await request.Content.GetODataMessageReaderAsync(requestContainer, cancellationToken);
var reader = await request.Content.GetODataMessageReaderAsync(requestContainer, cancellationToken).ConfigureAwait(false);
request.RegisterForDispose(reader);

List<ODataBatchRequestItem> requests = new List<ODataBatchRequestItem>();
ODataBatchReader batchReader = reader.CreateODataBatchReader();
Guid batchId = Guid.NewGuid();
var requests = new List<ODataBatchRequestItem>();
var batchReader = reader.CreateODataBatchReader();
var batchId = Guid.NewGuid();
while (batchReader.Read())
{
if (batchReader.State == ODataBatchReaderState.ChangesetStart)
{
IList<HttpRequestMessage> changeSetRequests =
await batchReader.ReadChangeSetRequestAsync(batchId, cancellationToken);
foreach (HttpRequestMessage changeSetRequest in changeSetRequests)
var changeSetRequests = await batchReader.ReadChangeSetRequestAsync(batchId, cancellationToken).ConfigureAwait(false);
foreach (var changeSetRequest in changeSetRequests)
{
changeSetRequest.CopyBatchRequestProperties(request);
changeSetRequest.DeleteRequestContainer(false);
Expand All @@ -67,8 +63,7 @@ ODataMessageReader reader
}
else if (batchReader.State == ODataBatchReaderState.Operation)
{
HttpRequestMessage operationRequest = await batchReader.ReadOperationRequestAsync(
batchId, true, cancellationToken);
var operationRequest = await batchReader.ReadOperationRequestAsync(batchId, true, cancellationToken).ConfigureAwait(false);
operationRequest.CopyBatchRequestProperties(request);
operationRequest.DeleteRequestContainer(false);
requests.Add(new OperationRequestItem(operationRequest));
Expand All @@ -83,10 +78,7 @@ ODataMessageReader reader
/// </summary>
/// <param name="changeSetRequests">The list of changeset requests.</param>
/// <returns>The created <see cref="RestierBatchChangeSetRequestItem"/> instance.</returns>
protected virtual RestierBatchChangeSetRequestItem CreateRestierBatchChangeSetRequestItem(
IList<HttpRequestMessage> changeSetRequests)
{
return new RestierBatchChangeSetRequestItem(changeSetRequests);
}
protected virtual RestierBatchChangeSetRequestItem CreateRestierBatchChangeSetRequestItem(IList<HttpRequestMessage> changeSetRequests) =>
new RestierBatchChangeSetRequestItem(changeSetRequests);
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.ComponentModel;
using System.Globalization;
using System.Threading.Tasks;
using Microsoft.AspNet.OData.Batch;
using Microsoft.AspNet.OData.Extensions;
using Microsoft.AspNet.OData.Routing;
Expand All @@ -9,12 +13,8 @@
using Microsoft.Restier.AspNet;
using Microsoft.Restier.AspNet.Batch;
using Microsoft.Restier.Core;
using System.Collections.Generic;
using System.ComponentModel;
using System.Globalization;
using System.Threading.Tasks;
using ServiceLifetime = Microsoft.OData.ServiceLifetime;
using RAResources = Microsoft.Restier.AspNet.Resources;
using ServiceLifetime = Microsoft.OData.ServiceLifetime;


namespace System.Web.Http
Expand All @@ -26,7 +26,9 @@ namespace System.Web.Http
public static class HttpConfigurationExtensions
{
private const string UseVerboseErrorsFlagKey = "Microsoft.Restier.UseVerboseErrorsFlag";
#pragma warning disable CA1823 // Do not declare static members on generic types
private const string RootContainerKey = "Microsoft.AspNet.OData.RootContainerMappingsKey";
#pragma warning restore CA1823 // Do not declare static members on generic types

/// TODO GitHubIssue#51 : Support model lazy loading
/// <summary>
Expand All @@ -47,9 +49,7 @@ public static Task<ODataRoute> MapRestierRoute<TApi>(
{
// This will be added a service to callback stored in ApiConfiguration
// Callback is called by ApiBase.AddApiServices method to add real services.
ApiBase.AddPublisherServices(
typeof(TApi),
services =>
ApiBase.AddPublisherServices(typeof(TApi), services =>
{
services.AddODataServices<TApi>();
});
Expand All @@ -64,8 +64,8 @@ public static Task<ODataRoute> MapRestierRoute<TApi>(
}

void configureAction(IContainerBuilder builder) => builder
.AddService<IEnumerable<IODataRoutingConvention>>(ServiceLifetime.Singleton, sp => conventions)
.AddService<ODataBatchHandler>(ServiceLifetime.Singleton, sp => batchHandler);
.AddService<IEnumerable<IODataRoutingConvention>>(ServiceLifetime.Singleton, sp => conventions)
.AddService<ODataBatchHandler>(ServiceLifetime.Singleton, sp => batchHandler);

var route = config.MapODataServiceRoute(routeName, routePrefix, configureAction);

Expand All @@ -81,8 +81,7 @@ public static bool GetUseVerboseErrors(this HttpConfiguration configuration)
{
if (configuration == null)
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture, RAResources.ArgumentCannotBeNull, "configuration"));
throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, RAResources.ArgumentCannotBeNull, "configuration"));
}

var useVerboseErrorsFlag = false;
Expand All @@ -105,8 +104,7 @@ public static void SetUseVerboseErrors(this HttpConfiguration configuration, boo
{
if (configuration == null)
{
throw new ArgumentException(string.Format(
CultureInfo.InvariantCulture, RAResources.ArgumentCannotBeNull, "configuration"));
throw new ArgumentException(string.Format(CultureInfo.InvariantCulture, RAResources.ArgumentCannotBeNull, "configuration"));
}

configuration.Properties[UseVerboseErrorsFlagKey] = useVerboseErrors;
Expand All @@ -125,8 +123,7 @@ private static IList<IODataRoutingConvention> CreateRestierRoutingConventions(
var index = 0;
for (; index < conventions.Count; index++)
{
var attributeRouting = conventions[index] as AttributeRoutingConvention;
if (attributeRouting != null)
if (conventions[index] is AttributeRoutingConvention attributeRouting)
{
break;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ internal static class HttpRequestMessageExtensions
/// <param name="changeSetProperty">The change set to be set.</param>
public static void SetChangeSet(this HttpRequestMessage request, RestierChangeSetProperty changeSetProperty)
{
Ensure.NotNull(request, "request");
Ensure.NotNull(request, nameof(request));
request.Properties.Add(ChangeSetKey, changeSetProperty);
}

Expand All @@ -34,10 +34,9 @@ public static void SetChangeSet(this HttpRequestMessage request, RestierChangeSe
/// <returns>The <see cref="RestierChangeSetProperty"/>.</returns>
public static RestierChangeSetProperty GetChangeSet(this HttpRequestMessage request)
{
Ensure.NotNull(request, "request");
Ensure.NotNull(request, nameof(request));

object value;
if (request.Properties.TryGetValue(ChangeSetKey, out value))
if (request.Properties.TryGetValue(ChangeSetKey, out var value))
{
return value as RestierChangeSetProperty;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public override async Task OnExceptionAsync(

foreach (var handler in Handlers)
{
var result = await handler.Invoke(actionExecutedContext, useVerboseErros, cancellationToken);
var result = await handler.Invoke(actionExecutedContext, useVerboseErros, cancellationToken).ConfigureAwait(false);

if (result != null)
{
Expand All @@ -66,7 +66,7 @@ private static async Task<HttpResponseMessage> HandleChangeSetValidationExceptio
bool useVerboseErros,
CancellationToken cancellationToken)
{
ChangeSetValidationException validationException = context.Exception as ChangeSetValidationException;
var validationException = context.Exception as ChangeSetValidationException;
if (validationException != null)
{
var exceptionResult = new NegotiatedContentResult<IEnumerable<ValidationResultDto>>(
Expand All @@ -75,7 +75,7 @@ private static async Task<HttpResponseMessage> HandleChangeSetValidationExceptio
context.ActionContext.RequestContext.Configuration.Services.GetContentNegotiator(),
context.Request,
new MediaTypeFormatterCollection());
return await exceptionResult.ExecuteAsync(cancellationToken);
return await exceptionResult.ExecuteAsync(cancellationToken).ConfigureAwait(false);
}

return null;
Expand All @@ -98,7 +98,7 @@ private static Task<HttpResponseMessage> HandleCommonException(
return Task.FromResult<HttpResponseMessage>(null);
}

HttpStatusCode code = HttpStatusCode.Unused;
var code = HttpStatusCode.Unused;
if (exception is ODataException)
{
code = HttpStatusCode.BadRequest;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ public override void WriteObject(
ODataMessageWriter messageWriter,
ODataSerializerContext writeContext)
{
PrimitiveResult primitiveResult = graph as PrimitiveResult;
if (primitiveResult != null)
if (graph is PrimitiveResult primitiveResult)
{
graph = primitiveResult.Result;
type = primitiveResult.Type;
Expand Down Expand Up @@ -80,7 +79,7 @@ public override ODataPrimitiveValue CreateODataPrimitiveValue(

internal static object ConvertToPayloadValue(object value, ODataSerializerContext writeContext)
{
Ensure.NotNull(writeContext, "writeContext");
Ensure.NotNull(writeContext, nameof(writeContext));

IEdmTypeReference edmTypeReference = null;
if (writeContext.Path != null)
Expand Down
Loading

0 comments on commit 85d0770

Please sign in to comment.