From 019fd0700d293106ac9a9fd32636980f345fee2b Mon Sep 17 00:00:00 2001 From: Eddy Nakamura Date: Wed, 6 Jan 2021 17:13:51 -0300 Subject: [PATCH] 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 | 2 ++ src/Sarif.Driver/Sdk/CommonOptionsBase.cs | 4 ++-- src/Sarif/OptionallyEmittedData.cs | 2 +- src/Sarif/SdkResources.Designer.cs | 4 ++-- src/Sarif/SdkResources.resx | 2 +- src/Sarif/Visitors/InsertOptionalDataVisitor.cs | 15 +++++++++------ .../Test.UnitTests.Sarif.csproj | 4 ++-- ...oreTests-Relative_VersionControlDetails.sarif} | 0 .../Visitors/InsertOptionalDataVisitorTests.cs | 6 +++--- 9 files changed, 22 insertions(+), 17 deletions(-) rename src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/{CoreTests-Relative_VersionControlInformation.sarif => CoreTests-Relative_VersionControlDetails.sarif} (100%) diff --git a/src/ReleaseHistory.md b/src/ReleaseHistory.md index daf8d3da9..5a144db70 100644 --- a/src/ReleaseHistory.md +++ b/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) diff --git a/src/Sarif.Driver/Sdk/CommonOptionsBase.cs b/src/Sarif.Driver/Sdk/CommonOptionsBase.cs index 5467af6f7..68206efd8 100644 --- a/src/Sarif.Driver/Sdk/CommonOptionsBase.cs +++ b/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 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 DataToRemove { get; set; } [Option( diff --git a/src/Sarif/OptionallyEmittedData.cs b/src/Sarif/OptionallyEmittedData.cs index 653ae3fca..899f7057b 100644 --- a/src/Sarif/OptionallyEmittedData.cs +++ b/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. diff --git a/src/Sarif/SdkResources.Designer.cs b/src/Sarif/SdkResources.Designer.cs index b617ad8f7..91cdc1e65 100644 --- a/src/Sarif/SdkResources.Designer.cs +++ b/src/Sarif/SdkResources.Designer.cs @@ -81,9 +81,9 @@ public static string CallGenericGetProperty { /// /// Looks up a localized string similar to Cannot provide version control information because the current directory '{0}' is not under a Git repository root directory.. /// - public static string CannotProvideVersionControlInformation { + public static string CannotProvideVersionControlDetails { get { - return ResourceManager.GetString("CannotProvideVersionControlInformation", resourceCulture); + return ResourceManager.GetString("CannotProvideVersionControlDetails", resourceCulture); } } diff --git a/src/Sarif/SdkResources.resx b/src/Sarif/SdkResources.resx index 483e154d0..f5d6add42 100644 --- a/src/Sarif/SdkResources.resx +++ b/src/Sarif/SdkResources.resx @@ -249,7 +249,7 @@ 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). - + Cannot provide version control information because the current directory '{0}' is not under a Git repository root directory. diff --git a/src/Sarif/Visitors/InsertOptionalDataVisitor.cs b/src/Sarif/Visitors/InsertOptionalDataVisitor.cs index 778be81f3..4b0507976 100644 --- a/src/Sarif/Visitors/InsertOptionalDataVisitor.cs +++ b/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(); + _run.OriginalUriBaseIds ??= new Dictionary(); 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(); _repoRootUris.Add(repoRootUri); - Uri repoRelativeUri = repoRootUri.MakeRelativeUri(uri); - node.Uri = repoRelativeUri; + node.Uri = repoRootUri.MakeRelativeUri(uri); node.UriBaseId = GetUriBaseIdForRepoRoot(repoRootUri); } } diff --git a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj index e0e8d329e..2de0ff466 100644 --- a/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj +++ b/src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj @@ -31,7 +31,7 @@ - + @@ -102,7 +102,7 @@ - + diff --git a/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlInformation.sarif b/src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlDetails.sarif similarity index 100% rename from src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlInformation.sarif rename to src/Test.UnitTests.Sarif/TestData/InsertOptionalDataVisitor/ExpectedOutputs/CoreTests-Relative_VersionControlDetails.sarif diff --git a/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs b/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs index 8b7c3ec4a..b99561869 100644 --- a/src/Test.UnitTests.Sarif/Visitors/InsertOptionalDataVisitorTests.cs +++ b/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);