Skip to content

Commit

Permalink
Fix #1600: Matched result should use current suppressions.
Browse files Browse the repository at this point in the history
  • Loading branch information
Larry Golding committed Aug 6, 2019
1 parent 0151b67 commit 9ecf550
Show file tree
Hide file tree
Showing 7 changed files with 242 additions and 14 deletions.
3 changes: 3 additions & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -428,3 +428,6 @@

## **v2.1.10** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.1.10) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.1.10) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.1.10) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.1.10)
* BUGFIX: Resolve a performance issue in web request parsing code. https://github.com/microsoft/sarif-sdk/issues/1608

## **v2.1.11** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.1.11) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.1.11) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.1.11) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.1.11)
* BUGFIX: Result matching should prefer the suppression info from the current run. https://github.com/microsoft/sarif-sdk/issues/1600
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Collections.Generic;
using System.Linq;

namespace Microsoft.CodeAnalysis.Sarif.Baseline.ResultMatching
{
Expand Down Expand Up @@ -130,7 +129,7 @@ private Result ConstructExistingResult(
// Result exists.
Result result = CurrentResult.Result.DeepClone();
result.CorrelationGuid = PreviousResult.Result.CorrelationGuid;
result.Suppressions = PreviousResult.Result.Suppressions;
result.Suppressions = CurrentResult.Result.Suppressions;
result.BaselineState = BaselineState.Unchanged;

if (!PreviousResult.Result.TryGetProperty(SarifLogResultMatcher.ResultMatchingResultPropertyName, out originalResultMatchingProperties))
Expand Down
80 changes: 70 additions & 10 deletions src/Test.UnitTests.Sarif/Baseline2/V2ResultMatcherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,28 +18,48 @@ public class V2ResultMatcherTests
private const string SampleFilePath = "elfie-arriba.sarif";
private Run SampleRun { get; }

private const string SuppressionTestPreviousFilePath = "SuppressionTestPrevious.sarif";
private SarifLog SuppressionTestPreviousLog { get; }

private const string SuppressionTestCurrentFilePath = "SuppressionTestCurrent.sarif";
private SarifLog SuppressionTestCurrentLog { get; }

private static readonly IResultMatcher matcher = new V2ResultMatcher();
private static readonly ResourceExtractor extractor = new ResourceExtractor(typeof(V2ResultMatcherTests));

public V2ResultMatcherTests()
{
string sampleFileContents = extractor.GetResourceText(SampleFilePath);
SampleRun = GetLogFromResource(SampleFilePath).Runs[0];

SuppressionTestPreviousLog = GetLogFromResource(SuppressionTestPreviousFilePath);
SuppressionTestCurrentLog = GetLogFromResource(SuppressionTestCurrentFilePath);
}

SampleRun = JsonConvert.DeserializeObject<SarifLog>(sampleFileContents).Runs[0];
private static SarifLog GetLogFromResource(string filePath)
{
string fileContents = extractor.GetResourceText(filePath);
return JsonConvert.DeserializeObject<SarifLog>(fileContents);
}

private static IEnumerable<MatchedResults> Match(Run previous, Run current)
private static IEnumerable<MatchedResults> CreateMatchedResults(Run previous, Run current)
{
return matcher.Match(
previous.Results.Select(r => new ExtractedResult(r, previous)).ToList(),
current.Results.Select(r => new ExtractedResult(r, current)).ToList()
);
}

private static Run CreateMatchedRun(SarifLog previousLog, SarifLog currentLog)
{
ISarifLogMatcher baseliner = ResultMatchingBaselinerFactory.GetDefaultResultMatchingBaseliner();
SarifLog matchedLog = baseliner.Match(new[] { previousLog }, new[] { currentLog }).Single();
return matchedLog.Runs[0];
}

[Fact]
public void V2ResultMatcher_Identical()
{
IEnumerable<MatchedResults> matches = Match(SampleRun, SampleRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, SampleRun);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().BeEmpty();
}

Expand All @@ -49,7 +69,7 @@ public void V2ResultMatcher_MoveWithinFile()
Run newRun = SampleRun.DeepClone();
newRun.Results[2].Locations[0].PhysicalLocation.Region.StartLine += 1;

IEnumerable<MatchedResults> matches = Match(SampleRun, newRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, newRun);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().BeEmpty();
}

Expand All @@ -60,7 +80,7 @@ public void V2ResultMatcher_RenameFile()
newRun.Results[2].Locations[0].PhysicalLocation.ArtifactLocation.Index = -1;
newRun.Results[2].Locations[0].PhysicalLocation.ArtifactLocation.Uri = new Uri("file:///C:/Code/elfie-arriba/XForm/XForm.Web/node_modules/public-encrypt/test/test_rsa_privkey_NEW.pem");

IEnumerable<MatchedResults> matches = Match(SampleRun, newRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, newRun);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().BeEmpty();
}

Expand All @@ -75,7 +95,7 @@ public void V2ResultMatcher_AddResult()

newRun.Results.Add(newResult);

IEnumerable<MatchedResults> matches = Match(SampleRun, newRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, newRun);
matches.Where(m => m.PreviousResult != null && m.CurrentResult != null).Should().HaveCount(5);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().HaveCount(1);

Expand All @@ -89,7 +109,7 @@ public void V2ResultMatcher_RemoveResult()
Run newRun = SampleRun.DeepClone();
newRun.Results.RemoveAt(2);

IEnumerable<MatchedResults> matches = Match(SampleRun, newRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, newRun);
matches.Where(m => m.PreviousResult != null && m.CurrentResult != null).Should().HaveCount(4);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().HaveCount(1);

Expand All @@ -109,7 +129,7 @@ public void V2ResultMatcher_RemovedAndAdded()
newResult.Message.Text = "Different Message";
newRun.Results.Add(newResult);

IEnumerable<MatchedResults> matches = Match(SampleRun, newRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, newRun);
matches.Where(m => m.PreviousResult != null && m.CurrentResult != null).Should().HaveCount(4);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().HaveCount(2);

Expand All @@ -135,9 +155,49 @@ public void V2ResultMatcher_TwoAdded()
newResult.Message.Text = "Different Message";
newRun.Results.Add(newResult);

IEnumerable<MatchedResults> matches = Match(SampleRun, newRun);
IEnumerable<MatchedResults> matches = CreateMatchedResults(SampleRun, newRun);
matches.Where(m => m.PreviousResult != null && m.CurrentResult != null).Should().HaveCount(5);
matches.Where(m => m.PreviousResult == null || m.CurrentResult == null).Should().HaveCount(2);
}

[Fact]
public void V2ResultMatcher_WhenNewIssueIsSuppressed_SupressesTheResultInTheOutput()
{
Run matchedRun = CreateMatchedRun(SuppressionTestPreviousLog, SuppressionTestCurrentLog);

Result newSuppressedResult = matchedRun.Results.Single(r => r.Message.Text == "New suppressed result.");

newSuppressedResult.Suppressions.Count.Should().Be(1);
}

[Fact]
public void V2ResultMatcher_WhenExistingUnsuppressedIssueIsNewlySuppressed_SupressesTheResultInTheOutput()
{
Run matchedRun = CreateMatchedRun(SuppressionTestPreviousLog, SuppressionTestCurrentLog);

Result existingResultNewlySuppressed = matchedRun.Results.Single(r => r.Message.Text == "Existing, originally unsuppressed result.");

existingResultNewlySuppressed.Suppressions.Count.Should().Be(1);
}

[Fact]
public void V2ResultMatcher_WhenIssueIsSuppressedInBothRuns_SupressesTheResultInTheOutput()
{
Run matchedRun = CreateMatchedRun(SuppressionTestPreviousLog, SuppressionTestCurrentLog);

Result existingResultNewlySuppressed = matchedRun.Results.Single(r => r.Message.Text == "Result suppressed in both runs.");

existingResultNewlySuppressed.Suppressions.Count.Should().Be(1);
}

[Fact]
public void V2ResultMatcher_WhenExistingSuppressedIssueIsNewlyUnsuppressed_DoesNotSupressTheResultInTheOutput()
{
Run matchedRun = CreateMatchedRun(SuppressionTestPreviousLog, SuppressionTestCurrentLog);

Result existingResultNewlyUnsuppressed = matchedRun.Results.Single(r => r.Message.Text == "Existing, originally suppressed result.");

existingResultNewlyUnsuppressed.Suppressions.Should().BeNull();
}
}
}
4 changes: 4 additions & 0 deletions src/Test.UnitTests.Sarif/Test.UnitTests.Sarif.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

<ItemGroup>
<None Remove="TestData\Baseline\elfie-arriba.sarif" />
<None Remove="TestData\Baseline\SuppressionTestCurrent.sarif" />
<None Remove="TestData\Baseline\SuppressionTestPrevious.sarif" />
<None Remove="TestData\Map\Sample.json" />
<None Remove="TestData\Map\TinyArray.json" />
<None Remove="TestData\PrereleaseCompatibilityTransformer\ExpectedOutputs\ArtifactsWithRoles.sarif" />
Expand Down Expand Up @@ -61,6 +63,8 @@

<ItemGroup>
<EmbeddedResource Include="TestData\Baseline\elfie-arriba.sarif" />
<EmbeddedResource Include="TestData\Baseline\SuppressionTestCurrent.sarif" />
<EmbeddedResource Include="TestData\Baseline\SuppressionTestPrevious.sarif" />
<EmbeddedResource Include="TestData\InsertOptionalDataVisitor\ExpectedOutputs\TopLevelOriginalUriBaseIdUriMissing_ContextRegionSnippets.sarif" />
<EmbeddedResource Include="TestData\InsertOptionalDataVisitor\Inputs\TopLevelOriginalUriBaseIdUriMissing.sarif" />
<EmbeddedResource Include="TestData\Map\TinyArray.json">
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
{
"$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "CodeScanner"
}
},
"results": [
{
"ruleId": "TST0001",
"message": {
"text": "New suppressed result."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file1.c"
}
}
}
],
"suppressions": [
{
"kind": "external"
}
]
},
{
"ruleId": "TST0002",
"message": {
"text": "Existing, originally unsuppressed result."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file2.c"
}
}
}
],
"suppressions": [
{
"kind": "external"
}
]
},
{
"ruleId": "TST0003",
"message": {
"text": "Result suppressed in both runs."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file3.c"
}
}
}
],
"suppressions": [
{
"kind": "external"
}
]
},
{
"ruleId": "TST0004",
"message": {
"text": "Existing, originally suppressed result."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file4.c"
}
}
}
]
}
],
"columnKind": "utf16CodeUnits"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
{
"$schema": "http://json.schemastore.org/sarif-2.1.0-rtm.4.json",
"version": "2.1.0",
"runs": [
{
"tool": {
"driver": {
"name": "CodeScanner"
}
},
"results": [
{
"ruleId": "TST0002",
"message": {
"text": "Existing, originally unsuppressed result."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file2.c"
}
}
}
]
},
{
"ruleId": "TST0003",
"message": {
"text": "Result suppressed in both runs."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file3.c"
}
}
}
],
"suppressions": [
{
"kind": "external"
}
]
},
{
"ruleId": "TST0004",
"message": {
"text": "Existing, originally suppressed result."
},
"locations": [
{
"physicalLocation": {
"artifactLocation": {
"uri": "file:///C:/code/file4.c"
}
}
}
],
"suppressions": [
{
"kind": "external"
}
]
}
],
"columnKind": "utf16CodeUnits"
}
]
}
4 changes: 2 additions & 2 deletions src/build.props
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
<Copyright Condition=" '$(Copyright)' == '' ">© Microsoft Corporation. All rights reserved.</Copyright>

<!-- VersionPrefix denotes the current Semantic Version for the SDK. Must be updated before every nuget drop. -->
<VersionPrefix>2.1.10</VersionPrefix>
<VersionPrefix>2.1.11</VersionPrefix>

<!-- SchemaVersionAsPublishedToSchemaStoreOrg identifies the current published version on json schema store at https://schemastore.azurewebsites.net/schemas/json/ -->
<SchemaVersionAsPublishedToSchemaStoreOrg>2.1.0-rtm.4</SchemaVersionAsPublishedToSchemaStoreOrg>
Expand All @@ -30,7 +30,7 @@
place. These properties are actually used by the PowerShell script that
hides the previous package versions on nuget.org.
-->
<PreviousVersionPrefix>2.1.9</PreviousVersionPrefix>
<PreviousVersionPrefix>2.1.10</PreviousVersionPrefix>
<PreviousSchemaVersionAsPublishedToSchemaStoreOrg>2.1.0-rtm.3</PreviousSchemaVersionAsPublishedToSchemaStoreOrg>
<PreviousStableSarifVersion>2.1.0</PreviousStableSarifVersion>
</PropertyGroup>
Expand Down

0 comments on commit 9ecf550

Please sign in to comment.