Skip to content

Commit 8a06cfd

Browse files
committed
[Fix rubocop#1410] Make registered cops aware of AllCops: MigratedSchemaVersion
When implementing rubocop#1383, the detection of `AllCops: MigratedSchemaVersion` was initially considered only for cops related to migrations. However, feedback received later, such as in rubocop#1410, indicated that it is also expected to apply to `Style`, `Lint`, and other categories. This suggestion is reasonable, as warnings for migrated files may not be limited to database columns but could also include Ruby programming logic. Excluding `Style` and `Lint` from this consideration would not align with this feedback. This PR modifies the behavior so that all registered cops can detect the value of `AllCops: MigratedSchemaVersion`. Fixes rubocop#1410.
1 parent b1fbd49 commit 8a06cfd

14 files changed

+81
-38
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
* [#1410](https://github.com/rubocop/rubocop-rails/issues/1410): Make registered cops aware of `AllCops: MigratedSchemaVersion`. ([@koic][])

lib/rubocop-rails.rb

+3
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,9 @@
1515

1616
require_relative 'rubocop/cop/rails_cops'
1717

18+
require_relative 'rubocop/rails/migration_file_skippable'
19+
RuboCop::Rails::MigrationFileSkippable.apply_to_cops!
20+
1821
RuboCop::Cop::Style::HashExcept.minimum_target_ruby_version(2.0)
1922

2023
RuboCop::Cop::Style::InverseMethods.singleton_class.prepend(

lib/rubocop/cop/mixin/migrations_helper.rb

-29
Original file line numberDiff line numberDiff line change
@@ -21,35 +21,6 @@ def in_migration?(node)
2121
migration_class?(class_node)
2222
end
2323
end
24-
25-
# rubocop:disable Style/DocumentDynamicEvalDefinition
26-
%i[on_send on_csend on_block on_numblock on_class].each do |method|
27-
class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
28-
def #{method}(node)
29-
return if already_migrated_file?
30-
31-
super if method(__method__).super_method
32-
end
33-
RUBY
34-
end
35-
# rubocop:enable Style/DocumentDynamicEvalDefinition
36-
37-
private
38-
39-
def already_migrated_file?
40-
return false unless migrated_schema_version
41-
42-
match_data = File.basename(processed_source.file_path).match(/(?<timestamp>\d{14})/)
43-
schema_version = match_data['timestamp'] if match_data
44-
45-
return false unless schema_version
46-
47-
schema_version <= migrated_schema_version.to_s # Ignore applied migration files.
48-
end
49-
50-
def migrated_schema_version
51-
config.for_all_cops.fetch('MigratedSchemaVersion', nil)
52-
end
5324
end
5425
end
5526
end

lib/rubocop/cop/rails/add_column_index.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ module Rails
1919
class AddColumnIndex < Base
2020
extend AutoCorrector
2121
include RangeHelp
22-
prepend MigrationsHelper
22+
include MigrationsHelper
2323

2424
MSG = '`add_column` does not accept an `index` key, use `add_index` instead.'
2525
RESTRICT_ON_SEND = %i[add_column].freeze

lib/rubocop/cop/rails/bulk_change_table.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ module Rails
6565
# end
6666
class BulkChangeTable < Base
6767
include DatabaseTypeResolvable
68-
prepend MigrationsHelper
68+
include MigrationsHelper
6969

7070
MSG_FOR_CHANGE_TABLE = <<~MSG.chomp
7171
You can combine alter queries using `bulk: true` options.

lib/rubocop/cop/rails/dangerous_column_names.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ module Rails
1414
# # good
1515
# add_column :users, :saved
1616
class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength
17-
prepend MigrationsHelper
17+
include MigrationsHelper
1818

1919
COLUMN_TYPE_METHOD_NAMES = %i[
2020
bigint

lib/rubocop/cop/rails/migration_class_name.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ module Rails
2020
#
2121
class MigrationClassName < Base
2222
extend AutoCorrector
23-
prepend MigrationsHelper
23+
include MigrationsHelper
2424

2525
MSG = 'Replace with `%<camelized_basename>s` that matches the file name.'
2626

lib/rubocop/cop/rails/not_null_column.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ module Rails
4141
# change_column_null :products, :category_id, false
4242
class NotNullColumn < Base
4343
include DatabaseTypeResolvable
44-
prepend MigrationsHelper
44+
include MigrationsHelper
4545

4646
MSG = 'Do not add a NOT NULL column without a default value.'
4747
RESTRICT_ON_SEND = %i[add_column add_reference].freeze

lib/rubocop/cop/rails/reversible_migration.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ module Rails
151151
# remove_index :users, column: :email
152152
# end
153153
class ReversibleMigration < Base
154-
prepend MigrationsHelper
154+
include MigrationsHelper
155155

156156
MSG = '%<action>s is not reversible.'
157157

lib/rubocop/cop/rails/reversible_migration_method_definition.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ module Rails
4343
# end
4444
# end
4545
class ReversibleMigrationMethodDefinition < Base
46-
prepend MigrationsHelper
46+
include MigrationsHelper
4747

4848
MSG = 'Migrations must contain either a `change` method, or both an `up` and a `down` method.'
4949

lib/rubocop/cop/rails/schema_comment.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ module Rails
2323
#
2424
class SchemaComment < Base
2525
include ActiveRecordMigrationsHelper
26-
prepend MigrationsHelper
26+
include MigrationsHelper
2727

2828
COLUMN_MSG = 'New database column without `comment`.'
2929
TABLE_MSG = 'New database table without `comment`.'

lib/rubocop/cop/rails/three_state_boolean_column.rb

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ module Rails
1818
# t.boolean :active, default: true, null: false
1919
#
2020
class ThreeStateBooleanColumn < Base
21-
prepend MigrationsHelper
21+
include MigrationsHelper
2222

2323
MSG = 'Boolean columns should always have a default value and a `NOT NULL` constraint.'
2424
RESTRICT_ON_SEND = %i[add_column column boolean].freeze
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
# frozen_string_literal: true
2+
3+
module RuboCop
4+
module Rails
5+
# This module allows cops to detect and ignore files that have already been migrated
6+
# by leveraging the `AllCops: MigratedSchemaVersion` configuration.
7+
#
8+
# [source,yaml]
9+
# -----
10+
# AllCops:
11+
# MigratedSchemaVersion: '20241225000000'
12+
# -----
13+
#
14+
# When applied to cops, it dynamically overrides the `on_*` methods for all supported
15+
# node types, ensuring that cops skip processing if the file is determined to be a
16+
# migrated file based on the schema version.
17+
#
18+
# @api private
19+
module MigrationFileSkippable
20+
Parser::Meta::NODE_TYPES.each do |node_type|
21+
method_name = :"on_#{node_type}"
22+
23+
class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
24+
def #{method_name}(node, *args) # def on_if(node, *args)
25+
return if already_migrated_file? # return if already_migrated_file?
26+
#
27+
super if method(__method__).super_method # super if method(__method__).super_method
28+
rescue NameError # rescue NameError
29+
# noop # # noop
30+
end # end
31+
RUBY
32+
end
33+
34+
def self.apply_to_cops!
35+
RuboCop::Cop::Registry.all.each { |cop| cop.prepend(MigrationFileSkippable) }
36+
end
37+
38+
private
39+
40+
def already_migrated_file?
41+
return false unless migrated_schema_version
42+
43+
match_data = File.basename(processed_source.file_path).match(/(?<timestamp>\d{14})/)
44+
schema_version = match_data['timestamp'] if match_data
45+
46+
return false unless schema_version
47+
48+
schema_version <= migrated_schema_version.to_s # Ignore applied migration files.
49+
end
50+
51+
def migrated_schema_version
52+
config.for_all_cops.fetch('MigratedSchemaVersion', nil)
53+
end
54+
end
55+
end
56+
end

spec/rubocop/cop/rails/presence_spec.rb

+12
Original file line numberDiff line numberDiff line change
@@ -441,4 +441,16 @@
441441
end
442442
RUBY
443443
end
444+
445+
context 'when inspection file that have already been migrated' do
446+
let(:config) do
447+
RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' })
448+
end
449+
450+
it 'does not register an offense and corrects when `a.present? ? a : nil`' do
451+
expect_no_offenses(<<~RUBY, '20190101010101_add_column_to_table.rb')
452+
a.present? ? a : nil
453+
RUBY
454+
end
455+
end
444456
end

0 commit comments

Comments
 (0)