[Backend] Increase <AnalysisMode> to Recommended (#2339)

Resolve CA1716
Give targeted exceptions to CA1720
Give global exception to CA1848

.editorconfig

@@ -10,6 +10,8 @@ indent_size = 4
 # CA1305 requires using a FormatProvider with int.Parse and string.Format.
 dotnet_diagnostic.CA1305.severity = none
 dotnet_diagnostic.CA1710.severity = warning
+# TODO: Implement LoggerMessage pattern to remove the CA1848 exception.
+dotnet_diagnostic.CA1848.severity = none
 # CS1591 is our only exception to EnforceCodeStyleInBuild+GenerateDocumentationFile.
 dotnet_diagnostic.CS1591.severity = none
 # IDE0005 requires both EnforceCodeStyleInBuild and GenerateDocumentationFile set to true.

Backend.Tests/Controllers/MergeControllerTests.cs

@@ -54,16 +54,16 @@ public void BlacklistAddTest()
 
             // Add two Lists of wordIds.
             _ = _mergeController.BlacklistAdd(ProjId, wordIdsA).Result;
-            var result = _mergeBlacklistRepo.GetAll(ProjId).Result;
+            var result = _mergeBlacklistRepo.GetAllEntries(ProjId).Result;
             Assert.That(result, Has.Count.EqualTo(1));
             Assert.That(result.First().WordIds, Is.EqualTo(wordIdsA));
             _ = _mergeController.BlacklistAdd(ProjId, wordIdsB).Result;
-            result = _mergeBlacklistRepo.GetAll(ProjId).Result;
+            result = _mergeBlacklistRepo.GetAllEntries(ProjId).Result;
             Assert.That(result, Has.Count.EqualTo(2));
 
             // Add a List of wordIds that contains both previous lists.
             _ = _mergeController.BlacklistAdd(ProjId, wordIdsC).Result;
-            result = _mergeBlacklistRepo.GetAll(ProjId).Result;
+            result = _mergeBlacklistRepo.GetAllEntries(ProjId).Result;
             Assert.That(result, Has.Count.EqualTo(1));
             Assert.That(result.First().WordIds, Is.EqualTo(wordIdsC));
         }

Backend.Tests/Mocks/BannerRepositoryMock.cs

@@ -17,7 +17,7 @@ public BannerRepositoryMock()
             _banners[BannerType.Login] = new Banner { Type = BannerType.Login };
         }
 
-        public Task<Banner> Get(BannerType type)
+        public Task<Banner> GetBanner(BannerType type)
         {
             return Task.FromResult(_banners[type]);
         }

Backend.Tests/Mocks/MergeBlacklistRepositoryMock.cs

@@ -17,7 +17,7 @@ public MergeBlacklistRepositoryMock()
             _mergeBlacklist = new List<MergeBlacklistEntry>();
         }
 
-        public Task<List<MergeBlacklistEntry>> GetAll(string projectId, string? userId = null)
+        public Task<List<MergeBlacklistEntry>> GetAllEntries(string projectId, string? userId = null)
         {
             var cloneList = _mergeBlacklist.Select(e => e.Clone()).ToList();
             var enumerable = userId is null ?
@@ -26,7 +26,7 @@ public Task<List<MergeBlacklistEntry>> GetAll(string projectId, string? userId =
             return Task.FromResult(enumerable.ToList());
         }
 
-        public Task<MergeBlacklistEntry?> Get(string projectId, string entryId)
+        public Task<MergeBlacklistEntry?> GetEntry(string projectId, string entryId)
         {
             try
             {
@@ -46,7 +46,7 @@ public Task<MergeBlacklistEntry> Create(MergeBlacklistEntry blacklistEntry)
             return Task.FromResult(blacklistEntry.Clone());
         }
 
-        public Task<bool> DeleteAll(string projectId)
+        public Task<bool> DeleteAllEntries(string projectId)
         {
             _mergeBlacklist.Clear();
             return Task.FromResult(true);

Backend.Tests/Services/MergeServiceTests.cs

@@ -199,10 +199,10 @@ public void UndoMergeMultiChildTest()
         [Test]
         public void AddMergeToBlacklistTest()
         {
-            _ = _mergeBlacklistRepo.DeleteAll(ProjId).Result;
+            _ = _mergeBlacklistRepo.DeleteAllEntries(ProjId).Result;
             var wordIds = new List<string> { "1", "2" };
             _ = _mergeService.AddToMergeBlacklist(ProjId, UserId, wordIds).Result;
-            var blacklist = _mergeBlacklistRepo.GetAll(ProjId).Result;
+            var blacklist = _mergeBlacklistRepo.GetAllEntries(ProjId).Result;
             Assert.That(blacklist, Has.Count.EqualTo(1));
             var expectedEntry = new MergeBlacklistEntry { ProjectId = ProjId, UserId = UserId, WordIds = wordIds };
             Assert.That(expectedEntry.ContentEquals(blacklist.First()));
@@ -211,7 +211,7 @@ public void AddMergeToBlacklistTest()
         [Test]
         public void AddMergeToBlacklistErrorTest()
         {
-            _ = _mergeBlacklistRepo.DeleteAll(ProjId).Result;
+            _ = _mergeBlacklistRepo.DeleteAllEntries(ProjId).Result;
             var wordIds0 = new List<string>();
             var wordIds1 = new List<string> { "1" };
             Assert.ThrowsAsync<MergeService.InvalidBlacklistEntryException>(
@@ -223,7 +223,7 @@ public void AddMergeToBlacklistErrorTest()
         [Test]
         public void IsInMergeBlacklistTest()
         {
-            _ = _mergeBlacklistRepo.DeleteAll(ProjId).Result;
+            _ = _mergeBlacklistRepo.DeleteAllEntries(ProjId).Result;
             var wordIds = new List<string> { "1", "2", "3" };
             var subWordIds = new List<string> { "3", "2" };
 
@@ -235,7 +235,7 @@ public void IsInMergeBlacklistTest()
         [Test]
         public void IsInMergeBlacklistErrorTest()
         {
-            _ = _mergeBlacklistRepo.DeleteAll(ProjId).Result;
+            _ = _mergeBlacklistRepo.DeleteAllEntries(ProjId).Result;
             var wordIds0 = new List<string>();
             var wordIds1 = new List<string> { "1" };
             Assert.ThrowsAsync<MergeService.InvalidBlacklistEntryException>(
@@ -265,7 +265,7 @@ public void UpdateMergeBlacklistTest()
             _ = _mergeBlacklistRepo.Create(entryA);
             _ = _mergeBlacklistRepo.Create(entryB);
 
-            var oldBlacklist = _mergeBlacklistRepo.GetAll(ProjId).Result;
+            var oldBlacklist = _mergeBlacklistRepo.GetAllEntries(ProjId).Result;
             Assert.That(oldBlacklist, Has.Count.EqualTo(2));
 
             // Make sure all wordIds are in the frontier EXCEPT 1.
@@ -282,7 +282,7 @@ public void UpdateMergeBlacklistTest()
             Assert.That(updatedEntriesCount, Is.EqualTo(2));
 
             // The only blacklistEntry with at least two ids in the frontier is A.
-            var newBlacklist = _mergeBlacklistRepo.GetAll(ProjId).Result;
+            var newBlacklist = _mergeBlacklistRepo.GetAllEntries(ProjId).Result;
             Assert.That(newBlacklist, Has.Count.EqualTo(1));
             Assert.AreEqual(newBlacklist.First().WordIds, new List<string> { "2", "3" });
         }

Backend/BackendFramework.csproj

@@ -4,10 +4,10 @@
     <LangVersion>10.0</LangVersion>
     <Nullable>enable</Nullable>
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
-    <AnalysisMode>Minimum</AnalysisMode>
+    <AnalysisMode>Recommended</AnalysisMode>
     <EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
-    <NoWarn>$(NoWarn);CA1305;CA1816;CS1591</NoWarn>
+    <NoWarn>$(NoWarn);CA1305;CA1816;CA1848;CS1591</NoWarn>
   </PropertyGroup>
   <ItemGroup>
     <None Remove="Data\sdList.txt" />

Backend/Controllers/BannerController.cs

@@ -30,7 +30,7 @@ public BannerController(IBannerRepository bannerRepo, IPermissionService permiss
         [ProducesResponseType(StatusCodes.Status200OK, Type = typeof(SiteBanner))]
         public async Task<IActionResult> GetBanner(BannerType type)
         {
-            var banner = await _bannerRepo.Get(type);
+            var banner = await _bannerRepo.GetBanner(type);
             return Ok(new SiteBanner { Type = type, Text = banner.Text });
         }
 

Backend/Interfaces/IBannerRepository.cs

@@ -6,7 +6,7 @@ namespace BackendFramework.Interfaces
 {
     public interface IBannerRepository
     {
-        Task<Banner> Get(BannerType type);
+        Task<Banner> GetBanner(BannerType type);
         Task<ResultOfUpdate> Update(SiteBanner banner);
     }
 }

Backend/Interfaces/IMergeBlacklistRepository.cs

@@ -7,11 +7,11 @@ namespace BackendFramework.Interfaces
 {
     public interface IMergeBlacklistRepository
     {
-        Task<List<MergeBlacklistEntry>> GetAll(string projectId, string? userId = null);
-        Task<MergeBlacklistEntry?> Get(string projectId, string entryId);
+        Task<List<MergeBlacklistEntry>> GetAllEntries(string projectId, string? userId = null);
+        Task<MergeBlacklistEntry?> GetEntry(string projectId, string entryId);
         Task<MergeBlacklistEntry> Create(MergeBlacklistEntry blacklistEntry);
         Task<bool> Delete(string projectId, string entryId);
-        Task<bool> DeleteAll(string projectId);
+        Task<bool> DeleteAllEntries(string projectId);
         Task<ResultOfUpdate> Update(MergeBlacklistEntry blacklistEntry);
     }
 }

Backend/Models/SemanticDomain.cs

@@ -16,7 +16,9 @@ public class SemanticDomain
         public string MongoId { get; set; }
         [Required]
         [BsonElement("guid")]
+#pragma warning disable CA1720
         public string Guid { get; set; }
+#pragma warning restore CA1720
         [Required]
         [BsonElement("name")]
         public string Name { get; set; }
@@ -145,7 +147,9 @@ public class SemanticDomainTreeNode
         public string Lang { get; set; }
         [Required]
         [BsonElement("guid")]
+#pragma warning disable CA1720
         public string Guid { get; set; }
+#pragma warning restore CA1720
         [Required]
         [BsonElement("name")]
         public string Name { get; set; }

Backend/Models/Sense.cs

@@ -16,7 +16,9 @@ public class Sense
         /// </summary>
         [Required]
         [BsonElement("guid")]
+#pragma warning disable CA1720
         public Guid Guid { get; set; }
+#pragma warning restore CA1720
 
         [Required]
         [BsonElement("accessibility")]

Backend/Models/UserEdit.cs

@@ -115,7 +115,9 @@ public class Edit
     {
         [Required]
         [BsonElement("guid")]
+#pragma warning disable CA1720
         public Guid Guid { get; set; }
+#pragma warning restore CA1720
 
         /// <summary> Integer representation of enum <see cref="Models.GoalType"/> </summary>
         [Required]

Backend/Models/Word.cs

@@ -21,7 +21,9 @@ public class Word
         /// </summary>
         [Required]
         [BsonElement("guid")]
+#pragma warning disable CA1720
         public Guid Guid { get; set; }
+#pragma warning restore CA1720
 
         [Required]
         [BsonElement("vernacular")]

Backend/Repositories/BannerRepository.cs

@@ -26,7 +26,7 @@ private async Task<Banner> CreateEmptyBanner(BannerType type)
             return emptyBanner;
         }
 
-        public async Task<Banner> Get(BannerType type)
+        public async Task<Banner> GetBanner(BannerType type)
         {
             var bannerList = await _bannerDatabase.Banners.FindAsync(x => x.Type == type);
             try
@@ -41,7 +41,7 @@ public async Task<Banner> Get(BannerType type)
 
         public async Task<ResultOfUpdate> Update(SiteBanner banner)
         {
-            var existingBanner = await Get(banner.Type);
+            var existingBanner = await GetBanner(banner.Type);
             var filter = Builders<Banner>.Filter.Eq(x => x.Id, existingBanner.Id);
             var updateDef = Builders<Banner>.Update
                 .Set(x => x.Type, banner.Type)

Backend/Repositories/MergeBlacklistRepository.cs

@@ -21,7 +21,7 @@ public MergeBlacklistRepository(IMergeBlacklistContext collectionSettings)
         }
 
         /// <summary> Finds all <see cref="MergeBlacklistEntry"/>s for specified <see cref="Project"/>. </summary>
-        public async Task<List<MergeBlacklistEntry>> GetAll(string projectId, string? userId = null)
+        public async Task<List<MergeBlacklistEntry>> GetAllEntries(string projectId, string? userId = null)
         {
             var listFind = userId is null ?
                 _mergeBlacklistDatabase.MergeBlacklist.Find(e => e.ProjectId == projectId) :
@@ -31,14 +31,14 @@ public async Task<List<MergeBlacklistEntry>> GetAll(string projectId, string? us
 
         /// <summary> Removes all <see cref="MergeBlacklistEntry"/>s for specified <see cref="Project"/>. </summary>
         /// <returns> A bool: success of operation. </returns>
-        public async Task<bool> DeleteAll(string projectId)
+        public async Task<bool> DeleteAllEntries(string projectId)
         {
             var deleted = await _mergeBlacklistDatabase.MergeBlacklist.DeleteManyAsync(u => u.ProjectId == projectId);
             return deleted.DeletedCount != 0;
         }
 
         /// <summary> Finds specified <see cref="MergeBlacklistEntry"/> for specified <see cref="Project"/>. </summary>
-        public async Task<MergeBlacklistEntry?> Get(string projectId, string entryId)
+        public async Task<MergeBlacklistEntry?> GetEntry(string projectId, string entryId)
         {
             var filterDef = new FilterDefinitionBuilder<MergeBlacklistEntry>();
             var filter = filterDef.And(

Backend/Services/MergeService.cs

@@ -125,7 +125,7 @@ public async Task<MergeBlacklistEntry> AddToMergeBlacklist(
                 throw new InvalidBlacklistEntryException("Cannot blacklist a list of fewer than 2 wordIds.");
             }
             // When we switch from individual to common blacklist, the userId argument here should be removed.
-            var blacklist = await _mergeBlacklistRepo.GetAll(projectId, userId);
+            var blacklist = await _mergeBlacklistRepo.GetAllEntries(projectId, userId);
             foreach (var entry in blacklist)
             {
                 if (entry.WordIds.All(wordIds.Contains))
@@ -146,7 +146,7 @@ public async Task<bool> IsInMergeBlacklist(string projectId, List<string> wordId
             {
                 throw new InvalidBlacklistEntryException("Cannot blacklist a list of fewer than 2 wordIds.");
             }
-            var blacklist = await _mergeBlacklistRepo.GetAll(projectId, userId);
+            var blacklist = await _mergeBlacklistRepo.GetAllEntries(projectId, userId);
             foreach (var entry in blacklist)
             {
                 if (wordIds.All(entry.WordIds.Contains))
@@ -165,7 +165,7 @@ public async Task<bool> IsInMergeBlacklist(string projectId, List<string> wordId
         /// <returns> Number of <see cref="MergeBlacklistEntry"/>s updated. </returns>
         public async Task<int> UpdateMergeBlacklist(string projectId)
         {
-            var oldBlacklist = await _mergeBlacklistRepo.GetAll(projectId);
+            var oldBlacklist = await _mergeBlacklistRepo.GetAllEntries(projectId);
             if (oldBlacklist.Count == 0)
             {
                 return 0;