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

Enable RemoveOptionalData for RewriteCommand #2160

Merged
merged 6 commits into from
Nov 24, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
4 changes: 4 additions & 0 deletions docs/multitool-usage.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ Sarif.Multitool convert Current.fpr -tool FortifyFpr -output Current.sarif
: Add file contents from analyzed files and snippets from result regions to SARIF
Sarif.Multitool rewrite Current.sarif --insert "TextFiles,RegionSnippets" --inline

: Remove codeFlows from results regions to SARIF
Sarif.Multitool rewrite Current.sarif --remove "CodeFlows" --inline

: Rebase URIs from local paths to a token (for later resolution against remote URLs)
Sarif.Multitool rebaseuri Current.sarif --base-path-value "C:\Local\Path\To\Repo" --base-path-token REPO

Expand Down Expand Up @@ -72,4 +75,5 @@ Run ```Sarif.Multitool convert --help``` for the current list.
| Name | Purpose |
| ---- | ------- |
| pretty-print | Produce pretty-printed JSON output rather than compact form. |
Copy link
Member

Choose a reason for hiding this comment

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

form [](start = 72, length = 4)

We should mention this is the default! :) Probably, this argument should be deprecated over time (i.e., we'd only have minify which would reverse our default pretty print behavior).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I took a look at our base class and pretty-print isn't default. What we could do is: in a separate PR remove the minify since minify is the opposite of pretty-print. What do you think?


In reply to: 529762644 [](ancestors = 529762644)

| minify | Produce compact JSON output (all white space removed) rather than pretty-printed output. |
| force | Force overwrite of output file if it exists. |
4 changes: 4 additions & 0 deletions src/ReleaseHistory.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# SARIF Package Release History (SDK, Driver, Converters, and Multitool)

* FEATURE: Multitool SARIF rewrite accepts `remove` parameter[#2160](https://github.com/microsoft/sarif-sdk/pull/2160)

## **v2.3.8** [Sdk](https://www.nuget.org/packages/Sarif.Sdk/2.3.8) | [Driver](https://www.nuget.org/packages/Sarif.Driver/2.3.8) | [Converters](https://www.nuget.org/packages/Sarif.Converters/2.3.8) | [Multitool](https://www.nuget.org/packages/Sarif.Multitool/2.3.8) | [Multitool Library](https://www.nuget.org/packages/Sarif.Multitool.Library/2.3.8)

* FEATURE: PACKAGE BREAKING: Upgrade from .NET Framework 4.5 to .NET Framework 4.5.2 [#2135](https://github.com/microsoft/sarif-sdk/pull/2135)
Copy link
Member

@michaelcfanning michaelcfanning Nov 24, 2020

Choose a reason for hiding this comment

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

FEATURE [](start = 2, length = 7)

Eliminate whitespace on line 6? Go ahead and add the v2.3.9 banner at the top of the file? #Closed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I was thinking about this and it may cause some confusion because it will show that we have already the version 2.3.9, which is not true. What i will do is change and create a ## Unreleased.
What do you think?


In reply to: 529763647 [](ancestors = 529763647)

Copy link
Member

Choose a reason for hiding this comment

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

Sure, that's a fine idea. We just need to be disciplined about updating it when we actually release.


In reply to: 529771234 [](ancestors = 529771234,529763647)

* FEATURE: Multitool SARIF merge accepts `threads` parameter [#2026](https://github.com/microsoft/sarif-sdk/pull/2026)
* FEATURE: Enable GitHub SourceLink to all project [#2148](https://github.com/microsoft/sarif-sdk/pull/2148)
Expand Down
6 changes: 3 additions & 3 deletions src/Sarif.Multitool.Library/RewriteCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
using Microsoft.CodeAnalysis.Sarif.Driver.Sdk;
using Microsoft.CodeAnalysis.Sarif.Visitors;

using Newtonsoft.Json;

namespace Microsoft.CodeAnalysis.Sarif.Multitool
{
public class RewriteCommand : CommandBase
Expand All @@ -35,9 +33,11 @@ public int Run(RewriteOptions rewriteOptions)
SarifLog actualLog = ReadSarifFile<SarifLog>(_fileSystem, rewriteOptions.InputFilePath);

OptionallyEmittedData dataToInsert = rewriteOptions.DataToInsert.ToFlags();
OptionallyEmittedData dataToRemove = rewriteOptions.DataToRemove.ToFlags();
IDictionary<string, ArtifactLocation> originalUriBaseIds = rewriteOptions.ConstructUriBaseIdsDictionary();

SarifLog reformattedLog = new InsertOptionalDataVisitor(dataToInsert, originalUriBaseIds).VisitSarifLog(actualLog);
SarifLog reformattedLog = new RemoveOptionalDataVisitor(dataToRemove).VisitSarifLog(actualLog);
reformattedLog = new InsertOptionalDataVisitor(dataToInsert, originalUriBaseIds).VisitSarifLog(reformattedLog);

string fileName = CommandUtilities.GetTransformedOutputFileName(rewriteOptions);

Expand Down
4 changes: 4 additions & 0 deletions src/Sarif/OptionallyEmittedData.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ public enum OptionallyEmittedData : int
// version of the code that was analyzed.
VersionControlInformation = 0x400,

// A set of threadFlows which together describe a pattern of code execution relevant
// to detecting a result.
CodeFlows = 0x800,

// A special enum value that indicates that insertion should overwrite any existing
// information in the SARIF log file. In the absence of this setting, any existing
// data that would otherwise have been overwritten by the insert operation will
Expand Down
8 changes: 7 additions & 1 deletion src/Sarif/Visitors/RemoveOptionalDataVisitor.cs
Original file line number Diff line number Diff line change
@@ -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;

namespace Microsoft.CodeAnalysis.Sarif.Visitors
Expand All @@ -11,7 +12,7 @@ public RemoveOptionalDataVisitor(OptionallyEmittedData optionallyEmittedData)
_dataToRemove = optionallyEmittedData;
}

readonly OptionallyEmittedData _dataToRemove;
private readonly OptionallyEmittedData _dataToRemove;

public override Invocation VisitInvocation(Invocation node)
{
Expand Down Expand Up @@ -71,6 +72,11 @@ public override Result VisitResult(Result node)
node.Guid = null;
}

if (_dataToRemove.HasFlag(OptionallyEmittedData.CodeFlows))
{
node.CodeFlows = null;
}

return base.VisitResult(node);
}
}
Expand Down