Enable .NET Analyzers for all managed projects. (#423)

* Enable .NET Analyzers for all managed projects.
Use SDL Recommended ruleset at Error level.
Report all code analysis warnings as errors.
Fix existing violations.

Author: jander-msft

build/CodeAnalysis/Sdl.Recommended.Error.ruleset

@@ -21,6 +21,9 @@
 
   <PropertyGroup>
     <DefineConstants Condition="'$(PublicRelease)' != 'True'">$(DefineConstants);ALLOWNOTSIGNED</DefineConstants>
+    <EnableNetAnalyzers>true</EnableNetAnalyzers>
+    <CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)CodeAnalysis\Sdl.Recommended.Error.ruleset</CodeAnalysisRuleSet>
+    <CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>
   </PropertyGroup>
 
   <Import Project="Sdk.props" Sdk="Microsoft.NET.Sdk" />

build/Managed.props

@@ -9,7 +9,7 @@ steps:
   inputs:
     continueOnError: true
     rulesetName: Custom
-    msBuildArchitecture: x64
+    msBuildArchitecture: amd64
     customRuleset: DevDivRoslynRequired.ruleset
     msBuildCommandline: >-
       dotnet

build/yaml/steps/codeanalysis/csharp.yaml

@@ -5,7 +5,7 @@
 
 namespace RemoteUnitTestExecutor.Host
 {
-    public class Program
+    public static class Program
     {
         public static void Main(string[] args)
         {

src/Tests/RemoteUnitTestExecutor.Host/Program.cs

@@ -4,19 +4,35 @@
 namespace RemoteUnitTestExecutor
 {
     using Microsoft.VisualStudio.TestTools.UnitTesting;
+    using System;
     using System.Collections.Generic;
 
     public static class AssertExtensions
     {
         public static void ClearInvokedMethods(IList<MethodInvocationInfo> invokedMethods)
         {
+            if (null == invokedMethods)
+            {
+                throw new ArgumentNullException(nameof(invokedMethods));
+            }
+
             // Clear the methods
             invokedMethods.Clear();
             Assert.AreEqual(0, invokedMethods.Count, "Invoked methods are not cleared.");
         }
 
         public static void InvokedMethodsAreEqual(IList<MethodInvocationInfo> actual, string[] expectedMethodNames)
         {
+            if (null == actual)
+            {
+                throw new ArgumentNullException(nameof(actual));
+            }
+
+            if (null == expectedMethodNames)
+            {
+                throw new ArgumentNullException(nameof(expectedMethodNames));
+            }
+
             Assert.AreEqual(expectedMethodNames.Length, actual.Count, "Actual and expected invoked methods count mismatch.");
 
             for (int i = 0; i < expectedMethodNames.Length; ++i)

src/Tests/RemoteUnitTestExecutor/AssertExtensions.cs

@@ -4,12 +4,21 @@
 namespace RemoteUnitTestExecutor
 {
     using System;
+    using System.Collections.Generic;
 
     [Serializable]
     public class MethodInvocationInfo
     {
+        [NonSerialized]
+        private List<object> _arguments = new List<object>();
+
         public string MethodName { get; set; }
 
-        [NonSerialized] public object[] Arguments;
+        public IEnumerable<object> Arguments => _arguments;
+
+        public void AddArgument(object value)
+        {
+            _arguments.Add(value);
+        }
     }
 }

src/Tests/RemoteUnitTestExecutor/MethodInvocationInfo.cs

@@ -5,10 +5,15 @@ namespace RemoteUnitTestExecutor
 {
     using System;
 
-    public class Program
+    public static class Program
     {
         public static void Main(string[] args)
         {
+            if (null == args)
+            {
+                throw new ArgumentNullException(nameof(args));
+            }
+
             if (args.Length < 2 || string.IsNullOrWhiteSpace(args[1]))
             {
                 throw new ArgumentException("Wrong number of arguments provided - expecting at least 2 arguments:\r\n arg[0] - test name; \r\n arg[1] - test results file name");

src/Tests/RemoteUnitTestExecutor/Program.cs

@@ -20,7 +20,7 @@ public ITestResult ExecuteTest(
         {
             if (true == string.IsNullOrWhiteSpace(testName))
             {
-                throw new ArgumentNullException("testName");
+                throw new ArgumentNullException(nameof(testName));
             }
 
             string testOutputFileName = "testOutput_" + Guid.NewGuid() + ".xml";

src/Tests/RemoteUnitTestExecutor/TestEngineBase.cs

@@ -5,8 +5,10 @@ namespace RemoteUnitTestExecutor
 {
     using System;
     using System.Collections.Generic;
+    using System.Diagnostics.CodeAnalysis;
     using System.IO;
     using System.Linq;
+    using System.Runtime.Serialization;
     using System.Runtime.Serialization.Formatters.Binary;
 
     /// <summary>
@@ -20,7 +22,7 @@ public TestResult()
             InvokedMethods = new List<MethodInvocationInfo>();
         }
 
-        public IList<MethodInvocationInfo> InvokedMethods { get; set; }
+        public IList<MethodInvocationInfo> InvokedMethods { get; }
 
         public bool Succeeded { get; set; }
 
@@ -36,16 +38,28 @@ public void Serialize(string filename)
             }
         }
 
+        /// CA2300 is set to Info level, which will not cause a build break. However, RoslynAnalyzer and PostAnalysis
+        /// pipeline tasks will fail on any level reported above None. Recommendation is to disable CA2300 and enable
+        /// CA2301 and CA2302 as mitigations. However, these are part of the SDL ruleset and we should not modify its
+        /// behavior. Thus, mitigate CA2301 and CA2302 and then suppress CA2300.
+        [SuppressMessage("Security", "CA2300", Justification = "Mitigated with fixes for CA2301 and CA2302")]
         public static ITestResult CreateFromFile(string testOutputFileName)
         {
             using (FileStream deserializationStream = File.OpenRead(testOutputFileName))
             {
-                return (ITestResult) new BinaryFormatter().Deserialize(deserializationStream);
+                BinaryFormatter formatter = new BinaryFormatter();
+                formatter.Binder = new TestResultSerializationBinder();
+                return (ITestResult)formatter.Deserialize(deserializationStream);
             }
         }
 
         public void AddProfilerTraces(IEnumerable<string> tracesIterator)
         {
+            if (null == tracesIterator)
+            {
+                throw new ArgumentNullException(nameof(tracesIterator));
+            }
+
             foreach (var trace in tracesIterator)
             {
                 this.ProfilerTraces.Add(trace);
@@ -71,5 +85,27 @@ public string InvokedMethodsSequence
                 return string.Empty;
             }
         }
+
+        private class TestResultSerializationBinder : SerializationBinder
+        {
+            private static IEnumerable<string> s_supportedTypes = new List<string>()
+            {
+                typeof(List<MethodInvocationInfo>).FullName,
+                typeof(List<string>).FullName,
+                typeof(MethodInvocationInfo).FullName,
+                typeof(TestResult).FullName
+            };
+
+            public override Type BindToType(string assemblyName, string typeName)
+            {
+                if (s_supportedTypes.Contains(typeName, StringComparer.Ordinal))
+                {
+                    // Tells serializer to use type from deserialized data.
+                    return null;
+                }
+
+                throw new NotSupportedException("Attempted to deserialize unexpected type: " + typeName);
+            }
+        }
     }
 }

src/Tests/RemoteUnitTestExecutor/TestResult.cs

@@ -71,22 +71,22 @@ private object OnBegin(object thisObj)
 
             public object OnEnd(object context, object retValue, object thisObj)
             {
-                TestResult.InvokedMethods.Add(new MethodInvocationInfo
-                    {
-                        MethodName = "OnEnd",
-                        Arguments = new[] { context, retValue, thisObj }
-                    });
+                MethodInvocationInfo info = new MethodInvocationInfo { MethodName = "OnEnd" };
+                info.AddArgument(context);
+                info.AddArgument(retValue);
+                info.AddArgument(thisObj);
+                TestResult.InvokedMethods.Add(info);
 
                 return retValue;
             }
 
             public void OnException(object context, Exception exc, object thisObj)
             {
-                TestResult.InvokedMethods.Add(new MethodInvocationInfo
-                    {
-                        MethodName = "OnException",
-                        Arguments = new[] { context, exc, thisObj }
-                    });
+                MethodInvocationInfo info = new MethodInvocationInfo { MethodName = "OnException" };
+                info.AddArgument(context);
+                info.AddArgument(exc);
+                info.AddArgument(thisObj);
+                TestResult.InvokedMethods.Add(info);
             }
         }