Enable suppress command in the multitool #2394
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,82 @@ | ||
| // 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.Diagnostics; | ||
|
|
||
| using Microsoft.CodeAnalysis.Sarif.Driver; | ||
| using Microsoft.CodeAnalysis.Sarif.Readers; | ||
| using Microsoft.CodeAnalysis.Sarif.Visitors; | ||
| using Microsoft.CodeAnalysis.Sarif.Writers; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.Sarif.Multitool | ||
| { | ||
| public class SuppressCommand : CommandBase | ||
| { | ||
| public SuppressCommand(IFileSystem fileSystem = null) : base(fileSystem) | ||
| { | ||
| } | ||
|
|
||
| public int Run(SuppressOptions options) | ||
| { | ||
| try | ||
| { | ||
| Console.WriteLine($"Suppress '{options.InputFilePath}' => '{options.OutputFilePath}'..."); | ||
| var w = Stopwatch.StartNew(); | ||
|
|
||
| bool valid = ValidateOptions(options); | ||
| if (!valid) | ||
| { | ||
| return FAILURE; | ||
| } | ||
|
|
||
| SarifLog currentSarifLog = PrereleaseCompatibilityTransformer.UpdateToCurrentVersion(FileSystem.FileReadAllText(options.InputFilePath), | ||
| options.Formatting, | ||
| out string _); | ||
|
|
||
| SarifLog reformattedLog = new SuppressVisitor(options.Justification, | ||
| options.Alias, | ||
| options.Guids, | ||
| options.Timestamps, | ||
| options.ExpiryInDays, | ||
| options.Status).VisitSarifLog(currentSarifLog); | ||
|
|
||
| string actualOutputPath = CommandUtilities.GetTransformedOutputFileName(options); | ||
| if (options.SarifOutputVersion == SarifVersion.OneZeroZero) | ||
| { | ||
| var visitor = new SarifCurrentToVersionOneVisitor(); | ||
| visitor.VisitSarifLog(reformattedLog); | ||
|
|
||
| WriteSarifFile(FileSystem, visitor.SarifLogVersionOne, actualOutputPath, options.Formatting, SarifContractResolverVersionOne.Instance); | ||
| } | ||
| else | ||
| { | ||
| WriteSarifFile(FileSystem, reformattedLog, actualOutputPath, options.Formatting); | ||
| } | ||
|
|
||
| w.Stop(); | ||
| Console.WriteLine($"Supress completed in {w.Elapsed}."); | ||
|
There was a problem hiding this comment. like the idea to capture the execution time, not all commands implemented this, maybe we can move it to command base to work for all commands? #WontFix There was a problem hiding this comment. we can create an issue to improve this. |
||
| } | ||
| catch (Exception ex) | ||
| { | ||
| Console.WriteLine(ex); | ||
| return FAILURE; | ||
| } | ||
|
|
||
| return SUCCESS; | ||
| } | ||
|
|
||
| private bool ValidateOptions(SuppressOptions options) | ||
| { | ||
| bool valid = true; | ||
|
|
||
| valid &= options.Validate(); | ||
| valid &= options.ExpiryInDays >= 0; | ||
| valid &= !string.IsNullOrWhiteSpace(options.Justification); | ||
| valid &= (options.Status == SuppressionStatus.Accepted || options.Status == SuppressionStatus.UnderReview); | ||
| valid &= DriverUtilities.ReportWhetherOutputFileCanBeCreated(options.OutputFilePath, options.Force, FileSystem); | ||
|
There was a problem hiding this comment. Even if every other item is false, this will return true if the last line is true, is this intended behavior? Conversely, if every other item is true, this will return false if the last item is false. I can see why that might be useful in this case, but again, is that intended behavior? #Resolved There was a problem hiding this comment. I improved the tests. |
||
|
|
||
| return valid; | ||
| } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| // Copyright (c) Microsoft. All rights reserved. | ||
| // Licensed under the MIT license. See LICENSE file in the project root for full license information. | ||
|
|
||
| using CommandLine; | ||
|
|
||
| using Microsoft.CodeAnalysis.Sarif.Driver; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.Sarif.Multitool | ||
| { | ||
| [Verb("suppress", HelpText = "Enrich a SARIF file with additional data.")] | ||
| public class SuppressOptions : SingleFileOptionsBase | ||
| { | ||
| [Option( | ||
| "justification", | ||
| HelpText = "A string that provides the rationale for the suppressions", | ||
| Required = true)] | ||
| public string Justification { get; set; } | ||
|
|
||
| [Option( | ||
| "alias", | ||
| HelpText = "The account name associated with the suppression.")] | ||
| public string Alias { get; set; } | ||
|
There was a problem hiding this comment. good question. |
||
|
|
||
| [Option( | ||
| "guids", | ||
| HelpText = "A UUID that will be associated with a suppression.")] | ||
| public bool Guids { get; set; } | ||
|
|
||
| [Option( | ||
| "timestamps", | ||
| HelpText = "The property 'timeUtc' that will be associated with a suppression.")] | ||
| public bool Timestamps { get; set; } | ||
|
|
||
| [Option( | ||
| "expiryInDays", | ||
| HelpText = "The property 'expiryUtc' that will be associated with a suppression from the 'timeUtc'.")] | ||
| public int ExpiryInDays { get; set; } | ||
|
|
||
| [Option( | ||
| "status", | ||
| HelpText = "The status that will be used in the suppression. Valid values include Accepted and UnderReview.")] | ||
| public SuppressionStatus Status { get; set; } | ||
| } | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| // 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; | ||
|
|
||
| namespace Microsoft.CodeAnalysis.Sarif.Visitors | ||
| { | ||
| public class SuppressVisitor : SarifRewritingVisitor | ||
| { | ||
| private readonly bool guids; | ||
| private readonly string alias; | ||
| private readonly bool timestamps; | ||
| private readonly DateTime timeUtc; | ||
| private readonly DateTime expiryUtc; | ||
| private readonly int expiryInDays; | ||
| private readonly string justification; | ||
| private readonly SuppressionStatus suppressionStatus; | ||
|
|
||
| public SuppressVisitor(string justification, | ||
| string alias, | ||
| bool guids, | ||
| bool timestamps, | ||
| int expiryInDays, | ||
| SuppressionStatus suppressionStatus) | ||
| { | ||
| this.alias = alias; | ||
| this.guids = guids; | ||
| this.timestamps = timestamps; | ||
| this.timeUtc = DateTime.UtcNow; | ||
| this.expiryInDays = expiryInDays; | ||
| this.justification = justification; | ||
| this.suppressionStatus = suppressionStatus; | ||
| this.expiryUtc = this.timeUtc.AddDays(expiryInDays); | ||
| } | ||
|
|
||
| public override Result VisitResult(Result node) | ||
| { | ||
| if (node.Suppressions == null) | ||
| { | ||
| node.Suppressions = new List<Suppression>(); | ||
| } | ||
|
|
||
| var suppression = new Suppression | ||
| { | ||
| Status = suppressionStatus, | ||
| Justification = justification | ||
| }; | ||
|
|
||
| if (!string.IsNullOrWhiteSpace(alias)) | ||
| { | ||
| suppression.SetProperty(nameof(alias), alias); | ||
|
There was a problem hiding this comment. it will be stored in the property bag |
||
| } | ||
|
|
||
| if (guids) | ||
| { | ||
| suppression.SetProperty("guid", Guid.NewGuid()); | ||
| } | ||
|
|
||
| if (timestamps) | ||
| { | ||
| //suppression.SetProperty("timeUtc", timeUtc.ToString(SarifUtilities.SarifDateTimeFormatMillisecondsPrecision, | ||
| // CultureInfo.InvariantCulture)); | ||
|
There was a problem hiding this comment. do we still need this comment #Closed There was a problem hiding this comment. no we don't. |
||
| suppression.SetProperty("timeUtc", timeUtc); | ||
|
There was a problem hiding this comment. actually, they will be printed in the same format. |
||
| } | ||
|
|
||
| if (expiryInDays > 0) | ||
| { | ||
| suppression.SetProperty("expiryUtc", expiryUtc.ToString(SarifUtilities.SarifDateTimeFormatMillisecondsPrecision, | ||
|
There was a problem hiding this comment. just updated! |
||
| CultureInfo.InvariantCulture)); | ||
| } | ||
|
|
||
| node.Suppressions.Add(suppression); | ||
| return base.VisitResult(node); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing a parameter "status" #Resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added.