From 34e703d26dd2adfa4f5d6c5561fe6f5b47592a32 Mon Sep 17 00:00:00 2001
From: Kassio Borges <kborges@gitlab.com>
Date: Thu, 19 Oct 2023 21:53:26 +0000
Subject: [PATCH] Use Danger to notify about outdated rubocop todos
When removing files, contributors might forget to remove the related
rubocop todos. To avoid that, this new Danger plugin will add comments
on MRs on deleted files that still have related rubocop todos.
Changelog: added
---
danger/plugins/todos.rb | 11 +++
danger/todos/Dangerfile | 3 +
.../tooling/danger/rubocop_todo/cop1.yml | 5 ++
.../tooling/danger/rubocop_todo/cop2.yml | 4 +
spec/tooling/danger/outdated_todo_spec.rb | 83 +++++++++++++++++++
tooling/danger/outdated_todo.rb | 58 +++++++++++++
6 files changed, 164 insertions(+)
create mode 100644 danger/plugins/todos.rb
create mode 100644 danger/todos/Dangerfile
create mode 100644 spec/fixtures/tooling/danger/rubocop_todo/cop1.yml
create mode 100644 spec/fixtures/tooling/danger/rubocop_todo/cop2.yml
create mode 100644 spec/tooling/danger/outdated_todo_spec.rb
create mode 100644 tooling/danger/outdated_todo.rb
diff --git a/danger/plugins/todos.rb b/danger/plugins/todos.rb
new file mode 100644
index 0000000000000..b31f147f2af28
--- /dev/null
+++ b/danger/plugins/todos.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+require_relative '../../tooling/danger/outdated_todo'
+
+module Danger
+ class Todos < ::Danger::Plugin
+ def check_outdated_todos(filenames)
+ Tooling::Danger::OutdatedTodo.new(filenames, context: self).check
+ end
+ end
+end
diff --git a/danger/todos/Dangerfile b/danger/todos/Dangerfile
new file mode 100644
index 0000000000000..45494e59bacfa
--- /dev/null
+++ b/danger/todos/Dangerfile
@@ -0,0 +1,3 @@
+# frozen_string_literal: true
+
+todos.check_outdated_todos(git.deleted_files)
diff --git a/spec/fixtures/tooling/danger/rubocop_todo/cop1.yml b/spec/fixtures/tooling/danger/rubocop_todo/cop1.yml
new file mode 100644
index 0000000000000..8f240b926823a
--- /dev/null
+++ b/spec/fixtures/tooling/danger/rubocop_todo/cop1.yml
@@ -0,0 +1,5 @@
+---
+Cop1:
+ Exclude:
+ - 'app/controllers/application_controller.rb'
+ - 'app/controllers/acme_challenges_controller.rb'
diff --git a/spec/fixtures/tooling/danger/rubocop_todo/cop2.yml b/spec/fixtures/tooling/danger/rubocop_todo/cop2.yml
new file mode 100644
index 0000000000000..9ab2c0dabb9b9
--- /dev/null
+++ b/spec/fixtures/tooling/danger/rubocop_todo/cop2.yml
@@ -0,0 +1,4 @@
+---
+Cop2:
+ Exclude:
+ - 'app/controllers/application_controller.rb'
diff --git a/spec/tooling/danger/outdated_todo_spec.rb b/spec/tooling/danger/outdated_todo_spec.rb
new file mode 100644
index 0000000000000..3a3909c69ac2c
--- /dev/null
+++ b/spec/tooling/danger/outdated_todo_spec.rb
@@ -0,0 +1,83 @@
+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'gitlab/dangerfiles/spec_helper'
+
+require_relative '../../../tooling/danger/outdated_todo'
+
+RSpec.describe Tooling::Danger::OutdatedTodo, feature_category: :tooling do
+ let(:fake_danger) { double }
+ let(:filenames) { ['app/controllers/application_controller.rb'] }
+
+ let(:todos) do
+ [
+ File.join('spec', 'fixtures', 'tooling', 'danger', 'rubocop_todo', '**', '*.yml')
+ ]
+ end
+
+ subject(:plugin) { described_class.new(filenames, context: fake_danger, todos: todos) }
+
+ context 'when the filenames are mentioned in single todo' do
+ let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] }
+
+ it 'warns about mentions' do
+ expect(fake_danger)
+ .to receive(:warn)
+ .with <<~MESSAGE
+ `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in:
+ - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5`
+ MESSAGE
+
+ plugin.check
+ end
+ end
+
+ context 'when the filenames are mentioned in multiple todos' do
+ let(:filenames) do
+ [
+ 'app/controllers/application_controller.rb',
+ 'app/controllers/acme_challenges_controller.rb'
+ ]
+ end
+
+ it 'warns about mentions' do
+ expect(fake_danger)
+ .to receive(:warn)
+ .with(<<~FIRSTMESSAGE)
+ `app/controllers/application_controller.rb` was removed but is mentioned in:
+ - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:4`
+ - `spec/fixtures/tooling/danger/rubocop_todo/cop2.yml:4`
+ FIRSTMESSAGE
+
+ expect(fake_danger)
+ .to receive(:warn)
+ .with(<<~SECONDMESSAGE)
+ `app/controllers/acme_challenges_controller.rb` was removed but is mentioned in:
+ - `spec/fixtures/tooling/danger/rubocop_todo/cop1.yml:5`
+ SECONDMESSAGE
+
+ plugin.check
+ end
+ end
+
+ context 'when the filenames are not mentioned in todos' do
+ let(:filenames) { ['any/inexisting/file.rb'] }
+
+ it 'does not warn' do
+ expect(fake_danger).not_to receive(:warn)
+
+ plugin.check
+ end
+ end
+
+ context 'when there is no todos' do
+ let(:filenames) { ['app/controllers/acme_challenges_controller.rb'] }
+ let(:todos) { [] }
+
+ it 'does not warn' do
+ expect(fake_danger).not_to receive(:warn)
+
+ plugin.check
+ end
+ end
+end
diff --git a/tooling/danger/outdated_todo.rb b/tooling/danger/outdated_todo.rb
new file mode 100644
index 0000000000000..a5f5cc897a9ae
--- /dev/null
+++ b/tooling/danger/outdated_todo.rb
@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+module Tooling
+ module Danger
+ class OutdatedTodo
+ TODOS_GLOBS = %w[
+ .rubocop_todo/**/*.yml
+ spec/support/rspec_order_todo.yml
+ ].freeze
+
+ def initialize(filenames, context:, todos: TODOS_GLOBS)
+ @filenames = filenames
+ @context = context
+ @todos_globs = todos
+ end
+
+ def check
+ filenames.each do |filename|
+ check_filename(filename)
+ end
+ end
+
+ private
+
+ attr_reader :filenames, :context
+
+ def check_filename(filename)
+ mentions = all_mentions_for(filename)
+
+ return if mentions.empty?
+
+ context.warn <<~MESSAGE
+ `#{filename}` was removed but is mentioned in:
+ #{mentions.join("\n")}
+ MESSAGE
+ end
+
+ def all_mentions_for(filename)
+ todos
+ .filter_map { |todo| mentioned_lines(filename, todo) }
+ .flatten
+ .map { |todo| "- `#{todo}`" }
+ end
+
+ def mentioned_lines(filename, todo)
+ File
+ .foreach(todo)
+ .with_index(1)
+ .select { |text, _line| text.match?(/.*#{filename}.*/) }
+ .map { |_text, line| "#{todo}:#{line}" }
+ end
+
+ def todos
+ @todos ||= @todos_globs.flat_map { |value| Dir.glob(value) }
+ end
+ end
+ end
+end
--
GitLab