From 20e30e9d3dc8bba3cfdaf984255a5c9d8003fbf0 Mon Sep 17 00:00:00 2001 From: Peter Leitzen <pleitzen@gitlab.com> Date: Fri, 5 Apr 2024 18:35:45 +0000 Subject: [PATCH] Danger: Reminder to revisit RuboCop documentation on new TODOs --- danger/plugins/rubocop.rb | 3 +- danger/rubocop/Dangerfile | 1 + spec/tooling/danger/rubocop_new_todo_spec.rb | 44 ++++++++++++++++++++ tooling/danger/rubocop_helper.rb | 20 ++++++--- tooling/danger/rubocop_new_todo.rb | 21 ++++++++++ 5 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 spec/tooling/danger/rubocop_new_todo_spec.rb create mode 100644 tooling/danger/rubocop_new_todo.rb diff --git a/danger/plugins/rubocop.rb b/danger/plugins/rubocop.rb index d1f63f356440..ab60228ccd97 100644 --- a/danger/plugins/rubocop.rb +++ b/danger/plugins/rubocop.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true -require_relative '../../tooling/danger/rubocop_inline_disable_suggestion' require_relative '../../tooling/danger/rubocop_discourage_todo_addition' +require_relative '../../tooling/danger/rubocop_inline_disable_suggestion' +require_relative '../../tooling/danger/rubocop_new_todo' require_relative '../../tooling/danger/rubocop_helper' module Danger diff --git a/danger/rubocop/Dangerfile b/danger/rubocop/Dangerfile index 86f8e56f0004..8e193bd59b62 100644 --- a/danger/rubocop/Dangerfile +++ b/danger/rubocop/Dangerfile @@ -5,3 +5,4 @@ return if helper.revert_mr? && !helper.stable_branch? rubocop.execute_inline_disable_suggestor rubocop.execute_todo_suggestor +rubocop.execute_new_todo_reminder diff --git a/spec/tooling/danger/rubocop_new_todo_spec.rb b/spec/tooling/danger/rubocop_new_todo_spec.rb new file mode 100644 index 000000000000..813bded9a89a --- /dev/null +++ b/spec/tooling/danger/rubocop_new_todo_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require 'gitlab/dangerfiles/spec_helper' + +require_relative '../../../tooling/danger/rubocop_new_todo' +require_relative '../../../tooling/danger/project_helper' + +RSpec.describe Tooling::Danger::RubocopNewTodo, feature_category: :tooling do + include_context "with dangerfile" + + let(:filename) { 'spec/foo_spec.rb' } + let(:fake_danger) { DangerSpecHelper.fake_danger } + let(:fake_project_helper) { instance_double('Tooling::Danger::ProjectHelper') } + let(:context) { fake_danger.new(helper: fake_helper) } + + let(:template) { described_class::SUGGESTION } + + let(:file_lines) do + <<~RUBY.split("\n") + --- + A/Rule/Name: + Details: grace period + Exclude: + - 'foo_spec.rb' + RUBY + end + + let(:changed_lines) { file_lines.map { |line| "+#{line}" } } + + subject(:new_todo) { described_class.new(filename, context: context) } + + before do + allow(context).to receive(:project_helper).and_return(fake_project_helper) + allow(context.helper).to receive(:changed_lines).with(filename).and_return(changed_lines) + allow(context.project_helper).to receive(:file_lines).and_return(file_lines) + end + + it 'adds comment once' do + expect(context).to receive(:markdown).with("\n#{template}".chomp, file: filename, line: 2) + + new_todo.suggest + end +end diff --git a/tooling/danger/rubocop_helper.rb b/tooling/danger/rubocop_helper.rb index 9104e17bf6f4..b46808116fb0 100644 --- a/tooling/danger/rubocop_helper.rb +++ b/tooling/danger/rubocop_helper.rb @@ -22,11 +22,19 @@ def execute_todo_suggestor # we'll assume a todo file modification only means regeneration/gem update/etc return if contains_rubocop_update_files? || only_rubocop_todo_files? - modified_rubocop_todo_files.each do |filename| + rubocop_todo_files(helper.modified_files).each do |filename| add_todo_suggestion_for(filename) end end + def execute_new_todo_reminder + return if helper.draft_mr? + + rubocop_todo_files(helper.added_files).each do |filename| + add_new_rubocop_reminder(filename) + end + end + private def contains_rubocop_update_files? @@ -39,10 +47,8 @@ def only_rubocop_todo_files? helper.all_changed_files.none? { |path| path !~ %r{\A\.rubocop_todo/.*/\w+.yml\b} } end - def modified_rubocop_todo_files - files = helper.modified_files - - files.select { |path| path =~ %r{\A\.rubocop_todo/.*/\w+.yml\b} } + def rubocop_todo_files(files) + files.grep(%r{\A\.rubocop_todo/.*/\w+.yml\b}) end def add_todo_suggestion_for(filename) @@ -52,6 +58,10 @@ def add_todo_suggestion_for(filename) def add_inline_disable_suggestions_for(filename) Tooling::Danger::RubocopInlineDisableSuggestion.new(filename, context: self).suggest end + + def add_new_rubocop_reminder(filename) + Tooling::Danger::RubocopNewTodo.new(filename, context: self).suggest + end end end end diff --git a/tooling/danger/rubocop_new_todo.rb b/tooling/danger/rubocop_new_todo.rb new file mode 100644 index 000000000000..98cd5ea97101 --- /dev/null +++ b/tooling/danger/rubocop_new_todo.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require_relative 'suggestion' + +module Tooling + module Danger + class RubocopNewTodo < Suggestion + # For example: `Gitlab/DocUrl:`. + MATCH = %r{^\+\w+/.*:} + REPLACEMENT = nil + + SUGGESTION = <<~MARKDOWN + Please review RuboCop documentation related to [Enabling a new cop](https://docs.gitlab.com/ee/development/rubocop_development_guide.html#enabling-a-new-cop) and ensure you have followed all of the steps before resolving this comment. + + ---- + + [Improve this message](https://gitlab.com/gitlab-org/gitlab/-/blob/master/tooling/danger/rubocop_new_todo.rb). + MARKDOWN + end + end +end -- GitLab