-
Notifications
You must be signed in to change notification settings - Fork 16
add support for the SARIF data format #39
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
Conversation
csgrep --mode=sarif
: use SARIF output formatcsgrep --mode=sarif
: use SARIF output format [WIP]
... that make-srpm.sh uses while it creates a source RPM
7293b71
to
ef28f13
Compare
csgrep --mode=sarif
: use SARIF output format [WIP]
The code slowly starts to work and does not obviously break anything that originally worked. So I would prefer to merge it as it is and continue with further tweaks of the encoder/decoder in separate pull requests. @lzaoral What do you think about this approach? It would help to avoid unnecessary conflicts with the other changes you are working on. |
Thanks for review! I have updated the pull request to reflect the review comments. |
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.
Thanks a lot, @kdudka! I think the PR is ready to be merged after the two remaining comments are addressed.
The writer is not yet implemented, only the output format switch. Closes: csutils#39
Thanks for review! Ale review comments should be addressed and fix-up commits squashed. |
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.
LGTM! Thanks, @kdudka!
... as an abstraction over various tree formats No changes in behavior intended by this commit.
The original fix worked only for lines ending with ','.
... as a placeholder for SARIF 2.1.0 writer
No changes in behavior intended by this commit.
... from csparser.cc No changes in behavior intended by this commit.
No changes in behavior intended by this commit.
It triggers assertion failure in a debug build.
... but stash them into `relatedLocations`, so that they do not disturb while visualizing the results on Github.
Suggested-by: Lukas Zaoral
This is a work in progress. The actual writer is not yet implemented.