feat: add baseline support for tracking and suppressing existing violations#2154
feat: add baseline support for tracking and suppressing existing violations#2154
Conversation
…ations Baselines allow you to track existing violations in a codebase and only report new violations. This enables: - Adopting sqruff in existing projects without fixing all violations first - Preventing regressions while allowing legacy issues to be addressed over time - Enabling incremental improvement of code quality Usage: 1. Generate a baseline: sqruff baseline <paths...> -o .sqruff-baseline 2. Lint using the baseline (only report new violations): sqruff lint <paths...> --baseline .sqruff-baseline The baseline uses a count-based approach (similar to elm-review and ESLint), tracking how many violations of each rule type exist per file. This is more stable than line-number-based approaches because it doesn't get invalidated when unrelated code changes shift line numbers.
Benchmark for 727f950Click to view benchmark
|
There was a problem hiding this comment.
Pull request overview
This PR adds baseline support for tracking and suppressing existing violations in a codebase, enabling incremental adoption of sqruff without requiring all violations to be fixed upfront. The baseline uses a count-based approach (similar to elm-review and ESLint) that tracks violation counts per rule per file, which is more stable than line-number-based approaches.
Changes:
- Added new
baselinemodule with serialization, filtering, and comparison logic for violation baselines - Added
sqruff baselinecommand to generate baseline files from existing violations - Extended
sqruff lintcommand with--baselineflag to filter violations against a baseline - Added comprehensive integration tests for baseline generation and lint filtering
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/cli-lib/src/baseline.rs | New module implementing baseline data structures, serialization/deserialization, and filtering logic |
| crates/cli-lib/src/commands_baseline.rs | New command handler for generating baselines from paths or stdin |
| crates/cli-lib/src/commands_lint.rs | Extended lint command to support baseline filtering with custom formatter creation and violation filtering |
| crates/cli-lib/src/commands.rs | Added BaselineArgs and baseline field to LintArgs for CLI argument parsing |
| crates/cli-lib/src/lib.rs | Integrated baseline command into main command dispatch logic |
| crates/cli/tests/baseline.rs | Added comprehensive integration tests for baseline generation and lint filtering scenarios |
| crates/cli/Cargo.toml | Added test configuration and serde_json dev-dependency |
| Cargo.lock | Updated dependencies with serde_json |
| docs/cli.md | Generated documentation for new baseline command and --baseline option |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| /// Run baseline generation from stdin. | ||
| pub(crate) fn run_baseline_stdin(config: FluffConfig, output: Option<std::path::PathBuf>) -> i32 { |
There was a problem hiding this comment.
The run_baseline_stdin function is missing the collect_parse_errors parameter. This function should accept this parameter like run_baseline does (line 16) to maintain API consistency, and it should be passed from the caller at line 140 in lib.rs.
| fn create_formatter(format: Format, config: &FluffConfig) -> Arc<dyn Formatter> { | ||
| match format { | ||
| Format::Human => { | ||
| let output_stream = std::io::stderr().into(); | ||
| let formatter = OutputStreamFormatter::new( | ||
| output_stream, | ||
| config.get("nocolor", "core").as_bool().unwrap_or_default(), | ||
| config.get("verbose", "core").as_int().unwrap_or_default(), | ||
| ); | ||
| Arc::new(formatter) | ||
| } | ||
| Format::GithubAnnotationNative => { | ||
| let output_stream = std::io::stderr(); | ||
| let formatter = GithubAnnotationNativeFormatter::new(output_stream); | ||
| Arc::new(formatter) | ||
| } | ||
| Format::Json => { | ||
| let formatter = JsonFormatter::default(); | ||
| Arc::new(formatter) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The create_formatter function duplicates the formatter creation logic from the linter function in lib.rs (lines 148-167). Consider extracting this into a shared utility function to reduce code duplication and ensure consistency when formatter creation logic changes.
| fn main() { | ||
| test_baseline_generation(); | ||
| test_baseline_lint_suppresses_existing(); | ||
| test_baseline_lint_detects_new_violations(); | ||
| test_baseline_lint_reports_fixed_violations(); | ||
| test_baseline_file_not_found(); | ||
| } |
There was a problem hiding this comment.
This test file uses a non-standard test structure with a main function calling individual test functions. While this pattern works with harness = false, it means test failures won't be properly reported - if one test fails, subsequent tests won't run. Consider using proper Rust test attributes (#[test]) or at least add error handling to ensure all tests run even if one fails.
| }; | ||
|
|
||
| // Create a linter WITHOUT a formatter (we don't want to output violations during baseline generation) | ||
| let linter = Linter::new(config, None, None, false); |
There was a problem hiding this comment.
The collect_parse_errors parameter is not used when creating the Linter on line 78. It's hardcoded to false. This should use the parameter passed from the caller to maintain consistency with the non-stdin baseline generation path (line 21 in run_baseline).
2 participants
Baselines allow you to track existing violations in a codebase and only
report new violations. This enables:
Usage:
Generate a baseline:
sqruff baseline <paths...> -o .sqruff-baseline
Lint using the baseline (only report new violations):
sqruff lint <paths...> --baseline .sqruff-baseline
The baseline uses a count-based approach (similar to elm-review and ESLint),
tracking how many violations of each rule type exist per file. This is more
stable than line-number-based approaches because it doesn't get invalidated
when unrelated code changes shift line numbers.