Merge pull request #1478 from github/cleanup/LogManager-CA1004

Fix a bunch of warnings in release build

Author: grokys

src/GitHub.App/Api/ApiClient.cs

@@ -242,7 +242,8 @@ public IObservable<PullRequest> GetPullRequestsForRepository(string owner, strin
             Guard.ArgumentNotEmptyString(name, nameof(name));
 
             return gitHubClient.PullRequest.GetAllForRepository(owner, name,
-                new PullRequestRequest {
+                new PullRequestRequest
+                {
                     State = ItemStateFilter.All,
                     SortProperty = PullRequestSort.Updated,
                     SortDirection = SortDirection.Descending
@@ -268,7 +269,7 @@ public IObservable<Branch> GetBranches(string owner, string repo)
             Guard.ArgumentNotEmptyString(owner, nameof(owner));
             Guard.ArgumentNotEmptyString(repo, nameof(repo));
 
-#pragma warning disable CS0618
+#pragma warning disable 618
             // GetAllBranches is obsolete, but don't want to introduce the change to fix the
             // warning in the PR, so disabling for now.
             return gitHubClient.Repository.GetAllBranches(owner, repo);

src/GitHub.Logging/Logging/LogManager.cs

@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Diagnostics.CodeAnalysis;
 using GitHub.Info;
 using Serilog;
 using Serilog.Core;
@@ -33,10 +34,8 @@ static Logger CreateLogger()
 
         static Lazy<Logger> Logger { get; } = new Lazy<Logger>(CreateLogger);
 
-        //Violates CA1004 - Generic methods should provide type parameter
-#pragma warning disable CA1004
+        [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")]
         public static ILogger ForContext<T>() => ForContext(typeof(T));
-#pragma warning restore CA1004
 
         public static ILogger ForContext(Type type) => Logger.Value.ForContext(type).ForContext("ShortSourceContext", type.Name);
     }

test/GitHub.InlineReviews.UnitTests/TestDoubles/FakeDiffService.cs

@@ -39,7 +39,9 @@ public string AddFile(string path, string contents)
             var directory = Path.GetDirectoryName(fullPath);
             Directory.CreateDirectory(directory);
             File.WriteAllText(fullPath, contents);
+#pragma warning disable 618 // Type or member is obsolete
             repository.Stage(path);
+#pragma warning restore 618 // Type or member is obsolete
             repository.Commit("Added " + path, signature, signature);
             return repository.Head.Tip.Sha;
         }
@@ -106,7 +108,9 @@ static IRepository CreateRepository()
             var signature = new Signature("user", "user@user", DateTimeOffset.Now);
 
             File.WriteAllText(Path.Combine(tempPath, ".gitattributes"), "* text=auto");
+#pragma warning disable 618 // Type or member is obsolete
             result.Stage("*");
+#pragma warning restore 618 // Type or member is obsolete
             result.Commit("Initial commit", signature, signature);
 
             return result;

test/UnitTests/GitHub.Api/LoginManagerTests.cs

@@ -250,7 +250,7 @@ public async Task ErasesLoginWhenNonOctokitExceptionThrown()
             var keychain = Substitute.For<IKeychain>();
             var tfa = new Lazy<ITwoFactorChallengeHandler>(() => Substitute.For<ITwoFactorChallengeHandler>());
             var oauthListener = Substitute.For<IOAuthCallbackListener>();
-          
+
             var target = new LoginManager(keychain, tfa, oauthListener, "id", "secret", scopes);
             Assert.ThrowsAsync<InvalidOperationException>(async () => await target.Login(host, client, "foo", "bar"));
 
@@ -283,7 +283,7 @@ public async Task ErasesLoginWhenNonOctokitExceptionThrownIn2FA()
         }
 
         [Test]
-        public async Task InvalidResponseScopesCauseException()
+        public void InvalidResponseScopesCauseException()
         {
             var client = CreateClient(responseScopes: new[] { "user", "repo" });
             client.Authorization.GetOrCreateApplicationAuthentication("id", "secret", Arg.Any<NewAuthorization>())

test/UnitTests/GitHub.App/Caches/ImageCacheTests.cs

@@ -55,7 +55,7 @@ public async Task WhenLoadingFromCacheFailsInvalidatesCacheEntry()
             Assert.That(retrieved, Is.Null);
             Assert.ThrowsAsync<KeyNotFoundException>(async () => await cache.Get("https://fake/"));
         }
-        
+
         [Test]
         public async Task DownloadsImageWhenMissingAndCachesIt()
         {
@@ -76,7 +76,7 @@ public async Task DownloadsImageWhenMissingAndCachesIt()
         }
 
         [Test]
-        public async Task ThrowsKeyNotFoundExceptionWhenItemNotInCacheAndImageFetchThrowsException()
+        public void ThrowsKeyNotFoundExceptionWhenItemNotInCacheAndImageFetchThrowsException()
         {
             var imageUri = new Uri("https://example.com/poop.gif");
             var cacheFactory = Substitute.For<IBlobCacheFactory>();
@@ -91,7 +91,7 @@ public async Task ThrowsKeyNotFoundExceptionWhenItemNotInCacheAndImageFetchThrow
         }
 
         [Test]
-        public async Task ThrowsKeyNotFoundExceptionWhenItemNotInCacheAndImageFetchReturnsEmpty()
+        public void ThrowsKeyNotFoundExceptionWhenItemNotInCacheAndImageFetchReturnsEmpty()
         {
             var imageUri = new Uri("https://example.com/poop.gif");
             var cache = new InMemoryBlobCache();

test/UnitTests/GitHub.App/Models/AccountModelTests.cs

@@ -119,26 +119,13 @@ public static bool BitmapSourcesAreEqual(BitmapSource image1, BitmapSource image
 
         public static byte[] BitmapSourceToBytes(BitmapSource image)
         {
-            byte[] data = new byte[] { };
-            if (image != null)
+            var encoder = new BmpBitmapEncoder();
+            encoder.Frames.Add(BitmapFrame.Create(image));
+            using (MemoryStream ms = new MemoryStream())
             {
-                try
-                {
-                    var encoder = new BmpBitmapEncoder();
-                    encoder.Frames.Add(BitmapFrame.Create(image));
-                    using (MemoryStream ms = new MemoryStream())
-                    {
-                        encoder.Save(ms);
-                        data = ms.ToArray();
-                    }
-                    return data;
-                }
-                catch (Exception ex)
-                {
-                }
+                encoder.Save(ms);
+                return ms.ToArray();
             }
-
-            return data;
         }
     }
 }

test/UnitTests/GitHub.App/Services/GitClientTests.cs

@@ -250,7 +250,9 @@ public async Task LocalBaseHeadAndMergeBase_DontFetch()
 
             var mergeBaseSha = await gitClient.GetPullRequestMergeBase(repo, targetCloneUrl, baseSha, headSha, baseRef, pullNumber);
 
+#pragma warning disable 618 // Type or member is obsolete
             repo.Network.DidNotReceiveWithAnyArgs().Fetch(null as Remote, null, null as FetchOptions);
+#pragma warning restore 618 // Type or member is obsolete
             Assert.That(expectMergeBaseSha, Is.EqualTo(mergeBaseSha));
         }
 
@@ -274,7 +276,9 @@ public async Task WhenToFetch(string baseSha, string headSha, string mergeBaseSh
             }
             catch (NotFoundException) { /* We're interested in calls to Fetch even if it throws */ }
 
+#pragma warning disable 618 // Type or member is obsolete
             repo.Network.Received(receivedFetch).Fetch(Arg.Any<Remote>(), Arg.Any<string[]>(), Arg.Any<FetchOptions>());
+#pragma warning restore 618 // Type or member is obsolete
         }
 
         [TestCase("baseSha", null, "mergeBaseSha", "baseRef", 777, "refs/pull/777/head")]
@@ -295,7 +299,9 @@ public async Task WhatToFetch(string baseSha, string headSha, string mergeBaseSh
             }
             catch (NotFoundException) { /* We're interested in calls to Fetch even if it throws */ }
 
+#pragma warning disable 618 // Type or member is obsolete
             repo.Network.Received(1).Fetch(Arg.Any<Remote>(), Arg.Is<IEnumerable<string>>(x => x.Contains(expectRefSpec)), Arg.Any<FetchOptions>());
+#pragma warning restore 618 // Type or member is obsolete
         }
 
         static IRepository MockRepo(string baseSha, string headSha, string mergeBaseSha)

test/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs

@@ -616,7 +616,7 @@ public async Task ExtractBase_MergeBaseAvailable_UseMergeBaseSha()
         }
 
         [Test]
-        public async Task MergeBaseNotAvailable_ThrowsNotFoundException()
+        public void MergeBaseNotAvailable_ThrowsNotFoundException()
         {
             var baseFileContent = "baseFileContent";
             var headFileContent = "headFileContent";

test/UnitTests/GitHub.VisualStudio/Services/ConnectionManagerTests.cs

@@ -47,7 +47,7 @@ public async Task ReturnsInvalidConnections()
             var result = await target.GetLoadedConnections();
 
             Assert.That(2, Is.EqualTo(result.Count));
-            Assert.That("https://github.com/",Is.EqualTo(result[0].HostAddress.WebUri.ToString()));
+            Assert.That("https://github.com/", Is.EqualTo(result[0].HostAddress.WebUri.ToString()));
             Assert.That("https://invalid.com/", Is.EqualTo(result[1].HostAddress.WebUri.ToString()));
             Assert.That(result[0].ConnectionError, Is.Null);
             Assert.That(result[1].ConnectionError, Is.Not.Null);
@@ -121,7 +121,7 @@ public async Task AddsLoggedInConnectionToConnections()
         }
 
         [Test]
-        public async Task ThrowsWhenLoginFails()
+        public void ThrowsWhenLoginFails()
         {
             var target = new ConnectionManager(
                 CreateProgram(),
@@ -136,7 +136,7 @@ public async Task ThrowsWhenLoginFails()
         }
 
         [Test]
-        public async Task ThrowsWhenExistingConnectionExists()
+        public void ThrowsWhenExistingConnectionExists()
         {
             var target = new ConnectionManager(
                 CreateProgram(),
@@ -208,7 +208,7 @@ public async Task RemovesConnectionFromConnections()
         }
 
         [Test]
-        public async Task ThrowsIfConnectionDoesntExist()
+        public void ThrowsIfConnectionDoesntExist()
         {
             var loginManager = CreateLoginManager();
             var target = new ConnectionManager(

test/UnitTests/TestDoubles/FakeCommitLog.cs

@@ -32,10 +32,12 @@ public ICommitLog QueryBy(CommitFilter filter)
         throw new NotImplementedException();
     }
 
+#pragma warning disable 618 // Type or member is obsolete
     public IEnumerable<LogEntry> QueryBy(string path, FollowFilter filter)
     {
         throw new NotImplementedException();
     }
+#pragma warning restore 618 // Type or member is obsolete
 
     public IEnumerable<LogEntry> QueryBy(string path, CommitFilter filter)
     {