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

@W-11111352@: SARIF and Table format now properly format output in the event of SFGE errors. #679

Merged
merged 2 commits into from May 16, 2022

Conversation

jfeingold35
Copy link
Collaborator

This PR does the following:

  • Gives SFGE internal error violations and timeout violations a synthetic rule name and category
  • Causes table and sarif output formats to not explode when a path-based violation lacks a sink vertex
  • Adds test coverage for the sarif-based error case

"sourceColumn": 8,
"sourceType": "BearController",
"sourceMethodName": "getAllBears",
"sinkFileName": null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would empty strings be safer than nulls?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Probably. Stand by for that.

"Source Location": `${relativeFile}:${violation.sourceLine}`,
"Source Line": violation.sourceLine,
"Source Column": violation.sourceColumn,
"Sink Location": null,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same with these nulls as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think we want the line and column to still be null, since those are supposed to be numbers. But the location can be an empty string, I believe.

Copy link
Collaborator

@rmohan20 rmohan20 left a comment

Choose a reason for hiding this comment

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

Changes look good. I had a minor suggestion to use "" instead of null.

@jfeingold35 jfeingold35 merged commit 2b2f971 into dev-3 May 16, 2022
@jfeingold35 jfeingold35 deleted the d/W-11111352 branch May 31, 2022 14:52
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.

None yet

2 participants