Merge branch 'maintenance/code-analysis' into develop

JeremyCaney · JeremyCaney · commit fb38d9b2b36b · 2021-02-19T12:54:25.000-08:00
In centralizing the common properties, we introduced both nullability context and code analysis to the unit test and `Host` projects. These previously didn't include these properties. This isn't strictly necessary, but helps us maintain consistent code and best practices. It also helps better expose potential issues in our nullability context early on when writing tests. This obviously exposed a lot of code analysis warnings, necessitating updating these projects to ensure they're both annotated for nullability and also adhering to consistent coding conventions.
diff --git a/OnTopic.All/Properties/AssemblyInfo.cs b/OnTopic.All/Properties/AssemblyInfo.cs
@@ -0,0 +1,15 @@
+﻿/*==============================================================================================================================
+| Author        Ignia, LLC
+| Client        Ignia, LLC
+| Project       Topics Library
+\=============================================================================================================================*/
+using System;
+using System.Runtime.InteropServices;
+
+/*==============================================================================================================================
+| DEFINE ASSEMBLY ATTRIBUTES
+>===============================================================================================================================
+| Declare and define attributes used in the compiling of the finished assembly.
+\-----------------------------------------------------------------------------------------------------------------------------*/
+[assembly: ComVisible(false)]
+[assembly: CLSCompliant(false)]
diff --git a/OnTopic.AspNetCore.Mvc.Host/Properties/AssemblyInfo.cs b/OnTopic.AspNetCore.Mvc.Host/Properties/AssemblyInfo.cs
@@ -0,0 +1,15 @@
+﻿/*==============================================================================================================================
+| Author        Ignia, LLC
+| Client        Ignia, LLC
+| Project       Topics Library
+\=============================================================================================================================*/
+using System;
+using System.Runtime.InteropServices;
+
+/*==============================================================================================================================
+| DEFINE ASSEMBLY ATTRIBUTES
+>===============================================================================================================================
+| Declare and define attributes used in the compiling of the finished assembly.
+\-----------------------------------------------------------------------------------------------------------------------------*/
+[assembly: ComVisible(false)]
+[assembly: CLSCompliant(false)]
diff --git a/OnTopic.AspNetCore.Mvc.Host/SampleActivator.cs b/OnTopic.AspNetCore.Mvc.Host/SampleActivator.cs
@@ -11,6 +11,7 @@
 using OnTopic.AspNetCore.Mvc.Host.Components;
 using OnTopic.Data.Caching;
 using OnTopic.Data.Sql;
+using OnTopic.Internal.Diagnostics;
 using OnTopic.Lookup;
 using OnTopic.Mapping;
 using OnTopic.Mapping.Hierarchical;
@@ -31,15 +32,15 @@ public class SampleActivator : IControllerActivator, IViewComponentActivator {
     /*==========================================================================================================================
     | PRIVATE INSTANCES
     \-------------------------------------------------------------------------------------------------------------------------*/
-    private readonly            ITypeLookupService              _typeLookupService              = null;
-    private readonly            ITopicMappingService            _topicMappingService            = null;
-    private readonly            ITopicRepository                _topicRepository                = null;
+    private readonly            ITypeLookupService              _typeLookupService;
+    private readonly            ITopicMappingService            _topicMappingService;
+    private readonly            ITopicRepository                _topicRepository;
     private                     DateTime                        _cacheLastUpdated               = DateTime.UtcNow;
 
     /*==========================================================================================================================
     | HIERARCHICAL TOPIC MAPPING SERVICE
     \-------------------------------------------------------------------------------------------------------------------------*/
-    private readonly IHierarchicalTopicMappingService<NavigationTopicViewModel> _hierarchicalMappingService = null;
+    private readonly IHierarchicalTopicMappingService<NavigationTopicViewModel> _hierarchicalMappingService;
 
     /*==========================================================================================================================
     | CONSTRUCTOR
@@ -90,6 +91,11 @@ public SampleActivator(string connectionString) {
     /// <returns>A concrete instance of an <see cref="IController"/>.</returns>
     public object Create(ControllerContext context) {
 
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate parameters
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      Contract.Requires(context, nameof(context));
+
       /*------------------------------------------------------------------------------------------------------------------------
       | Determine controller type
       \-----------------------------------------------------------------------------------------------------------------------*/
@@ -100,7 +106,7 @@ public object Create(ControllerContext context) {
       \-----------------------------------------------------------------------------------------------------------------------*/
       if (DateTime.UtcNow > _cacheLastUpdated.AddMinutes(1)) {
         var currentUpdate = DateTime.UtcNow;
-        _topicRepository.Refresh(_topicRepository.Load(), _cacheLastUpdated);
+        _topicRepository.Refresh(_topicRepository.Load()!, _cacheLastUpdated);
         _cacheLastUpdated = currentUpdate;
       }
 
@@ -125,6 +131,11 @@ public object Create(ControllerContext context) {
     /// <returns>A concrete instance of an <see cref="IController"/>.</returns>
     public object Create(ViewComponentContext context) {
 
+      /*------------------------------------------------------------------------------------------------------------------------
+      | Validate parameters
+      \-----------------------------------------------------------------------------------------------------------------------*/
+      Contract.Requires(context, nameof(context));
+
       /*------------------------------------------------------------------------------------------------------------------------
       | Determine view component type
       \-----------------------------------------------------------------------------------------------------------------------*/
diff --git a/OnTopic.AspNetCore.Mvc.Host/Views/Shared/Components/Menu/Default.cshtml b/OnTopic.AspNetCore.Mvc.Host/Views/Shared/Components/Menu/Default.cshtml
@@ -3,7 +3,7 @@
 <h2>Menu</h2>
 <nav id="PrimaryNavigationSmallScreen" role="navigation" vocab="http://schema.org" typeof="SiteNavigationElement">
   <ul>
-    @foreach (var topic in Model.NavigationRoot.Children) {
+    @foreach (var topic in Model.NavigationRoot!.Children) {
       @WriteMenu(topic);
     }
   </ul>
@@ -12,7 +12,7 @@
 
 @{
 
-  IHtmlContent Body(Func<object, IHtmlContent> body) => body(null);
+  IHtmlContent Body(Func<object?, IHtmlContent> body) => body(null);
 
   IHtmlContent WriteMenu(NavigationTopicViewModel topic, int indentLevel = 1) => Body(
 
diff --git a/OnTopic.AspNetCore.Mvc.Tests/OnTopic.AspNetCore.Mvc.Tests.csproj b/OnTopic.AspNetCore.Mvc.Tests/OnTopic.AspNetCore.Mvc.Tests.csproj
@@ -3,6 +3,7 @@
   <PropertyGroup>
     <TargetFrameworks>net5.0</TargetFrameworks>
     <IsPackable>false</IsPackable>
+    <NoWarn>CA1707</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>
diff --git a/OnTopic.AspNetCore.Mvc.Tests/Properties/AssemblyInfo.cs b/OnTopic.AspNetCore.Mvc.Tests/Properties/AssemblyInfo.cs
@@ -0,0 +1,15 @@
+﻿/*==============================================================================================================================
+| Author        Ignia, LLC
+| Client        Ignia, LLC
+| Project       Topics Library
+\=============================================================================================================================*/
+using System;
+using System.Runtime.InteropServices;
+
+/*==============================================================================================================================
+| DEFINE ASSEMBLY ATTRIBUTES
+>===============================================================================================================================
+| Declare and define attributes used in the compiling of the finished assembly.
+\-----------------------------------------------------------------------------------------------------------------------------*/
+[assembly: ComVisible(false)]
+[assembly: CLSCompliant(false)]
diff --git a/OnTopic.AspNetCore.Mvc.Tests/TopicControllerTest.cs b/OnTopic.AspNetCore.Mvc.Tests/TopicControllerTest.cs
@@ -91,12 +91,12 @@ public async Task TopicController_IndexAsync_ReturnsTopicViewResult() {
       };
 
       var result                = await controller.IndexAsync(_topic.GetWebPath()).ConfigureAwait(false) as TopicViewResult;
-      var model                 = result.Model as PageTopicViewModel;
+      var model                 = result?.Model as PageTopicViewModel;
 
       controller.Dispose();
 
       Assert.IsNotNull(model);
-      Assert.AreEqual<string>("Web_0_1_1", model.Title);
+      Assert.AreEqual<string?>("Web_0_1_1", model?.Title);
 
     }
 
@@ -115,8 +115,8 @@ public void RedirectController_TopicRedirect_ReturnsRedirectResult() {
       controller.Dispose();
 
       Assert.IsNotNull(result);
-      Assert.IsTrue(result.Permanent);
-      Assert.AreEqual<string>("/Web/Web_1/Web_1_1/Web_1_1_1/", result.Url);
+      Assert.IsTrue(result?.Permanent?? false);
+      Assert.AreEqual<string?>("/Web/Web_1/Web_1_1/Web_1_1_1/", result?.Url);
 
     }
 
@@ -138,13 +138,13 @@ public void SitemapController_Index_ReturnsSitemapXml() {
         ControllerContext       = new(actionContext)
       };
       var result                = controller.Index() as ContentResult;
-      var model                 = result.Content as string;
+      var model                 = result?.Content as string;
 
       controller.Dispose();
 
       Assert.IsNotNull(model);
-      Assert.IsTrue(model.StartsWith("<?xml version=\"1.0\" encoding=\"utf-8\" standalone=\"no\"?>"));
-      Assert.IsTrue(model.Contains("/Web/Web_1/Web_1_1/Web_1_1_1/</loc>"));
+      Assert.IsTrue(model!.StartsWith("<?xml version=\"1.0\" encoding=\"utf-8\" standalone=\"no\"?>", StringComparison.Ordinal));
+      Assert.IsTrue(model!.Contains("/Web/Web_1/Web_1_1/Web_1_1_1/</loc>", StringComparison.Ordinal));
 
     }
 
@@ -177,20 +177,20 @@ public void SitemapController_Index_ExcludesContentTypes() {
         ControllerContext       = new(actionContext)
       };
       var result                = controller.Index(false, true) as ContentResult;
-      var model                 = result.Content as string;
+      var model                 = result?.Content as string;
 
       controller.Dispose();
 
       Assert.IsNotNull(model);
-      Assert.IsTrue(model.Contains("<DataObject type=\"Attributes\">"));
-      Assert.IsFalse(model.Contains("<DataObject type=\"List\">"));
-      Assert.IsFalse(model.Contains("<DataObject type=\"Container\">"));
-      Assert.IsFalse(model.Contains("<DataObject type=\"PageGroup\">"));
-      Assert.IsTrue(model.Contains("/Web/Web_0/Web_0_0/Web_0_0_1/</loc>"));
-      Assert.IsTrue(model.Contains("/Web/Web_1/Web_1_1/Web_1_1_0/</loc>"));
-      Assert.IsFalse(model.Contains("/Web/Web_1/Web_1_0/Web_1_0_0/</loc>"));
-      Assert.IsFalse(model.Contains("/Web/Web_1/Web_1_1/Web_1_1_1/</loc>"));
-      Assert.IsFalse(model.Contains("/Web/Web_0/Web_0_0/</loc>"));
+      Assert.IsTrue(model!.Contains("<DataObject type=\"Attributes\">", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("<DataObject type=\"List\">", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("<DataObject type=\"Container\">", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("<DataObject type=\"PageGroup\">", StringComparison.Ordinal));
+      Assert.IsTrue(model!.Contains("/Web/Web_0/Web_0_0/Web_0_0_1/</loc>", StringComparison.Ordinal));
+      Assert.IsTrue(model!.Contains("/Web/Web_1/Web_1_1/Web_1_1_0/</loc>", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("/Web/Web_1/Web_1_0/Web_1_0_0/</loc>", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("/Web/Web_1/Web_1_1/Web_1_1_1/</loc>", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("/Web/Web_0/Web_0_0/</loc>", StringComparison.Ordinal));
 
     }
 
@@ -220,15 +220,15 @@ public void SitemapController_Index_ExcludesAttributes() {
         ControllerContext       = new(actionContext)
       };
       var result                = controller.Index(false, true) as ContentResult;
-      var model                 = result.Content as string;
+      var model                 = result?.Content as string;
 
       controller.Dispose();
 
       Assert.IsNotNull(model);
-      Assert.IsTrue(model.Contains("<Attribute name=\"Title\">"));
-      Assert.IsTrue(model.Contains("<Attribute name=\"LastModified\">"));
-      Assert.IsFalse(model.Contains("<Attribute name=\"Body\">"));
-      Assert.IsFalse(model.Contains("<Attribute name=\"IsHidden\">"));
+      Assert.IsTrue(model!.Contains("<Attribute name=\"Title\">", StringComparison.Ordinal));
+      Assert.IsTrue(model!.Contains("<Attribute name=\"LastModified\">", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("<Attribute name=\"Body\">", StringComparison.Ordinal));
+      Assert.IsFalse(model!.Contains("<Attribute name=\"IsHidden\">", StringComparison.Ordinal));
 
     }
 
diff --git a/OnTopic.AspNetCore.Mvc.Tests/TopicRepositoryExtensionsTest.cs b/OnTopic.AspNetCore.Mvc.Tests/TopicRepositoryExtensionsTest.cs
@@ -61,7 +61,7 @@ public void Load_ByRoute_ReturnsTopic() {
 
       Assert.IsNotNull(currentTopic);
       Assert.ReferenceEquals(topic, currentTopic);
-      Assert.AreEqual<string>("Web_0_1_1", currentTopic.Key);
+      Assert.AreEqual<string?>("Web_0_1_1", currentTopic?.Key);
 
     }
 
@@ -83,7 +83,7 @@ public void Load_ByRoute_ReturnsRootTopic() {
 
       Assert.IsNotNull(currentTopic);
       Assert.ReferenceEquals(topic, currentTopic);
-      Assert.AreEqual<string>("Root", currentTopic.Key);
+      Assert.AreEqual<string?>("Root", currentTopic?.Key);
 
     }
 
diff --git a/OnTopic.AspNetCore.Mvc.Tests/TopicViewComponentTest.cs b/OnTopic.AspNetCore.Mvc.Tests/TopicViewComponentTest.cs
@@ -41,7 +41,7 @@ public class TopicViewComponentTest {
     /*==========================================================================================================================
     | HIERARCHICAL TOPIC MAPPING SERVICE
     \-------------------------------------------------------------------------------------------------------------------------*/
-    private readonly IHierarchicalTopicMappingService<NavigationTopicViewModel> _hierarchicalMappingService = null;
+    private readonly IHierarchicalTopicMappingService<NavigationTopicViewModel> _hierarchicalMappingService;
 
     /*==========================================================================================================================
     | CONSTRUCTOR
@@ -110,13 +110,13 @@ public async Task Menu_Invoke_ReturnsNavigationViewModel() {
 
       var result                = await viewComponent.InvokeAsync().ConfigureAwait(false);
       var concreteResult        = result as ViewViewComponentResult;
-      var model                 = concreteResult.ViewData.Model as NavigationViewModel<NavigationTopicViewModel>;
+      var model                 = concreteResult?.ViewData.Model as NavigationViewModel<NavigationTopicViewModel>;
 
       Assert.IsNotNull(model);
-      Assert.AreEqual<string>(_topic.GetUniqueKey(), model.CurrentKey);
-      Assert.AreEqual<string>("Root:Web", model.NavigationRoot.UniqueKey);
-      Assert.AreEqual<int>(3, model.NavigationRoot.Children.Count);
-      Assert.IsTrue(model.NavigationRoot.IsSelected(_topic.GetUniqueKey()));
+      Assert.AreEqual<string?>(_topic.GetUniqueKey(), model?.CurrentKey);
+      Assert.AreEqual<string?>("Root:Web", model?.NavigationRoot?.UniqueKey);
+      Assert.AreEqual<int?>(3, model?.NavigationRoot?.Children.Count);
+      Assert.IsTrue(model?.NavigationRoot?.IsSelected(_topic.GetUniqueKey())?? false);
 
     }
 
@@ -135,13 +135,13 @@ public async Task PageLevelNavigation_Invoke_ReturnsNavigationViewModel() {
 
       var result                = await viewComponent.InvokeAsync().ConfigureAwait(false);
       var concreteResult        = result as ViewViewComponentResult;
-      var model                 = concreteResult.ViewData.Model as NavigationViewModel<NavigationTopicViewModel>;
+      var model                 = concreteResult?.ViewData.Model as NavigationViewModel<NavigationTopicViewModel>;
 
       Assert.IsNotNull(model);
-      Assert.AreEqual<string>(_topic.GetUniqueKey(), model.CurrentKey);
-      Assert.AreEqual<string>("Root:Web:Web_3", model.NavigationRoot.UniqueKey);
-      Assert.AreEqual<int>(2, model.NavigationRoot.Children.Count);
-      Assert.IsTrue(model.NavigationRoot.IsSelected(_topic.GetUniqueKey()));
+      Assert.AreEqual<string?>(_topic.GetUniqueKey(), model?.CurrentKey);
+      Assert.AreEqual<string?>("Root:Web:Web_3", model?.NavigationRoot?.UniqueKey);
+      Assert.AreEqual<int?>(2, model?.NavigationRoot?.Children.Count);
+      Assert.IsTrue(model?.NavigationRoot?.IsSelected(_topic.GetUniqueKey())?? false);
 
     }
 
diff --git a/OnTopic.AspNetCore.Mvc.Tests/ValidateTopicAttributeTest.cs b/OnTopic.AspNetCore.Mvc.Tests/ValidateTopicAttributeTest.cs
@@ -61,6 +61,7 @@ public static ActionExecutingContext GetActionExecutingContext(Controller contro
     /// <summary>
     ///   Generates a barebones <see cref="ControllerContext"/> for testing a controller.
     /// </summary>
+    #pragma warning disable CA1024 // Use properties where appropriate
     public static ControllerContext GetControllerContext() =>
       new(
         new() {
@@ -69,14 +70,15 @@ public static ControllerContext GetControllerContext() =>
           ActionDescriptor          = new ControllerActionDescriptor()
         }
       );
+    #pragma warning restore CA1024 // Use properties where appropriate
 
     /*==========================================================================================================================
     | METHOD: GET TOPIC CONTROLLER
     \-------------------------------------------------------------------------------------------------------------------------*/
     /// <summary>
     ///   Generates a barebones <see cref="ControllerContext"/> for testing a controller.
     /// </summary>
-    public static TopicController GetTopicController(Topic topic) =>
+    public static TopicController GetTopicController(Topic? topic) =>
       new(
           new DummyTopicRepository(),
           new DummyTopicMappingService()
@@ -127,7 +129,7 @@ public void NullTopic_ReturnsNotFound() {
 
       controller.Dispose();
 
-      Assert.AreEqual(typeof(NotFoundObjectResult), context.Result.GetType());
+      Assert.AreEqual(typeof(NotFoundObjectResult), context.Result?.GetType());
 
     }
 
@@ -151,7 +153,7 @@ public void DisabledTopic_ReturnsNotFound() {
 
       controller.Dispose();
 
-      Assert.AreEqual(typeof(UnauthorizedResult), context.Result.GetType());
+      Assert.AreEqual(typeof(UnauthorizedResult), context.Result?.GetType());
 
     }
 
@@ -176,7 +178,7 @@ public void TopicWithUrl_ReturnsRedirect() {
 
       controller.Dispose();
 
-      Assert.AreEqual(typeof(RedirectResult), context.Result.GetType());
+      Assert.AreEqual(typeof(RedirectResult), context.Result?.GetType());
 
     }
 
@@ -202,7 +204,7 @@ public void NestedTopic_Returns403() {
       var result                = context.Result as StatusCodeResult;
 
       Assert.IsNotNull(result);
-      Assert.AreEqual(403, result.StatusCode);
+      Assert.AreEqual(403, result?.StatusCode);
 
     }
 
@@ -227,7 +229,7 @@ public void PageGroupTopic_ReturnsRedirect() {
 
       controller.Dispose();
 
-      Assert.AreEqual(typeof(RedirectResult), context.Result.GetType());
+      Assert.AreEqual(typeof(RedirectResult), context.Result?.GetType());
 
     }
 

Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ public void Load_ByRoute_ReturnsTopic() {`
`61`	`61`
`62`	`62`	`Assert.IsNotNull(currentTopic);`
`63`	`63`	`Assert.ReferenceEquals(topic, currentTopic);`
`64`		`- Assert.AreEqual<string>("Web_0_1_1", currentTopic.Key);`
	`64`	`+ Assert.AreEqual<string?>("Web_0_1_1", currentTopic?.Key);`
`65`	`65`
`66`	`66`	`}`
`67`	`67`
`@@ -83,7 +83,7 @@ public void Load_ByRoute_ReturnsRootTopic() {`
`83`	`83`
`84`	`84`	`Assert.IsNotNull(currentTopic);`
`85`	`85`	`Assert.ReferenceEquals(topic, currentTopic);`
`86`		`- Assert.AreEqual<string>("Root", currentTopic.Key);`
	`86`	`+ Assert.AreEqual<string?>("Root", currentTopic?.Key);`
`87`	`87`
`88`	`88`	`}`
`89`	`89`