Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new visitor to get deterministic SARIF log by sorting results #2422

Merged
merged 29 commits into from
Mar 2, 2022
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
22ec32b
Add new visitor to get deterministic SARIF log by sorting results
yongyan-gh Jan 17, 2022
44a9f46
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Jan 17, 2022
3c597ff
Fix dotnet format issue
yongyan-gh Jan 17, 2022
ca5c356
updating format
yongyan-gh Jan 18, 2022
043079f
remove unnecessary using
yongyan-gh Jan 20, 2022
ea82e82
Add Run Comparer to support sorting logs with multiple runs.
yongyan-gh Jan 21, 2022
3961924
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Jan 27, 2022
d568f85
Add command argument unit tests
yongyan-gh Jan 27, 2022
07e1b4c
Merge branch 'users/yongyan-gh/sortingvisitor' of https://github.com/…
yongyan-gh Jan 27, 2022
cfebb85
use ContainsKey to avoid allocating variable
yongyan-gh Feb 2, 2022
351ef43
Fixing `AnalyzeCommandBase` and `MultithreadedAnalyzeCommandBase` art…
eddynaka Jan 31, 2022
9f140e4
`SarifLogger` now emits an artifacts table entry if `artifactLocation…
eddynaka Feb 1, 2022
0727702
Fix `ArgumentException` when recurse is enabled and two file target s…
eddynaka Feb 1, 2022
6306faf
Addressing PR review issues
yongyan-gh Feb 8, 2022
f192c36
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Feb 9, 2022
bb3ce50
Merge branch 'main' of https://github.com/microsoft/sarif-sdk
yongyan-gh Feb 9, 2022
196cc0a
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Feb 9, 2022
9fa0ca7
Fix issues in PR review
yongyan-gh Feb 11, 2022
9d1ce42
Add xml comments
yongyan-gh Feb 11, 2022
305570e
Fix test issues
yongyan-gh Feb 14, 2022
4747ec1
fix dotnet format
yongyan-gh Feb 14, 2022
9281b44
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Feb 14, 2022
314bd53
Addressing review feedbacks
yongyan-gh Feb 18, 2022
d37dd18
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Feb 18, 2022
eb150f7
Merge branch 'main' into users/yongyan-gh/sortingvisitor
yongyan-gh Feb 18, 2022
7c77019
Fix tests
yongyan-gh Feb 18, 2022
966b7f8
Update extension methods names
yongyan-gh Feb 19, 2022
f07f4af
Change xml doc comments to normal comments
yongyan-gh Feb 20, 2022
f18e005
Merge branch 'main' into users/yongyan-gh/sortingvisitor
michaelcfanning Mar 2, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* BREAKING: Fix `InvalidOperationException` when using PropertiesDictionary in a multithreaded application, and remove `[Serializable]` from it. Now use of BinaryFormatter on it will result in `SerializationException`: Type `PropertiesDictionary` is not marked as serializable. [#2415](https://github.com/microsoft/sarif-sdk/pull/2415)
* BREAKING: `SarifLogger` now emits an artifacts table entry if `artifactLocation` is not null for tool configuration and tool execution notifications. [#2437](https://github.com/microsoft/sarif-sdk/pull/2437)
* 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)
* FEATURE: Add `--sort-results` argument to `rewrite` command to get sorted SARIF results. [#2422](https://github.com/microsoft/sarif-sdk/pull/2422)

## **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)

Expand Down
5 changes: 5 additions & 0 deletions src/Sarif.Multitool.Library/RewriteCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,11 @@ public int Run(RewriteOptions options)
SarifLog reformattedLog = new RemoveOptionalDataVisitor(dataToRemove).VisitSarifLog(actualLog);
reformattedLog = new InsertOptionalDataVisitor(dataToInsert, originalUriBaseIds, insertProperties: options.InsertProperties).VisitSarifLog(reformattedLog);

if (options.SortResults)
{
reformattedLog = new SortingVisitor().VisitSarifLog(reformattedLog);
}

if (options.SarifOutputVersion == SarifVersion.OneZeroZero)
{
var visitor = new SarifCurrentToVersionOneVisitor();
Expand Down
6 changes: 6 additions & 0 deletions src/Sarif.Multitool.Library/RewriteOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,11 @@ namespace Microsoft.CodeAnalysis.Sarif.Multitool
[Verb("rewrite", HelpText = "Enrich a SARIF file with additional data.")]
public class RewriteOptions : SingleFileOptionsBase
{
[Option(
's',
"sort-results",
Copy link
Member

@michaelcfanning michaelcfanning Feb 2, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

results

Why isn't this in a base class? #Pending

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future work?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open issue #2443 for tracking

Default = false,
HelpText = "Sort results in the final output file.")]
public bool SortResults { get; set; }
}
}
112 changes: 112 additions & 0 deletions src/Sarif/Comparers/ArtifactComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
// 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;

/// <summary>
/// Warning this comparer may not have all properties compared. Will be replaced by comprehensive
/// comparer generated by JSchema as part of EqualityComparer in a planned comprehensive solution.
/// Tracking by issue: https://github.com/microsoft/jschema/issues/141
/// </summary>
namespace Microsoft.CodeAnalysis.Sarif.Comparers
{
internal class ArtifactComparer : IComparer<Artifact>
{
internal static readonly ArtifactComparer Instance = new ArtifactComparer();

public int Compare(Artifact left, Artifact right)
{
if (ComparerHelper.CompareReference(left, right, out int compareResult))
{
return compareResult;
}

compareResult = MessageComparer.Instance.Compare(left.Description, right.Description);

if (compareResult != 0)
{
return compareResult;
}

compareResult = ArtifactLocationComparer.Instance.Compare(left.Location, right.Location);

if (compareResult != 0)
{
return compareResult;
}

compareResult = left.ParentIndex.CompareTo(right.ParentIndex);

if (compareResult != 0)
{
return compareResult;
}

compareResult = left.Offset.CompareTo(right.Offset);

if (compareResult != 0)
{
return compareResult;
}

compareResult = left.Length.CompareTo(right.Length);

if (compareResult != 0)
{
return compareResult;
}

compareResult = left.Roles.CompareTo(right.Roles);

if (compareResult != 0)
{
return compareResult;
}

compareResult = string.Compare(left.MimeType, right.MimeType);

if (compareResult != 0)
{
return compareResult;
}

compareResult = ArtifactContentComparer.Instance.Compare(left.Contents, right.Contents);

if (compareResult != 0)
{
return compareResult;
}

compareResult = string.Compare(left.Encoding, right.Encoding);

if (compareResult != 0)
{
return compareResult;
}

compareResult = string.Compare(left.SourceLanguage, right.SourceLanguage);

if (compareResult != 0)
{
return compareResult;
}

compareResult = ComparerHelper.CompareDictionary(left.Hashes, right.Hashes);

if (compareResult != 0)
{
return compareResult;
}

compareResult = left.LastModifiedTimeUtc.CompareTo(right.LastModifiedTimeUtc);

if (compareResult != 0)
{
return compareResult;
}

// Warning there may be properties are not compared.
return compareResult;
}
}
}
49 changes: 49 additions & 0 deletions src/Sarif/Comparers/ArtifactContentComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// 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;

/// <summary>
/// Warning this comparer may not have all properties compared. Will be replaced by comprehensive
/// comparer generated by JSchema as part of EqualityComparer in a planned comprehensive solution.
/// Tracking by issue: https://github.com/microsoft/jschema/issues/141
/// </summary>
namespace Microsoft.CodeAnalysis.Sarif.Comparers
{
internal class ArtifactContentComparer : IComparer<ArtifactContent>
{
internal static readonly ArtifactContentComparer Instance = new ArtifactContentComparer();

public int Compare(ArtifactContent left, ArtifactContent right)
{
if (ComparerHelper.CompareReference(left, right, out int compareResult))
{
return compareResult;
}

compareResult = string.Compare(left.Text, right.Text);

if (compareResult != 0)
{
return compareResult;
}

compareResult = string.Compare(left.Binary, right.Binary);

if (compareResult != 0)
{
return compareResult;
}

compareResult = MultiformatMessageStringComparer.Instance.Compare(left.Rendered, right.Rendered);

if (compareResult != 0)
{
return compareResult;
}

// Warning there may be properties are not compared.
return compareResult;
}
}
}
56 changes: 56 additions & 0 deletions src/Sarif/Comparers/ArtifactLocationComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// 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;

/// <summary>
/// Warning this comparer may not have all properties compared. Will be replaced by comprehensive
/// comparer generated by JSchema as part of EqualityComparer in a planned comprehensive solution.
/// Tracking by issue: https://github.com/microsoft/jschema/issues/141
/// </summary>
namespace Microsoft.CodeAnalysis.Sarif.Comparers
{
internal class ArtifactLocationComparer : IComparer<ArtifactLocation>
{
internal static readonly ArtifactLocationComparer Instance = new ArtifactLocationComparer();

public int Compare(ArtifactLocation left, ArtifactLocation right)
{
if (ComparerHelper.CompareReference(left, right, out int compareResult))
{
return compareResult;
}

compareResult = ComparerHelper.CompareUri(left.Uri, right.Uri);

if (compareResult != 0)
{
return compareResult;
}

compareResult = string.Compare(left.UriBaseId, right.UriBaseId);

if (compareResult != 0)
{
return compareResult;
}

compareResult = left.Index.CompareTo(right.Index);

if (compareResult != 0)
{
return compareResult;
}

compareResult = MessageComparer.Instance.Compare(left.Description, right.Description);

if (compareResult != 0)
{
return compareResult;
}

// Warning there may be properties are not compared.
return compareResult;
}
}
}
42 changes: 42 additions & 0 deletions src/Sarif/Comparers/CodeFlowComparer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// 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;

/// <summary>
/// Warning this comparer may not have all properties compared. Will be replaced by comprehensive
/// comparer generated by JSchema as part of EqualityComparer in a planned comprehensive solution.
/// Tracking by issue: https://github.com/microsoft/jschema/issues/141
/// </summary>
namespace Microsoft.CodeAnalysis.Sarif.Comparers
{
internal class CodeFlowComparer : IComparer<CodeFlow>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

internal

All of these classes s/be public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

was following the same pattern of Sarif SDK's EqualityComparers. The EqualityComparer class is internal and have a public reference in Sarif types, e.g.
public static IEqualityComparer ValueComparer => ArtifactLocationEqualityComparer.Instance;

{
internal static readonly CodeFlowComparer Instance = new CodeFlowComparer();

public int Compare(CodeFlow left, CodeFlow right)
{
if (ComparerHelper.CompareReference(left, right, out int compareResult))
{
return compareResult;
}

compareResult = MessageComparer.Instance.Compare(left.Message, right.Message);

if (compareResult != 0)
{
return compareResult;
}

compareResult = ComparerHelper.CompareList(left.ThreadFlows, right.ThreadFlows, ThreadFlowComparer.Instance);

if (compareResult != 0)
{
return compareResult;
}

// Warning there may be properties are not compared.
return compareResult;
}
}
}
Loading