Fix first instance rule being used as rule description for all violations of that rule and other SARIF improvements#7640
Conversation
|
Thanks for you contribution. I added a remark on how to keep this in sync for future changes. Possibly not something which should addressed before/in this PR. Also something like |
danmar
left a comment
There was a problem hiding this comment.
Thanks for your contribution!!
I believe we'll need to rethink that getRuleDescription somehow but unfortunately I don't see a quick/simple method..
|
In the future.. if I don't respond for couple of days.. feel free to ping me. |
|
@danmar I have simplified the approach significantly. I added genericMessage output for ErrorMessage class like you recommended and have reduced the regex to a minimum. I also removed the security violation list of errors and now I just add security tag if a CWE ID exists. |
|
Actually I just made a discovery. If I make all the descriptions for the rules blank strings, github will default to the instance specific description for each and this solves the problem. So this should greatly simplify the addition. |
That is so much better 👍 Sounds like it will both simplify our job and make the user experience better. |
@danmar this is ready for review again |
for your information I prefer if people use |
|
CI: I think you can fix the "dmake" failures by just running |
|
I have released 2.18.0 but if you fix this I think a 2.18.1 can be released with this fix. |
@danmar I fixed the dmake issue. |
|
@danmar this is ready for review |
|
@Nettozx there are 5 failing tests in the CI have you looked at those? |
I made an update to move sarif reporter out to its own header because it was in an anonymous class so it couldn't be used for unit test originally. Now that I moved it, I updated the tests so it no longer relies on cppcheck executable and it can use internal libraries. |
|
|
I am very sorry it took so long to review. Please feel free to ping me if it takes more than 3-4 days to get a review. |
…nge tests so it doesnt need to run executable
… declaration - Include json.h in header - Remove duplicate include
|
@danmar This should be ready for review again now |
|
@danmar the one windows failure is a github action timeout not related to PR, i think you may have to just re-run that test. |
|



Before:

After:
