Fixing null reference when using VersionControlInformation (#2222)

* Fixing null reference when using VersionControlInformation

* renaming and adding to release history

* renaming versioncontrolinformation

* fixing test

src/ReleaseHistory.md

@@ -1,5 +1,7 @@
 # SARIF Package Release History (SDK, Driver, Converters, and Multitool)
 
+* BREAKING: Rename flag `VersionControlInformation` to `VersionControlDetails` from `OptionallyEmittedData`. [#2222](https://github.com/microsoft/sarif-sdk/pull/2222)
+
 ## **v2.3.14** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.14) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.14) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.14) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.14) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.3.14)
 * BUGFIX: Fix concurrency issue in when using `Cache`. [#2215](https://github.com/microsoft/sarif-sdk/pull/2215)
 * FEATURE: `ConsoleLogger` will print exception if that exists. [#2217](https://github.com/microsoft/sarif-sdk/pull/2217)

src/Sarif.Driver/Sdk/CommonOptionsBase.cs

@@ -43,7 +43,7 @@ public class CommonOptionsBase
             HelpText =
             "Optionally present data, expressed as a semicolon-delimited list, that should be inserted into the log file. " +
             "Valid values include Hashes, TextFiles, BinaryFiles, EnvironmentVariables, RegionSnippets, ContextRegionSnippets, " +
-            "Guids, VersionControlInformation, and NondeterministicProperties.")]
+            "Guids, VersionControlDetails, and NondeterministicProperties.")]
         public IEnumerable<OptionallyEmittedData> DataToInsert { get; set; }
 
         [Option(
@@ -52,7 +52,7 @@ public class CommonOptionsBase
             HelpText =
             "Optionally present data, expressed as a semicolon-delimited list, that should be not be persisted to or which " +
             "should be removed from the log file. Valid values include Hashes, TextFiles, BinaryFiles, EnvironmentVariables, " +
-            "RegionSnippets, ContextRegionSnippets, Guids, VersionControlInformation, and NondeterministicProperties.")]
+            "RegionSnippets, ContextRegionSnippets, Guids, VersionControlDetails, and NondeterministicProperties.")]
         public IEnumerable<OptionallyEmittedData> DataToRemove { get; set; }
 
         [Option(

src/Sarif/OptionallyEmittedData.cs

@@ -71,7 +71,7 @@ public enum OptionallyEmittedData : int
 
         // Specifies the repository, branch, and other information describing the source controlled
         // version of the code that was analyzed.
-        VersionControlInformation = 0x400,
+        VersionControlDetails = 0x400,
 
         // A set of threadFlows which together describe a pattern of code execution relevant
         // to detecting a result.

src/Sarif/SdkResources.resx

@@ -249,7 +249,7 @@
   <data name="PartioningVisitHappensAtSarifLogLevel" xml:space="preserve">
     <value>Could not identify the log being partitioned. Call VisitSarifLog and provide the log to partition. This class is designed to create log files on a per-run basis (i.e., all partioned logs will contain a single run only).</value>
   </data>
-  <data name="CannotProvideVersionControlInformation" xml:space="preserve">
+  <data name="CannotProvideVersionControlDetails" xml:space="preserve">
     <value>Cannot provide version control information because the current directory '{0}' is not under a Git repository root directory.</value>
   </data>
   <data name="GitHelperDefaultInstanceDoesNotPermitCaching" xml:space="preserve">

src/Sarif/Visitors/InsertOptionalDataVisitor.cs

@@ -1,5 +1,6 @@
 // Copyright (c) Microsoft. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
 using System;
 using System.Collections.Generic;
 using System.Globalization;
@@ -40,6 +41,7 @@ public InsertOptionalDataVisitor(
             _dataToInsert = dataToInsert;
             _originalUriBaseIds = originalUriBaseIds;
             _ruleIndex = -1;
+            _gitHelper = new GitHelper();
         }
 
         public override Run VisitRun(Run node)
@@ -50,7 +52,7 @@ public override Run VisitRun(Run node)
 
             if (_originalUriBaseIds != null)
             {
-                _run.OriginalUriBaseIds = _run.OriginalUriBaseIds ?? new Dictionary<string, ArtifactLocation>();
+                _run.OriginalUriBaseIds ??= new Dictionary<string, ArtifactLocation>();
 
                 foreach (string key in _originalUriBaseIds.Keys)
                 {
@@ -73,7 +75,7 @@ public override Run VisitRun(Run node)
             Run visited = base.VisitRun(node);
 
             // After all the ArtifactLocations have been visited,
-            if (_run.VersionControlProvenance == null && _dataToInsert.HasFlag(OptionallyEmittedData.VersionControlInformation))
+            if (_run.VersionControlProvenance == null && _dataToInsert.HasFlag(OptionallyEmittedData.VersionControlDetails))
             {
                 _run.VersionControlProvenance = CreateVersionControlProvenance();
             }
@@ -98,7 +100,7 @@ public override PhysicalLocation VisitPhysicalLocation(PhysicalLocation node)
                 Region expandedRegion;
                 ArtifactLocation artifactLocation = node.ArtifactLocation;
 
-                _fileRegionsCache = _fileRegionsCache ?? new FileRegionsCache();
+                _fileRegionsCache ??= new FileRegionsCache();
 
                 if (artifactLocation.Uri == null && artifactLocation.Index >= 0)
                 {
@@ -188,7 +190,7 @@ public override Artifact VisitArtifact(Artifact node)
 
         public override ArtifactLocation VisitArtifactLocation(ArtifactLocation node)
         {
-            if (_dataToInsert.HasFlag(OptionallyEmittedData.VersionControlInformation))
+            if (_dataToInsert.HasFlag(OptionallyEmittedData.VersionControlDetails))
             {
                 node = ExpressRelativeToRepoRoot(node);
             }
@@ -276,10 +278,11 @@ private ArtifactLocation ExpressRelativeToRepoRoot(ArtifactLocation node)
                 if (repoRootPath != null)
                 {
                     var repoRootUri = new Uri(repoRootPath + @"\", UriKind.Absolute);
+
+                    _repoRootUris ??= new HashSet<Uri>();
                     _repoRootUris.Add(repoRootUri);
 
-                    Uri repoRelativeUri = repoRootUri.MakeRelativeUri(uri);
-                    node.Uri = repoRelativeUri;
+                    node.Uri = repoRootUri.MakeRelativeUri(uri);
                     node.UriBaseId = GetUriBaseIdForRepoRoot(repoRootUri);
                 }
             }

src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj

@@ -31,7 +31,7 @@
     <None Remove="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Absolute_TextFiles.sarif" />
     <None Remove="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Relative_All.sarif" />
     <None Remove="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Relative_Guids.sarif" />
-    <None Remove="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Relative_VersionControlInformation.sarif" />
+    <None Remove="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Relative_VersionControlDetails.sarif" />
     <None Remove="TestData\InsertOptionalDataVisitor\Inputs\CoreTests-Absolute.sarif" />
     <None Remove="TestData\JsonConverters\UriConverterTests.json" />
     <None Remove="TestData\Map\Sample.json" />
@@ -102,7 +102,7 @@
     <EmbeddedResource Include="TestData\GitHubIngestionVisitor\Inputs\ThreadFlowLocations.sarif" />
     <EmbeddedResource Include="TestData\GitHubIngestionVisitor\Inputs\WithArtifacts.sarif" />
     <EmbeddedResource Include="TestData\GitHubIngestionVisitor\Inputs\TooManyResults.sarif" />
-    <EmbeddedResource Include="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Relative_VersionControlInformation.sarif" />
+    <EmbeddedResource Include="TestData\InsertOptionalDataVisitor\ExpectedOutputs\CoreTests-Relative_VersionControlDetails.sarif" />
     <EmbeddedResource Include="TestData\Readers\elfie-arriba-utf8-bom.sarif" />
     <EmbeddedResource Include="TestData\Baseline\elfie-arriba.sarif" />
     <EmbeddedResource Include="TestData\Baseline\SuppressionTestCurrent.sarif" />

src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs

@@ -108,7 +108,7 @@ protected override string ConstructTestOutputFromInputResource(string inputResou
                 }
             }
 
-            if (_currentOptionallyEmittedData.HasFlag(OptionallyEmittedData.VersionControlInformation))
+            if (_currentOptionallyEmittedData.HasFlag(OptionallyEmittedData.VersionControlDetails))
             {
                 VersionControlDetails versionControlDetails = actualLog.Runs[0].VersionControlProvenance[0];
 
@@ -206,7 +206,7 @@ public void InsertOptionalDataVisitor_PersistsGuids()
         [Fact]
         public void InsertOptionalDataVisitor_PersistsVersionControlInformation()
         {
-            RunTest("CoreTests-Relative.sarif", OptionallyEmittedData.VersionControlInformation);
+            RunTest("CoreTests-Relative.sarif", OptionallyEmittedData.VersionControlDetails);
         }
 
         [Fact]
@@ -325,7 +325,7 @@ public void InsertOptionalDataVisitor_SkipsExistingRepoRootSymbolsAndHandlesMult
             };
 
             var visitor = new InsertOptionalDataVisitor(
-                OptionallyEmittedData.VersionControlInformation,
+                OptionallyEmittedData.VersionControlDetails,
                 originalUriBaseIds: null,
                 fileSystem: mockFileSystem.Object,
                 processRunner: mockProcessRunner.Object);