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.

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

Address all fxcop violations and wire up command line support to run it in CI #7

Merged
merged 5 commits into from
Feb 26, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions Microsoft.Rest/ClientRuntime.Etw/ClientRuntime.Etw.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
</PropertyGroup>
<Import Project="$(SolutionDir)\tools\AutoRest.Settings.targets" />
<ItemGroup>
<Compile Include="GlobalSuppressions.cs" />
<Compile Include="HttpOperationEventSource.cs" />
<Compile Include="EtwTracingInterceptor.cs" />
<Compile Include="Properties\AssemblyInfo.cs" />
Expand Down
4 changes: 4 additions & 0 deletions Microsoft.Rest/ClientRuntime.Etw/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA2210:AssembliesShouldHaveValidStrongNames", Justification="We do give it strong name and sign it when build in CI server and verify it")]
3 changes: 2 additions & 1 deletion Microsoft.Rest/ClientRuntime.Log4Net/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,5 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "log4net.ILog.DebugFormat(System.String,System.Object[])", Scope = "member", Target = "Microsoft.Rest.Tracing.Log4Net.Log4NetTracingInterceptor.#Enter(System.String,System.Object,System.String,System.Collections.Generic.IDictionary`2<System.String,System.Object>)")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object)", Scope = "member", Target = "Microsoft.Rest.Tracing.Log4Net.Log4NetTracingInterceptor.#Exit(System.String,System.Object)")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object)", Scope = "member", Target = "Microsoft.Rest.Tracing.Log4Net.Log4NetTracingInterceptor.#Exit(System.String,System.Object)")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA2210:AssembliesShouldHaveValidStrongNames", Justification="We do give it strong name and sign it when build in CI server and verify it")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just say
"Signed before publishing."

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" InitialTargets="TurnOffCodeAnalysis">
<Import Project="..\..\packages\xunit.runner.visualstudio.0.99.9-build1021\build\net20\xunit.runner.visualstudio.props" Condition="Exists('..\..\packages\xunit.runner.visualstudio.0.99.9-build1021\build\net20\xunit.runner.visualstudio.props')" />
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<PropertyGroup>
Expand Down
6 changes: 3 additions & 3 deletions Microsoft.Rest/ClientRuntime.Tests/ServiceClientTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ public void RetryHandlerRetriesWith500Errors()
int attemptsFailed = 0;

fakeClient.SetRetryPolicy(new RetryPolicy<HttpStatusCodeErrorDetectionStrategy>(2));
var retryHandler = fakeClient.GetHttpPipeline().OfType<RetryDelegatingHandler>().FirstOrDefault();
var retryHandler = fakeClient.HttpPipelines.OfType<RetryDelegatingHandler>().FirstOrDefault();
retryHandler.Retrying += (sender, args) => { attemptsFailed++; };

var result = fakeClient.DoStuff();
Expand All @@ -79,7 +79,7 @@ public void RetryHandlerRetriesWith500ErrorsAndSucceeds()
int attemptsFailed = 0;

fakeClient.SetRetryPolicy(new RetryPolicy<HttpStatusCodeErrorDetectionStrategy>(2));
var retryHandler = fakeClient.GetHttpPipeline().OfType<RetryDelegatingHandler>().FirstOrDefault();
var retryHandler = fakeClient.HttpPipelines.OfType<RetryDelegatingHandler>().FirstOrDefault();
retryHandler.Retrying += (sender, args) => { attemptsFailed++; };

var result = fakeClient.DoStuff();
Expand All @@ -94,7 +94,7 @@ public void RetryHandlerDoesntRetryFor400Errors()
int attemptsFailed = 0;

fakeClient.SetRetryPolicy(new RetryPolicy<HttpStatusCodeErrorDetectionStrategy>(2));
var retryHandler = fakeClient.GetHttpPipeline().OfType<RetryDelegatingHandler>().FirstOrDefault();
var retryHandler = fakeClient.HttpPipelines.OfType<RetryDelegatingHandler>().FirstOrDefault();
retryHandler.Retrying += (sender, args) => { attemptsFailed++; };

var result = fakeClient.DoStuff();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class DefaultHttpErrorDetectionStrategyTests
public void ResponseCodeIsConsideredTransient(HttpStatusCode code)
{
var strategy = new HttpStatusCodeErrorDetectionStrategy();
Assert.True(strategy.IsTransient(new HttpRequestExceptionWithStatus { StatusCode = code }));
Assert.True(strategy.IsTransient(new HttpRequestWithStatusException { StatusCode = code }));
}

[Theory]
Expand All @@ -30,7 +30,7 @@ public void ResponseCodeIsConsideredTransient(HttpStatusCode code)
public void ResponseCodeIsNotConsideredTransient(HttpStatusCode code)
{
var strategy = new HttpStatusCodeErrorDetectionStrategy();
Assert.False(strategy.IsTransient(new HttpRequestExceptionWithStatus { StatusCode = code }));
Assert.False(strategy.IsTransient(new HttpRequestWithStatusException { StatusCode = code }));
}

[Fact]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<Project ToolsVersion="12.0" DefaultTargets="Build" xmlns="http://schemas.microsoft.com/developer/msbuild/2003" InitialTargets="TurnOffCodeAnalysis">
<Import Project="..\..\packages\xunit.runner.visualstudio.0.99.9-build1021\build\net20\xunit.runner.visualstudio.props" Condition="Exists('..\..\packages\xunit.runner.visualstudio.0.99.9-build1021\build\net20\xunit.runner.visualstudio.props')" />
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props" Condition="Exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.props')" />
<PropertyGroup>
Expand Down
11 changes: 10 additions & 1 deletion Microsoft.Rest/ClientRuntime/BasicAuthenticationCredentials.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License. See License.txt in the project root for license information.

using System;
using System.Globalization;
using System.Net.Http;
using System.Net.Http.Headers;
using System.Text;
Expand All @@ -18,6 +19,8 @@ public class BasicAuthenticationCredentials : ServiceClientCredentials
/// <summary>
/// Basic auth username.
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming",
"CA1702:CompoundWordsShouldBeCasedCorrectly", MessageId = "Username")]
public string Username { get; set; }

/// <summary>
Expand All @@ -34,9 +37,15 @@ public class BasicAuthenticationCredentials : ServiceClientCredentials
public override Task ProcessHttpRequestAsync(HttpRequestMessage request,
CancellationToken cancellationToken)
{
if (request == null)
{
throw new ArgumentNullException("request");
}
// Add username and password to "Basic" header of each request.
request.Headers.Authorization = new AuthenticationHeaderValue("Basic",
Convert.ToBase64String(Encoding.UTF8.GetBytes(string.Format("{0}:{1}",
Convert.ToBase64String(Encoding.UTF8.GetBytes(string.Format(
CultureInfo.InvariantCulture,
"{0}:{1}",
Username,
Password).ToCharArray())));
return PlatformTaskEx.FromResult(null);
Expand Down
1 change: 1 addition & 0 deletions Microsoft.Rest/ClientRuntime/ClientRuntime.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
<Import Project="$(SolutionDir)\tools\AutoRest.Settings.targets" />
<ItemGroup>
<Compile Include="BasicAuthenticationCredentials.cs" />
<Compile Include="GlobalSuppressions.cs" />
<Compile Include="HttpExtensions.cs" />
<Compile Include="IDeserializationModel.cs" />
<Compile Include="TypeConversion.cs" />
Expand Down
6 changes: 6 additions & 0 deletions Microsoft.Rest/ClientRuntime/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA1021:AvoidOutParameters", MessageId = "2#", Scope = "member", Target = "Microsoft.Rest.TransientFaultHandling.ShouldRetryHandler.#Invoke(System.Int32,System.Exception,System.TimeSpan&)")]
[assembly: System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Design", "CA2210:AssembliesShouldHaveValidStrongNames", Justification="We do give it strong name and sign it when build in CI server and verify it")]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would just say
"Signed before publishing."


2 changes: 1 addition & 1 deletion Microsoft.Rest/ClientRuntime/HttpExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static HttpHeaders GetContentHeaders(this HttpRequestMessage request)
/// <returns>The content headers.</returns>
public static HttpHeaders GetContentHeaders(this HttpResponseMessage response)
{
if (response.Content != null)
if (response != null && response.Content != null)
{
return response.Content.Headers;
}
Expand Down
16 changes: 15 additions & 1 deletion Microsoft.Rest/ClientRuntime/HttpOperationException.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License. See License.txt in the project root for license information.

using Microsoft.Rest.Properties;
using System;
using System.Net.Http;

Expand All @@ -10,6 +9,13 @@ namespace Microsoft.Rest
/// <summary>
/// Exception thrown for an invalid response with custom error information.
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage(
"Microsoft.Design",
"CA1032:ImplementStandardExceptionConstructors"),
System.Diagnostics.CodeAnalysis.SuppressMessage(
"Microsoft.Usage",
"CA2237:MarkISerializableTypesWithSerializable",
Justification = "All properties of this class get serialized manually")]
public class HttpOperationException<T> : HttpRequestException
{
/// <summary>
Expand All @@ -27,6 +33,14 @@ public class HttpOperationException<T> : HttpRequestException
/// </summary>
public T Body { get; set; }

/// <summary>
/// Initializes a new instance of the HttpOperationException class.
/// </summary>
public HttpOperationException()
: base()
{
}

/// <summary>
/// Initializes a new instance of the HttpOperationException class.
/// </summary>
Expand Down
2 changes: 2 additions & 0 deletions Microsoft.Rest/ClientRuntime/ILazyCollection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ namespace Microsoft.Rest
/// <summary>
/// Represents a collection that supports on-demand initialization.
/// </summary>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming",
"CA1711:IdentifiersShouldNotHaveIncorrectSuffix")]
public interface ILazyCollection
{
/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ public interface IServiceClientTracingInterceptor
/// </summary>
/// <param name="invocationId">Method invocation identifier.</param>
/// <param name="exception">The error.</param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming",
"CA1716:IdentifiersShouldNotMatchKeywords",
MessageId = "Error",
Justification="The primary client applications will be C#")]
void Error(string invocationId, Exception exception);

/// <summary>
Expand All @@ -64,6 +68,10 @@ public interface IServiceClientTracingInterceptor
/// </summary>
/// <param name="invocationId">Method invocation identifier.</param>
/// <param name="returnValue">Method return value.</param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming",
"CA1716:IdentifiersShouldNotMatchKeywords",
MessageId = "Exit",
Justification="The primary client applications will be C#")]
void Exit(string invocationId, object returnValue);
}
}
2 changes: 2 additions & 0 deletions Microsoft.Rest/ClientRuntime/LazyList.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ namespace Microsoft.Rest
/// Represents an object list that supports on-demand initialization.
/// </summary>
/// <typeparam name="T"></typeparam>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming",
"CA1710:IdentifiersShouldHaveCorrectSuffix")]
public class LazyList<T> : IList<T>, ILazyCollection
{
private IList<T> _list;
Expand Down
14 changes: 7 additions & 7 deletions Microsoft.Rest/ClientRuntime/PlatformTaskEx.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,11 @@

namespace Microsoft.Rest
{
public class PlatformTaskEx
[System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming",
"CA1711:IdentifiersShouldNotHaveIncorrectSuffix",
Justification="We think with Ex is better than using 2")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Well, it is providing a platform neutral way of calling Task or TaskEx. We could just call it PlatformTask.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

agree.

public static class PlatformTaskEx
{
//Per FxCop performance rule to prevent the compiler from generating a default constructor.
private PlatformTaskEx() { }

public static Task FromResult(object result)
{
#if NET45
Expand All @@ -21,12 +21,12 @@ public static Task FromResult(object result)
#endif
}

public static Task Delay(TimeSpan delay)
public static Task Delay(TimeSpan delayTime)
{
#if NET45
return Task.Delay(delay);
return Task.Delay(delayTime);
#else
return TaskEx.Delay(delay);
return TaskEx.Delay(delayTime);
#endif
}

Expand Down
5 changes: 5 additions & 0 deletions Microsoft.Rest/ClientRuntime/Properties/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
// Licensed under the MIT License. See License.txt in the project root for license information.


using System;
using System.Reflection;
using System.Resources;
using System.Runtime.InteropServices;

[assembly: AssemblyTitle("Microsoft Rest Client Library")]
[assembly: AssemblyDescription("Provides an HTTP pipeline for authentication, tracing, logging and transient fault handling.")]
Expand All @@ -18,3 +20,6 @@
[assembly: AssemblyTrademark("")]
[assembly: AssemblyCulture("")]
[assembly: NeutralResourcesLanguage("en")]

[assembly: ComVisible(false)]
[assembly: CLSCompliant(false)]
19 changes: 14 additions & 5 deletions Microsoft.Rest/ClientRuntime/Properties/Resources.Designer.cs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Microsoft.Rest/ClientRuntime/Properties/Resources.resx
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@
<data name="ExceptionRetryManagerNotSet" xml:space="preserve">
<value>The default RetryManager has not been set. Set it by invoking the RetryManager.SetDefault static method, or if you are using declarative configuration, you can invoke the RetryPolicyFactory.CreateDefault() method to automatically create the retry manager from the configuration file.</value>
</data>
<data name="ITransientErrorDetectionStrategyNotImplemented" xml:space="preserve">
<value>The error detection strategy type must implement the ITransientErrorDetectionStrategy interface.</value>
</data>
<data name="ResponseStatusCodeError" xml:space="preserve">
<value>Response status code indicates server error: {0} ({1}).</value>
</data>
Expand Down
2 changes: 1 addition & 1 deletion Microsoft.Rest/ClientRuntime/RetryDelegatingHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ await RetryPolicy.ExecuteAsync(async () =>

if (!responseMessage.IsSuccessStatusCode)
{
throw new HttpRequestExceptionWithStatus(string.Format(
throw new HttpRequestWithStatusException(string.Format(
CultureInfo.InvariantCulture,
Resources.ResponseStatusCodeError,
(int)responseMessage.StatusCode,
Expand Down
Loading