Merge pull request #2247 from microsoft/users/v-jwinkler/#2228_base-l…

…ogger

#2228 Renaming, remove quiet from Logging Options

src/ReleaseHistory.md

@@ -1,7 +1,8 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)
 
-## **v2.3.18** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.18) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.18) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.18) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.18) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.3.18)
-* BREAKING: Entirely remove "verbose" whose fuctionality has been replaced by --level and --kind.  
+## **v2.4.0** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.0) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.0) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.0) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.0) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.0)
+* BREAKING: Entirely remove "verbose" whose fuctionality has been replaced by --level and --kind.
+* BREAKING: Rename "LoggingOptions" to "LogFilePersistenceOptions"
 * FEATURE: --quiet will now suppress all console messages except for errors.
 
 ## **v2.3.17** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.17) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.17) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.17) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.17) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.3.17)

src/Sarif.Converters/GlobalSuppressions.cs

@@ -3,11 +3,5 @@
 
 using System.Diagnostics.CodeAnalysis;
 
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.AndroidStudioConverter.#Convert(System.IO.Stream,Microsoft.CodeAnalysis.Sarif.IResultLogWriter,Microsoft.CodeAnalysis.Sarif.Writers.LoggingOptions)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.ClangAnalyzerConverter.#Convert(System.IO.Stream,Microsoft.CodeAnalysis.Sarif.IResultLogWriter,Microsoft.CodeAnalysis.Sarif.Writers.LoggingOptions)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.CppCheckConverter.#Convert(System.IO.Stream,Microsoft.CodeAnalysis.Sarif.IResultLogWriter,Microsoft.CodeAnalysis.Sarif.Writers.LoggingOptions)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.FortifyConverter.#Convert(System.IO.Stream,Microsoft.CodeAnalysis.Sarif.IResultLogWriter,Microsoft.CodeAnalysis.Sarif.Writers.LoggingOptions)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.FortifyFprConverter.#ParseAuditStream(System.IO.Stream)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.FxCopLogReader.#Read(Microsoft.CodeAnalysis.Sarif.Converters.FxCopLogReader+Context,System.IO.Stream)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.PREfastConverter.#Convert(System.IO.Stream,Microsoft.CodeAnalysis.Sarif.IResultLogWriter,Microsoft.CodeAnalysis.Sarif.Writers.LoggingOptions)")]
-[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "Microsoft.CodeAnalysis.Sarif.Converters.SparseReader.#CreateFromStream(Microsoft.CodeAnalysis.Sarif.Converters.SparseReaderDispatchTable,System.IO.Stream,System.Xml.Schema.XmlSchemaSet)")]
+[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "~M:Microsoft.CodeAnalysis.Sarif.Converters.FxCopLogReader.Read(Microsoft.CodeAnalysis.Sarif.Converters.FxCopLogReader.Context,System.IO.Stream)")]
+[module: SuppressMessage("Microsoft.Security.Xml", "CA3053:UseXmlSecureResolver", Scope = "member", Target = "~M:Microsoft.CodeAnalysis.Sarif.Converters.SparseReader.CreateFromStream(Microsoft.CodeAnalysis.Sarif.Converters.SparseReaderDispatchTable,System.IO.Stream,System.Xml.Schema.XmlSchemaSet)~Microsoft.CodeAnalysis.Sarif.Converters.SparseReader")]

src/Sarif.Converters/ToolFormatConverter.cs

@@ -38,7 +38,7 @@ public void ConvertToStandardFormat(
             string toolFormat,
             string inputFileName,
             string outputFileName,
-            LoggingOptions loggingOptions = LoggingOptions.None,
+            LogFilePersistenceOptions logFilePersistenceOptions = LogFilePersistenceOptions.None,
             OptionallyEmittedData dataToInsert = OptionallyEmittedData.None,
             string pluginAssemblyPath = null)
         {
@@ -52,7 +52,7 @@ public void ConvertToStandardFormat(
             using (var outputTextWriter = new StreamWriter(outputTextStream))
             using (var outputJson = new JsonTextWriter(outputTextWriter))
             {
-                if (loggingOptions.HasFlag(LoggingOptions.PrettyPrint))
+                if (logFilePersistenceOptions.HasFlag(LogFilePersistenceOptions.PrettyPrint))
                 {
                     outputJson.Formatting = Formatting.Indented;
                 }

src/Sarif.Driver/DriverExtensionMethods.cs

@@ -10,17 +10,16 @@ namespace Microsoft.CodeAnalysis.Sarif.Driver
 {
     public static class DriverExtensionMethods
     {
-        public static LoggingOptions ConvertToLoggingOptions(this AnalyzeOptionsBase analyzeOptions)
+        public static LogFilePersistenceOptions ConvertToLogFilePersistenceOptions(this AnalyzeOptionsBase analyzeOptions)
         {
-            LoggingOptions loggingOptions = LoggingOptions.PrettyPrint;
+            LogFilePersistenceOptions logFilePersistenceOptions = LogFilePersistenceOptions.PrettyPrint;
 
-            if (analyzeOptions.Force) { loggingOptions |= LoggingOptions.OverwriteExistingOutputFile; }
-            if (analyzeOptions.Minify) { loggingOptions ^= LoggingOptions.PrettyPrint; }
-            if (analyzeOptions.Quiet) { loggingOptions |= LoggingOptions.Quiet; }
-            if (analyzeOptions.Optimize) { loggingOptions |= LoggingOptions.Optimize; }
-            if (analyzeOptions.PrettyPrint) { loggingOptions |= LoggingOptions.PrettyPrint; }
+            if (analyzeOptions.Force) { logFilePersistenceOptions |= LogFilePersistenceOptions.OverwriteExistingOutputFile; }
+            if (analyzeOptions.Minify) { logFilePersistenceOptions ^= LogFilePersistenceOptions.PrettyPrint; }
+            if (analyzeOptions.Optimize) { logFilePersistenceOptions |= LogFilePersistenceOptions.Optimize; }
+            if (analyzeOptions.PrettyPrint) { logFilePersistenceOptions |= LogFilePersistenceOptions.PrettyPrint; }
 
-            return loggingOptions;
+            return logFilePersistenceOptions;
         }
 
         /// <summary>
@@ -139,12 +138,22 @@ public static bool ValidateOutputOptions(this MultipleFilesOptionsBase options)
         {
             bool valid = true;
 
+            //  TODO:  Is it correct to modify options in a "validate" method?
             if (options.Inline)
             {
                 options.Force = true;
             }
 
             return valid;
         }
+
+        public static bool ValidateOutputOptions(this AnalyzeOptionsBase options)
+        {
+            bool valid = true;
+
+            valid &= !(options.Quiet && string.IsNullOrWhiteSpace(options.OutputFilePath));
+
+            return valid;
+        }
     }
 }

src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs

@@ -173,9 +173,12 @@ protected virtual void ValidateOptions(TContext context, TOptions analyzeOptions
             succeeded &= ValidateFiles(context, analyzeOptions.PluginFilePaths, shouldExist: true);
             succeeded &= ValidateInvocationPropertiesToLog(context, analyzeOptions.InvocationPropertiesToLog);
             succeeded &= ValidateOutputFileCanBeCreated(context, analyzeOptions.OutputFilePath, analyzeOptions.Force);
+            succeeded &= analyzeOptions.ValidateOutputOptions();
 
             if (!succeeded)
             {
+                //  TODO: This seems like uninformative error output.  All these errors get squished into one generic message
+                //  whenever something goes wrong.
                 ThrowExitApplicationException(context, ExitReason.InvalidCommandLineOption);
             }
         }
@@ -408,7 +411,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
                 (
                     () =>
                     {
-                        LoggingOptions loggingOptions = analyzeOptions.ConvertToLoggingOptions();
+                        LogFilePersistenceOptions logFilePersistenceOptions = analyzeOptions.ConvertToLogFilePersistenceOptions();
 
                         OptionallyEmittedData dataToInsert = analyzeOptions.DataToInsert.ToFlags();
                         OptionallyEmittedData dataToRemove = analyzeOptions.DataToRemove.ToFlags();
@@ -419,7 +422,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
                         {
                             sarifLogger = new SarifLogger(
                                     analyzeOptions.OutputFilePath,
-                                    loggingOptions,
+                                    logFilePersistenceOptions,
                                     dataToInsert,
                                     dataToRemove,
                                     tool: _tool,
@@ -434,7 +437,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
                         {
                             sarifLogger = new SarifOneZeroZeroLogger(
                                     analyzeOptions.OutputFilePath,
-                                    loggingOptions,
+                                    logFilePersistenceOptions,
                                     dataToInsert,
                                     dataToRemove,
                                     tool: _tool,

src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs

@@ -365,7 +365,7 @@ private async Task<bool> FindFilesAsync(TOptions options, TContext rootContext)
                         _fileContexts.Add(
                             CreateContext(
                                 options,
-                                new CachingLogger(),
+                                new CachingLogger(options.Level, options.Kind),
                                 rootContext.RuntimeErrors,
                                 rootContext.Policy,
                                 filePath: file)
@@ -642,7 +642,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
                 (
                     () =>
                     {
-                        LoggingOptions loggingOptions = analyzeOptions.ConvertToLoggingOptions();
+                        LogFilePersistenceOptions logFilePersistenceOptions = analyzeOptions.ConvertToLogFilePersistenceOptions();
 
                         OptionallyEmittedData dataToInsert = analyzeOptions.DataToInsert.ToFlags();
                         OptionallyEmittedData dataToRemove = analyzeOptions.DataToRemove.ToFlags();
@@ -655,7 +655,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
                         {
                             sarifLogger = new SarifLogger(
                                     analyzeOptions.OutputFilePath,
-                                    loggingOptions,
+                                    logFilePersistenceOptions,
                                     dataToInsert,
                                     dataToRemove,
                                     tool: _tool,
@@ -668,7 +668,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
                         {
                             sarifLogger = new SarifOneZeroZeroLogger(
                                     analyzeOptions.OutputFilePath,
-                                    loggingOptions,
+                                    logFilePersistenceOptions,
                                     dataToInsert,
                                     dataToRemove,
                                     tool: _tool,

src/Sarif.Multitool.Library/ConvertCommand.cs

@@ -40,25 +40,25 @@ public int Run(ConvertOptions convertOptions, IFileSystem fileSystem = null)
 
                 if (!ValidateOptions(convertOptions, fileSystem)) { return FAILURE; }
 
-                LoggingOptions loggingOptions = LoggingOptions.None;
+                LogFilePersistenceOptions logFilePersistenceOptions = LogFilePersistenceOptions.None;
 
                 OptionallyEmittedData dataToInsert = convertOptions.DataToInsert.ToFlags();
 
                 if (convertOptions.PrettyPrint)
                 {
-                    loggingOptions |= LoggingOptions.PrettyPrint;
+                    logFilePersistenceOptions |= LogFilePersistenceOptions.PrettyPrint;
                 }
 
                 if (convertOptions.Force)
                 {
-                    loggingOptions |= LoggingOptions.OverwriteExistingOutputFile;
+                    logFilePersistenceOptions |= LogFilePersistenceOptions.OverwriteExistingOutputFile;
                 }
 
                 new ToolFormatConverter().ConvertToStandardFormat(
                                                 convertOptions.ToolFormat,
                                                 convertOptions.InputFilePath,
                                                 convertOptions.OutputFilePath,
-                                                loggingOptions,
+                                                logFilePersistenceOptions,
                                                 dataToInsert,
                                                 convertOptions.PluginAssemblyPath);
 

src/Sarif/Writers/CacheByFileHashLogger.cs

@@ -79,7 +79,7 @@ private void CacheResult(ReportingDescriptor rule, Result result)
 
         public void LogConfigurationNotification(Notification notification)
         {
-            if (!cacheLoggingData) { return; }
+            if (!cacheLoggingData || !ShouldLog(notification)) { return; }
 
             if (!HashToNotificationsMap.TryGetValue(currentFileHash, out List<Notification> notifications))
             {
@@ -90,6 +90,12 @@ public void LogConfigurationNotification(Notification notification)
 
         public void LogToolNotification(Notification notification)
         {
+            //  In case someone adds something here:
+            if (!ShouldLog(notification))
+            {
+                return;
+            }
+            //  Add actual functionality below
         }
     }
 }

src/Sarif/Writers/CachingLogger.cs

@@ -12,8 +12,12 @@ namespace Microsoft.CodeAnalysis.Sarif.Writers
     /// instances. The driver framework uses this mechanism to merge results
     /// produced by a multi-threaded analysis into a single output file.
     /// </summary>
-    public class CachingLogger : IAnalysisLogger
+    public class CachingLogger : BaseLogger, IAnalysisLogger
     {
+        public CachingLogger(IEnumerable<FailureLevel> level, IEnumerable<ResultKind> kind) : base(level, kind)
+        {
+        }
+
         public IDictionary<ReportingDescriptor, IList<Result>> Results { get; set; }
 
         public IList<Notification> ConfigurationNotifications { get; set; }
@@ -65,14 +69,20 @@ public void Log(ReportingDescriptor rule, Result result)
 
         public void LogConfigurationNotification(Notification notification)
         {
-            ConfigurationNotifications ??= new List<Notification>();
-            ConfigurationNotifications.Add(notification);
+            if (ShouldLog(notification))
+            {
+                ConfigurationNotifications ??= new List<Notification>();
+                ConfigurationNotifications.Add(notification);
+            }
         }
 
         public void LogToolNotification(Notification notification)
         {
-            ToolNotifications ??= new List<Notification>();
-            ToolNotifications.Add(notification);
+            if (ShouldLog(notification))
+            {
+                ToolNotifications ??= new List<Notification>();
+                ToolNotifications.Add(notification);
+            }
         }
     }
 }

src/Sarif/Writers/ConsoleLogger.cs

@@ -11,9 +11,9 @@ namespace Microsoft.CodeAnalysis.Sarif.Writers
 {
     public class ConsoleLogger : BaseLogger, IAnalysisLogger
     {
-        public ConsoleLogger(bool quiet, string toolName, IEnumerable<FailureLevel> level = null, IEnumerable<ResultKind> kind = null) : base(level, kind)
+        public ConsoleLogger(bool quietConsole, string toolName, IEnumerable<FailureLevel> level = null, IEnumerable<ResultKind> kind = null) : base(level, kind)
         {
-            _quiet = quiet;
+            _quietConsole = quietConsole;
             _toolName = toolName.ToUpperInvariant();
         }
 
@@ -24,11 +24,11 @@ public ConsoleLogger(bool quiet, string toolName, IEnumerable<FailureLevel> leve
 
         public string CapturedOutput => _capturedOutput?.ToString();
 
-        private readonly bool _quiet;
+        private readonly bool _quietConsole;
 
-        private void WriteLineToConsole(string text = null, bool ignoreQuiet = false)
+        private void WriteLineToConsole(string text = null, bool forceEmitOfErrorNotifications = false)
         {
-            if (!_quiet || ignoreQuiet)
+            if (!_quietConsole || forceEmitOfErrorNotifications)
             {
                 Console.WriteLine(text);
 
@@ -97,7 +97,7 @@ public void Log(ReportingDescriptor rule, Result result)
                 throw new ArgumentNullException(nameof(result));
             }
 
-            if (!ShouldLog(result))
+            if (!ShouldLog(result) || _quietConsole)
             {
                 return;
             }
@@ -202,7 +202,7 @@ private void WriteToConsole(Notification notification)
                     WriteLineToConsole(FormatNotificationMessage(notification, _toolName));
                     break;
                 case FailureLevel.Error:
-                    WriteLineToConsole(FormatNotificationMessage(notification, _toolName), true);
+                    WriteLineToConsole(FormatNotificationMessage(notification, _toolName), forceEmitOfErrorNotifications: true);
                     break;
                 default:
                     throw new InvalidOperationException();

src/Sarif/Writers/LoggingOptions.cs → src/Sarif/Writers/LogFilePersistenceOptions.cs

@@ -6,22 +6,20 @@
 namespace Microsoft.CodeAnalysis.Sarif.Writers
 {
     [Flags]
-    public enum LoggingOptions
+    public enum LogFilePersistenceOptions
+
     {
         None = 0,
 
         // Indent persisted JSON for easy file viewing.
         PrettyPrint = 0x1,
 
-        // Suppress all non-errors to the console and results file
-        Quiet = 0x2,
-
         // Overwrite previous version of log file, if it exists.
         OverwriteExistingOutputFile = 0x4,
 
         // Omit redundant properties, producing a smaller but non-human-readable log.
         Optimize = 0x8,
 
-        All = PrettyPrint | Quiet | OverwriteExistingOutputFile | Optimize
+        All = PrettyPrint | OverwriteExistingOutputFile | Optimize
     }
 }