Add SARIF output format for qlty check #1595
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implement a new SARIF (Static Analysis Results Interchange Format) output for qlty check. - Add --sarif argument to the check command (conflicts with --json) - Implement SarifFormatter to generate valid SARIF 2.1.0 compliant output - Include relevant issues data in the SARIF format with proper rule mapping 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Diff Coverage for cli: The code coverage on the diff in this pull request is 65.3%. Total Coverage for cli: This PR will decrease coverage by 0.19%. File Coverage Changes
🛟 Help
|
- Add insta as dependency with JSON feature - Convert manual assertions to snapshot test for better readability and maintenance - Include snapshot file in source control 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Update new method to return Self instead of Box<dyn Formatter> - Add boxed method to return a boxed instance - Update references to use boxed method instead 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Remove all comments from sarif.rs for cleaner and more consistent code. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add tool to the ruleId (tool:ruleKey format) - Add fingerprints support using source_checksum - Add partial fingerprints support - Add related locations from other_locations - Add fixes from suggestions - Add category in taxa - Add tags in properties - Add all properties from the Issue - Add sourceLanguage to region when available 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Extract issue processing logic from create_sarif_document into a separate function to improve code organization and readability. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Rename process_issue to serialize_issue - Remove comments for cleaner code - Move source checksum to partialFingerprints instead of fingerprints - Fix test to work with new fingerprint structure 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Adds redactions for version, version_string, and date fields in the SARIF output to ensure snapshot tests remain stable regardless of build date or version number changes. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Uses insta::sorted_redaction to ensure consistent ordering of fingerprint keys in the snapshot test regardless of the order they appear in the SARIF output. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
brynary
force-pushed
the
feature/sarif-output
branch
from
0a346af to
2fa6e5c
Compare
- Move the auth module and all related files from qlty-cloud to qlty-cli - Add necessary dependencies to qlty-cli - Added auth functions to public exports in qlty-cli 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Added auth_token field to qlty-check Settings - Updated Fixer to use auth_token from Settings instead of Client::authenticated() - Modified check.rs and build.rs to get auth token and pass it to Settings - Changed whoami.rs to use Client::new with auth token from qlty-cli 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Remove auth module import - Remove auth function exports - Remove authenticated method from Client impl - Code now uses the auth module from qlty-cli 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Move CoverageExport class from qlty-cloud to qlty-coverage - Update references to use the new location - Add necessary dependencies to qlty-coverage - Update login/logout to use auth from qlty-cli 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Created a new crate to hold common formatters used across multiple crates. This helps avoid circular dependencies between qlty-cli and qlty-cloud. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Removed formatter implementations from qlty-cloud/src/format/ since they're now in qlty-formats. Also moved the test snapshots to qlty-formats. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Moved the SarifFormatter from qlty-formats to qlty-cli. Created a stub implementation in qlty-cloud for backward compatibility. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Removed the SarifFormatter stub from qlty-cloud/src/format.rs since all references to it have been updated to use the one in qlty-cli. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Removed the auth module from qlty-cloud since it was previously moved to qlty-cli. This completes the migration of authentication functionality to qlty-cli. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Moved the AnalysisExport from qlty-cloud/src/export/analysis.rs to qlty-cli/src/export/analysis.rs. Cleaned up unnecessary comments in qlty-cloud/src/export.rs. Simplified format re-exports in qlty-cloud/src/format.rs. Updated imports in qlty-cli/src/commands/build.rs to use the new location. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
This refactoring completes our crate reorganization, moving functionality out of qlty-cloud: - Moved format.rs to the qlty-formats crate - Moved export/analysis.rs to the qlty-cli/src/export module - Moved export/coverage.rs to the qlty-coverage/src/export module - Moved auth module to qlty-cli/src/auth - Updated imports and fixed circular dependencies - Simplified qlty-cloud to only contain the Client struct 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Updated formatters to use slices instead of Vec references in qlty-formats - Fixed format implementation to avoid reference lifetime issues in qlty-cli/export/analysis.rs - Fixed test module import issues with integration tests 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Changed SarifFormatter to use qlty_check::Report instead of qlty_analysis::Report - Removed from_issues and boxed_from_issues constructors - Updated usage in commands/check.rs - Fixed test code to properly create a qlty_check::Report instance 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
2 new issues
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
Comment on lines
+1
to
+512
| end_column: 20, | ||
| ..Default::default() | ||
| }), | ||
| }), | ||
| }], | ||
| }, | ||
| Suggestion { | ||
| id: "suggestion-2".to_string(), | ||
| description: "Alternative fix".to_string(), | ||
| patch: "alternative patch content".to_string(), | ||
| r#unsafe: true, | ||
| source: SuggestionSource::Llm.into(), | ||
| replacements: vec![Replacement { | ||
| data: "differentSolution()".to_string(), | ||
| location: Some(Location { | ||
| path: "src/test.rs".to_string(), | ||
| range: Some(Range { | ||
| start_line: 10, | ||
| start_column: 5, | ||
| end_line: 10, | ||
| end_column: 20, | ||
| ..Default::default() | ||
| }), | ||
| }), | ||
| }], | ||
| }, | ||
| ], | ||
| tags, | ||
| mode: Mode::Block.into(), | ||
| on_added_line: true, | ||
| effort_minutes: 60, | ||
| value: 100, | ||
| ..Default::default() | ||
| }; | ||
|
|
||
| let simple_issue = Issue { | ||
| rule_key: "test-rule-2".to_string(), | ||
| message: "Test message 2".to_string(), | ||
| level: Level::Medium.into(), | ||
| location: Some(Location { | ||
| path: "src/test2.rs".to_string(), | ||
| range: Some(Range { | ||
| start_line: 15, | ||
| start_column: 1, | ||
| end_line: 20, | ||
| end_column: 2, | ||
| ..Default::default() | ||
| }), | ||
| }), | ||
| tool: "test-tool".to_string(), | ||
| language: Language::Typescript.into(), | ||
| category: Category::Lint.into(), | ||
| ..Default::default() | ||
| }; | ||
|
|
||
| let info_message = Message { | ||
| message: "Info message".to_string(), | ||
| details: "Detailed info".to_string(), | ||
| level: MessageLevel::Info.into(), | ||
| module: "test-module".to_string(), | ||
| ty: "INFO".to_string(), | ||
| timestamp: None, | ||
| ..Default::default() | ||
| }; | ||
|
|
||
| let warning_message = Message { | ||
| message: "Warning message".to_string(), | ||
| details: "".to_string(), | ||
| level: MessageLevel::Warning.into(), | ||
| module: "warning-module".to_string(), | ||
| ty: "WARNING".to_string(), | ||
| ..Default::default() | ||
| }; | ||
|
|
||
| let report = Report { | ||
| verb: ExecutionVerb::Check, | ||
| target_mode: TargetMode::default(), | ||
| messages: vec![info_message, warning_message], | ||
| invocations: vec![], | ||
| issues: vec![comprehensive_issue, simple_issue], | ||
| formatted: vec![], | ||
| fixed: HashSet::new(), | ||
| fixable: HashSet::new(), | ||
| counts: IssueCount::default(), | ||
| }; | ||
|
|
||
| let formatter = SarifFormatter::boxed(report); | ||
| let output = formatter.read().unwrap(); | ||
| let output_str = String::from_utf8_lossy(&output); | ||
|
|
||
| let json_value: Value = serde_json::from_str(&output_str).unwrap(); | ||
|
|
||
| insta::assert_json_snapshot!(json_value, { | ||
| ".runs[0].tool.driver.semanticVersion" => "[version]", | ||
| ".runs[0].tool.driver.version" => "[version_string]", | ||
| ".runs[0].tool.driver.releaseDateUtc" => "[date]", | ||
| ".runs[0].results[0].partialFingerprints" => insta::sorted_redaction() | ||
| }); | ||
| } | ||
| } |
Comment on lines
+49
to
+51
| if lang != Language::Unspecified { | ||
| let lang_str = format!("{:?}", lang).to_lowercase(); | ||
| region_obj["sourceLanguage"] = json!(lang_str); |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
qlty checkwith--sarifflagSarifFormatterto generate SARIF 2.1.0 compliant outputTesting
qlty check --sarif [file]to confirm valid SARIF output is generated🤖 Generated with Claude Code