-
Notifications
You must be signed in to change notification settings - Fork 114
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
Running suppress command can edit files whitespace even with no suppressions #631
Comments
I think I've identified the issue here. If there is a file that has detected issue in the sarif but their rule ids don't match the ones selected for suppression the file wasn't being skipped, so it was just being rewritten in place. #632 adds a check for this to skip processing those files since doing so is essentially a no-op (other than possibly messing with trailing whitespace as you encountered). |
Thanks for the quick fix! There's probably technically 2 bugs here, unless there's magic in the code I haven't seen. The unaddressed issue is that the whitespace is changed at all for a suppression command. #632 will help with the UX when filtering specific issues. The whitespace changes could still be painful and noisy when large numbers of files are getting suppressed. |
That's a fair point. I'll try to see this afternoon if I can do something about modifying whitespace when there actually is an issue detected as well. We use ReadAllLines and then glue the lines back together with Environment.NewLine, but that inserts newlines based on the OS being used so there's definitely some edge cases. |
@JustinSchneiderPBI Added a second fix for the whitespace bug to the same PR. |
Thank you very much! |
Describe the bug
Suppress command edits the whitespace in many files listed in a sarif result (a problem on its own). Surprisingly, this even affects files with violations outside the --rules list. Looks like EOF newlines removed sometimes. There might have been an encoding change on a file too?
To Reproduce
devskim suppress --source-code
Expected behavior
Only files with DS173237 rule violations would be edited
Versions(please complete the following information):
Screenshots
Additional context
In this case I was expecting just 2 files to be edited. Instead I had to sort through my changes and these whitespace changes to git commit.
The text was updated successfully, but these errors were encountered: