Skip to content

Commit

Permalink
Introduce AllCops: MigratedSchemaVersion config
Browse files Browse the repository at this point in the history
This PR introduces `AllCops: MigratedSchemaVersion` config.

By specifying the `MigratedSchemaVersion` option, migration files that have already been run can be ignored.
When `MigratedSchemaVersion: '20241225000000'` is set, migration files lower than or equal to '20241225000000' will be ignored.
For example, to ignore db/migrate/20241225000000_create_articles.rb and earlier migrations you would configure it the following way:

```yaml
AllCops
  MigratedSchemaVersion: '20241225000000'
```

This prevents inspecting schema settings for already applied migration files.
Changing already applied migrations should be avoided because it can lead to the schema getting out of sync
between your local copy and what it actually is in production, depending on when `bin/rails db:migrate` was executed.
If you want to modify your schema to comply with the cops, you should instead create new migrations.
  • Loading branch information
koic committed Dec 25, 2024
1 parent ad3287a commit 86b7a55
Show file tree
Hide file tree
Showing 22 changed files with 224 additions and 54 deletions.
16 changes: 16 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,22 @@ gems.locked file to find the version of Rails that has been bound to the
application. If neither of those files exist, RuboCop will use Rails 5.0
as the default.

### `AllCops: MigratedSchemaVersion`

By specifying the `MigratedSchemaVersion` option, migration files that have already been run can be ignored.
When `MigratedSchemaVersion: '20241225000000'` is set, migration files lower than or equal to '20241225000000' will be ignored.
For example, to ignore db/migrate/20241225000000_create_articles.rb and earlier migrations you would configure it the following way:
```yaml
AllCops
MigratedSchemaVersion: '20241225000000'
```
This prevents inspecting schema settings for already applied migration files.
Changing already applied migrations should be avoided because it can lead to the schema getting out of sync
between your local copy and what it actually is in production, depending on when `bin/rails db:migrate` was executed.
If you want to modify your schema to comply with the cops, you should instead create new migrations.

## Rails configuration tip

In Rails 6.1+, add the following `config.generators.after_generate` setting to
Expand Down
1 change: 1 addition & 0 deletions changelog/new_intro_migrated_schema_version.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* [#1383](https://github.com/rubocop/rubocop-rails/pull/1383): Introduce `AllCops: MigratedSchemaVersion` config. ([@koic][])
4 changes: 4 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ AllCops:
# application. If neither of those files exist, RuboCop will use Rails 5.0
# as the default.
TargetRailsVersion: ~
# By specifying `MigratedSchemaVersion` option, migration files that have been migrated can be ignored.
# When `MigratedSchemaVersion: '20241231000000'` is set. Migration files lower than or equal to '20250101000000' will be ignored.
# For example, this is the timestamp in db/migrate/20250101000000_create_articles.rb.
MigratedSchemaVersion: ~

Lint/NumberConversion:
# Add Rails' duration methods to the ignore list for `Lint/NumberConversion`
Expand Down
17 changes: 17 additions & 0 deletions docs/modules/ROOT/pages/usage.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,23 @@ gems.locked file to find the version of Rails that has been bound to the
application. If neither of those files exist, RuboCop will use Rails 5.0
as the default.

=== `AllCops: MigratedSchemaVersion`

By specifying the `MigratedSchemaVersion` option, migration files that have already been run can be ignored.
When `MigratedSchemaVersion: '20241225000000'` is set, migration files lower than or equal to '20241225000000' will be ignored.
For example, to ignore db/migrate/20241225000000_create_articles.rb and earlier migrations you would configure it the following way:

[source,yaml]
----
AllCops
MigratedSchemaVersion: '20241225000000'
----

This prevents inspecting schema settings for already applied migration files.
Changing already applied migrations should be avoided because it can lead to the schema getting out of sync
between your local copy and what it actually is in production, depending on when `bin/rails db:migrate` was executed.
If you want to modify your schema to comply with the cops, you should instead create new migrations.

== Rails configuration tip

In Rails 6.1+, add the following `config.generators.after_generate` setting to
Expand Down
29 changes: 29 additions & 0 deletions lib/rubocop/cop/mixin/migrations_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,35 @@ def in_migration?(node)
migration_class?(class_node)
end
end

# rubocop:disable Style/DocumentDynamicEvalDefinition
%i[on_send on_csend on_block on_numblock on_class].each do |method|
class_eval(<<~RUBY, __FILE__, __LINE__ + 1)
def #{method}(node)
return if already_migrated_file?
super if method(__method__).super_method
end
RUBY
end
# rubocop:enable Style/DocumentDynamicEvalDefinition

private

def already_migrated_file?
return false unless migrated_schema_version

match_data = File.basename(processed_source.file_path).match(/(?<timestamp>\d{14})/)
schema_version = match_data['timestamp'] if match_data

return false unless schema_version

schema_version <= migrated_schema_version.to_s # Ignore applied migration files.
end

def migrated_schema_version
config.for_all_cops.fetch('MigratedSchemaVersion', nil)
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/add_column_index.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ module Rails
class AddColumnIndex < Base
extend AutoCorrector
include RangeHelp
prepend MigrationsHelper

MSG = '`add_column` does not accept an `index` key, use `add_index` instead.'
RESTRICT_ON_SEND = %i[add_column].freeze
Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/bulk_change_table.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ module Rails
# end
class BulkChangeTable < Base
include DatabaseTypeResolvable
prepend MigrationsHelper

MSG_FOR_CHANGE_TABLE = <<~MSG.chomp
You can combine alter queries using `bulk: true` options.
Expand Down
2 changes: 2 additions & 0 deletions lib/rubocop/cop/rails/dangerous_column_names.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ module Rails
# # good
# add_column :users, :saved
class DangerousColumnNames < Base # rubocop:disable Metrics/ClassLength
prepend MigrationsHelper

COLUMN_TYPE_METHOD_NAMES = %i[
bigint
binary
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/migration_class_name.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ module Rails
#
class MigrationClassName < Base
extend AutoCorrector
include MigrationsHelper
prepend MigrationsHelper

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

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/not_null_column.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ module Rails
# change_column_null :products, :category_id, false
class NotNullColumn < Base
include DatabaseTypeResolvable
prepend MigrationsHelper

MSG = 'Do not add a NOT NULL column without a default value.'
RESTRICT_ON_SEND = %i[add_column add_reference].freeze
Expand Down
2 changes: 1 addition & 1 deletion lib/rubocop/cop/rails/reversible_migration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ module Rails
# remove_index :users, column: :email
# end
class ReversibleMigration < Base
include MigrationsHelper
prepend MigrationsHelper

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ module Rails
# end
# end
class ReversibleMigrationMethodDefinition < Base
include MigrationsHelper
prepend MigrationsHelper

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

Expand Down
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails/schema_comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ module Rails
#
class SchemaComment < Base
include ActiveRecordMigrationsHelper
prepend MigrationsHelper

COLUMN_MSG = 'New database column without `comment`.'
TABLE_MSG = 'New database table without `comment`.'
Expand Down
3 changes: 2 additions & 1 deletion lib/rubocop/cop/rails/three_state_boolean_column.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,9 @@ module Rails
# t.boolean :active, default: true, null: false
#
class ThreeStateBooleanColumn < Base
MSG = 'Boolean columns should always have a default value and a `NOT NULL` constraint.'
prepend MigrationsHelper

MSG = 'Boolean columns should always have a default value and a `NOT NULL` constraint.'
RESTRICT_ON_SEND = %i[add_column column boolean].freeze

def_node_matcher :three_state_boolean?, <<~PATTERN
Expand Down
34 changes: 28 additions & 6 deletions spec/rubocop/cop/rails/add_column_index_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::AddColumnIndex, :config do
let(:config) do
RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' })
end

it 'registers an offense and corrects when an `add_column` call has `index: true`' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer, default: 0, index: true
^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
RUBY
Expand All @@ -14,7 +18,7 @@
end

it 'registers an offense and corrects when an `add_column` call has `index:` with a hash' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer, default: 0, index: { unique: true, name: 'my_unique_index' }
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
RUBY
Expand All @@ -26,7 +30,7 @@
end

it 'registers an offense and corrects with there is another hash key after `index`' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer, index: true, default: 0
^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
RUBY
Expand All @@ -38,7 +42,7 @@
end

it 'registers an offense and corrects with string keys' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer, 'index' => true, default: 0
^^^^^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
RUBY
Expand All @@ -50,7 +54,7 @@
end

it 'registers an offense and corrects when on multiple lines' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer,
index: true,
^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
Expand All @@ -65,7 +69,7 @@
end

it 'can correct multiple `add_column` calls' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer, default: 0, index: true
^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
add_column :table, :column2, :integer, default: 0, index: true
Expand All @@ -85,4 +89,22 @@
add_column :table, :column, :integer, default: 0
RUBY
end

context '`MigratedSchemaVersion` is an integer' do
let(:config) do
RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => 20240101010101 }) # rubocop:disable Style/NumericLiterals
end

it 'registers an offense and corrects when an `add_column` call has `index: true`' do
expect_offense(<<~RUBY, '20250101010101_add_column_to_table.rb')
add_column :table, :column, :integer, default: 0, index: true
^^^^^^^^^^^ `add_column` does not accept an `index` key, use `add_index` instead.
RUBY

expect_correction(<<~RUBY)
add_column :table, :column, :integer, default: 0
add_index :table, :column
RUBY
end
end
end
16 changes: 14 additions & 2 deletions spec/rubocop/cop/rails/dangerous_column_names_spec.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::DangerousColumnNames, :config do
let(:config) do
RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' })
end

context 'with non-dangerous column name' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
Expand All @@ -18,6 +22,14 @@
end
end

context 'with dangerous column name on `add_column` when migration file was migrated' do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY, '20190101010101_add_save_to_users.rb')
add_column :users, :save, :string
RUBY
end
end

context 'with dangerous column name on `rename_column`' do
it 'registers an offense' do
expect_offense(<<~RUBY)
Expand All @@ -29,7 +41,7 @@

context 'with dangerous column name on `t.string`' do
it 'registers an offense' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_create_users.rb')
create_table :users do |t|
t.string :save
^^^^^ Avoid dangerous column names.
Expand All @@ -40,7 +52,7 @@

context 'with dangerous column name on `t.rename`' do
it 'registers an offense' do
expect_offense(<<~RUBY)
expect_offense(<<~RUBY, '20250101010101_create_users.rb')
create_table :users do |t|
t.rename :name, :save
^^^^^ Avoid dangerous column names.
Expand Down
7 changes: 5 additions & 2 deletions spec/rubocop/cop/rails/migration_class_name_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::MigrationClassName, :config do
let(:filename) { 'db/migrate/20220101050505_create_users.rb' }
let(:config) do
RuboCop::Config.new('AllCops' => { 'MigratedSchemaVersion' => '20240101010101' })
end
let(:filename) { 'db/migrate/20250101010101_create_users.rb' }

context 'when the class name matches its file name' do
it 'does not register an offense' do
Expand Down Expand Up @@ -85,7 +88,7 @@ class AddBlobs < ActiveRecord::Migration[7.0]
# end
#
context 'when `ActiveSupport::Inflector` is applied to the class name and the case is different' do
let(:filename) { 'db/migrate/20210623095243_remove_unused_oauth_scope_grants.rb' }
let(:filename) { 'db/migrate/20250101010101_remove_unused_oauth_scope_grants.rb' }

it 'does not register an offense' do
expect_no_offenses(<<~RUBY, filename)
Expand Down
Loading

0 comments on commit 86b7a55

Please sign in to comment.