-
Notifications
You must be signed in to change notification settings - Fork 23
Add SARIF output format support with comprehensive tests #290
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
base: main
Are you sure you want to change the base?
Conversation
Adds support for outputting verification results in the SARIF (Static Analysis Results Interchange Format) v2.1.0 JSON format via a new output module with complete data structures and conversion functions for transforming VCResults to SARIF format. Command-line options (`--sarif` and `--output-format=sarif`) enable SARIF output generation.
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.
Style seems reasonable to me. I had a few changes; the most important is related to #guard_msgs.
| driver : Driver | ||
| deriving Repr, ToJson, FromJson | ||
|
|
||
| instance : Inhabited Tool where |
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.
Removing the explicit instance and adding deriving Inhabited to line 111 should work here (and most other places).
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.
Done.
| structure Driver where | ||
| name : String | ||
| version : String := "0.1.0" | ||
| informationUri : String := "https://github.com/tautschnig/strata-private" |
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.
Change the informationUri?
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.
Oh, thanks, done!
| /-! ## VCResult to SARIF Conversion Tests -/ | ||
|
|
||
| -- Test converting a successful VCResult | ||
| #eval |
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.
You can use #guard_msgs in to test the output is expected. I'd add that to all the #eval statements to squelch output.
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.
I believe to have addressed this, if I understood correctly?
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.
Pull request overview
This PR adds support for outputting verification results in SARIF (Static Analysis Results Interchange Format) v2.1.0 JSON format, enabling integration with tools that consume SARIF output. The implementation includes comprehensive data structures, conversion functions, and test coverage.
Key Changes:
- New SARIF output module with complete data structures for SARIF v2.1.0 format
- Command-line options
--sarifand--output-format=sarifto enable SARIF output generation - Comprehensive test suite validating SARIF conversion and JSON serialization
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| StrataVerify.lean | Adds command-line parsing for SARIF options and integrates SARIF output generation into the main verification workflow, with special handling for C_Simp files |
| Strata/Languages/Boogie/SarifOutput.lean | Implements complete SARIF v2.1.0 data structures and conversion functions to transform VCResults to SARIF format |
| StrataTest/Languages/Boogie/SarifOutputTests.lean | Provides comprehensive test coverage including level conversion, message generation, location extraction, and JSON serialization |
| Strata.lean | Adds import for the new SarifOutput module |
| Examples/SarifTest.boogie.st | Adds example Boogie program for testing SARIF output functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| match uri?, startLine?, startColumn? with | ||
| | some uri, some startLine, some startColumn => pure { uri, startLine, startColumn } | ||
| | some uri, some startLine, none => pure { uri, startLine, startColumn := 1 } | ||
| | some uri, none, some startColumn => pure { uri, startLine := 1, startColumn } | ||
| | some uri, none, none => pure { uri, startLine := 1, startColumn := 1 } | ||
| | none, _, _ => none |
Copilot
AI
Dec 22, 2025
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.
The extractLocation function provides fallback default values when line or column information is missing (defaulting to line 1, column 1). While this allows SARIF output to be generated, it may be misleading to report a location with incorrect line/column numbers. Consider either returning None when essential location information is missing, or documenting this fallback behavior clearly in the function's docstring.
| { fld := .label "file", value := .msg "/test/file.st" }, | ||
| { fld := .label "startLine", value := .msg "10" } | ||
| ] | ||
| (extractLocation md == none) |
Copilot
AI
Dec 22, 2025
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.
This test expects extractLocation to return none when column information is missing, but the actual implementation (lines 167-169 in SarifOutput.lean) provides fallback default values and returns Some with startColumn := 1. This test will fail. Either update the test to match the implementation's behavior, or change the implementation to match the test's expectations.
| (extractLocation md == none) | |
| match extractLocation md with | |
| | some loc => | |
| loc.uri = "/test/file.st" && loc.startLine = 10 && loc.startColumn = 1 | |
| | none => false |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Description of changes:
Adds support for outputting verification results in the SARIF (Static Analysis Results Interchange Format) v2.1.0 JSON format via a new output module with complete data structures and conversion functions for transforming VCResults to SARIF format. Command-line options (
--sarifand--output-format=sarif) enable SARIF output generation.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.