From 35da2baaa2a0dfae89f069ef21971b779e2896eb Mon Sep 17 00:00:00 2001 From: Peter Leitzen <pleitzen@gitlab.com> Date: Tue, 11 Feb 2025 07:35:01 +0100 Subject: [PATCH] RuboCop: Auto-enable grace period for new/increased violations When generating TODOs, automatically enable grace period for RuboCop rules that are: - Newly added - Have an increased number of violations This helps manage the gradual adoption of code style rules while preventing immediate enforcement of rules with growing violations. --- doc/development/rubocop_development_guide.md | 7 ++++++- rubocop/formatter/todo_formatter.rb | 12 +++++++++++- spec/rubocop/formatter/todo_formatter_spec.rb | 5 +++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/doc/development/rubocop_development_guide.md b/doc/development/rubocop_development_guide.md index e4dbd4f845e9..3ad0f2d43afe 100644 --- a/doc/development/rubocop_development_guide.md +++ b/doc/development/rubocop_development_guide.md @@ -106,6 +106,12 @@ On the default branch, offenses from cops in the [grace period](rake_tasks.md#ru A grace period can safely be lifted as soon as there are no warnings for 1 week in the `#f_rubocop` channel on Slack. +When [generating TODOs](rake_tasks.md#generate-initial-rubocop-todo-list), RuboCop cop rules are placed in a grace period under the following conditions: + +- The rule was previously in a grace period +- The rule is newly added +- The number of violations for the rule has increased since the last generation + ## Proposing a new cop or cop change If you want to make a proposal to enforce a new cop or change existing cop configuration use the @@ -120,7 +126,6 @@ a structured way of communicating the consequences of the new rule. 1. Enable the new cop in `.rubocop.yml` (if not already done via [`gitlab-styles`](https://gitlab.com/gitlab-org/ruby/gems/gitlab-styles)). 1. [Generate TODOs for the new cop](rake_tasks.md#generate-initial-rubocop-todo-list). -1. [Set the new cop to `grace period`](#cop-grace-period). 1. Create an issue to fix TODOs and encourage community contributions (via ~"quick win" and/or ~"Seeking community contributions"). [See some examples](https://gitlab.com/gitlab-org/gitlab/-/issues/?sort=created_date&state=opened&label_name%5B%5D=quick%20win&label_name%5B%5D=static%20code%20analysis&first_page_size=20). 1. Create an issue to remove `grace period` after 1 week of silence in the `#f_rubocop` Slack channel. [See an example](https://gitlab.com/gitlab-org/gitlab/-/issues/374903). diff --git a/rubocop/formatter/todo_formatter.rb b/rubocop/formatter/todo_formatter.rb index ee81c5daf064..9f2902980aef 100644 --- a/rubocop/formatter/todo_formatter.rb +++ b/rubocop/formatter/todo_formatter.rb @@ -104,9 +104,19 @@ def grace_period?(todo) GracefulFormatter.grace_period?(todo.cop_name, config) end + def todos_increased?(todo) + return false if todo.previously_disabled + + config = config_for(todo) + before = (config['Exclude'] || []).size + after = todo.files.size + + after > before + end + def configure_and_validate_todo(todo) todo.previously_disabled = previously_disabled?(todo) - todo.grace_period = grace_period?(todo) + todo.grace_period = grace_period?(todo) || todos_increased?(todo) if todo.previously_disabled && todo.grace_period raise "#{todo.cop_name}: Cop must be enabled to use `#{GracefulFormatter.grace_period_key_value}`." diff --git a/spec/rubocop/formatter/todo_formatter_spec.rb b/spec/rubocop/formatter/todo_formatter_spec.rb index 146dc31518dc..b65717391936 100644 --- a/spec/rubocop/formatter/todo_formatter_spec.rb +++ b/spec/rubocop/formatter/todo_formatter_spec.rb @@ -77,6 +77,7 @@ def run_formatter expect(todo_yml('A/Offense')).to eq(<<~YAML) --- A/Offense: + Details: grace period Exclude: - 'a.rb' YAML @@ -85,6 +86,7 @@ def run_formatter --- # Cop supports --autocorrect. B/AutoCorrect: + Details: grace period Exclude: - 'd.rb' YAML @@ -92,6 +94,7 @@ def run_formatter expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) --- B/TooManyOffenses: + Details: grace period Exclude: - 'a.rb' - 'c.rb' @@ -164,6 +167,7 @@ def run_formatter expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) --- B/TooManyOffenses: + Details: grace period Exclude: - 'a.rb' - 'c.rb' @@ -252,6 +256,7 @@ def run_formatter expect(todo_yml('B/TooManyOffenses')).to eq(<<~YAML) --- B/TooManyOffenses: + Details: grace period Exclude: - 'a.rb' - 'c.rb' -- GitLab