Multitool --force option: advertised default is false, actual default is true. #1340
Labels
Comments
|
Not good. |
ghost
self-assigned this
|
I ran all the multitool commands, and the only ones that respect |
This was referenced Aug 14, 2019
ghost
pushed a commit
that referenced
this issue
Aug 22, 2019
This addresses one of a set of about 10 bugs I recently filed in the course of fixing #1340 ("Multitool --force option: advertised default is false, actual default is true"). I discovered that there were widespread inconsistencies in the behaviors and underlying implementations of the various commands related to their treatment of command line options. This was one of them. As usual, I took the opportunity to address IDE code warnings, improve documentation, and generally clean up the code I came in contact with while investigating the bug. The core of the fix is the introduction of the method(s) `ValidateOutputOptions` in DriverExtensionMethods.cs. The code in each Multitool command is rearranged or enhanced to invoke these methods. Notes: 1. I implemented the options validation methods in Sarif.Driver rather than in Sarif.Multitool because that's where the options classes themselves are defined. It's not obvious that that's where they belong. 2. The validation methods use a helper method `GetOptionDescription` that takes an option property such as `SingleFileOptionsBase.Force` and creates a description for the option like `"-p, --pretty-print"`, which is the format that the CommandLine package uses. This method was originally defined in Sarif.Multitool, but since it is now needed in Sarif.Driver, I moved it there. The corresponding unit test file moved to Test.UnitTests.Sarif.Driver. 4. As I've mentioned before, our unit test namespaces are a hodgepodge. In the same assembly, you can find tests in the namespaces `Microsoft.CodeAnalysis.Sarif.Multitool`, `Microsoft.CodeAnalysis.Test.UnitTests.Sarif.Multitool`, and `Test.UnitTests.Sarif.Multitool`. This makes it hard to find your tests in the Test Explorer. Per @michaelcfanning I'm standardizing on `Microsoft.CodeAnalysis.Sarif.Multitool`. I'll modify the test project files (or build.props, if I can think of a way to automate it) accordingly in a later PR.
This issue was closed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Repeat invocations of
smt transform input.sarif --output transformed.sarifwill overwrite the output file.Expected: Since --force flag was not set, transformed.sarif should not be overwritten.
The text was updated successfully, but these errors were encountered: