Skip to content

Fix bug in SARIF comparison#3317

Merged
koesie10 merged 2 commits intomainfrom
koesie10/sarif-comparison
Feb 6, 2024
Merged

Fix bug in SARIF comparison#3317
koesie10 merged 2 commits intomainfrom
koesie10/sarif-comparison

Conversation

@koesie10
Copy link
Copy Markdown
Member

@koesie10 koesie10 commented Feb 6, 2024

The SARIF comparison code was comparing the index of the artifact location, which is not useful for comparison and may differ between runs of very similar queries. This adds a function to convert a SARIF result to a canonical form, which removes the index from the artifact location.

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

The SARIF comparison code was comparing the index of the artifact
location, which is not useful for comparison and may differ between runs
of very similar queries. This adds a function to convert a SARIF result
to a canonical form, which removes the index from the artifact location.
@koesie10 koesie10 marked this pull request as ready for review February 6, 2024 13:07
@koesie10 koesie10 requested a review from a team as a code owner February 6, 2024 13:07
@koesie10 koesie10 requested a review from a team February 6, 2024 13:07
Copy link
Copy Markdown
Contributor

@robertbrignull robertbrignull left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me.

If we find we're wanting to do more of these modifications then I wonder if it's worth investing in a real deepCopy function so the code is simpler. For just this I think what's in this PR is good.

@koesie10 koesie10 merged commit a03863c into main Feb 6, 2024
@koesie10 koesie10 deleted the koesie10/sarif-comparison branch February 6, 2024 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants