From f7b456da42e320c977ce9b42c9f9a667f918693f Mon Sep 17 00:00:00 2001 From: Andreas Brandl <abrandl@gitlab.com> Date: Wed, 25 Aug 2021 14:15:18 +0200 Subject: [PATCH] Cop for database migration class --- .../migration/versioned_migration_class.rb | 55 +++++++++++++++ .../versioned_migration_class_spec.rb | 69 +++++++++++++++++++ 2 files changed, 124 insertions(+) create mode 100644 rubocop/cop/migration/versioned_migration_class.rb create mode 100644 spec/rubocop/cop/migration/versioned_migration_class_spec.rb diff --git a/rubocop/cop/migration/versioned_migration_class.rb b/rubocop/cop/migration/versioned_migration_class.rb new file mode 100644 index 0000000000000..33debf716b408 --- /dev/null +++ b/rubocop/cop/migration/versioned_migration_class.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require_relative '../../migration_helpers' + +module RuboCop + module Cop + module Migration + class VersionedMigrationClass < RuboCop::Cop::Cop + include MigrationHelpers + + ENFORCED_SINCE = 2021_08_30_00_00_00 + + MSG_INHERIT = 'Don\'t inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead.' + MSG_INCLUDE = 'Don\'t include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead.' + + MIGRATION_CLASS = 'Gitlab::Database::Migration' + + def_node_search :includes_helpers?, <<~PATTERN + (send nil? :include + (const + (const + (const nil? :Gitlab) :Database) :MigrationHelpers)) + PATTERN + + def on_class(node) + return unless relevant_migration?(node) + + add_offense(node, location: :expression, message: MSG_INHERIT) unless gitlab_migration_class?(node) + end + + def on_send(node) + return unless relevant_migration?(node) + + add_offense(node, location: :expression, message: MSG_INCLUDE) if includes_helpers?(node) + end + + private + + def relevant_migration?(node) + in_migration?(node) && version(node) >= ENFORCED_SINCE + end + + def gitlab_migration_class?(node) + superclass(node) == MIGRATION_CLASS + end + + def superclass(class_node) + _, *others = class_node.descendants + + others.find { |node| node.const_type? && node&.const_name != 'Types' }&.const_name + end + end + end + end +end diff --git a/spec/rubocop/cop/migration/versioned_migration_class_spec.rb b/spec/rubocop/cop/migration/versioned_migration_class_spec.rb new file mode 100644 index 0000000000000..89b0d1a9c8552 --- /dev/null +++ b/spec/rubocop/cop/migration/versioned_migration_class_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'fast_spec_helper' +require_relative '../../../../rubocop/cop/migration/versioned_migration_class' + +RSpec.describe RuboCop::Cop::Migration::VersionedMigrationClass do + subject(:cop) { described_class.new } + + let(:migration) do + <<~SOURCE + class TestMigration < Gitlab::Database::Migration[1.0] + def up + execute 'select 1' + end + + def down + execute 'select 1' + end + end + SOURCE + end + + shared_examples 'a disabled cop' do + it 'does not register any offenses' do + expect_no_offenses(migration) + end + end + + context 'outside of a migration' do + it_behaves_like 'a disabled cop' + end + + context 'in migration' do + before do + allow(cop).to receive(:in_migration?).and_return(true) + end + + context 'in an old migration' do + before do + allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE - 5) + end + + it_behaves_like 'a disabled cop' + end + + context 'that is recent' do + before do + allow(cop).to receive(:version).and_return(described_class::ENFORCED_SINCE + 5) + end + + it 'adds an offence if inheriting from ActiveRecord::Migration' do + expect_offense(<<~RUBY) + class MyMigration < ActiveRecord::Migration[6.1] + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't inherit from ActiveRecord::Migration but use Gitlab::Database::Migration[1.0] instead. + end + RUBY + end + + it 'adds an offence if including Gitlab::Database::MigrationHelpers directly' do + expect_offense(<<~RUBY) + class MyMigration < Gitlab::Database::Migration[1.0] + include Gitlab::Database::MigrationHelpers + ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Don't include migration helper modules directly. Inherit from Gitlab::Database::Migration[1.0] instead. + end + RUBY + end + end + end +end -- GitLab