Initial code analyzer for Microsoft.ML, use limited StyleCop (#557)

* Initial code analyzer for Microsoft.ML

Adds code analysis initially for correct usage of common Contracts.Except/Check
patterns, naming conventions, variable usage and initializations, and other idioms
used throughout the Microsoft.ML codebase. Enables analysis on Microsoft.ML projects.

Also added StyleCop, with most rules currently disabled, but rules on whitespace,
declaration modifier ordering, explicit access modifiers, and inteface naming check.

* Analyzer is .NET Standard 1.3 to avoid problems with IDE1003.

dotnet/roslyn#22368

* Projects in `src` can not use analyzer by setting `UseMLCodeAnalyzer` project property to false.

Author: TomFinley

Microsoft.ML.sln

@@ -5,6 +5,9 @@ MinimumVisualStudioVersion = 10.0.40219.1
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.ML.Core", "src\Microsoft.ML.Core\Microsoft.ML.Core.csproj", "{A6CA6CC6-5D7C-4D7F-A0F5-35E14B383B0A}"
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{09EADF06-BE25-4228-AB53-95AE3E15B530}"
+	ProjectSection(SolutionItems) = preProject
+		src\Source.ruleset = src\Source.ruleset
+	EndProjectSection
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{AED9C836-31E3-4F3F-8ABC-929555D3F3C4}"
 EndProject
@@ -88,6 +91,12 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Microsoft.ML.CpuMath", "Mic
 		pkg\Microsoft.ML.CpuMath\Microsoft.ML.CpuMath.symbols.nupkgproj = pkg\Microsoft.ML.CpuMath\Microsoft.ML.CpuMath.symbols.nupkgproj
 	EndProjectSection
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "tools-local", "tools-local", "{7F13E156-3EBA-4021-84A5-CD56BA72F99E}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.ML.CodeAnalyzer", "tools-local\Microsoft.ML.CodeAnalyzer\Microsoft.ML.CodeAnalyzer.csproj", "{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}"
+EndProject
+Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.ML.CodeAnalyzer.Tests", "test\Microsoft.ML.CodeAnalyzer.Tests\Microsoft.ML.CodeAnalyzer.Tests.csproj", "{3E4ABF07-7970-4BE6-B45B-A13D3C397545}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -304,6 +313,22 @@ Global
 		{DCF46B79-1FDB-4DBA-A263-D3D64E3AAA27}.Release|Any CPU.Build.0 = Release|Any CPU
 		{DCF46B79-1FDB-4DBA-A263-D3D64E3AAA27}.Release-Intrinsics|Any CPU.ActiveCfg = Release|Any CPU
 		{DCF46B79-1FDB-4DBA-A263-D3D64E3AAA27}.Release-Intrinsics|Any CPU.Build.0 = Release|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Debug-Intrinsics|Any CPU.ActiveCfg = Debug|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Debug-Intrinsics|Any CPU.Build.0 = Debug|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Release|Any CPU.Build.0 = Release|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Release-Intrinsics|Any CPU.ActiveCfg = Release|Any CPU
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440}.Release-Intrinsics|Any CPU.Build.0 = Release|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Debug-Intrinsics|Any CPU.ActiveCfg = Debug|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Debug-Intrinsics|Any CPU.Build.0 = Debug|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Release-Intrinsics|Any CPU.ActiveCfg = Release|Any CPU
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545}.Release-Intrinsics|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -340,6 +365,8 @@ Global
 		{001F3B4E-FBE4-4001-AFD2-A6A989CD1C25} = {09EADF06-BE25-4228-AB53-95AE3E15B530}
 		{DCF46B79-1FDB-4DBA-A263-D3D64E3AAA27} = {09EADF06-BE25-4228-AB53-95AE3E15B530}
 		{BF66A305-DF10-47E4-8D81-42049B149D2B} = {D3D38B03-B557-484D-8348-8BADEE4DF592}
+		{B4E55B2D-2A92-46E7-B72F-E76D6FD83440} = {7F13E156-3EBA-4021-84A5-CD56BA72F99E}
+		{3E4ABF07-7970-4BE6-B45B-A13D3C397545} = {AED9C836-31E3-4F3F-8ABC-929555D3F3C4}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {41165AF1-35BB-4832-A189-73060F82B01D}

build/Dependencies.props

@@ -7,6 +7,6 @@
     <SystemCodeDomPackageVersion>4.4.0</SystemCodeDomPackageVersion>
     <SystemReflectionEmitLightweightPackageVersion>4.3.0</SystemReflectionEmitLightweightPackageVersion>
     <PublishSymbolsPackageVersion>1.0.0-beta-62824-02</PublishSymbolsPackageVersion>
-	<LightGBMPackageVersion>2.1.2.2</LightGBMPackageVersion>
+    <LightGBMPackageVersion>2.1.2.2</LightGBMPackageVersion>
   </PropertyGroup>
 </Project>

src/Directory.Build.props

@@ -11,7 +11,18 @@
     <NoWarn>$(NoWarn);1591</NoWarn>
     <WarningsNotAsErrors>$(WarningsNotAsErrors);1591</WarningsNotAsErrors>
 
-
+    <CodeAnalysisRuleSet>$(MSBuildThisFileDirectory)\Source.ruleset</CodeAnalysisRuleSet>
   </PropertyGroup>
 
+  <ItemGroup>
+    <ProjectReference
+      Condition="'$(UseMLCodeAnalyzer)' != 'false' and '$(MSBuildProjectExtension)' == '.csproj'"
+      Include="$(MSBuildThisFileDirectory)\..\tools-local\Microsoft.ML.CodeAnalyzer\Microsoft.ML.CodeAnalyzer.csproj">
+      <ReferenceOutputAssembly>false</ReferenceOutputAssembly>
+      <OutputItemType>Analyzer</OutputItemType>
+    </ProjectReference>
+    <PackageReference Condition="'$(UseStyleCopAnalyzer)' != 'false' and '$(MSBuildProjectExtension)' == '.csproj'"
+      Include="StyleCop.Analyzers" Version="1.1.0-beta008" PrivateAssets="All" />
+  </ItemGroup>
+
 </Project>

src/Microsoft.ML.Api/ApiUtils.cs

@@ -46,7 +46,7 @@ private static OpCode GetAssignmentOpCode(Type t)
 
         /// <summary>
         /// Each of the specialized 'peek' methods copies the appropriate field value of an instance of T
-        /// into the provided buffer. So, the call is 'peek(userObject, ref destination)' and the logic is 
+        /// into the provided buffer. So, the call is 'peek(userObject, ref destination)' and the logic is
         /// indentical to 'destination = userObject.##FIELD##', where ##FIELD## is defined per peek method.
         /// </summary>
         internal static Delegate GeneratePeek<TOwn, TRow>(InternalSchemaDefinition.Column column)
@@ -83,7 +83,7 @@ private static Delegate GeneratePeek<TOwn, TRow, TValue>(FieldInfo fieldInfo, Op
 
         /// <summary>
         /// Each of the specialized 'poke' methods sets the appropriate field value of an instance of T
-        /// to the provided value. So, the call is 'peek(userObject, providedValue)' and the logic is 
+        /// to the provided value. So, the call is 'peek(userObject, providedValue)' and the logic is
         /// indentical to 'userObject.##FIELD## = providedValue', where ##FIELD## is defined per poke method.
         /// </summary>
         internal static Delegate GeneratePoke<TOwn, TRow>(InternalSchemaDefinition.Column column)

src/Microsoft.ML.Api/CodeGenerationUtils.cs

@@ -97,12 +97,12 @@ public static string GetCSharpString(CSharpCodeProvider codeProvider, string val
         }
 
         /// <summary>
-        /// Gets the C# strings representing the type name for a variable corresponding to 
-        /// the <see cref="IDataView"/> column type. 
-        /// 
-        /// If the type is a vector, then <paramref name="useVBuffer"/> controls whether the array field is 
+        /// Gets the C# strings representing the type name for a variable corresponding to
+        /// the <see cref="IDataView"/> column type.
+        ///
+        /// If the type is a vector, then <paramref name="useVBuffer"/> controls whether the array field is
         /// generated or <see cref="VBuffer{T}"/>.
-        /// 
+        ///
         /// If additional attributes are required, they are appended to the <paramref name="attributes"/> list.
         /// </summary>
         private static string GetBackingTypeName(ColumnType colType, bool useVBuffer, List<string> attributes)

src/Microsoft.ML.Api/ComponentCreation.cs

@@ -11,14 +11,14 @@
 namespace Microsoft.ML.Runtime.Api
 {
     /// <summary>
-    /// This class defines extension methods for an <see cref="IHostEnvironment"/> to facilitate creating 
+    /// This class defines extension methods for an <see cref="IHostEnvironment"/> to facilitate creating
     /// components (loaders, transforms, trainers, scorers, evaluators, savers).
     /// </summary>
     public static class ComponentCreation
     {
         /// <summary>
         /// Create a new data view which is obtained by appending all columns of all the source data views.
-        /// If the data views are of different length, the resulting data view will have the length equal to the 
+        /// If the data views are of different length, the resulting data view will have the length equal to the
         /// length of the shortest source.
         /// </summary>
         /// <param name="env">The host environment to use.</param>
@@ -59,11 +59,11 @@ public static RoleMappedData CreateExamples(this IHostEnvironment env, IDataView
         /// Create a new <see cref="IDataView"/> over an in-memory collection of the items of user-defined type.
         /// The user maintains ownership of the <paramref name="data"/> and the resulting data view will
         /// never alter the contents of the <paramref name="data"/>.
-        /// Since <see cref="IDataView"/> is assumed to be immutable, the user is expected to not 
+        /// Since <see cref="IDataView"/> is assumed to be immutable, the user is expected to not
         /// modify the contents of <paramref name="data"/> while the data view is being actively cursored.
-        /// 
+        ///
         /// One typical usage for in-memory data view could be: create the data view, train a predictor.
-        /// Once the predictor is fully trained, modify the contents of the underlying collection and 
+        /// Once the predictor is fully trained, modify the contents of the underlying collection and
         /// train another predictor.
         /// </summary>
         /// <typeparam name="TRow">The user-defined item type.</typeparam>
@@ -88,9 +88,9 @@ public static IDataView CreateDataView<TRow>(this IHostEnvironment env, IList<TR
         /// Since <see cref="IDataView"/> is assumed to be immutable, the user is expected to support
         /// multiple enumeration of the <paramref name="data"/> that would return the same results, unless
         /// the user knows that the data will only be cursored once.
-        /// 
+        ///
         /// One typical usage for streaming data view could be: create the data view that lazily loads data
-        /// as needed, then apply pre-trained transformations to it and cursor through it for transformation 
+        /// as needed, then apply pre-trained transformations to it and cursor through it for transformation
         /// results. This is how <see cref="BatchPredictionEngine{TSrc,TDst}"/> is implemented.
         /// </summary>
         /// <typeparam name="TRow">The user-defined item type.</typeparam>
@@ -191,7 +191,7 @@ public static PredictionEngine<TSrc, TDst> CreatePredictionEngine<TSrc, TDst>(th
         /// <summary>
         /// Create a prediction engine.
         /// This encapsulates the 'classic' prediction problem, where the input is denoted by the float array of features,
-        /// and the output is a float score. For binary classification predictors that can output probability, there are output 
+        /// and the output is a float score. For binary classification predictors that can output probability, there are output
         /// fields that report the predicted label and probability.
         /// </summary>
         /// <param name="env">The host environment to use.</param>
@@ -207,7 +207,7 @@ public static SimplePredictionEngine CreateSimplePredictionEngine(this IHostEnvi
 
         /// <summary>
         /// Load the transforms (but not loader) from the model steram and apply them to the specified data.
-        /// It is acceptable to have no transforms in the model stream: in this case the original 
+        /// It is acceptable to have no transforms in the model stream: in this case the original
         /// <paramref name="data"/> will be returned.
         /// </summary>
         /// <param name="env">The host environment to use.</param>

src/Microsoft.ML.Api/DataViewConstructionUtils.cs

@@ -285,7 +285,7 @@ private Delegate CreateGetter(int index)
                 }
 
                 // REVIEW: The converting getter invokes a type conversion delegate on every call, so it's inherently slower
-                // than the 'direct' getter. We don't have good indication of this to the user, and the selection 
+                // than the 'direct' getter. We don't have good indication of this to the user, and the selection
                 // of affected types is pretty arbitrary (signed integers and bools, but not uints and floats).
                 private Delegate CreateConvertingArrayGetterDelegate<TSrc, TDst>(int index, Func<TSrc, TDst> convert)
                 {
@@ -396,7 +396,7 @@ private void CheckColumnInRange(int columnIndex)
         }
 
         /// <summary>
-        /// An in-memory data view based on the IList of data. 
+        /// An in-memory data view based on the IList of data.
         /// Supports shuffling.
         /// </summary>
         private sealed class ListDataView<TRow> : DataViewBase<TRow>
@@ -492,11 +492,11 @@ protected override bool MoveManyCore(long count)
         }
 
         /// <summary>
-        /// An in-memory data view based on the IEnumerable of data. 
+        /// An in-memory data view based on the IEnumerable of data.
         /// Doesn't support shuffling.
-        /// 
+        ///
         /// This class is public because prediction engine wants to call its <see cref="SetData"/>
-        /// for performance reasons. 
+        /// for performance reasons.
         /// </summary>
         public sealed class StreamingDataView<TRow> : DataViewBase<TRow>
             where TRow : class
@@ -578,7 +578,7 @@ protected override bool MoveNextCore()
 
         /// <summary>
         /// This represents the 'infinite data view' over one (mutable) user-defined object.
-        /// The 'current row' object can be updated at any time, this will affect all the 
+        /// The 'current row' object can be updated at any time, this will affect all the
         /// newly created cursors, but not the ones already existing.
         /// </summary>
         public sealed class SingleRowLoopDataView<TRow> : DataViewBase<TRow>
@@ -731,7 +731,7 @@ public abstract partial class MetadataInfo
         /// </summary>
         public ColumnType MetadataType;
         /// <summary>
-        /// The string identifier of the metadata. Some identifiers have special meaning, 
+        /// The string identifier of the metadata. Some identifiers have special meaning,
         /// like "SlotNames", but any other identifiers can be used.
         /// </summary>
         public readonly string Kind;
@@ -757,7 +757,7 @@ public sealed class MetadataInfo<T> : MetadataInfo
         /// <summary>
         /// Constructor for metadata of value type T.
         /// </summary>
-        /// <param name="kind">The string identifier of the metadata. Some identifiers have special meaning, 
+        /// <param name="kind">The string identifier of the metadata. Some identifiers have special meaning,
         /// like "SlotNames", but any other identifiers can be used.</param>
         /// <param name="value">Metadata value.</param>
         /// <param name="metadataType">Type of the metadata.</param>

src/Microsoft.ML.Api/GenerateCodeCommand.cs

@@ -21,7 +21,7 @@ namespace Microsoft.ML.Runtime.Api
 {
     /// <summary>
     /// Generates the sample prediction code for a given model file, with correct input and output classes.
-    /// 
+    ///
     /// REVIEW: Consider adding support for generating VBuffers instead of arrays, maybe for high dimensionality vectors.
     /// </summary>
     public sealed class GenerateCodeCommand : ICommand
@@ -45,7 +45,7 @@ public sealed class Arguments
                 ShortName = "sparse", SortOrder = 102)]
             public bool SparseVectorDeclaration;
 
-            // REVIEW: currently, it's only used in unit testing to not generate the paths into the test output folder. 
+            // REVIEW: currently, it's only used in unit testing to not generate the paths into the test output folder.
             // However, it might be handy for automation scenarios, so I've added this as a hidden option.
             [Argument(ArgumentType.AtMostOnce, HelpText = "A location of the model file to put into generated file", Hide = true)]
             public string ModelNameOverride;

src/Microsoft.ML.Api/InternalSchemaDefinition.cs

@@ -76,12 +76,12 @@ private Column(string columnName, ColumnType columnType, FieldInfo fieldInfo = n
             }
 
             /// <summary>
-            /// Function that checks whether the InternalSchemaDefinition.Column is a valid one. 
+            /// Function that checks whether the InternalSchemaDefinition.Column is a valid one.
             /// To be valid, the Column must:
             ///     1. Have non-empty values for ColumnName and ColumnType
             ///     2. Have a non-empty value for FieldInfo iff it is a field column, else
             ///        ReturnParameterInfo and Generator iff it is a computed column
-            ///     3. Generator must have the method inputs (TRow rowObject, 
+            ///     3. Generator must have the method inputs (TRow rowObject,
             ///        long position, ref TValue outputValue) in that order.
             ///  </summary>
             [Conditional("DEBUG")]
@@ -133,7 +133,7 @@ private InternalSchemaDefinition(Column[] columns)
         /// <summary>
         /// Given a field info on a type, returns whether this appears to be a vector type,
         /// and also the associated data kind for this type. If a data kind could not
-        /// be determined, this will throw. 
+        /// be determined, this will throw.
         /// </summary>
         /// <param name="fieldInfo">The field info to inspect.</param>
         /// <param name="isVector">Whether this appears to be a vector type.</param>
@@ -149,7 +149,7 @@ public static void GetVectorAndKind(FieldInfo fieldInfo, out bool isVector, out
         /// <summary>
         /// Given a parameter info on a type, returns whether this appears to be a vector type,
         /// and also the associated data kind for this type. If a data kind could not
-        /// be determined, this will throw. 
+        /// be determined, this will throw.
         /// </summary>
         /// <param name="parameterInfo">The parameter info to inspect.</param>
         /// <param name="isVector">Whether this appears to be a vector type.</param>
@@ -165,7 +165,7 @@ public static void GetVectorAndKind(ParameterInfo parameterInfo, out bool isVect
         /// <summary>
         /// Given a type and name for a variable, returns whether this appears to be a vector type,
         /// and also the associated data kind for this type. If a data kind could not
-        /// be determined, this will throw. 
+        /// be determined, this will throw.
         /// </summary>
         /// <param name="rawType">The type of the variable to inspect.</param>
         /// <param name="name">The name of the variable to inspect.</param>
@@ -222,7 +222,7 @@ public static InternalSchemaDefinition Create(Type userType, SchemaDefinition us
                             col.MemberName,
                             userType.FullName);
 
-                    //Clause to handle the field that may be used to expose the cursor channel. 
+                    //Clause to handle the field that may be used to expose the cursor channel.
                     //This field does not need a column.
                     if (fieldInfo.FieldType == typeof(IChannel))
                         continue;
@@ -251,7 +251,7 @@ public static InternalSchemaDefinition Create(Type userType, SchemaDefinition us
                 }
                 else
                 {
-                    // Make sure that the types are compatible with the declared type, including 
+                    // Make sure that the types are compatible with the declared type, including
                     // whether it is a vector type.
                     if (isVector != col.ColumnType.IsVector)
                     {