Fixing AnalyzeCommandBase and MultithreadedAnalyzeCommandBase artifacts generation

src/ReleaseHistory.md

@@ -2,6 +2,7 @@
 
 ## Unreleased
 
+* BUGFIX: Fix `AnalyzeCommandBase` and `MultithreadedAnalyzeCommandBase` from outputting all artifacts to SARIF even if no results were produced when Hashes is enabled. [#2433](https://github.com/microsoft/sarif-sdk/pull/2433)
 * BUGFIX: Adjust Json Serialization field order for ReportingDescriptor and skip emit empty AutomationDetails node. [#2420](https://github.com/microsoft/sarif-sdk/pull/2420)
 * BREAKING: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application, and remove `[Serializable]` from it. Now use of BinaryFormatter on it will result in `SerializationException`: Type `PropertiesDictionary` is not marked as serializable. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415)
 

src/Sarif.Driver/Sdk/AnalyzeCommandBase.cs

@@ -169,7 +169,7 @@ private void Analyze(TOptions options, AggregatingLogger logger)
             targets = ValidateTargetsExist(_rootContext, targets);
 
             // 5. Initialize report file, if configured.
-            InitializeOutputFile(options, _rootContext, targets);
+            InitializeOutputFile(options, _rootContext);
 
             // 6. Instantiate skimmers.
             ISet<Skimmer<TContext>> skimmers = CreateSkimmers(options, _rootContext);
@@ -287,13 +287,14 @@ private ISet<string> ValidateTargetsExist(TContext context, ISet<string> targets
 
                 if ((options.DataToInsert.ToFlags() & OptionallyEmittedData.Hashes) != 0)
                 {
-                    if (_pathToHashDataMap != null && _pathToHashDataMap.TryGetValue(filePath, out HashData hashData))
+                    if (_pathToHashDataMap?.TryGetValue(filePath, out HashData hashData) == true)
                     {
                         context.Hashes = hashData;
                     }
                     else
                     {
                         context.Hashes = HashUtilities.ComputeHashes(filePath);
+                        _pathToHashDataMap?.Add(filePath, context.Hashes);
                     }
                 }
             }
@@ -352,7 +353,7 @@ protected virtual void InitializeConfiguration(TOptions options, TContext contex
             }
         }
 
-        private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISet<string> targets)
+        private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
         {
             string filePath = analyzeOptions.OutputFilePath;
             AggregatingLogger aggregatingLogger = (AggregatingLogger)context.Logger;
@@ -388,7 +389,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
                                     dataToRemove,
                                     tool: _tool,
                                     run: _run,
-                                    analysisTargets: targets,
+                                    analysisTargets: null,
                                     quiet: analyzeOptions.Quiet,
                                     invocationTokensToRedact: GenerateSensitiveTokensList(),
                                     invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
@@ -404,7 +405,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context, ISe
                                     dataToRemove,
                                     tool: _tool,
                                     run: _run,
-                                    analysisTargets: targets,
+                                    analysisTargets: null,
                                     invocationTokensToRedact: GenerateSensitiveTokensList(),
                                     invocationPropertiesToLog: analyzeOptions.InvocationPropertiesToLog,
                                     levels: analyzeOptions.Level,

src/Sarif.Driver/Sdk/MultithreadedAnalyzeCommandBase.cs

@@ -40,6 +40,7 @@ public abstract class MultithreadedAnalyzeCommandBase<TContext, TOptions> : Plug
         private Channel<int> _resultsWritingChannel;
         private Channel<int> _fileEnumerationChannel;
         private Dictionary<string, List<string>> _hashToFilesMap;
+        private IDictionary<string, HashData> _pathToHashDataMap;
         private ConcurrentDictionary<int, TContext> _fileContexts;
 
         public Exception ExecutionException { get; set; }
@@ -485,12 +486,9 @@ private async Task<bool> HashAsync()
                             _hashToFilesMap[hashData.Sha256] = paths;
                         }
 
-                        _run?.GetFileIndex(new ArtifactLocation { Uri = context.TargetUri },
-                                           dataToInsert: _dataToInsert,
-                                           hashData: hashData);
-
                         paths.Add(localPath);
                         context.Hashes = hashData;
+                        _pathToHashDataMap?.Add(localPath, hashData);
                     }
 
                     await _fileEnumerationChannel.Writer.WriteAsync(index);
@@ -705,6 +703,7 @@ private void InitializeOutputFile(TOptions analyzeOptions, TContext context)
                                                                      kinds: analyzeOptions.Kind,
                                                                      insertProperties: analyzeOptions.InsertProperties);
                         }
+                        _pathToHashDataMap = sarifLogger.AnalysisTargetToHashDataMap;
                         sarifLogger.AnalysisStarted();
                         aggregatingLogger.Loggers.Add(sarifLogger);
                     },

src/Sarif/HashUtilities.cs

@@ -31,15 +31,14 @@ internal static IFileSystem FileSystem
 
         public static IDictionary<string, HashData> MultithreadedComputeTargetFileHashes(IEnumerable<string> analysisTargets, bool suppressConsoleOutput = false)
         {
-            if (analysisTargets == null) { return null; }
+            var fileToHashDataMap = new ConcurrentDictionary<string, HashData>();
+            if (analysisTargets == null) { return fileToHashDataMap; }
 
             if (!suppressConsoleOutput)
             {
                 Console.WriteLine("Computing file hashes...");
             }
 
-            var fileToHashDataMap = new ConcurrentDictionary<string, HashData>();
-
             var queue = new ConcurrentQueue<string>(analysisTargets);
 
             var tasks = new List<Task>();

src/Sarif/OptionallyEmittedData.cs

@@ -80,6 +80,9 @@ public enum OptionallyEmittedData : int
         // Enrich SARIF log with git blame information
         GitBlameInformation = 0x1000,
 
+        // Enrich with artifacts only.
+        Artifacts = 0x2000,
+
         // A special enum value that indicates that insertion should overwrite any existing
         // information in the SARIF log file. In the absence of this setting, any existing
         // data that would otherwise have been overwritten by the insert operation will

src/Sarif/Writers/SarifLogger.cs

@@ -119,6 +119,7 @@ public class SarifLogger : BaseLogger, IDisposable, IAnalysisLogger
 
             _persistArtifacts =
                 (_dataToInsert & OptionallyEmittedData.Hashes) != 0 ||
+                (_dataToInsert & OptionallyEmittedData.Artifacts) != 0 ||
                 (_dataToInsert & OptionallyEmittedData.TextFiles) != 0 ||
                 (_dataToInsert & OptionallyEmittedData.BinaryFiles) != 0;
         }
@@ -447,12 +448,16 @@ private void CaptureArtifact(ArtifactLocation fileLocation)
                 catch (ArgumentException) { } // Unrecognized encoding name
             }
 
+            HashData hashData = null;
+            AnalysisTargetToHashDataMap?.TryGetValue(fileLocation.Uri.OriginalString, out hashData);
+
             // Ensure Artifact is in Run.Artifacts and ArtifactLocation.Index is set to point to it
             int index = _run.GetFileIndex(
                 fileLocation,
                 addToFilesTableIfNotPresent: _persistArtifacts,
                 _dataToInsert,
-                encoding);
+                encoding,
+                hashData);
 
             // Remove redundant Uri and UriBaseId once index has been set
             if (index > -1 && this.Optimize)

src/Test.FunctionalTests.Sarif/Multitool/BaselineOptionTests.cs

@@ -112,6 +112,7 @@ protected override string ConstructTestOutputFromInputResource(string inputResou
                 PrettyPrint = true,
                 Optimize = true,
                 Kind = new List<ResultKind> { ResultKind.Fail },
+                DataToInsert = new List<OptionallyEmittedData> { OptionallyEmittedData.Artifacts },
                 Level = new List<FailureLevel> { FailureLevel.Error, FailureLevel.Warning, FailureLevel.Note, FailureLevel.None },
             };
 

src/Test.FunctionalTests.Sarif/Multitool/ValidateCommandTests.cs

@@ -480,6 +480,7 @@ protected override string ConstructTestOutputFromInputResource(string inputResou
                 PrettyPrint = true,
                 Optimize = true,
                 Kind = new List<ResultKind> { ResultKind.Fail },
+                DataToInsert = new List<OptionallyEmittedData> { OptionallyEmittedData.Artifacts },
                 Level = new List<FailureLevel> { FailureLevel.Error, FailureLevel.Warning, FailureLevel.Note, FailureLevel.None },
             };
 

src/Test.FunctionalTests.Sarif/TestData/Multitool/BaselineOption/ExpectedOutputs/TEST1001.ValidateWithBaseline.sarif

@@ -9,42 +9,42 @@
           "rules": [
             {
               "id": "SARIF2003",
-              "name": "ProvideVersionControlProvenance",
               "fullDescription": {
                 "text": "Provide 'versionControlProvenance' to record which version of the code was analyzed, and to enable paths to be expressed relative to the root of the repository."
               },
+              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
               "messageStrings": {
                 "Note_Default": {
                   "text": "{0}: This run does not provide 'versionControlProvenance'. As a result, it is not possible to determine which version of code was analyzed, nor to map relative paths to their locations within the repository."
                 }
               },
+              "name": "ProvideVersionControlProvenance",
               "defaultConfiguration": {
                 "level": "note"
-              },
-              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
+              }
             },
             {
               "id": "SARIF2002",
-              "name": "ProvideMessageArguments",
               "fullDescription": {
                 "text": "In result messages, use the 'message.id' and 'message.arguments' properties rather than 'message.text'. This has several advantages. If 'text' is lengthy, using 'id' and 'arguments' makes the SARIF file smaller. If the rule metadata is stored externally to the SARIF log file, the message text can be improved (for example, by adding more text, clarifying the phrasing, or fixing typos), and the result messages will pick up the improvements the next time it is displayed. Finally, SARIF supports localizing messages into different languages, which is possible if the SARIF file contains 'message.id' and 'message.arguments', but not if it contains 'message.text' directly."
               },
+              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
               "messageStrings": {
                 "Note_Default": {
                   "text": "{0}: The 'message' property of this result contains a 'text' property. Consider replacing it with 'id' and 'arguments' properties. This potentially reduces the log file size, allows the message text to be improved without modifying the log file, and enables localization."
                 }
               },
+              "name": "ProvideMessageArguments",
               "defaultConfiguration": {
                 "level": "note"
-              },
-              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
+              }
             },
             {
               "id": "SARIF2005",
-              "name": "ProvideToolProperties",
               "fullDescription": {
                 "text": "Provide information that makes it easy to identify the name and version of your tool.\r\n\r\nThe tool's 'name' property should be no more than three words long. This makes it easy to remember and allows it to fit into a narrow column when displaying a list of results. If you need to provide more information about your tool, use the 'fullName' property.\r\n\r\nThe tool should provide either or both of the 'version' and 'semanticVersion' properties. This enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions.\r\n\r\nIf 'version' is used, facilitate comparison between versions by specifying a version number that starts with an integer, optionally followed by any desired characters."
               },
+              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
               "messageStrings": {
                 "Warning_ProvideToolVersion": {
                   "text": "{0}: The tool '{1}' does not provide any of the version-related properties {2}. Providing version information enables the log file consumer to determine whether the file was produced by an up to date version, and to avoid accidentally comparing log files produced by different tool versions."
@@ -59,14 +59,14 @@
                   "text": "{0}: The tool '{1}' does not provide 'informationUri'. This property helps the developer responsible for addessing a result by providing a way to learn more about the tool."
                 }
               },
-              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
+              "name": "ProvideToolProperties"
             },
             {
               "id": "SARIF2012",
-              "name": "ProvideRuleProperties",
               "fullDescription": {
                 "text": "Rule metadata should provide information that makes it easy to understand and fix the problem.\r\n\r\nProvide the 'name' property, which contains a \"friendly name\" that helps users see at a glance the purpose of the rule. For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal-case identifier, for example, 'ProvideRuleFriendlyName'.\r\n\r\nProvide the 'helpUri' property, which contains a URI where users can find detailed information about the rule. This information should include a detailed description of the invalid pattern, an explanation of why the pattern is poor practice (particularly in contexts such as security or accessibility where driving considerations might not be readily apparent), guidance for resolving the problem (including describing circumstances in which ignoring the problem altogether might be appropriate), examples of invalid and valid patterns, and special considerations (such as noting when a violation should never be ignored or suppressed, noting when a violation could cause downstream tool noise, and noting when a rule can be configured in some way to refine or alter the analysis)."
               },
+              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html",
               "messageStrings": {
                 "Note_FriendlyNameNotAPascalIdentifier": {
                   "text": "{0}: '{1}' is not a Pascal-case identifier. For uniformity of experience across all tools that produce SARIF, the friendly name should be a single Pascal-case identifier, for example, 'ProvideRuleFriendlyName'."
@@ -84,10 +84,10 @@
                   "text": "'{0}' does not provide metadata for rule '{1}'. Rule metadata contains information that helps the user understand why each rule fires and what the user can do to fix it."
                 }
               },
+              "name": "ProvideRuleProperties",
               "defaultConfiguration": {
                 "level": "note"
-              },
-              "helpUri": "http://docs.oasis-open.org/sarif/sarif/v2.1.0/sarif-v2.1.0.html"
+              }
             }
           ]
         }