From 2a24892d3df49263a5130c1503e3b27d7a55bc8b Mon Sep 17 00:00:00 2001 From: Eric Silverberg Date: Sun, 13 Sep 2026 14:38:42 -0400 Subject: [PATCH] [GH-8922] Add support for printing only one error per file --- README.md | 24 +++++++++++++++++++++- lib/rubyzen/assertions/assert_zen_empty.rb | 10 ++++++--- lib/rubyzen/assertions/assert_zen_false.rb | 10 ++++++--- lib/rubyzen/assertions/assert_zen_true.rb | 10 ++++++--- lib/rubyzen/expectation_helpers.rb | 24 +++++++++++++++++++--- lib/rubyzen/matchers/zen_empty_matcher.rb | 20 +++++++++++------- lib/rubyzen/matchers/zen_false_matcher.rb | 14 +++++++++---- lib/rubyzen/matchers/zen_true_matcher.rb | 16 ++++++++++----- spec/matchers/zen_empty_matcher_spec.rb | 19 +++++++++++++++++ 9 files changed, 118 insertions(+), 29 deletions(-) diff --git a/README.md b/README.md index 7435ea5..89df455 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,29 @@ Rubyzen provides three checks for your architectural lint rules: | `zen_true { \|item\| }` | `assert_zen_true(collection) { \|item\| }` | the block returns true for every element | | `zen_false { \|item\| }` | `assert_zen_false(collection) { \|item\| }` | the block returns false for every element | -All three accept an `allowlist:` of exceptions that are permanently exempt from the rule, a `baseline:` of known existing violations (technical debt) to fix over time, and a custom failure message. +All three accept: +- `allowlist:` of exceptions that are permanently exempt from the rule +- `baseline:` of known existing violations (technical debt) to fix over time +- `one_per_file:` (Boolean) to display only the first violation per file, useful when generating an initial baseline from many violations +- A custom failure message + +### Compact failure output with `one_per_file` + +When a rule has many violations across many files, the default output can be overwhelming. Use `one_per_file: true` to show only the first violation from each file, making it easier to generate a baseline: + +```ruby +# RSpec +it 'does not assign local variables in it blocks' do + expect(violations).to zen_empty(one_per_file: true) +end + +# Minitest +def test_does_not_assign_local_variables_in_it_blocks + assert_zen_empty(violations, one_per_file: true) +end +``` + +The rule still **fails** when there are multiple violations in a file; the option only limits the diagnostic output for readability. All violations are considered when evaluating baseline and allowlist entries. ## Run your lint rules diff --git a/lib/rubyzen/assertions/assert_zen_empty.rb b/lib/rubyzen/assertions/assert_zen_empty.rb index 1c9d90f..1a9cc76 100644 --- a/lib/rubyzen/assertions/assert_zen_empty.rb +++ b/lib/rubyzen/assertions/assert_zen_empty.rb @@ -10,6 +10,7 @@ module Assertions # @param message [String, nil] optional custom failure message # @param allowlist [Array, nil] items to permanently ignore # @param baseline [Array, nil] known violations for gradual adoption + # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # @return [true] when the assertion passes # @raise [Minitest::Assertion] when there are live violations or stale entries # @@ -18,12 +19,15 @@ module Assertions # # @example With a baseline for gradual adoption # assert_zen_empty(violations, baseline: ['LegacyController']) - def assert_zen_empty(collection, message: nil, allowlist: nil, baseline: nil) + # + # @example Compact output to one per file + # assert_zen_empty(violations, one_per_file: true) + def assert_zen_empty(collection, message: nil, allowlist: nil, baseline: nil, one_per_file: false) @failure_message = nil @custom_message = message - @classified_items = classify_items(collection, allowlist: allowlist, baseline: baseline) + @classified_items = classify_items(collection, allowlist: allowlist, baseline: baseline, one_per_file: one_per_file) - violations = @classified_items[:violations] + violations = @classified_items[:violations_raw] stale_baseline = @classified_items[:stale_baseline] stale_allowlist = @classified_items[:stale_allowlist] diff --git a/lib/rubyzen/assertions/assert_zen_false.rb b/lib/rubyzen/assertions/assert_zen_false.rb index f98d999..25ac623 100644 --- a/lib/rubyzen/assertions/assert_zen_false.rb +++ b/lib/rubyzen/assertions/assert_zen_false.rb @@ -7,6 +7,7 @@ module Assertions # @param message [String, nil] optional custom failure message # @param allowlist [Array, nil] items to permanently ignore # @param baseline [Array, nil] known violations for gradual adoption + # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # @yield [item] block that should return false for each item # @return [true] when the assertion passes # @raise [ArgumentError] when no block is given @@ -17,15 +18,18 @@ module Assertions # # @example With a baseline for gradual adoption # assert_zen_false(classes, baseline: ['LegacyModel']) { |k| k.lines_of_code > 200 } - def assert_zen_false(collection, message: nil, allowlist: nil, baseline: nil, &block) + # + # @example Compact output to one per file + # assert_zen_false(items, one_per_file: true) { |x| x.bad_condition? } + def assert_zen_false(collection, message: nil, allowlist: nil, baseline: nil, one_per_file: false, &block) raise ArgumentError, 'assert_zen_false requires a block' unless block @failure_message = nil @custom_message = message failing_items = Array(collection).filter { |item| block.call(item) } - @classified_items = classify_items(failing_items, allowlist: allowlist, baseline: baseline) + @classified_items = classify_items(failing_items, allowlist: allowlist, baseline: baseline, one_per_file: one_per_file) - violations = @classified_items[:violations] + violations = @classified_items[:violations_raw] stale_baseline = @classified_items[:stale_baseline] stale_allowlist = @classified_items[:stale_allowlist] diff --git a/lib/rubyzen/assertions/assert_zen_true.rb b/lib/rubyzen/assertions/assert_zen_true.rb index 3a60aae..eea0d42 100644 --- a/lib/rubyzen/assertions/assert_zen_true.rb +++ b/lib/rubyzen/assertions/assert_zen_true.rb @@ -7,6 +7,7 @@ module Assertions # @param message [String, nil] optional custom failure message # @param allowlist [Array, nil] items to permanently ignore # @param baseline [Array, nil] known violations for gradual adoption + # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # @yield [item] block that should return true for each item # @return [true] when the assertion passes # @raise [ArgumentError] when no block is given @@ -17,15 +18,18 @@ module Assertions # # @example With a custom failure message # assert_zen_true(services, message: 'All services must inherit from BaseService') { |s| s.superclass_name == 'BaseService' } - def assert_zen_true(collection, message: nil, allowlist: nil, baseline: nil, &block) + # + # @example Compact output to one per file + # assert_zen_true(items, one_per_file: true) { |m| m.valid? } + def assert_zen_true(collection, message: nil, allowlist: nil, baseline: nil, one_per_file: false, &block) raise ArgumentError, 'assert_zen_true requires a block' unless block @failure_message = nil @custom_message = message failing_items = Array(collection).filter { |item| !block.call(item) } - @classified_items = classify_items(failing_items, allowlist: allowlist, baseline: baseline) + @classified_items = classify_items(failing_items, allowlist: allowlist, baseline: baseline, one_per_file: one_per_file) - violations = @classified_items[:violations] + violations = @classified_items[:violations_raw] stale_baseline = @classified_items[:stale_baseline] stale_allowlist = @classified_items[:stale_allowlist] diff --git a/lib/rubyzen/expectation_helpers.rb b/lib/rubyzen/expectation_helpers.rb index 9db3439..611ca1b 100644 --- a/lib/rubyzen/expectation_helpers.rb +++ b/lib/rubyzen/expectation_helpers.rb @@ -66,9 +66,10 @@ def exception_entry_matches_item?(entry, item) # @param subject_collection [Array, Object] items to classify # @param allowlist [Array, nil] allowed exception entries # @param baseline [Array, nil] baseline exception entries + # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # @return [Hash{Symbol => Array}] keys: :violations, :baseline, :allowlist, - # :stale_baseline, :stale_allowlist - def classify_items(subject_collection, allowlist: nil, baseline: nil) + # :stale_baseline, :stale_allowlist, :violations_raw + def classify_items(subject_collection, allowlist: nil, baseline: nil, one_per_file: false) items = Array(subject_collection).compact normalized_allowlist = normalize_exception_entries(allowlist) normalized_baseline = normalize_exception_entries(baseline) @@ -97,10 +98,14 @@ def classify_items(subject_collection, allowlist: nil, baseline: nil) end end + raw_violations = Array(grouped_items[:violations]) + violations_to_display = one_per_file ? filter_one_per_file(raw_violations) : raw_violations + classifications = { baseline: Array(grouped_items[:baseline]).map { |item| element_name(item) }, allowlist: Array(grouped_items[:allowlist]).map { |item| element_name(item) }, - violations: Array(grouped_items[:violations]).map { |item| element_name(item) } + violations: violations_to_display.map { |item| element_name(item) }, + violations_raw: raw_violations } classifications.merge( @@ -109,6 +114,19 @@ def classify_items(subject_collection, allowlist: nil, baseline: nil) ) end + private + + # Filters violations to keep only the first violation per file. + # + # @param violations [Array] raw violation declaration objects + # @return [Array] violations with at most one per file_path + def filter_one_per_file(violations) + violations + .group_by { |item| item_details(item)[:file_path] } + .transform_values(&:first) + .values + end + # Formats a human-readable description of an item for failure messages. # # @param item [Object] a declaration object diff --git a/lib/rubyzen/matchers/zen_empty_matcher.rb b/lib/rubyzen/matchers/zen_empty_matcher.rb index a07740a..f68f346 100644 --- a/lib/rubyzen/matchers/zen_empty_matcher.rb +++ b/lib/rubyzen/matchers/zen_empty_matcher.rb @@ -10,37 +10,43 @@ # # @param custom_message [String, nil] optional failure message # # @param allowlist [Array, nil] items to permanently ignore # # @param baseline [Array, nil] known violations for gradual adoption +# # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # # # # @example Ensure no controllers use .where # # expect(controllers.all_methods.call_sites.with_name('where')).to zen_empty # # # # @example With baseline for gradual adoption # # expect(violations).to zen_empty(baseline: ['LegacyController']) -# def zen_empty(custom_message = nil, allowlist: nil, baseline: nil); end +# # +# # @example Compact output to one per file +# # expect(violations).to zen_empty(one_per_file: true) +# def zen_empty(custom_message = nil, allowlist: nil, baseline: nil, one_per_file: false); end # end # end -RSpec::Matchers.define :zen_empty do |custom_message=nil, allowlist: nil, baseline: nil| +RSpec::Matchers.define :zen_empty do |custom_message=nil, allowlist: nil, baseline: nil, one_per_file: false| include Rubyzen::ExpectationHelpers match do |subject_collection| options = custom_message.is_a?(Hash) ? custom_message : {} resolved_allowlist = allowlist || options[:allowlist] || options['allowlist'] resolved_baseline = baseline || options[:baseline] || options['baseline'] + resolved_one_per_file = one_per_file || options[:one_per_file] || options['one_per_file'] || false @custom_message = options[:message] || options['message'] || (custom_message unless custom_message.is_a?(Hash)) @classified_items = classify_items( subject_collection, allowlist: resolved_allowlist, - baseline: resolved_baseline + baseline: resolved_baseline, + one_per_file: resolved_one_per_file ) - @offenders = @classified_items[:violations] + @offenders = @classified_items[:violations_raw] stale_exception_groups = [] stale_exception_groups << 'baseline entries' if @classified_items[:stale_baseline].any? stale_exception_groups << 'allowlist entries' if @classified_items[:stale_allowlist].any? - @failure_reason = if @classified_items[:violations].any? && stale_exception_groups.any? + @failure_reason = if @classified_items[:violations_raw].any? && stale_exception_groups.any? "Expected to be empty, but found live violations and stale #{stale_exception_groups.join(' and ')}." - elsif @classified_items[:violations].any? + elsif @classified_items[:violations_raw].any? if resolved_baseline || resolved_allowlist 'Expected to be empty, but found live violations.' else @@ -50,7 +56,7 @@ "Expected to be empty, but found stale #{stale_exception_groups.join(' and ')}." end - @classified_items[:violations].empty? && + @classified_items[:violations_raw].empty? && @classified_items[:stale_baseline].empty? && @classified_items[:stale_allowlist].empty? end diff --git a/lib/rubyzen/matchers/zen_false_matcher.rb b/lib/rubyzen/matchers/zen_false_matcher.rb index a362e94..3a43ed9 100644 --- a/lib/rubyzen/matchers/zen_false_matcher.rb +++ b/lib/rubyzen/matchers/zen_false_matcher.rb @@ -9,6 +9,7 @@ # # @param custom_message [String, nil] optional failure message # # @param allowlist [Array, nil] items to permanently ignore # # @param baseline [Array, nil] known violations for gradual adoption +# # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # # @yield [item] block that should return false for each item # # # # @example Ensure no methods have more than 5 parameters @@ -16,16 +17,20 @@ # # # # @example With a baseline for gradual adoption # # expect(classes).to zen_false(baseline: ['LegacyModel']) { |k| k.lines_of_code > 200 } -# def zen_false(custom_message = nil, allowlist: nil, baseline: nil, &block); end +# # +# # @example Compact output to one per file +# # expect(items).to zen_false(one_per_file: true) { |x| x.bad_condition? } +# def zen_false(custom_message = nil, allowlist: nil, baseline: nil, one_per_file: false, &block); end # end # end -RSpec::Matchers.define :zen_false do |custom_message=nil, allowlist: nil, baseline: nil| +RSpec::Matchers.define :zen_false do |custom_message=nil, allowlist: nil, baseline: nil, one_per_file: false| include Rubyzen::ExpectationHelpers match do |subject_collection| options = custom_message.is_a?(Hash) ? custom_message : {} resolved_allowlist = allowlist || options[:allowlist] || options['allowlist'] resolved_baseline = baseline || options[:baseline] || options['baseline'] + resolved_one_per_file = one_per_file || options[:one_per_file] || options['one_per_file'] || false @custom_message = options[:message] || options['message'] || (custom_message unless custom_message.is_a?(Hash)) @offenders = [] @@ -36,9 +41,10 @@ @classified_items = classify_items( failing_items, allowlist: resolved_allowlist, - baseline: resolved_baseline + baseline: resolved_baseline, + one_per_file: resolved_one_per_file ) - @offenders = @classified_items[:violations] + @offenders = @classified_items[:violations_raw] stale_exception_groups = [] stale_baseline = @classified_items[:stale_baseline] diff --git a/lib/rubyzen/matchers/zen_true_matcher.rb b/lib/rubyzen/matchers/zen_true_matcher.rb index 5a771f1..70067c2 100644 --- a/lib/rubyzen/matchers/zen_true_matcher.rb +++ b/lib/rubyzen/matchers/zen_true_matcher.rb @@ -6,6 +6,7 @@ # # @param custom_message [String, nil] optional failure message # # @param allowlist [Array, nil] items to permanently ignore # # @param baseline [Array, nil] known violations for gradual adoption +# # @param one_per_file [Boolean] if true, displays only the first violation per file (default: false) # # @yield [item] block that should return true for each item # # # # @example Ensure all methods have parameters @@ -13,29 +14,34 @@ # # # # @example With a custom failure message # # expect(services).to zen_true("All services must inherit from BaseService") { |s| s.superclass_name == 'BaseService' } -# def zen_true(custom_message = nil, allowlist: nil, baseline: nil, &block); end +# # +# # @example Compact output to one per file +# # expect(items).to zen_true(one_per_file: true) { |m| m.valid? } +# def zen_true(custom_message = nil, allowlist: nil, baseline: nil, one_per_file: false, &block); end # end # end -RSpec::Matchers.define :zen_true do |custom_message=nil, allowlist: nil, baseline: nil| +RSpec::Matchers.define :zen_true do |custom_message=nil, allowlist: nil, baseline: nil, one_per_file: false| include Rubyzen::ExpectationHelpers match do |subject_collection| options = custom_message.is_a?(Hash) ? custom_message : {} resolved_allowlist = allowlist || options[:allowlist] || options['allowlist'] resolved_baseline = baseline || options[:baseline] || options['baseline'] + resolved_one_per_file = one_per_file || options[:one_per_file] || options['one_per_file'] || false @custom_message = options[:message] || options['message'] || (custom_message unless custom_message.is_a?(Hash)) @offenders = [] if block_arg != nil - items = Array(subject_collection) # to handle one or multiple subjects + items = Array(subject_collection) failing_items = items.filter { |item| !block_arg.call(item) } @classified_items = classify_items( failing_items, allowlist: resolved_allowlist, - baseline: resolved_baseline + baseline: resolved_baseline, + one_per_file: resolved_one_per_file ) - @offenders = @classified_items[:violations] + @offenders = @classified_items[:violations_raw] stale_exception_groups = [] stale_baseline = @classified_items[:stale_baseline] diff --git a/spec/matchers/zen_empty_matcher_spec.rb b/spec/matchers/zen_empty_matcher_spec.rb index 53c420f..c2c0919 100644 --- a/spec/matchers/zen_empty_matcher_spec.rb +++ b/spec/matchers/zen_empty_matcher_spec.rb @@ -27,6 +27,25 @@ expect(non_empty_collection).not_to zen_empty end + describe 'with one_per_file' do + let(:classes) do + file = parse_ruby(<<~RUBY, file_path: '/app/controllers/foo_controller.rb') + class FooController; end + class BarController; end + RUBY + Rubyzen::Collections::ClassesCollection.new(file.classes) + end + + it 'displays only the first violation from each file' do + expect { + expect(classes).to zen_empty(one_per_file: true) + }.to raise_error(RSpec::Expectations::ExpectationNotMetError) { |error| + expect(error.message).to include('FooController') + expect(error.message).not_to include('BarController') + } + end + end + describe 'with allowlist' do let(:classes) do file = parse_ruby(<<~RUBY, file_path: '/app/controllers/foo_controller.rb')