Skip to content

Commit

Permalink
#2228 Create --kind and --level arguments for more granular control (#…
Browse files Browse the repository at this point in the history
…2241)

* Creating BaseLogger to share methods

* removing logmessage from interface

* updating more loggers

* Fix parameters

* Replace verbose in ConsoleLogger.  It should know about quiet instead

* Remove verbose

* Remove unused method

* Remove unnecessary nullcheck

* fixing functional tests

* back to quiet

* fixing tests

* Use ShouldLog in more locations

* Add to release history for breaking change

* Fix formatting

* Make more loggers children of BaseLogger.  Cleanup some functionality with --quiet.  Correct some tests

* fixing spacing

* Rename some parameters, classes, and files.  Remove "quiet" from LoggingOptions, and correct unit tests affected

* More complete renaming

* Rewording, minor cleanup

* fixing sample build

* Correct validate logic.  Remove redundant "none"s

* Rename parameters, reword helper text

* Add unit tests for DriverExtensionMethod and BaseLogger

* Fix casting error

* fixing samples compilation issue

* Small changes to address comments

* Formatting fixes

* Address renaming and refactoring comments

* More renaming

* Change validation in BaseLogger and correct unit tests and code  (#2262)

* Change validation in BaseLogger and correct unit tests and code broken as a result

* Correct formatting

* Test SarifLogger honors Kind/Level

* Refactor, remove redundant code

Co-authored-by: Eddy Nakamura <ednakamu@microsoft.com>
Co-authored-by: Eddy Nakamura <eddynaka@gmail.com>
  • Loading branch information
3 people committed Jan 25, 2021
1 parent df39327 commit 6c931ba
Show file tree
Hide file tree
Showing 33 changed files with 572 additions and 444 deletions.
3 changes: 3 additions & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# SARIF Package Release History (SDK, Driver, Converters, and Multitool)

* BREAKING: Entirely remove "verbose" whose fuctionality has been replaced by --level and --kind. [#2241](https://github.com/microsoft/sarif-sdk/pull/2241)
* BREAKING: Rename "LoggingOptions" to "LogFilePersistenceOptions". [#2241](https://github.com/microsoft/sarif-sdk/pull/2241)
* FEATURE: --quiet will now suppress all console messages except for errors. [#2241](https://github.com/microsoft/sarif-sdk/pull/2241)
* BUGFIX: Fix NullReference in SARIF1012 rule validation [#2254](https://github.com/microsoft/sarif-sdk/pull/2254)

## **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)
Expand Down
2 changes: 1 addition & 1 deletion src/Samples/Sarif.Sdk.Sample/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ internal static int CreateSarifLogFile(CreateOptions options)
{
using (var sarifLogger = new SarifLogger(
textWriter,
loggingOptions: LoggingOptions.PrettyPrint, // Use PrettyPrint to generate readable (multi-line, indented) JSON
logFilePersistenceOptions: LogFilePersistenceOptions.PrettyPrint, // Use PrettyPrint to generate readable (multi-line, indented) JSON
dataToInsert:
OptionallyEmittedData.TextFiles | // Embed source file content directly in the log file -- great for portability of the log!
OptionallyEmittedData.Hashes |
Expand Down
2 changes: 1 addition & 1 deletion src/Samples/SarifTrim/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ private static void Main(string[] args)
consolidator.RemoveWebResponses = (removeParts.Contains("WebResponses"));

// Consolidate the SarifLog per settings
using (SarifLogger logger = new SarifLogger(outputFilePath, LoggingOptions.OverwriteExistingOutputFile, tool: run.Tool, run: run))
using (SarifLogger logger = new SarifLogger(outputFilePath, LogFilePersistenceOptions.OverwriteExistingOutputFile, tool: run.Tool, run: run))
{
foreach (Result result in run.Results)
{
Expand Down
10 changes: 2 additions & 8 deletions src/Sarif.Converters/GlobalSuppressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
4 changes: 2 additions & 2 deletions src/Sarif.Converters/ToolFormatConverter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
}
Expand Down
25 changes: 17 additions & 8 deletions src/Sarif.Driver/DriverExtensionMethods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.Verbose) { loggingOptions |= LoggingOptions.Verbose; }
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>
Expand Down Expand Up @@ -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;
}
}
}
8 changes: 0 additions & 8 deletions src/Sarif.Driver/Sdk/AggregatingLogger.cs
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,6 @@ public void AnalyzingTarget(IAnalysisContext context)
}
}

public void LogMessage(bool verbose, string message)
{
foreach (IAnalysisLogger logger in Loggers)
{
logger.LogMessage(verbose, message);
}
}

public void Log(ReportingDescriptor rule, Result result)
{
foreach (IAnalysisLogger logger in Loggers)
Expand Down
21 changes: 14 additions & 7 deletions src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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. #2260 https://github.com/microsoft/sarif-sdk/issues/2260
ThrowExitApplicationException(context, ExitReason.InvalidCommandLineOption);
}
}
Expand Down Expand Up @@ -267,13 +270,13 @@ internal AggregatingLogger InitializeLogger(AnalyzeOptionsBase analyzeOptions)

if (!analyzeOptions.Quiet)
{
_consoleLogger = new ConsoleLogger(analyzeOptions.Verbose, _tool.Driver.Name) { CaptureOutput = _captureConsoleOutput };
_consoleLogger = new ConsoleLogger(analyzeOptions.Quiet, _tool.Driver.Name, analyzeOptions.Level, analyzeOptions.Kind) { CaptureOutput = _captureConsoleOutput };
logger.Loggers.Add(_consoleLogger);
}

if ((analyzeOptions.DataToInsert.ToFlags() & OptionallyEmittedData.Hashes) != 0)
{
_cacheByFileHashLogger = new CacheByFileHashLogger(analyzeOptions.Verbose);
_cacheByFileHashLogger = new CacheByFileHashLogger(analyzeOptions.Level, analyzeOptions.Kind);
logger.Loggers.Add(_cacheByFileHashLogger);
}

Expand Down Expand Up @@ -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();
Expand All @@ -419,27 +422,31 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
{
sarifLogger = new SarifLogger(
analyzeOptions.OutputFilePath,
loggingOptions,
logFilePersistenceOptions,
dataToInsert,
dataToRemove,
tool: _tool,
run: null,
analysisTargets: targets,
invocationTokensToRedact: GenerateSensitiveTokensList(),
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog);
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
levels: analyzeOptions.Level,
kinds: analyzeOptions.Kind);
}
else
{
sarifLogger = new SarifOneZeroZeroLogger(
analyzeOptions.OutputFilePath,
loggingOptions,
logFilePersistenceOptions,
dataToInsert,
dataToRemove,
tool: _tool,
run: null,
analysisTargets: targets,
invocationTokensToRedact: GenerateSensitiveTokensList(),
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog);
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
levels: analyzeOptions.Level,
kinds: analyzeOptions.Kind);
}
_pathToHashDataMap = sarifLogger.AnalysisTargetToHashDataMap;
sarifLogger.AnalysisStarted();
Expand Down
21 changes: 15 additions & 6 deletions src/Sarif.Driver/Sdk/AnalyzeOptionsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ public abstract class AnalyzeOptionsBase : CommonOptionsBase
HelpText = "File path to which analysis output will be written.")]
public string OutputFilePath { get; set; }

[Option(
"verbose",
HelpText = "Emit verbose output. The resulting comprehensive report is designed to provide appropriate evidence for compliance scenarios.")]
public bool Verbose { get; set; }

[Option(
'r',
"recurse",
Expand All @@ -41,7 +36,7 @@ public abstract class AnalyzeOptionsBase : CommonOptionsBase
[Option(
'q',
"quiet",
HelpText = "Do not log results to the console.")]
HelpText = "Suppress all console output (except for catastrophic tool runtime or configuration errors).")]
public bool Quiet { get; set; }

[Option(
Expand Down Expand Up @@ -95,5 +90,19 @@ public abstract class AnalyzeOptionsBase : CommonOptionsBase
"should be emitted to the console and log file (if appropriate). " +
"Valid values: ScanTime.")]
public virtual IEnumerable<string> Traces { get; set; }

[Option(
"level",
Separator = ';',
Default = new FailureLevel[] { FailureLevel.Error, FailureLevel.Warning },
HelpText = "Filter output of scan results to one or more failure levels. Valid values: Error, Warning and Note.")]
public IEnumerable<FailureLevel> Level { get; set; }

[Option(
"kind",
Separator = ';',
Default = new ResultKind[] { ResultKind.Fail },
HelpText = "Filter output one or more result kinds. Valid values: Fail (for literal scan results), Pass, Review, Open, NotApplicable and Informational.")]
public IEnumerable<ResultKind> Kind { get; set; }
}
}
18 changes: 11 additions & 7 deletions src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -551,7 +551,7 @@ internal AggregatingLogger InitializeLogger(AnalyzeOptionsBase analyzeOptions)

if (!analyzeOptions.Quiet)
{
_consoleLogger = new ConsoleLogger(analyzeOptions.Verbose, _tool.Driver.Name) { CaptureOutput = _captureConsoleOutput };
_consoleLogger = new ConsoleLogger(analyzeOptions.Quiet, _tool.Driver.Name, analyzeOptions.Level, analyzeOptions.Kind) { CaptureOutput = _captureConsoleOutput };
logger.Loggers.Add(_consoleLogger);
}

Expand Down Expand Up @@ -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();
Expand All @@ -655,27 +655,31 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
{
sarifLogger = new SarifLogger(
analyzeOptions.OutputFilePath,
loggingOptions,
logFilePersistenceOptions,
dataToInsert,
dataToRemove,
tool: _tool,
run: _run,
analysisTargets: null,
invocationTokensToRedact: GenerateSensitiveTokensList(),
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog);
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
levels: analyzeOptions.Level,
kinds: analyzeOptions.Kind);
}
else
{
sarifLogger = new SarifOneZeroZeroLogger(
analyzeOptions.OutputFilePath,
loggingOptions,
logFilePersistenceOptions,
dataToInsert,
dataToRemove,
tool: _tool,
run: _run,
analysisTargets: null,
invocationTokensToRedact: GenerateSensitiveTokensList(),
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog);
invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
levels: analyzeOptions.Level,
kinds: analyzeOptions.Kind);
}
_pathToHashDataMap = sarifLogger.AnalysisTargetToHashDataMap;
sarifLogger.AnalysisStarted();
Expand Down
Loading

0 comments on commit 6c931ba

Please sign in to comment.