Added code analysis nuget in Serilog.Sinks.MSSqlServer and Serilog.Sinks.MSSqlServer.Tests projects and fixed all problems possible and suppressed some that could not be fixed.

Author: ckadluba

src/Serilog.Sinks.MSSqlServer/Configuration/Extensions/Hybrid/LoggerConfigurationMSSqlServerExtensions.cs

@@ -35,7 +35,7 @@ public static class LoggerConfigurationMSSqlServerExtensions
         /// <summary>
         /// The configuration section name for app.config or web.config configuration files.
         /// </summary>
-        public static string AppConfigSectionName = "MSSqlServerSettingsSection";
+        public const string AppConfigSectionName = "MSSqlServerSettingsSection";
 
         /// <summary>
         /// Adds a sink that writes log events to a table in a MSSqlServer database.

src/Serilog.Sinks.MSSqlServer/Configuration/Extensions/System.Configuration/LoggerConfigurationMSSqlServerExtensions.cs

@@ -18,6 +18,7 @@
 using Serilog.Sinks.MSSqlServer;
 using System.Configuration;
 using Serilog.Formatting;
+using System.Diagnostics.CodeAnalysis;
 
 // System.Configuration support for .NET Framework 4.5.2 libraries and apps.
 
@@ -31,6 +32,8 @@ public static class LoggerConfigurationMSSqlServerExtensions
         /// <summary>
         /// The configuration section name for app.config or web.config configuration files.
         /// </summary>
+        // TODO: Make this non-static. It could cause race condition in unit tests!
+        [SuppressMessage("Usage", "CA2211:Non-constant fields should not be visible", Justification = "Too hard to change. Accepted for now.")]
         public static string AppConfigSectionName = "MSSqlServerSettingsSection";
 
         /// <summary>

src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/SetProperty.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Globalization;
 
 namespace Serilog.Sinks.MSSqlServer
 {
@@ -22,10 +23,10 @@ public static partial class SetProperty
         /// </summary>
         public static void IfNotNull<T>(string value, PropertySetter<T> setter)
         {
-            if (value == null) return;
+            if (value == null || setter == null) return;
             try
             {
-                T setting = (T)Convert.ChangeType(value, typeof(T));
+                T setting = (T)Convert.ChangeType(value, typeof(T), CultureInfo.InvariantCulture);
                 setter(setting);
             }
             // don't change the property if the conversion fails 

src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/ColumnCollection.cs

@@ -28,7 +28,7 @@ protected override ConfigurationElement CreateNewElement()
 
         protected override object GetElementKey(ConfigurationElement element)
         {
-            return ((ColumnConfig)element).ColumnName;
+            return ((ColumnConfig)element)?.ColumnName;
         }
     }
 }

src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/SetPropertyValueOrigin.cs

@@ -12,6 +12,11 @@ public static partial class SetProperty
         /// </summary>
         public static void IfProvided<T>(ConfigurationElement element, string propertyName, PropertySetter<T> setter)
         {
+            if (element == null)
+            {
+                return;
+            }
+
             var property = element.ElementInformation.Properties[propertyName];
             if (property.ValueOrigin == PropertyValueOrigin.Default) return;
             IfNotNull((string)property.Value, setter);
@@ -22,6 +27,11 @@ public static void IfProvided<T>(ConfigurationElement element, string propertyNa
         /// </summary>
         public static void IfProvidedNotEmpty<T>(ConfigurationElement element, string propertyName, PropertySetter<T> setter)
         {
+            if (element == null)
+            {
+                return;
+            }
+
             var property = element.ElementInformation.Properties[propertyName];
             if (property.ValueOrigin == PropertyValueOrigin.Default) return;
             IfNotNullOrEmpty((string)property.Value, setter);

src/Serilog.Sinks.MSSqlServer/Configuration/Implementations/System.Configuration/StandardColumnCollection.cs

@@ -14,7 +14,7 @@ protected override ConfigurationElement CreateNewElement()
 
         protected override object GetElementKey(ConfigurationElement element)
         {
-            return ((StandardColumnConfig)element).Name;
+            return ((StandardColumnConfig)element)?.Name;
         }
     }
 }

src/Serilog.Sinks.MSSqlServer/GlobalSuppressions.cs

@@ -0,0 +1,20 @@
+// This file is used by Code Analysis to maintain SuppressMessage
+// attributes that are applied to this project.
+// Project-level suppressions either have no target or are given
+// a specific target and scoped to a namespace, type, member, etc.
+
+using System.Diagnostics.CodeAnalysis;
+
+[assembly: SuppressMessage("Globalization", "CA1303:Do not pass literals as localized parameters", Justification = "Supplying string literals and not using resources is accepted within this project.", Scope = "namespaceanddescendants", Target = "Serilog.Sinks.MSSqlServer")]
+[assembly: SuppressMessage("Security", "CA2100:Review SQL queries for security vulnerabilities", Justification = "Too hard to change. Accepted for now.", Scope = "namespaceanddescendants", Target = "Serilog.Sinks.MSSqlServer")]
+[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerSinkTraits.TryChangeType(System.Object,System.Type,System.Object@)~System.Boolean")]
+[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerSink.EmitBatchAsync(System.Collections.Generic.IEnumerable{Serilog.Events.LogEvent})~System.Threading.Tasks.Task")]
+[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerSinkTraits.ConvertPropertiesToXmlStructure(System.Collections.Generic.IEnumerable{System.Collections.Generic.KeyValuePair{System.String,Serilog.Events.LogEventPropertyValue}})~System.String")]
+[assembly: SuppressMessage("Design", "CA1031:Do not catch general exception types", Justification = "Too hard to change. Accepted for now.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerSinkTraits.#ctor(System.String,System.String,System.String,Serilog.Sinks.MSSqlServer.ColumnOptions,System.IFormatProvider,System.Boolean,Serilog.Formatting.ITextFormatter,Serilog.Sinks.MSSqlServer.Platform.ISqlTableCreator)")]
+[assembly: SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "Serilog core guarantees to call Emit() with non-null logEvent parameter.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerAuditSink.Emit(Serilog.Events.LogEvent)")]
+[assembly: SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "Serilog core guarantees to call EmitBatchAsync() with non-null logEvent parameter.", Scope = "member", Target = "~M:Serilog.Sinks.MSSqlServer.MSSqlServerSink.EmitBatchAsync(System.Collections.Generic.IEnumerable{Serilog.Events.LogEvent})~System.Threading.Tasks.Task")]
+[assembly: SuppressMessage("Design", "CA1034:Nested types should not be visible", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "namespaceanddescendants", Target = "Serilog.Sinks.MSSqlServer")]
+[assembly: SuppressMessage("Usage", "CA2227:Collection properties should be read only", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "type", Target = "~T:Serilog.Sinks.MSSqlServer.ColumnOptions")]
+[assembly: SuppressMessage("Design", "CA1010:Collections should implement generic interface", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "namespaceanddescendants", Target = "Serilog.Sinks.MSSqlServer")]
+[assembly: SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "Too hard to change. Accepted for now.", Scope = "type", Target = "~T:Serilog.LoggerConfigurationMSSqlServerExtensions")]
+[assembly: SuppressMessage("Performance", "CA1822: Member AllowNull does not access instance data and can be marked as static (Shared in VisualBasic)", Justification = "Cannot be changed on public classes for backward compatibility reasons.", Scope = "member", Target = "~P:Serilog.Sinks.MSSqlServer.ColumnOptions.IdColumnOptions.AllowNull")]

src/Serilog.Sinks.MSSqlServer/Serilog.Sinks.MSSqlServer.csproj

@@ -26,6 +26,10 @@
 
   <ItemGroup>
     <!-- Whenever these are updated, also update versions in packages.config for net452 -->
+    <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
     <PackageReference Include="Serilog" Version="2.5.0" />
     <PackageReference Include="Serilog.Sinks.PeriodicBatching" Version="2.1.1" />
   </ItemGroup>
@@ -76,7 +80,7 @@
   </ItemGroup>
 
   <ItemGroup>
-    <None Include="Images\serilog-sink-nuget.png" Pack="true" PackagePath=""/>
+    <None Include="Images\serilog-sink-nuget.png" Pack="true" PackagePath="" />
   </ItemGroup>
 
 </Project>

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerAuditSink.cs

@@ -50,7 +50,7 @@ public MSSqlServerAuditSink(
             string schemaName = "dbo",
             ITextFormatter logEventFormatter = null)
         {
-            columnOptions.FinalizeConfigurationForSinkConstructor();
+            columnOptions?.FinalizeConfigurationForSinkConstructor();
 
             if (columnOptions != null)
             {

src/Serilog.Sinks.MSSqlServer/Sinks/MSSqlServer/MSSqlServerSink.cs

@@ -16,6 +16,7 @@
 using System.Collections.Generic;
 using System.Data;
 using System.Data.SqlClient;
+using System.Globalization;
 using System.Linq;
 using System.Threading.Tasks;
 using Serilog.Debugging;
@@ -67,7 +68,7 @@ public MSSqlServerSink(
             ITextFormatter logEventFormatter = null)
             : base(batchPostingLimit, period)
         {
-            columnOptions.FinalizeConfigurationForSinkConstructor();
+            columnOptions?.FinalizeConfigurationForSinkConstructor();
             _traits = new MSSqlServerSinkTraits(connectionString, tableName, schemaName, columnOptions, formatProvider, autoCreateSqlTable, logEventFormatter);
         }
 
@@ -95,7 +96,7 @@ protected override async Task EmitBatchAsync(IEnumerable<LogEvent> events)
                             : new SqlBulkCopy(cn, SqlBulkCopyOptions.CheckConstraints | SqlBulkCopyOptions.FireTriggers, null)
                     )
                     {
-                        copy.DestinationTableName = string.Format("[{0}].[{1}]", _traits.SchemaName, _traits.TableName);
+                        copy.DestinationTableName = string.Format(CultureInfo.InvariantCulture, "[{0}].[{1}]", _traits.SchemaName, _traits.TableName);
                         foreach (var column in _traits.EventTable.Columns)
                         {
                             var columnName = ((DataColumn)column).ColumnName;