Fix NullReferenceException in WorkItemFiler when all results baseline state are unchange/absent/updated

src/ReleaseHistory.md

@@ -9,6 +9,7 @@
 * BUGFIX: Fix `ArgumentException` when `--recurse` is enabled and two file target specifiers generates the same file path. [#2438](https://github.com/microsoft/sarif-sdk/pull/2438)
 * BUGFIX: Fix 'InvalidOperationException' with message `Collection was modified; enumeration operation may not execute` in `MultithreadedAnalyzeCommandBase`, which is raised when analyzing with the `--hashes` switch. [#2447](https://github.com/microsoft/sarif-sdk/pull/2447)
 * BUGFIX: Fix `Merge` command produces empty SARIF file in Linux when providing file name only without path. [#2408](https://github.com/microsoft/sarif-sdk/pull/2408)
+* BUGFIX: Fix `NullReferenceException` when filing work item with a SARIF file which has no filable results. [#2412](https://github.com/microsoft/sarif-sdk/pull/2412)
 
 ## **v2.4.12** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.4.12) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.4.12) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.4.12) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.4.12) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.4.12)
 

src/Sarif.WorkItems/SarifWorkItemFiler.cs

@@ -26,6 +26,8 @@ public class SarifWorkItemFiler : IDisposable
         private readonly object m_syncRoot = new object();
         private FilingClient m_filingClient = null;
 
+        internal static readonly string s_multipleLocationsTextPattern = "{0} (+{1} locations)";
+
         /// <summary>
         /// Initializes a new instance of the <see cref="SarifWorkItemFiler"> class.</see>
         /// </summary>
@@ -199,17 +201,17 @@ public virtual IReadOnlyList<SarifLog> SplitLogFile(SarifLog sarifLog)
 
                     Logger.LogInformation($"Splitting strategy - {splittingStrategy}");
 
-                    if (splittingStrategy == SplittingStrategy.None)
-                    {
-                        return new[] { sarifLog };
-                    }
-
                     PartitionFunction<string> partitionFunction = null;
 
                     Stopwatch splittingStopwatch = Stopwatch.StartNew();
 
                     switch (splittingStrategy)
                     {
+                        case SplittingStrategy.None:
+                        {
+                            partitionFunction = (result) => result.ShouldBeFiled() ? "Include" : null;
+                            break;
+                        }
                         case SplittingStrategy.PerRun:
                         {
                             partitionFunction = (result) => result.ShouldBeFiled() ? "Include" : null;
@@ -332,6 +334,13 @@ public SarifWorkItemModel FileWorkItemInternal(SarifLog sarifLog, SarifWorkItemC
                         }
                     }
 
+                    if (string.IsNullOrWhiteSpace(sarifWorkItemModel.Title) || string.IsNullOrWhiteSpace(sarifWorkItemModel.BodyOrDescription))
+                    {
+                        this.Logger.LogWarning("Attempt to call work item client with invalid work item values.", logId);
+                        this.LogMetricsForProcessedModel(sarifLog, sarifWorkItemModel, FilingResult.Canceled);
+                        return null;
+                    }
+
                     Task<IEnumerable<WorkItemModel>> task = filingClient.FileWorkItems(new[] { sarifWorkItemModel });
                     task.Wait();
                     this.FiledWorkItems.AddRange(task.Result);
@@ -387,23 +396,23 @@ private void LogMetricsForProcessedModel(SarifLog sarifLog, SarifWorkItemModel s
                 : "";
 
             var workItemMetrics = new Dictionary<string, object>
-                {
-                    { "LogId", logId },
-                    { "WorkItemModelId", sarifWorkItemModel.Id },
-                    { nameof(sarifWorkItemModel.Area), sarifWorkItemModel.Area },
-                    { nameof(sarifWorkItemModel.BodyOrDescription), sarifWorkItemModel.BodyOrDescription },
-                    { "FilingResult", filingResult.ToString() },
-                    { nameof(sarifWorkItemModel.CommentOrDiscussion), sarifWorkItemModel.CommentOrDiscussion },
-                    { nameof(sarifWorkItemModel.HtmlUri), sarifWorkItemModel.HtmlUri },
-                    { nameof(sarifWorkItemModel.Iteration), sarifWorkItemModel.Iteration },
-                    { "LabelsOrTags", tags },
-                    { "LocationUri", uris },
-                    { nameof(sarifWorkItemModel.Milestone), sarifWorkItemModel.Milestone },
-                    { nameof(sarifWorkItemModel.OwnerOrAccount), sarifWorkItemModel.OwnerOrAccount },
-                    { nameof(sarifWorkItemModel.RepositoryOrProject), sarifWorkItemModel.RepositoryOrProject },
-                    { nameof(sarifWorkItemModel.Title), sarifWorkItemModel.Title },
-                    { nameof(sarifWorkItemModel.Uri), sarifWorkItemModel.Uri },
-                };
+            {
+                { "LogId", logId },
+                { "WorkItemModelId", sarifWorkItemModel.Id },
+                { nameof(sarifWorkItemModel.Area), sarifWorkItemModel.Area },
+                { nameof(sarifWorkItemModel.BodyOrDescription), sarifWorkItemModel.BodyOrDescription },
+                { "FilingResult", filingResult.ToString() },
+                { nameof(sarifWorkItemModel.CommentOrDiscussion), sarifWorkItemModel.CommentOrDiscussion },
+                { nameof(sarifWorkItemModel.HtmlUri), sarifWorkItemModel.HtmlUri },
+                { nameof(sarifWorkItemModel.Iteration), sarifWorkItemModel.Iteration },
+                { "LabelsOrTags", tags },
+                { "LocationUri", uris },
+                { nameof(sarifWorkItemModel.Milestone), sarifWorkItemModel.Milestone },
+                { nameof(sarifWorkItemModel.OwnerOrAccount), sarifWorkItemModel.OwnerOrAccount },
+                { nameof(sarifWorkItemModel.RepositoryOrProject), sarifWorkItemModel.RepositoryOrProject },
+                { nameof(sarifWorkItemModel.Title), sarifWorkItemModel.Title },
+                { nameof(sarifWorkItemModel.Uri), sarifWorkItemModel.Uri },
+            };
 
             foreach (string key in additionalCustomDimensions.Keys)
             {

src/Sarif.WorkItems/SarifWorkItemModel.cs

@@ -41,13 +41,14 @@ public class SarifWorkItemModel : WorkItemModel<SarifWorkItemContext>
                 }
             }
 
-            bool? resultContainsWorkItemUri = sarifLog.Runs?.Any(run => run.Results.Any(result => result.WorkItemUris?.Count > 0));
-            if (resultContainsWorkItemUri == true)
+            foreach (Run run in sarifLog.Runs)
             {
-                Uri workItemUri = sarifLog.Runs?.Where(run => run.Results != null && run.Results.Any(result => result.WorkItemUris?.Count > 0)).LastOrDefault()
-                    .Results.Where(result => result.WorkItemUris != null && result.WorkItemUris.Count > 0).LastOrDefault()
-                    .WorkItemUris.LastOrDefault();
-                this.Uri = workItemUri;
+                Result resultContainsWorkItemUri = run.Results.LastOrDefault(result => result.WorkItemUris?.Count > 0);
+                if (resultContainsWorkItemUri != null)
+                {
+                    this.Uri = resultContainsWorkItemUri.WorkItemUris.LastOrDefault();
+                    break;
+                }
             }
 
             // Shared GitHub/Azure DevOps concepts

src/Sarif.WorkItems/SarifWorkItemsExtensions.cs

@@ -6,6 +6,8 @@
 using System.IO;
 using System.Linq;
 
+using Microsoft.VisualStudio.Services.Common;
+
 namespace Microsoft.CodeAnalysis.Sarif.WorkItems
 {
     public static class SarifWorkItemsExtensions
@@ -143,33 +145,44 @@ public static int GetAggregateFilableResultsCount(this SarifLog log)
 
         public static string CreateWorkItemDescription(this SarifLog log, SarifWorkItemContext context)
         {
-            int totalResults = log.GetAggregateFilableResultsCount();
+            int totalResults = 0;
+            int artifactLocationCount = 0;
+            Uri firstDetectedLocation = null;
+
+            IEnumerable<Result> results = log?.Runs?.SelectMany(run => run.Results.Where(r => r.ShouldBeFiled()));
+
+            foreach (Result result in results.AsEmptyIfNull())
+            {
+                totalResults++;
+                foreach (Location location in result.Locations.AsEmptyIfNull())
+                {
+                    firstDetectedLocation ??= location.PhysicalLocation?.ArtifactLocation?.Uri;
+                    artifactLocationCount += location.PhysicalLocation?.ArtifactLocation?.Uri != null ? 1 : 0;
+                }
+            }
+
+            if (totalResults == 0)
+            {
+                return null;
+            }
+
             List<string> toolNames = log.GetToolNames();
             string phrasedToolNames = toolNames.ToAndPhrase();
             string multipleToolsFooter = toolNames.Count > 1 ? WorkItemsResources.MultipleToolsFooter : string.Empty;
 
-            IEnumerable<Result> results = log?.Runs?[0]?.Results.Where(r => r.ShouldBeFiled());
             Uri runRepositoryUri = log?.Runs.FirstOrDefault()?.VersionControlProvenance?.FirstOrDefault().RepositoryUri;
-            Uri detectionLocationUri = !string.IsNullOrEmpty(runRepositoryUri?.OriginalString) ?
-                                       runRepositoryUri :
-                                       results?.FirstOrDefault().Locations?[0].PhysicalLocation?.ArtifactLocation?.Uri;
+            Uri detectionLocationUri = !string.IsNullOrEmpty(runRepositoryUri?.OriginalString)
+                ? runRepositoryUri
+                : firstDetectedLocation;
 
             string detectionLocation = (detectionLocationUri?.IsAbsoluteUri == true && detectionLocationUri?.Scheme == "https")
                 ? context.CreateLinkText(detectionLocationUri.OriginalString, detectionLocationUri?.OriginalString)
                 : detectionLocationUri?.OriginalString;
 
-            int locCount = results == null ? 0 :
-                           results
-                           .Where(r => r.Locations != null)
-                           .SelectMany(r => r.Locations)
-                           .Where(l => l.PhysicalLocation != null && l.PhysicalLocation.ArtifactLocation != null && l.PhysicalLocation.ArtifactLocation.Uri != null)
-                           .Count();
-
-            if (locCount > 1)
-            {
-                int additionalLocations = locCount - 1;
-                detectionLocation = $"{detectionLocation} (+{additionalLocations} locations)";
-            }
+            // "{0} (+{1} locations)"
+            detectionLocation = artifactLocationCount > 1
+                ? string.Format(SarifWorkItemFiler.s_multipleLocationsTextPattern, detectionLocation, artifactLocationCount - 1)
+                : detectionLocation;
 
             // This work item contains {0} {1} issue(s) detected in {2}{3}. Click the 'Scans' tab to review results.
             string description =