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

Multitool page command --force defaults to true and can't be changed #1630

Closed
ghost opened this issue Aug 8, 2019 · 0 comments · Fixed by #1663
Closed

Multitool page command --force defaults to true and can't be changed #1630

ghost opened this issue Aug 8, 2019 · 0 comments · Fixed by #1663
Labels
area-multitool Multitool command implementations bug

Comments

@ghost
Copy link

ghost commented Aug 8, 2019

The page command has force set to true by default. I don't understand the argument in favor of this (see the comment in PageOptions.cs), and there seems to be no way to provide an explicitly false value to a "switch" option in the command line parsing package we use.

@ghost ghost added bug area-multitool Multitool command implementations labels Aug 8, 2019
ghost pushed a commit that referenced this issue Aug 23, 2019
@ghost ghost closed this as completed in #1663 Aug 27, 2019
ghost pushed a commit that referenced this issue Aug 27, 2019
This change fixes the treatment of logical locations in the `RemapIndicesVisitor`.

As the code stood, if a result implicated a nested logical location (and most logical locations _are_ nested, for example `Namespace.Class.Method`), the visitor would populate `run.logicalLocations` with only that logical location, omitting its ancestors (`Namespace.Class` and `Namespace`). Worse yet, the visitor left the `logicalLocation` object's `parentIndex` property alone, so it pointed to whatever array index the parent had occupied in the original run (that is, in the baseline or current run, as opposed to the merged run). This led to index out of range errors in the `SarifLogResultMatcher`.

The visitor treated nested artifacts correctly, copying all the ancestors from the original run, and adjusting both `artifact.parentIndex` and `artifact.location.index` correctly. We rewrote the treatment of logical locations to mirror the treatment of artifacts.

Also:
- Fix #1630: Multitool page command --force defaults to true and can't be changed.
- Make success/failure symbols ALL_CAPS and internal.
- Rename "file" to "artifact" throughout.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-multitool Multitool command implementations bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

0 participants