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 `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.

```yaml
AllCops
  MigratedSchemaVersion: 20250101000000
```

This prevents inspecting schema settings for already applied migration files and avoids having different database schemas
depending on when `bin/rails db:migrate` is executed.
  • Loading branch information
koic committed Nov 8, 2024
1 parent ad3287a commit b2bbf6f
Show file tree
Hide file tree
Showing 22 changed files with 220 additions and 54 deletions.
14 changes: 14 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,20 @@ 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 `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.
```yaml
AllCops
MigratedSchemaVersion: '20250101000000'
```
This prevents inspecting schema settings for already applied migration files and avoids having different database schemas
depending on when `bin/rails db:migrate` is executed.

## 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
15 changes: 15 additions & 0 deletions docs/modules/ROOT/pages/usage.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,21 @@ 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 `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.

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

This prevents inspecting schema settings for already applied migration files and avoids having different database schemas
depending on when `bin/rails db:migrate` is executed.

== 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 b2bbf6f

Please sign in to comment.