diff --git a/src/Sarif.Multitool/Sarif.Multitool.csproj b/src/Sarif.Multitool/Sarif.Multitool.csproj
index 7d01b8655..5b49ddb82 100644
--- a/src/Sarif.Multitool/Sarif.Multitool.csproj
+++ b/src/Sarif.Multitool/Sarif.Multitool.csproj
@@ -13,10 +13,6 @@
-
-
-
-
diff --git a/src/Sarif.UnitTests/Baseline/ResultMatching/SarifLogResultMatcherTests.cs b/src/Sarif.UnitTests/Baseline/ResultMatching/SarifLogResultMatcherTests.cs
index 9eb6964e4..c09436da9 100644
--- a/src/Sarif.UnitTests/Baseline/ResultMatching/SarifLogResultMatcherTests.cs
+++ b/src/Sarif.UnitTests/Baseline/ResultMatching/SarifLogResultMatcherTests.cs
@@ -309,72 +309,54 @@ public void SarifLogResultMatcher_MultipleLogsDuplicateData_WorksAsExpected()
new Run()
{
Tool = new Tool { Name = "TestTool" },
- Files = new Dictionary()
+ Files = new List
{
- { "testfile", new FileData() { Contents = new FileContent() { Text = "TestFileContents" } } }
+ new FileData() { Contents = new FileContent() { Text = "TestFileContents" } }
},
- Results = new Result[0]
- }
- }
- };
- SarifLog current2 = new SarifLog()
- {
- Runs = new Run[]
- {
- new Run()
- {
- Tool = new Tool { Name = "TestTool" },
- Files = new Dictionary()
+ Results = new List
{
- { "testfile", new FileData() { Contents = new FileContent() { Text = "TestFileContents" } } }
- },
- Results = new Result[0],
+ new Result
+ {
+ Message = new Message { Text = "Some testing occurred."},
+ Locations = new List
+ {
+ new Location
+ {
+ PhysicalLocation = new PhysicalLocation
+ {
+ FileLocation = new FileLocation
+ {
+ FileIndex = 0
+ }
+ }
+ }
+ }
+ }
+ }
}
}
};
+
+ SarifLog current2 = current1.DeepClone();
SarifLog result = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[0], new SarifLog[] { current1, current2 }).First();
+ // In first match operation, the file data objects are identical, they should be collapsed into a single array entry
result.Runs[0].Files.Should().HaveCount(1);
-
- }
- [Fact]
- public void SarifLogResultMatcher_MultipleLogsInvalidData_ThrowsInvalidOperationException()
- {
- SarifLog current1 = new SarifLog()
- {
- Runs = new Run[]
- {
- new Run()
- {
- Tool = new Tool { Name = "TestTool" },
- Files = new Dictionary()
- {
- { "testfile", new FileData() { Contents = new FileContent() { Text = "TestFileContents" } } }
- },
- Results = new Result[0]
- }
- }
- };
- SarifLog current2 = new SarifLog()
- {
- Runs = new Run[]
- {
- new Run()
- {
- Tool = new Tool { Name = "TestTool" },
- Files = new Dictionary()
- {
- { "testfile", new FileData() { Contents = new FileContent() { Text = "DifferentTestFileContents" } } }
- },
- Results = new Result[0],
- }
- }
- };
- Assert.Throws(() => s_preserveOldestPropertyBagMatcher.Match(new SarifLog[0], new SarifLog[] { current1, current2 }));
+ current2 = current1.DeepClone();
- }
+ // Now we differentiate a files object
+ current2.Runs[0].Files[0].Contents.Text = Guid.NewGuid().ToString();
+ result = s_preserveOldestPropertyBagMatcher.Match(new SarifLog[0], new SarifLog[] { current1, current2 }).First();
+
+ // Both file data objects should be present
+ result.Runs[0].Files.Should().HaveCount(2);
+ // Merged results should each refer to differentiated file data object
+ result.Runs[0].Results[0].Locations[0].PhysicalLocation.FileLocation.FileIndex.Should().Be(0);
+ result.Runs[0].Results[1].Locations[0].PhysicalLocation.FileLocation.FileIndex.Should().Be(1);
+ }
+
[Fact]
public void SarifLogResultMatcher_PreservesPropertiesProperly ()
{
@@ -388,15 +370,14 @@ public void SarifLogResultMatcher_PreservesPropertiesProperly ()
SetPropertyOnAllFileAndResultObjects(baselineLog, "Key", baselinePropertyValue);
SetPropertyOnAllFileAndResultObjects(currentLog, "Key", currentPropertyValue);
-
SarifLog matchedLog = s_preserveOldestPropertyBagMatcher.Match(baselineLog.DeepClone(), currentLog.DeepClone());
matchedLog.Runs[0].Results.Where((r) => { return r.GetProperty("Key") == baselinePropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Results.Count);
- matchedLog.Runs[0].Files.Values.Where((r) => { return r.GetProperty("Key") == baselinePropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Files.Count);
+ matchedLog.Runs[0].Files.Where((r) => { return r.GetProperty("Key") == baselinePropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Files.Count);
// Retain property bag values from most current run
matchedLog = s_preserveMostRecentPropertyBagMatcher.Match(baselineLog.DeepClone(), currentLog.DeepClone());
matchedLog.Runs[0].Results.Where((r) => { return r.GetProperty("Key") == currentPropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Results.Count);
- matchedLog.Runs[0].Files.Values.Where((r) => { return r.GetProperty("Key") == currentPropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Files.Count);
+ matchedLog.Runs[0].Files.Where((r) => { return r.GetProperty("Key") == currentPropertyValue; }).Count().Should().Be(matchedLog.Runs[0].Files.Count);
}
private void SetPropertyOnAllFileAndResultObjects(SarifLog sarifLog, string propertyKey, string propertyValue)
@@ -410,7 +391,7 @@ private void SetPropertyOnAllFileAndResultObjects(SarifLog sarifLog, string prop
if (run.Files != null)
{
- foreach (FileData file in run.Files.Values)
+ foreach (FileData file in run.Files)
{
file.SetProperty(propertyKey, propertyValue);
}
diff --git a/src/Sarif.UnitTests/OrderSensitiveValueComparisonListTests.cs b/src/Sarif.UnitTests/OrderSensitiveValueComparisonListTests.cs
new file mode 100644
index 000000000..9b2cbe3d8
--- /dev/null
+++ b/src/Sarif.UnitTests/OrderSensitiveValueComparisonListTests.cs
@@ -0,0 +1,113 @@
+// 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 FluentAssertions;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.Sarif.UnitTests
+{
+ ///
+ /// A comparer, analagous to the internal .NET ObjectEqualityComparer, that
+ /// provides comparison semantics roughly equivalent to the default .NET
+ /// behavior provided by Object.Equals and friends (this functionality is
+ /// what's invoked when calling List.Contains). Our comparer only exists
+ /// for testing, because ObjectEqualityComparer is an internal type. We
+ /// don't cover all possible cases to make the types equivalent; the
+ /// implementation covers enough for core validation.
+ ///
+ ///
+ internal class DefaultObjectComparer : IEqualityComparer
+ {
+ public bool Equals(T x, T y)
+ {
+ return Object.Equals(x, y);
+ }
+
+ public int GetHashCode(T obj)
+ {
+ if (obj == null) { return 0; }
+ return obj.GetHashCode();
+ }
+ }
+
+ public class OrderSensitiveValueComparisonListTests
+ {
+ [Fact]
+ public void OrderSensitiveValueComparisonListTests_DefaultObjectComparer()
+ {
+ var equalityComparer = new DefaultObjectComparer();
+
+ var listOne = CreateTestList(equalityComparer);
+
+ // Populate the second list with references from the first.
+ var listTwo = new OrderSensitiveValueComparisonList(equalityComparer);
+ for (int i = 0; i < listOne.Count; i++)
+ {
+ listTwo.Add(listOne[i]);
+ }
+
+ // Every list s/be equal to itself.
+ listOne.Equals(listOne).Should().Be(true);
+
+ // Two lists with shared objects, by reference, in the same
+ // order should be regarded as equivalent.
+ listOne.Equals(listTwo).Should().Be(true);
+
+ FileChange toSwap = listTwo[0];
+ listTwo[0] = listTwo[1];
+ listTwo[1] = toSwap;
+
+ // We have reordered two objects that are themselves identical.
+ // The comparison should fail as the order of references changed.
+ listOne.Equals(listTwo).Should().Be(false);
+ }
+
+ [Fact]
+ public void OrderSensitiveValueComparisonListTests_ValueComparer()
+ {
+ // Two identical lists with elements that are
+ // distinct objects, by reference.
+ var listOne = CreateTestList(FileChange.ValueComparer);
+ var listTwo = CreateTestList(FileChange.ValueComparer);
+
+ // Every list s/be equal to itself
+ listOne.Equals(listOne).Should().Be(true);
+
+ // As initialized, these objects are different, due
+ // to a unique GUID property on each list
+ listOne.Equals(listTwo).Should().Be(false);
+
+ // Make the two lists equivalent, by value
+ listTwo[2].SetProperty(DIFFERENTIATING_PROPERTY_NAME, listOne[2].GetProperty(DIFFERENTIATING_PROPERTY_NAME));
+ listOne.Equals(listTwo).Should().Be(true);
+
+ FileChange toSwap = listTwo[0];
+ listTwo[0] = listTwo[1];
+ listTwo[1] = toSwap;
+
+ // We have reordered two objects that are themselves identical.
+ // by value. The comparison should still succeed.
+ listOne.Equals(listTwo).Should().Be(true);
+ }
+
+ private const string DIFFERENTIATING_PROPERTY_NAME = nameof(DIFFERENTIATING_PROPERTY_NAME);
+
+ private OrderSensitiveValueComparisonList CreateTestList(IEqualityComparer equalityComparer)
+ {
+ // Test list. First two elements are identical. The third element is unique.
+ var fileChangeOne = new FileChange();
+ var fileChangeTwo = new FileChange();
+
+ var fileChangeThree = new FileChange();
+ Guid differentiatingProperty = Guid.NewGuid();
+ fileChangeThree.SetProperty(DIFFERENTIATING_PROPERTY_NAME, differentiatingProperty);
+
+ var list = new OrderSensitiveValueComparisonList(equalityComparer);
+ list.AddRange(new[] { fileChangeOne, fileChangeTwo, fileChangeThree });
+
+ return list;
+ }
+ }
+}
\ No newline at end of file
diff --git a/src/Sarif.UnitTests/RandomSarifLogGenerator.cs b/src/Sarif.UnitTests/RandomSarifLogGenerator.cs
index 163ce7db6..f52a02854 100644
--- a/src/Sarif.UnitTests/RandomSarifLogGenerator.cs
+++ b/src/Sarif.UnitTests/RandomSarifLogGenerator.cs
@@ -92,6 +92,7 @@ public static IEnumerable GenerateFakeFiles(string baseAddress, int coun
public static IList GenerateFakeResults(Random random, List ruleIds, List filePaths, int resultCount)
{
List results = new List();
+ int fileIndex = random.Next(filePaths.Count);
for (int i = 0; i < resultCount; i++)
{
results.Add(new Result()
@@ -105,7 +106,8 @@ public static IList GenerateFakeResults(Random random, List rule
{
FileLocation = new FileLocation
{
- Uri = filePaths[random.Next(filePaths.Count)]
+ Uri = filePaths[fileIndex],
+ FileIndex = fileIndex
},
}
}
diff --git a/src/Sarif.UnitTests/Sarif.UnitTests.csproj b/src/Sarif.UnitTests/Sarif.UnitTests.csproj
index e609b0a43..d0ef23e63 100644
--- a/src/Sarif.UnitTests/Sarif.UnitTests.csproj
+++ b/src/Sarif.UnitTests/Sarif.UnitTests.csproj
@@ -14,12 +14,6 @@
-
-
-
-
-
-
diff --git a/src/Sarif.UnitTests/Visitors/RemapIndicesVisitorTests.cs b/src/Sarif.UnitTests/Visitors/RemapIndicesVisitorTests.cs
new file mode 100644
index 000000000..64c8c3370
--- /dev/null
+++ b/src/Sarif.UnitTests/Visitors/RemapIndicesVisitorTests.cs
@@ -0,0 +1,155 @@
+// Copyright (c) Microsoft. All rights reserved.
+// Licensed under the MIT license. See LICENSE file in the project root for full license information.
+
+using System.Collections.Generic;
+using FluentAssertions;
+using Xunit;
+
+namespace Microsoft.CodeAnalysis.Sarif.Visitors
+{
+ public class RemapIndicesVisitorTests
+ {
+ private Run CreateRunWithNestedFilesForTesting()
+ {
+ var run = new Run()
+ {
+ Tool = new Tool { Name = "Test Tool" },
+ Files = new List
+ {
+ new FileData{ FileLocation = new FileLocation{ FileIndex = 0 }, Contents = new FileContent { Text = "1" } },
+ new FileData{ FileLocation = new FileLocation{ FileIndex = 1 }, Contents = new FileContent { Text = "1.2" }, ParentIndex = 0 },
+ new FileData{ FileLocation = new FileLocation{ FileIndex = 2 }, Contents = new FileContent { Text = "1.2.3." }, ParentIndex = 1 }
+ },
+ Results = new List
+ {
+ new Result
+ {
+ Locations = new List
+ {
+ new Location
+ {
+ PhysicalLocation = new PhysicalLocation
+ {
+ FileLocation = new FileLocation
+ {
+ FileIndex = 2
+ }
+ }
+ }
+ }
+ }
+ }
+ };
+
+ return run;
+ }
+
+ [Fact]
+ public void RemapIndicesVisitor_RemapsNestedFilesProperly()
+ {
+ // This run has a single result that points to a doubly nested file
+ Run baselineRun = CreateRunWithNestedFilesForTesting();
+
+ // This run has a single result pointing to a single file location
+ var currentRun = new Run()
+ {
+ Tool = new Tool { Name = "Test Tool" },
+ Files = new List
+ {
+ new FileData{ FileLocation = new FileLocation{ FileIndex = 0 }, Contents = new FileContent { Text = "New" } },
+ },
+ Results = new List
+ {
+ new Result
+ {
+ Locations = new List
+ {
+ new Location
+ {
+ PhysicalLocation = new PhysicalLocation
+ {
+ FileLocation = new FileLocation
+ {
+ FileIndex = 0
+ }
+ }
+ }
+ }
+ }
+ }
+ };
+
+ Run mergedRun = currentRun.DeepClone();
+
+ // How does this remapping work? First, we initialize the indices remapper
+ // with a set of existing files (analagous to the files list of the most
+ // recent log in a baselining situation). Next, visit a set of results
+ // from the baseline itself, providing the historical set of file data
+ // objects from the baseline. The visit does two things: 1) updates the
+ // visitor files table with the superset of file data objects between
+ // the baseline and current run, 2) updates individual results from the
+ // baseline so that their file index values are correct.
+
+ // 1. At this point the merged run consists of a copy of the current run.
+ // After visitor construction, we should see that the visitor's
+ // CurrentFiles property is equivalent to mergedRun.Files. The visitor
+ // has also been initialized with a dictionary that uses the complete
+ // file hierarchy as a key into the files array
+ var visitor = new RemapIndicesVisitor(mergedRun.Files);
+ visitor.CurrentFiles.Should().BeEquivalentTo(mergedRun.Files);
+ visitor.RemappedFiles.Count.Should().Be(mergedRun.Files.Count);
+
+
+ // 2. We set HistoricalFiles to point to the old files array from the
+ // baseline run, then visit each baseline result. After each result
+ // visit, any file data objects (including parent files) for the
+ // result have been added to visitor.CurrentFiles, if missing. Each
+ // result fileIndex data is updated. We move the updated result
+ // objects to the merged run.
+
+ // Various operations mutate state. We will make a copy so that we can
+ // compare our ulitmate results to an unaltered original baseline
+ var baselineRunCopy = baselineRun.DeepClone();
+
+ visitor.HistoricalFiles = baselineRunCopy.Files;
+ foreach (Result result in baselineRunCopy.Results)
+ {
+ visitor.VisitResult(result);
+ mergedRun.Results.Add(result);
+ }
+
+ // 3. After completing the results array visit, we'll grab the
+ // files array that represents all merged files from the runs.
+ mergedRun.Files = visitor.CurrentFiles;
+
+ // We expect the merged files to be a superset of file data objects from the two runs
+ mergedRun.Files.Count.Should().Be(baselineRun.Files.Count + currentRun.Files.Count);
+
+ // We expect that every merged file data has a corrected file index
+ for (int i = 0; i < mergedRun.Files.Count; i++)
+ {
+ mergedRun.Files[i].FileLocation.FileIndex.Should().Be(i);
+ }
+
+ // We should see that the file index of the result we added should be pushed out by 1,
+ // to account for the second run's file data objects being appended to the single
+ // file data object in the first run.
+ mergedRun.Files[mergedRun.Results[0].Locations[0].PhysicalLocation.FileLocation.FileIndex].Contents.Text.Should()
+ .Be(currentRun.Files[currentRun.Results[0].Locations[0].PhysicalLocation.FileLocation.FileIndex].Contents.Text);
+
+ // Similarly, we expect that all file data objects from the first run have been offset by one
+ for (int i = 0; i < 3; i++)
+ {
+ baselineRun.Files[i].Contents.Text.Should().Be(mergedRun.Files[i + 1].Contents.Text);
+ }
+
+ // Finally, we should ensure that the parent index chain was updated properly on merging
+ FileData fileData = mergedRun.Files[mergedRun.Results[1].Locations[0].PhysicalLocation.FileLocation.FileIndex];
+
+ // Most nested
+ fileData.ParentIndex.Should().Be(2);
+ mergedRun.Files[fileData.ParentIndex].ParentIndex.Should().Be(1);
+ mergedRun.Files[mergedRun.Files[fileData.ParentIndex].ParentIndex].ParentIndex.Should().Be(-1);
+ }
+ }
+}
diff --git a/src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs b/src/Sarif.UnitTests/Visitors/UpdateIndicesFromLegacyDataVisitorTests.cs
similarity index 78%
rename from src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs
rename to src/Sarif.UnitTests/Visitors/UpdateIndicesFromLegacyDataVisitorTests.cs
index 1f183c6b3..f0642d456 100644
--- a/src/Sarif.UnitTests/Visitors/UpdateIndicesVisitorTests.cs
+++ b/src/Sarif.UnitTests/Visitors/UpdateIndicesFromLegacyDataVisitorTests.cs
@@ -8,7 +8,7 @@
namespace Microsoft.CodeAnalysis.Sarif.Visitors
{
- public class UpdateIndicesVisitorTests
+ public class UpdateIndicesFromLegacyDataVisitorTests
{
private readonly string _remappedUriBaseId = Guid.NewGuid().ToString();
private readonly string _remappedFullyQualifiedName = Guid.NewGuid().ToString();
@@ -16,7 +16,7 @@ public class UpdateIndicesVisitorTests
private readonly string _remappedFullyQualifiedLogicalName = Guid.NewGuid().ToString();
private readonly Result _result;
- public UpdateIndicesVisitorTests()
+ public UpdateIndicesFromLegacyDataVisitorTests()
{
_result = new Result
{
@@ -37,11 +37,11 @@ public UpdateIndicesVisitorTests()
[Fact]
- public void UpdateIndicesVisitor_FunctionsWithNullMaps()
+ public void UpdateIndicesFromLegacyDataVisitor_FunctionsWithNullMaps()
{
var result = _result.DeepClone();
- var visitor = new UpdateIndicesVisitor(null, null, null);
+ var visitor = new UpdateIndicesFromLegacyDataVisitor(null, null, null);
visitor.VisitResult(result);
result.Locations[0].LogicalLocationIndex.Should().Be(Int32.MaxValue);
@@ -49,7 +49,7 @@ public void UpdateIndicesVisitor_FunctionsWithNullMaps()
}
[Fact]
- public void UpdateIndicesVisitor_RemapsFullyQualifiedogicalLNames()
+ public void UpdateIndicesFromLegacyDataVisitor_RemapsFullyQualifiedogicalLNames()
{
var result = _result.DeepClone();
int remappedIndex = 42;
@@ -59,7 +59,7 @@ public void UpdateIndicesVisitor_RemapsFullyQualifiedogicalLNames()
[_remappedFullyQualifiedLogicalName] = remappedIndex
};
- var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, fileLocationKeyToIndexMap: null, ruleKeyToIndexMap: null);
+ var visitor = new UpdateIndicesFromLegacyDataVisitor(fullyQualifiedLogicalNameToIndexMap, fileLocationKeyToIndexMap: null, ruleKeyToIndexMap: null);
visitor.VisitResult(result);
result.Locations[0].LogicalLocationIndex.Should().Be(remappedIndex);
@@ -67,7 +67,7 @@ public void UpdateIndicesVisitor_RemapsFullyQualifiedogicalLNames()
}
[Fact]
- public void UpdateIndicesVisitor_RemapsFileLocations()
+ public void UpdateIndicesFromLegacyDataVisitor_RemapsFileLocations()
{
var result = _result.DeepClone();
int remappedIndex = 42 * 42;
@@ -79,7 +79,7 @@ public void UpdateIndicesVisitor_RemapsFileLocations()
["#" + fileLocation.UriBaseId + "#" + fileLocation.Uri.OriginalString] = remappedIndex
};
- var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap, ruleKeyToIndexMap: null);
+ var visitor = new UpdateIndicesFromLegacyDataVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap, ruleKeyToIndexMap: null);
visitor.VisitResult(result);
result.Locations[0].LogicalLocationIndex.Should().Be(Int32.MaxValue);
@@ -87,7 +87,7 @@ public void UpdateIndicesVisitor_RemapsFileLocations()
}
[Fact]
- public void UpdateIndicesVisitor_RemapsRuleIds()
+ public void UpdateIndicesFromLegacyDataVisitor_RemapsRuleIds()
{
int remappedIndex = 0;
string actualRuleId = "ActualId";
@@ -116,7 +116,7 @@ public void UpdateIndicesVisitor_RemapsRuleIds()
}
};
- var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: null, ruleKeyToIndexMap: ruleKeyToIndexMap);
+ var visitor = new UpdateIndicesFromLegacyDataVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: null, ruleKeyToIndexMap: ruleKeyToIndexMap);
visitor.VisitRun(run);
result.RuleId.Should().Be(actualRuleId);
@@ -124,7 +124,7 @@ public void UpdateIndicesVisitor_RemapsRuleIds()
}
[Fact]
- public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedLogicalLocation()
+ public void UpdateIndicesFromLegacyDataVisitor_DoesNotMutateUnrecognizedLogicalLocation()
{
var result = ConstructNewResult();
Result originalResult = result.DeepClone();
@@ -136,14 +136,14 @@ public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedLogicalLocation()
[_remappedFullyQualifiedLogicalName] = remappedIndex
};
- var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap, null, null);
+ var visitor = new UpdateIndicesFromLegacyDataVisitor(fullyQualifiedLogicalNameToIndexMap, null, null);
visitor.VisitResult(result);
result.ValueEquals(originalResult).Should().BeTrue();
}
[Fact]
- public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedFileLocation()
+ public void UpdateIndicesFromLegacyDataVisitor_DoesNotMutateUnrecognizedFileLocation()
{
var result = ConstructNewResult();
Result originalResult = result.DeepClone();
@@ -155,7 +155,7 @@ public void UpdateIndicesVisitor_DoesNotMutateUnrecognizedFileLocation()
["#" + _remappedUriBaseId + "#" + _remappedUri] = remappedIndex
};
- var visitor = new UpdateIndicesVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap, null);
+ var visitor = new UpdateIndicesFromLegacyDataVisitor(fullyQualifiedLogicalNameToIndexMap: null, fileLocationKeyToIndexMap: fileLocationKeyToIndexMap, null);
visitor.VisitResult(result);
result.ValueEquals(originalResult).Should().BeTrue();
diff --git a/src/Sarif/Baseline/ResultMatching/DataStructures/MatchedResults.cs b/src/Sarif/Baseline/ResultMatching/DataStructures/MatchedResults.cs
index a99e18996..bd2d20a6a 100644
--- a/src/Sarif/Baseline/ResultMatching/DataStructures/MatchedResults.cs
+++ b/src/Sarif/Baseline/ResultMatching/DataStructures/MatchedResults.cs
@@ -21,7 +21,7 @@ public class MatchedResults
public IResultMatcher MatchingAlgorithm { get; set; }
- public IList LogicalLocations { get; set; }
+ public Run Run{ get; set; }
///
/// Creates a new SARIF Result object with contents from the
@@ -52,7 +52,7 @@ public Result CalculateBasedlinedResult(DictionaryMergeBehavior propertyBagMerge
}
else
{
- throw new InvalidOperationException("Cannot generate a Result for a new baseline where both results are null.");
+ throw new InvalidOperationException("Cannot generate a result for a new baseline where both results are null.");
}
ResultMatchingProperties = MergeDictionaryPreferFirst(ResultMatchingProperties, OriginalResultMatchingProperties);
@@ -85,7 +85,7 @@ private Result ConstructAbsentResult(
ResultMatchingProperties.Add(MatchedResults.MatchResultMetadata_RunKeyName, PreviousResult.OriginalRun.Id.InstanceGuid);
}
- LogicalLocations = PreviousResult.OriginalRun.LogicalLocations;
+ Run = PreviousResult.OriginalRun;
return result;
}
@@ -115,7 +115,7 @@ private Result ConstructNewResult(
ResultMatchingProperties.Add(MatchedResults.MatchResultMetadata_FoundDateName, CurrentResult.OriginalRun.Invocations[0].StartTimeUtc);
}
- LogicalLocations = CurrentResult.OriginalRun.LogicalLocations;
+ Run = CurrentResult.OriginalRun;
return result;
}
@@ -140,7 +140,7 @@ private Result ConstructExistingResult(
ResultMatchingProperties.Add(MatchedResults.MatchResultMetadata_RunKeyName, CurrentResult.OriginalRun.Id.InstanceGuid);
}
- LogicalLocations = CurrentResult.OriginalRun.LogicalLocations;
+ Run = CurrentResult.OriginalRun;
return result;
}
diff --git a/src/Sarif/Baseline/ResultMatching/DictionaryMergeBehaviors.cs b/src/Sarif/Baseline/ResultMatching/DictionaryMergeBehaviors.cs
index 6d0ee44a9..f00750adb 100644
--- a/src/Sarif/Baseline/ResultMatching/DictionaryMergeBehaviors.cs
+++ b/src/Sarif/Baseline/ResultMatching/DictionaryMergeBehaviors.cs
@@ -5,12 +5,12 @@
namespace Microsoft.CodeAnalysis.Sarif.Baseline.ResultMatching
{
+ [Flags]
public enum DictionaryMergeBehavior
{
- None = 0, // By default, we will always exclusively preserve the earliest
- // properties that have been generated. We will not attempt any merging.
- InitializeFromOldest = None,
- InitializeFromMostRecent = 0x1,// On setting this bit, we will discard earlier properties
+ None = 0,
+ InitializeFromOldest = 0x1, // On setting this bit, we retain oldest property bags for matched items
+ InitializeFromMostRecent = 0x2,// On setting this bit, we will discard earlier properties
// in favor of those that are most recently generated.
}
}
diff --git a/src/Sarif/Baseline/ResultMatching/SarifLogMatcher.cs b/src/Sarif/Baseline/ResultMatching/SarifLogMatcher.cs
index 51b8cd217..3ebf20c3e 100644
--- a/src/Sarif/Baseline/ResultMatching/SarifLogMatcher.cs
+++ b/src/Sarif/Baseline/ResultMatching/SarifLogMatcher.cs
@@ -211,7 +211,8 @@ private SarifLog ConstructSarifLogFromMatchedResults(
Run run = new Run()
{
Tool = tool,
- Id = currentRuns.First().Id
+ Id = currentRuns.First().Id,
+ Resources = currentRuns.First().Resources ?? new Resources()
};
IDictionary properties = null;
@@ -223,7 +224,8 @@ private SarifLog ConstructSarifLogFromMatchedResults(
run.BaselineInstanceGuid = previousRuns.First().Id?.InstanceGuid;
}
- if (PropertyBagMergeBehavior.HasFlag(DictionaryMergeBehavior.InitializeFromOldest))
+ bool initializeFromOldest = PropertyBagMergeBehavior.HasFlag(DictionaryMergeBehavior.InitializeFromOldest);
+ if (initializeFromOldest)
{
// Find the 'oldest' log file and initialize properties from that log property bag
properties = previousRuns.FirstOrDefault() != null
@@ -237,7 +239,10 @@ private SarifLog ConstructSarifLogFromMatchedResults(
properties = currentRuns.Last().Properties;
}
- var remappedLogicalLocations = new Dictionary();
+ Dictionary rulesMetadata = new Dictionary(Rule.ValueComparer);
+ run.Resources.Rules = run.Resources.Rules ?? new List();
+
+ var indexRemappingVisitor = new RemapIndicesVisitor(currentFiles: null);
properties = properties ?? new Dictionary();
@@ -245,46 +250,45 @@ private SarifLog ConstructSarifLogFromMatchedResults(
foreach (MatchedResults resultPair in results)
{
Result result = resultPair.CalculateBasedlinedResult(PropertyBagMergeBehavior);
- newRunResults.Add(result);
- var logicalLocationIndexRemapper = new LogicalLocationIndexRemapper(resultPair.LogicalLocations, remappedLogicalLocations);
- logicalLocationIndexRemapper.VisitResult(result);
+ IList files =
+ (PropertyBagMergeBehavior.HasFlag(DictionaryMergeBehavior.InitializeFromOldest) && result.BaselineState == BaselineState.Existing)
+ ? resultPair.PreviousResult.OriginalRun.Files
+ : resultPair.Run.Files;
+
+ indexRemappingVisitor.HistoricalFiles = files;
+ indexRemappingVisitor.HistoricalLogicalLocations = resultPair.Run.LogicalLocations;
+ indexRemappingVisitor.VisitResult(result);
+
+ if (result.RuleIndex != -1)
+ {
+ Rule rule = resultPair.Run.Resources.Rules[0];
+ if (!rulesMetadata.TryGetValue(rule, out int ruleIndex))
+ {
+ rulesMetadata[rule] = run.Resources.Rules.Count;
+ run.Resources.Rules.Add(rule);
+ }
+ result.RuleIndex = ruleIndex;
+ }
+
+ newRunResults.Add(result);
}
run.Results = newRunResults;
+ run.Files = indexRemappingVisitor.CurrentFiles;
- // Merge run File data, resources, etc...
var graphs = new Dictionary();
var ruleData = new Dictionary();
- var fileData = new Dictionary();
+ var invocations = new List();
var messageData = new Dictionary();
- var invocations = new List();
-
- // Generally, we need to maintain all data from previous runs that may be associated with
- // results. Later, we can consider eliding information that relates to absent
- // messages (along with the baseline messages themselves that no longer recur).
- foreach (Run previousRun in previousRuns)
- {
- MergeDictionaryInto(fileData, previousRun.Files, FileDataEqualityComparer.Instance);
- }
foreach (Run currentRun in currentRuns)
{
- if (currentRun.Files != null)
- {
- MergeDictionaryInto(fileData, currentRun.Files, FileDataEqualityComparer.Instance);
- }
-
if (currentRun.Resources != null)
{
- if (currentRun.Resources.Rules != null)
- {
- MergeDictionaryInto(ruleData, currentRun.Resources.Rules, RuleEqualityComparer.Instance);
- }
if (currentRun.Resources.MessageStrings != null)
{
- // Autogenerated code does not currently mark this properly as a string, string dictionary.
- IDictionary converted = currentRun.Resources.MessageStrings as Dictionary;
+ IDictionary converted = currentRun.Resources.MessageStrings;
if (converted == null)
{
throw new ArgumentException("Message Strings did not deserialize properly into a dictionary mapping strings to strings.");
@@ -309,10 +313,9 @@ private SarifLog ConstructSarifLogFromMatchedResults(
}
}
- run.Files = fileData;
run.Graphs = graphs;
- run.LogicalLocations = new List(remappedLogicalLocations.Keys);
- run.Resources = new Resources() { MessageStrings = messageData, Rules = ruleData };
+ run.LogicalLocations = new List(indexRemappingVisitor.RemappedLogicalLocationIndices.Keys);
+ //run.Resources = new Resources() { MessageStrings = messageData, Rules = ruleData }; TODO
run.Invocations = invocations;
if (properties != null && properties.Count > 0)
diff --git a/src/Sarif/OrderSensitiveValueComparisonList.cs b/src/Sarif/OrderSensitiveValueComparisonList.cs
new file mode 100644
index 000000000..2d9d04c3d
--- /dev/null
+++ b/src/Sarif/OrderSensitiveValueComparisonList.cs
@@ -0,0 +1,77 @@
+// 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.Text;
+
+namespace Microsoft.CodeAnalysis.Sarif
+{
+ public class OrderSensitiveValueComparisonList : List, IEqualityComparer>
+ {
+ private IEqualityComparer _equalityComparer;
+
+ public OrderSensitiveValueComparisonList(IEqualityComparer equalityComparer)
+ {
+ _equalityComparer = equalityComparer;
+ }
+
+ public override bool Equals(object obj)
+ {
+ return Equals(this, (List)obj);
+ }
+
+ public override int GetHashCode()
+ {
+ return GetHashCode(this);
+ }
+
+ public bool Equals(List left, List right)
+ {
+ if (ReferenceEquals(left, right))
+ {
+ return true;
+ }
+
+ if (ReferenceEquals(left, null) || ReferenceEquals(right, null))
+ {
+ return false;
+ }
+
+ if (left.Count != right.Count)
+ {
+ return false;
+ }
+
+ for (int i = 0; i < left.Count; i++)
+ {
+ if (!_equalityComparer.Equals(left[i], right[i]))
+ {
+ return false;
+ }
+ }
+
+ return true;
+ }
+
+ public int GetHashCode(List obj)
+ {
+ if (ReferenceEquals(obj, null))
+ {
+ return 0;
+ }
+
+ int result = 17;
+ unchecked
+ {
+ result = (result * 31) + obj.Count.GetHashCode();
+
+ for (int i = 0; i < obj.Count; i++)
+ {
+ result = (result * 31) + _equalityComparer.GetHashCode(obj[i]);
+ }
+ }
+ return result;
+ }
+ }
+}
diff --git a/src/Sarif/Sarif.csproj b/src/Sarif/Sarif.csproj
index 496043616..1bfd7ec1e 100644
--- a/src/Sarif/Sarif.csproj
+++ b/src/Sarif/Sarif.csproj
@@ -24,10 +24,7 @@
-
-
-
diff --git a/src/Sarif/Visitors/LogicalLocationIndexRemapper.cs b/src/Sarif/Visitors/LogicalLocationIndexRemapper.cs
deleted file mode 100644
index 483093355..000000000
--- a/src/Sarif/Visitors/LogicalLocationIndexRemapper.cs
+++ /dev/null
@@ -1,45 +0,0 @@
-// Copyright (c) Microsoft. All rights reserved.
-// Licensed under the MIT license. See LICENSE file in the project root for full license information.
-
-using System.Collections.Generic;
-
-namespace Microsoft.CodeAnalysis.Sarif.Visitors
-{
- public class LogicalLocationIndexRemapper : SarifRewritingVisitor
- {
- private IList _previousLogicalLocations;
-
- public LogicalLocationIndexRemapper(IList previousLogicalLocations, IDictionary remappedLogicalLocations)
- {
- _previousLogicalLocations = previousLogicalLocations;
- RemappedLogicalLocations = remappedLogicalLocations;
- }
-
- public IDictionary RemappedLogicalLocations { get; private set; }
-
- public override Result VisitResult(Result node)
- {
- if (_previousLogicalLocations == null) { return node; }
-
- return base.VisitResult(node);
- }
-
- public override Location VisitLocation(Location node)
- {
- if (node.LogicalLocationIndex != -1)
- {
- LogicalLocation logicalLocation = _previousLogicalLocations[node.LogicalLocationIndex];
-
- if (!RemappedLogicalLocations.TryGetValue(logicalLocation, out int remappedIndex))
- {
- remappedIndex = RemappedLogicalLocations.Count;
- RemappedLogicalLocations[logicalLocation] = remappedIndex;
- }
-
- node.LogicalLocationIndex = remappedIndex;
- }
-
- return node;
- }
- }
-}
diff --git a/src/Sarif/Visitors/RemapIndicesVisitor.cs b/src/Sarif/Visitors/RemapIndicesVisitor.cs
new file mode 100644
index 000000000..5e30f1b56
--- /dev/null
+++ b/src/Sarif/Visitors/RemapIndicesVisitor.cs
@@ -0,0 +1,156 @@
+// 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.Diagnostics;
+
+namespace Microsoft.CodeAnalysis.Sarif.Visitors
+{
+ ///
+ /// This class is used to update indices for data that is merged for various
+ /// reasons, including result matching operations.
+ ///
+ public class RemapIndicesVisitor : SarifRewritingVisitor
+ {
+ public RemapIndicesVisitor(IList currentFiles)
+ {
+ BuildRemappedFiles(currentFiles);
+ RemappedLogicalLocationIndices = new Dictionary(LogicalLocation.ValueComparer);
+ }
+
+ private void BuildRemappedFiles(IList currentFiles)
+ {
+ RemappedFiles = new Dictionary, int>();
+
+ if (currentFiles != null)
+ {
+ foreach (FileData fileData in currentFiles)
+ {
+ CacheFileData(fileData);
+ }
+ }
+ }
+
+ public IList CurrentFiles { get; set; }
+
+ public IList HistoricalFiles { get; set; }
+
+ public IList HistoricalLogicalLocations { get; set; }
+
+ public IDictionary RemappedLogicalLocationIndices { get; private set; }
+
+ public IDictionary, int> RemappedFiles { get; private set; }
+
+ public override Result VisitResult(Result node)
+ {
+ // We suspend the visit if there's insufficient data to perform any work
+ if (HistoricalFiles == null && HistoricalLogicalLocations == null)
+ {
+ return node;
+ }
+
+ return base.VisitResult(node);
+ }
+
+ public override Location VisitLocation(Location node)
+ {
+ if (node.LogicalLocationIndex != -1 && HistoricalLogicalLocations != null)
+ {
+ LogicalLocation logicalLocation = HistoricalLogicalLocations[node.LogicalLocationIndex];
+
+ if (!RemappedLogicalLocationIndices.TryGetValue(logicalLocation, out int remappedIndex))
+ {
+ remappedIndex = RemappedLogicalLocationIndices.Count;
+ RemappedLogicalLocationIndices[logicalLocation] = remappedIndex;
+ }
+
+ node.LogicalLocationIndex = remappedIndex;
+ }
+ return base.VisitLocation(node);
+ }
+
+ public override FileData VisitFileData(FileData node)
+ {
+ return base.VisitFileData(node);
+ }
+
+ public override FileLocation VisitFileLocation(FileLocation node)
+ {
+ if (node.FileIndex != -1 && HistoricalFiles != null)
+ {
+ node.FileIndex = CacheFileData(HistoricalFiles[node.FileIndex]);
+ }
+ return node;
+ }
+
+ private int CacheFileData(FileData fileData)
+ {
+ this.CurrentFiles = this.CurrentFiles ?? new List();
+
+ int parentIndex = fileData.ParentIndex;
+
+ // Ensure all parent nodes are already remapped
+ if (parentIndex != -1)
+ {
+ // Important: the input results we are rewriting need to refer
+ // to the historical files index in order to understand parenting
+ fileData.ParentIndex = CacheFileData(HistoricalFiles[parentIndex]);
+ }
+
+ OrderSensitiveValueComparisonList fileChain;
+
+ // Equally important, the file chain is a specially constructed key that
+ // operates against the newly constructed files array in CurrentFiles
+ fileChain = ConstructFilesChain(CurrentFiles, fileData);
+
+ if (!RemappedFiles.TryGetValue(fileChain, out int remappedIndex))
+ {
+ remappedIndex = RemappedFiles.Count;
+
+ this.CurrentFiles.Add(fileData);
+ RemappedFiles[fileChain] = remappedIndex;
+
+ Debug.Assert(RemappedFiles.Count == this.CurrentFiles.Count);
+
+ if (fileData.FileLocation != null)
+ {
+ fileData.FileLocation.FileIndex = remappedIndex;
+ }
+ }
+ return remappedIndex;
+ }
+
+ private static OrderSensitiveValueComparisonList ConstructFilesChain(IList existingFiles, FileData currentFile)
+ {
+ var fileChain = new OrderSensitiveValueComparisonList(FileData.ValueComparer);
+
+ int parentIndex;
+
+ do
+ {
+ currentFile = currentFile.DeepClone();
+ parentIndex = currentFile.ParentIndex;
+
+ // Index information is entirely irrelevant for the identity of nested files,
+ // as each element of the chain could be stored at arbitrary locations in
+ // the run.files table. And so we elide this information.
+ currentFile.ParentIndex = -1;
+ if (currentFile.FileLocation != null)
+ {
+ currentFile.FileLocation.FileIndex = -1;
+ }
+
+ fileChain.Add(currentFile);
+
+ if (parentIndex != -1)
+ {
+ currentFile = existingFiles[parentIndex];
+ }
+
+ } while (parentIndex != -1);
+
+ return fileChain;
+ }
+ }
+}
diff --git a/src/Sarif/Visitors/UpdateIndicesVisitor.cs b/src/Sarif/Visitors/UpdateIndicesFromLegacyDataVisitor.cs
similarity index 90%
rename from src/Sarif/Visitors/UpdateIndicesVisitor.cs
rename to src/Sarif/Visitors/UpdateIndicesFromLegacyDataVisitor.cs
index 47fd3f5c4..30ed9984d 100644
--- a/src/Sarif/Visitors/UpdateIndicesVisitor.cs
+++ b/src/Sarif/Visitors/UpdateIndicesFromLegacyDataVisitor.cs
@@ -6,14 +6,18 @@
namespace Microsoft.CodeAnalysis.Sarif.Visitors
{
- public class UpdateIndicesVisitor : SarifRewritingVisitor
+ ///
+ /// Utility class that helps provide current SARIF v2 rule, file and logical location
+ /// index data, when transform SARIF v1 or SARIF v2 prerelease data.
+ ///
+ public class UpdateIndicesFromLegacyDataVisitor : SarifRewritingVisitor
{
private readonly IDictionary _fullyQualifiedLogicalNameToIndexMap;
private readonly IDictionary _fileLocationKeyToIndexMap;
private readonly IDictionary _ruleKeyToIndexMap;
private Resources _resources;
- public UpdateIndicesVisitor(
+ public UpdateIndicesFromLegacyDataVisitor(
IDictionary fullyQualifiedLogicalNameToIndexMap,
IDictionary fileLocationKeyToIndexMap,
IDictionary ruleKeyToIndexMap)
diff --git a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs
index bb405976d..1de243f47 100644
--- a/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs
+++ b/src/Sarif/Writers/PrereleaseCompatibilityTransformer.cs
@@ -83,7 +83,7 @@ public static SarifLog UpdateToCurrentVersion(
{
transformedSarifLog = JsonConvert.DeserializeObject(sarifLog.ToString());
- var indexUpdatingVisitor = new UpdateIndicesVisitor(
+ var indexUpdatingVisitor = new UpdateIndicesFromLegacyDataVisitor(
fullyQualifiedLogicalNameToIndexMap,
fileLocationKeyToIndexMap,
ruleKeyToIndexMap);
diff --git a/src/Sarif/Writers/SarifLogger.cs b/src/Sarif/Writers/SarifLogger.cs
index 81167a446..89a3fe8fb 100644
--- a/src/Sarif/Writers/SarifLogger.cs
+++ b/src/Sarif/Writers/SarifLogger.cs
@@ -323,7 +323,7 @@ private int LogRule(IRule iRule)
ruleIndex = _ruleToIndexMap.Count;
_ruleToIndexMap[rule] = ruleIndex;
_run.Resources = _run.Resources ?? new Resources();
- _run.Resources.Rules = _run.Resources.Rules ?? new List();
+ _run.Resources.Rules = _run.Resources.Rules ?? new OrderSensitiveValueComparisonList(Rule.ValueComparer);
_run.Resources.Rules.Add(rule);
}