From 2631963f6f955a462381f9601a7040033c3ae32b Mon Sep 17 00:00:00 2001
From: Peter Leitzen <pleitzen@gitlab.com>
Date: Thu, 13 Oct 2022 11:05:14 +0200
Subject: [PATCH] Generate RuboCop TODO config even without any offenses
Sometimes we enable new cop rules without any offenses and put this rule
in "grace period". This is done in the TODO config in
`.rubocop_todo/**/*.yml` because it's a temporary configuration which
should be removed short after.
However, when regenerating TODOs for cops without any offenses we
discarded to generate them.
This commit fixes this issue and generates a cop TODO config if:
* Cop was previously disabled due to too many offenses
* Cop is in grace period
* Cop excludes files
---
rubocop/cop_todo.rb | 12 +++-
rubocop/formatter/todo_formatter.rb | 26 +++++--
spec/rubocop/cop_todo_spec.rb | 33 ++++++++-
spec/rubocop/formatter/todo_formatter_spec.rb | 68 +++++++++++++++++--
4 files changed, 125 insertions(+), 14 deletions(-)
diff --git a/rubocop/cop_todo.rb b/rubocop/cop_todo.rb
index a36afc08673a3..91f5e302d5700 100644
--- a/rubocop/cop_todo.rb
+++ b/rubocop/cop_todo.rb
@@ -26,6 +26,10 @@ def autocorrectable?
@cop_class&.support_autocorrect?
end
+ def generate?
+ previously_disabled || grace_period || files.any?
+ end
+
def to_yaml
yaml = []
yaml << '---'
@@ -39,8 +43,12 @@ def to_yaml
end
yaml << " #{RuboCop::Formatter::GracefulFormatter.grace_period_key_value}" if grace_period
- yaml << ' Exclude:'
- yaml.concat files.sort.map { |file| " - '#{file}'" }
+
+ if files.any?
+ yaml << ' Exclude:'
+ yaml.concat files.sort.map { |file| " - '#{file}'" }
+ end
+
yaml << ''
yaml.join("\n")
diff --git a/rubocop/formatter/todo_formatter.rb b/rubocop/formatter/todo_formatter.rb
index b1c6d1c1688b2..5e49e2dc08241 100644
--- a/rubocop/formatter/todo_formatter.rb
+++ b/rubocop/formatter/todo_formatter.rb
@@ -31,6 +31,7 @@ def initialize(output, _options = {})
@config_inspect_todo_dir = load_config_inspect_todo_dir
@config_old_todo_yml = load_config_old_todo_yml
check_multiple_configurations!
+ create_empty_todos(@config_inspect_todo_dir)
super
end
@@ -47,11 +48,9 @@ def file_finished(file, offenses)
def finished(_inspected_files)
@todos.values.sort_by(&:cop_name).each do |todo|
- todo.previously_disabled = previously_disabled?(todo)
- todo.grace_period = grace_period?(todo)
- validate_todo!(todo)
- path = @todo_dir.write(todo.cop_name, todo.to_yaml)
+ next unless configure_and_validate_todo(todo)
+ path = @todo_dir.write(todo.cop_name, todo.to_yaml)
output.puts "Written to #{relative_path(path)}\n"
end
end
@@ -82,6 +81,14 @@ def check_multiple_configurations!
raise "Multiple configurations found for cops:\n#{list}\n"
end
+ # For each inspected cop TODO config create a TODO object to make sure
+ # the cop TODO config will be written even without any offenses.
+ def create_empty_todos(inspected_cop_config)
+ inspected_cop_config.each_key do |cop_name|
+ @todos[cop_name]
+ end
+ end
+
def config_for(todo)
cop_name = todo.cop_name
@@ -101,10 +108,15 @@ def grace_period?(todo)
GracefulFormatter.grace_period?(todo.cop_name, config)
end
- def validate_todo!(todo)
- return unless todo.previously_disabled && todo.grace_period
+ def configure_and_validate_todo(todo)
+ todo.previously_disabled = previously_disabled?(todo)
+ todo.grace_period = grace_period?(todo)
+
+ if todo.previously_disabled && todo.grace_period
+ raise "#{todo.cop_name}: Cop must be enabled to use `#{GracefulFormatter.grace_period_key_value}`."
+ end
- raise "#{todo.cop_name}: Cop must be enabled to use `#{GracefulFormatter.grace_period_key_value}`."
+ todo.generate?
end
def load_config_inspect_todo_dir
diff --git a/spec/rubocop/cop_todo_spec.rb b/spec/rubocop/cop_todo_spec.rb
index 3f9c378b3036e..b02cd6ce5c8a5 100644
--- a/spec/rubocop/cop_todo_spec.rb
+++ b/spec/rubocop/cop_todo_spec.rb
@@ -66,6 +66,38 @@
end
end
+ describe '#generate?' do
+ subject { cop_todo.generate? }
+
+ context 'when empty todo' do
+ it { is_expected.to eq(false) }
+ end
+
+ context 'when previously disabled' do
+ before do
+ cop_todo.previously_disabled = true
+ end
+
+ it { is_expected.to eq(true) }
+ end
+
+ context 'when in grace period' do
+ before do
+ cop_todo.grace_period = true
+ end
+
+ it { is_expected.to eq(true) }
+ end
+
+ context 'with offenses recorded' do
+ before do
+ cop_todo.record('a.rb', 1)
+ end
+
+ it { is_expected.to eq(true) }
+ end
+ end
+
describe '#to_yaml' do
subject(:yaml) { cop_todo.to_yaml }
@@ -79,7 +111,6 @@
---
# Cop supports --auto-correct.
#{cop_name}:
- Exclude:
YAML
end
end
diff --git a/spec/rubocop/formatter/todo_formatter_spec.rb b/spec/rubocop/formatter/todo_formatter_spec.rb
index edd846324096a..152987beefb15 100644
--- a/spec/rubocop/formatter/todo_formatter_spec.rb
+++ b/spec/rubocop/formatter/todo_formatter_spec.rb
@@ -309,18 +309,78 @@ def run_formatter
context 'without offenses detected' do
before do
+ todo_dir.write('A/Cop', yaml) if yaml
+ todo_dir.inspect_all
+
formatter.started(%w[a.rb b.rb])
formatter.file_finished('a.rb', [])
formatter.file_finished('b.rb', [])
formatter.finished(%w[a.rb b.rb])
+
+ todo_dir.delete_inspected
end
- it 'does not output anything' do
- expect(stdout.string).to eq('')
+ context 'without existing TODOs' do
+ let(:yaml) { nil }
+
+ it 'does not output anything' do
+ expect(stdout.string).to eq('')
+ end
+
+ it 'does not write any YAML files' do
+ expect(rubocop_todo_dir_listing).to be_empty
+ end
end
- it 'does not write any YAML files' do
- expect(rubocop_todo_dir_listing).to be_empty
+ context 'with existing TODOs' do
+ context 'when existing offenses only' do
+ let(:yaml) do
+ <<~YAML
+ ---
+ A/Cop:
+ Exclude:
+ - x.rb
+ YAML
+ end
+
+ it 'does not output anything' do
+ expect(stdout.string).to eq('')
+ end
+
+ it 'does not write any YAML files' do
+ expect(rubocop_todo_dir_listing).to be_empty
+ end
+ end
+
+ context 'when in grace period' do
+ let(:yaml) do
+ <<~YAML
+ ---
+ A/Cop:
+ Details: grace period
+ Exclude:
+ - x.rb
+ YAML
+ end
+
+ it 'outputs its actions' do
+ expect(stdout.string).to eq(<<~OUTPUT)
+ Written to .rubocop_todo/a/cop.yml
+ OUTPUT
+ end
+
+ it 'creates YAML file with Details only', :aggregate_failures do
+ expect(rubocop_todo_dir_listing).to contain_exactly(
+ 'a/cop.yml'
+ )
+
+ expect(todo_yml('A/Cop')).to eq(<<~YAML)
+ ---
+ A/Cop:
+ Details: grace period
+ YAML
+ end
+ end
end
end
--
GitLab