Code Analysis for C#

• CA1801: Unused parameters

(Object sender, EventArgs e) cannot be removed from UWP but it shows warnings. There is some discussions to avoid warning, and then I set (_1, _2)

https://github.com/dotnet/roslyn-analyzers/issues/2338#issuecomment-489806299

Violation

``` private void btnCopyFiles_Click(object sender, RoutedEventArgs e) { DisableCopyButton(); } ```

Violation fixed

``` private void btnCopyFiles_Click(object _1, RoutedEventArgs _2) { DisableCopyButton(); } ```

• CA1303: Do not pass literals as localized parameters

When: Exception message "Failed to obtain the token" is passed as string

Cause: A method passes a string literal as a parameter to a .NET constructor or method and that string should be localizable. String literals that are embedded in source code are difficult to localize.

Solution: Create Resources.resw and retrieve exception messages from rsource

Violation

``` try { var authResult = await authContext.AcquireTokenAsync(apiResource, _powershellClientId, new Uri(_oauth20RedirectUri), new PlatformParameters(PromptBehavior.Always, false)); if (authResult == null) { throw new InvalidOperationException("Failed to obtain the token"); } return authResult.AccessToken; } ```

Violation fixed

``` try { var authResult = await authContext.AcquireTokenAsync(apiResource, _powershellClientId, new Uri(_oauth20RedirectUri), new PlatformParameters(PromptBehavior.Always, false)); if (authResult == null) { ResourceLoader loader = new ResourceLoader("Resources"); throw new InvalidOperationException(loader.GetString("InvalidOperationExceptionMessage")); } return authResult.AccessToken; } ```

• CA1054: URI parameters should not be strings

A type declares a method with a string parameter whose name contains "uri", "Uri", "urn", "Urn", "url", or "Url" and the type does not declare a corresponding overload that takes a System.Uri parameter

Violation

``` string oauth20RedirectUri ```

Violation fixed

``` string oauth20Redirect ```

• CA1062: An externally visible method dereferences one of its reference arguments without verifying whether that argument is null

You can suppress a warning from this rule if you are sure that the dereferenced parameter has been validated by another method call in the function. If you're running this rule from FxCop analyzers (and not with legacy analysis), you can configure the analysis for this rule. This rule can lead to false positives if your code calls special null check validation methods in referenced libraries or projects. You can avoid these false positives by specifying the name or signature of null check validation methods. The analysis will then assume that arguments passed to this method are non-null after the call. For example, to mark all methods named Validate as null check validation methods, you can add the following key-value pair to an .editorconfig file in your project:

Violation

``` pipelineResult.Status = pipelineRun.Status; pipelineResult.Message = pipelineRun.Message; ```

Violation fixed

``` if (pipelineResult == null) throw new ArgumentNullException(nameof(pipelineResult)); pipelineResult.Status = pipelineRun.Status; pipelineResult.Message = pipelineRun.Message; ```

• PRI257: 0xdef00522 - Resources found for language(s) 'en' but no resources found for default language(s): 'en-US'. Change the default language or qualify resources with the default language. http://go.microsoft.com/fwlink/?LinkId=231899

You are hitting a known issue when NuGet packages with multiple supported languages are included in a UWP project targeting different language than en-US.There should be no runtime impact, and those warnings can be ignored. Please reopen the issue if there is a runtime impact

• CA1031

Do not catch general exception types. General exceptions should not be caught.

Violation

``` try { HttpResponseMessage response = await httpClient.SendAsync(request).ConfigureAwait(false); if (!response.IsSuccessStatusCode) { var statusCode = response.StatusCode; throw new Exception(statusCode.ToString()); } var content = await response.Content.ReadAsStringAsync().ConfigureAwait(false); SecretBundle secret = JsonConvert.DeserializeObject<secretbundle>(content); return secret.Value; } catch (Exception e) { Debug.WriteLine("Message :{0} ", e.Message); return null; } ```

Violation fixed

``` try { HttpResponseMessage response = await httpClient.SendAsync(request).ConfigureAwait(false); if (!response.IsSuccessStatusCode) { var statusCode = response.StatusCode; throw new WebException(statusCode.ToString()); } var content = await response.Content.ReadAsStringAsync().ConfigureAwait(false); SecretBundle secret = JsonConvert.DeserializeObject<secretbundle>(content); return secret.Value; } catch (WebException e) { Debug.WriteLine("Message :{0} ", e.Message); return null; } ```

• CA2000 HttpClient

HttpClient should not be disposed. Although HttpClient does indirectly implement the IDisposable interface, the standard usage of HttpClient is not to dispose of it after every request. The HttpClient object is intended to live for as long as your application needs to make HTTP requests.

https://github.com/mono/aspnetwebstack/blob/master/src/System.Net.Http.Formatting/HttpClientFactory.cs

Fixed

``` [SuppressMessage("Microsoft.Reliability", "CA2000:Dispose objects before losing scope", Justification = "Handler is disposed with HttpClient")] public async Task<string> GetKeyVaultAsync(string secretName) { var httpClient = new HttpClient(); var request = new HttpRequestMessage(HttpMethod.Get, Url); ```

• CA1062

In externally visible method, validate parameter is non-null before using it. If appropriate, throw an ArgumentNullException when the argument is null or add a Code Contract precondition asserting non-null argument.

Warning

``` public async Task WaitPipeline(PipelineResult pipelineResult, string runId) { pipelineResult.Status = Newtonsoft.Json.Linq.JObject.Parse(content)["status"].ToString(); pipelineResult.Message = Newtonsoft.Json.Linq.JObject.Parse(content)["message"].ToString(); ```

Fixed

``` public async Task WaitPipeline(PipelineResult pipelineResult, string runId) { if (pipelineResult == null) throw new ArgumentNullException(nameof(pipelineResult)); pipelineResult.Status = Newtonsoft.Json.Linq.JObject.Parse(content)["status"].ToString(); pipelineResult.Message = Newtonsoft.Json.Linq.JObject.Parse(content)["message"].ToString(); ```

• CA1819: Properties should not return arrays / CA2227: Change to be read-only by removing the property setter

CA1819 violation
Arrays returned by properties are not write-protected, even if the property is read-only. To keep the array tamper-proof, the property must return a copy of the array. To fix a violation of this rule, either make the property a method or change the property to return a collection.

``` public class AppConfiguration { public string[] srcFolderName { get; set; } public static AppConfiguration Config(string file = "appsettings.json") { if (_singleton == null) { var settingsFileContent = File.ReadAllText(file); _singleton = JsonConvert.DeserializeObject<appconfiguration>(settingsFileContent); } return _singleton; } } ```

How to fix CA1819 violations

To fix a violation of this rule, either make the property a method or change the property to return a collection

CA2227 violation
A writable collection property allows a user to replace the collection with a completely different collection. A read-only property stops the collection from being replaced, but still allows the individual members to be set. If replacing the collection is a goal, the preferred design pattern is to include a method to remove all the elements from the collection, and a method to repopulate the collection.

``` public class AppConfiguration { public List<string> srcFolderName { get; set; } public static AppConfiguration Config(string file = "appsettings.json") { if (_singleton == null) { var settingsFileContent = File.ReadAllText(file); _singleton = JsonConvert.DeserializeObject<appconfiguration>(settingsFileContent); } return _singleton; } } ```

How to fix CA2227 violations To fix a violation of this rule, make the property read-only. If the design requires it, add methods to clear and repopulate the collection.

CA1819 violation
After making the property a method, it still has the CA1819 violation because it still returns the array.

``` public class AppConfiguration { private string[] _SrcFolderName; public AppConfiguration(string[] srcFolderName) { _SrcFolderName = srcFolderName; } public string[] srcFolderName { get { return _SrcFolderName; } set { _SrcFolderName = value; } } public static AppConfiguration Config(string file = "appsettings.json") { if (_singleton == null) { var settingsFileContent = File.ReadAllText(file); _singleton = JsonConvert.DeserializeObject<appconfiguration>(settingsFileContent); } return _singleton; } } ```

Violation fixed

``` public class AppConfiguration { private List<string> _SrcFolderName; public AppConfiguration(string[] srcFolderName) { _SrcFolderName = new List<string>(srcFolderName); } public List<string> srcFolderName { get { return _SrcFolderName; } } public static AppConfiguration Config(string file = "appsettings.json") { if (_singleton == null) { var settingsFileContent = File.ReadAllText(file); _singleton = JsonConvert.DeserializeObject<appconfiguration>(settingsFileContent); } return _singleton; } } ```

• CA1001

Types that own disposable fields should be disposable. To fixt the violation, it should implement IDisposable and call the Dispose method of the field.
Violation

``` public class KeyVaultDataFactoryConfig { private string _kvUrl; private KeyVaultClient _kvClient; public KeyVaultDataFactoryConfig(string kvName, string accessToken) { this._kvUrl = $"https://{kvName}.vault.azure.net"; this._kvClient = new KeyVaultClient(async (authority, resource, scope) => { return accessToken; }); } ```

Violation fixed

``` public class KeyVaultDataFactoryConfig : IDisposable { private string _kvUrl; private KeyVaultClient _kvClient; public KeyVaultDataFactoryConfig(string kvName, string accessToken) { this._kvUrl = $"https://{kvName}.vault.azure.net"; this._kvClient = new KeyVaultClient(async (authority, resource, scope) => { await Task.Delay(0).ConfigureAwait(false); return accessToken; }); } protected virtual void Dispose(bool disposing) { if (disposing) { _kvClient.Dispose(); } } public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } ```

• CA2000

Dispose objects before losing scope. This happens becaause a local object of a IDisposable type is created but the object is not disposed before all references to the object are out of scope. To fix a violation of this rule, it has to call Dispose on the object before all references to it are out of scope. In this case, it will use the using statement to wrap objects that implement IDisposable and that will automatically be disposed at the close of the using block.

Violation

``` public class KeyVaultDataFactoryConfig : IDisposable { public KeyVaultDataFactoryConfig(string kvName, string accessToken) { ... } protected virtual void Dispose(bool disposing) { if (disposing) { _kvClient.Dispose(); } } public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } } var kvDfConfig = new KeyVaultDataFactoryConfig(config.kvName, kvAccessToken); subscriptionId = await kvDfConfig.GetKeyVaultAsync(config.kvSecretDfSubscriptionId).ConfigureAwait(false); ```

Violation fixed

``` public class KeyVaultDataFactoryConfig : IDisposable { public KeyVaultDataFactoryConfig(string kvName, string accessToken) { ... } protected virtual void Dispose(bool disposing) { if (disposing) { _kvClient.Dispose(); } } public void Dispose() { Dispose(true); GC.SuppressFinalize(this); } } using (var kvDfConfig = new KeyVaultDataFactoryConfig(config.kvName, kvAccessToken)) { subscriptionId = await kvDfConfig.GetKeyVaultAsync(config.kvSecretDfSubscriptionId).ConfigureAwait(false); } ```